Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Manage hot region label for the tikv created by auto-scaler #1801
Manage hot region label for the tikv created by auto-scaler #1801
Changes from 8 commits
b49f33e
610be18
f3a87d9
023c56d
5af0d19
d85d4a3
f896259
b376524
ed78917
881e5ff
6777dc4
6e563c4
c7b0a19
8083282
d0b205f
7970691
ed150d2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
When the auto-scaling result is scale-out, we would record the ordinal of the tikv pod which is going to be created by auto-scaler in tidbcluster annotations in order to add hot region labels during tikv scaling process in tidbcluster controller.
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.
describe the purpose of this special label here? why we need to add this special label on newly created TiKV instances. If PD has public docs about this label, we can link too.
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.
Nice catch, I will add
TODO
to mark there should be some docs about this label after I finish the entire auto-scaling document.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.
great! comments describing the purpose or reason behind will make it easier to understand the code.
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.
Reclaimed the ordinal slots in the tidbclusters annotations which belongs to the pod already been eliminated by scale-in.
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.
Is it necessary to check if the label is already set?
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.
Not necessary.
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.
can you copy this
encode
method here and make it public?tidb-operator/pkg/manager/member/utils.go
Lines 100 to 106 in 4cbffa7
this is a more generic
JSON marshal to string
method, then we don't need to add many different but similar methods. in your case, you can passsets.List()
to it.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.
updated.