-
Notifications
You must be signed in to change notification settings - Fork 981
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
Add abitility to set QPS and Burst limits for api client #2667
base: master
Are you sure you want to change the base?
Conversation
Any updates ? |
Can you not make these variables also configurable similar to NoDatabaseAccess and NoTeamsAPI. We can use 5 and 10 as default values. |
@FxKu I have reverted my previous changes because we can't change qps and burst limits for kube client after initialization. |
Config changes always require a replacement of the operator pod. Doesn't this initialize the client as well each time? |
@FxKu I mean that you can't change kubernetes client parameters after client has been initilized. |
@FxKu We have been testing this fix in our k8s cluster with 200+ postgres clusters for 2 weeks now and it really helps. |
@FxKu any updates? |
flag.IntVar(&config.KubeQPS, "kubeqps", 5, "Kubernetes api requests per second.") | ||
flag.IntVar(&config.KubeBurst, "kubeburst", 10, "Kubernetes api requests burst limit.") |
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.
flag.IntVar(&config.KubeQPS, "kubeqps", 5, "Kubernetes api requests per second.") | |
flag.IntVar(&config.KubeBurst, "kubeburst", 10, "Kubernetes api requests burst limit.") | |
flag.IntVar(&config.KubeQPS, "kubeqps", 50, "Kubernetes api requests per second.") | |
flag.IntVar(&config.KubeBurst, "kubeburst", 100, "Kubernetes api requests burst limit.") |
I read, that default is 50 and 100. Can we use these values instead? For some reason the owner references e2e test keeps failing and I don't know why.
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.
Sorry, these were values from kubelet and not REST config. 5 and 10 are probably right. But maybe we can still increase to see if e2e tests succeed.
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.
No problem, you can increase it for tests.
But for default values I recommend use smaller like default values because it can affect on api servers.
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.
ok then better keep the defaults here. Buty any idea, why the owner references check e2e test starts failing with this change. Maybe we add these 2 new config values to the config of our kind
cluster, too.
Can you add a chapter in the docs on how to do this (administrator.md is a good place, I think)? I guess many users would be interested. |
We have a problem in our big kubernetes clusters with 300+ postgres clusters.
When operator starts reconcile it can stack creating new clusters for 10-15 minutes and We can see in the logs messages like that:
Especially in working hours when our developers create new environments.
This PR adds ability to set kubernetes qps and burst limits for client.