Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Increase the default QPS and Burst value of the controller manager #1461

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/controller-manager/app/controller-manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ member clusters and do the necessary reconciliation`,
flags.StringVar(&kubeFedConfig, "kubefed-config", "", "Path to a KubeFedConfig yaml file. Test only.")
flags.StringVar(&kubeconfig, "kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.")
flags.StringVar(&masterURL, "master", "", "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.")
flags.Float32Var(&restConfigQPS, "rest-config-qps", 5.0, "Maximum QPS to the api-server from this client.")
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.

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.

Copy link
Contributor

@xunpan xunpan Oct 25, 2021

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 says Clusters larger than 500 nodes use 100 QPS client limit for both components: scheduler and controller manager. Consider kubefed is a manager of multiple clusters. I think this 100 for 500 nodes can be a value as minimum for kubefed. And burst can be double of it: 200.

So, my proposal is:

"rest-config-qps": 100 
"rest-config-burst": 200

@yuswift @hectorj2f @iawia002 what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with that 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, my proposal is:

"rest-config-qps": 100 
"rest-config-burst": 200

Updated.

flags.IntVar(&restConfigBurst, "rest-config-burst", 10, "Maximum burst for throttle to the api-server from this client.")
flags.Float32Var(&restConfigQPS, "rest-config-qps", 100.0, "Maximum QPS to the api-server from this client.")
flags.IntVar(&restConfigBurst, "rest-config-burst", 200, "Maximum burst for throttle to the api-server from this client.")

local := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
klog.InitFlags(local)
Expand Down