-
Notifications
You must be signed in to change notification settings - Fork 313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cluster: support IngoreInitConfigComps #1987
base: master
Are you sure you want to change the base?
Conversation
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
Codecov ReportBase: 56.31% // Head: 50.94% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1987 +/- ##
==========================================
- Coverage 56.31% 50.94% -5.37%
==========================================
Files 313 312 -1
Lines 33492 33481 -11
==========================================
- Hits 18858 17055 -1803
- Misses 12415 14212 +1797
+ Partials 2219 2214 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
I can't fix this unit test, can someone help me? |
I think we should resolve that it takes a lot of time(several hours) to generate config, rather than provide a flag to ignore it |
I don't know why we should generate configs in all nodes when scale in. The config seems unchanged. Could you please explain it to me? |
PTAL |
If any PD node is scaled in, we should re-generate configs for all TiKV and TiDB nodes as they are in the startup scripts. And the Prometheus config is always updated if any node is added or removed from the cluster. I agree that we don't have to regenerate configs for all nodes in some cases, but that could be quite complex to implement, the current approach is a reasonable workaround. Could you rename the |
Thanks for your explanation @AstroProfundis |
Sorry for the delay...
I agree, and I think TiFlash is also safe to be ignored, but I'm not 100% sure about that... |
In our production environment(only TiKV cluster), I have used this code many times and found nothing unusual, how about adding this feature as optional? Our cluster has hundreds of nodes, and init config for every node when scaling is really slow. |
I agree that adding it as an optional switch for users to decide what components should be ignored when updating configs could be reasonable.
How about like this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have updated name @AstroProfundis
And I don't know what's the meaning of mark it as hidden
@AstroProfundis It's been a long time, Do you still interested in this PR? |
What problem does this PR solve?
If we have a large number of tikv-server, it will cost a lot of time(several hours) to generate config
What is changed and how it works?
./tiup-cluster scale-in Kvstore_UAT_0 --node <IP:port> -y --ignore-config-roles tikv
Check List
Tests
Code changes
Side effects
Related changes
Release notes: