This repository has been archived by the owner on Apr 25, 2023. It is now read-only.
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.
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.
These values come from https://github.com/kubernetes/client-go/blob/master/rest/config.go#L45 as any other client. I suggest you simply adapt them in your deployment to other values using the flags.
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.
Yes, I understand that, but based on the actual situation, for the kubefed-controller-manager component, 5 and 10 are too small. For most users, it will be more convenient for us to set the default value to a more appropriate value. WDYT?
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.
I'm not sure if there are anyone who are using v0.8.1, after we upgraded to this version, we did encounter the slowing start up problem of controller. IMO, increasing it is necessary for the users who are using v0.8.1.
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.
I agree that default qps is small. We need to set a value that makes more sense to
kubefed
controller by default.It is not easy to get a suitable value. I think we can refer to
https://cloud.google.com/kubernetes-engine/docs/best-practices/scalability
. It saysClusters larger than 500 nodes use 100 QPS client limit for both components
: scheduler and controller manager. Considerkubefed
is a manager of multiple clusters. I think this100
for 500 nodes can be a value as minimum forkubefed
. And burst can be double of it:200
.So, my proposal is:
@yuswift @hectorj2f @iawia002 what do you think?
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.
agree with that 😉
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.