Skip to content
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

Allow customizing QPS/Burst in the kubeapps-api service #4069

Merged
merged 4 commits into from
Jan 13, 2022

Conversation

antgamdia
Copy link
Contributor

@antgamdia antgamdia commented Jan 12, 2022

Description of the change

When talking to the k8s API it is critical to configure the rate limit accordingly rather than relying on the default (5 requests per second, AFAIK). This PR allows changing the QPS/Burst values as we already did in Kubeops a long time ago.

Benefits

More customization.

Possible drawbacks

N/A

Applicable issues

Additional information

Required by #4062 to work decently (setting qps to ~100 is enough)

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

When talking to the k8s API it is critical to configure the rate limit accordingly rather than relying on the default (5 requests per second, AFAIK)

Heh, yes, 5 per second is a sensible default for a human client serving one keyboard, not a service :) Good find.

Required by #4062 to work decently (setting qps to ~100 is enough)

Hmm, why default to 10.0 then?

@@ -1658,6 +1660,12 @@ kubeappsapis:
## @param kubeappsapis.replicaCount Number of frontend replicas to deploy
##
replicaCount: 2
## @param kubeappsapis.qps KubeappsAPIs Kubernetes API client QPS limit
##
qps: "10.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of interest, I notice that for kubeops, the chart defaults are "" (ie. not set by the chart) with the kubeops binary setting defaults of 10 and 15 when the args aren't set, but here for kubeapps apis you're explicitly setting the defaults here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, that's precisely the intention: in kubeops, we were modifying the default rate limit values directly in the code, so users wouldn't notice the actual value being used.

If we set the values in the values.yaml, it will appear directly in the readme, so users will immediately know which is the default value without digging into the code.

Hmm, why default to 10.0 then?

In this PR I'm just using the same values as we were defining in Kubeops. The proper value for the kubeappsapis service to work decently is yet to be analyzed and defined (100 was just a random feasible value, but we have to give it a thought)

@antgamdia antgamdia merged commit dbfb082 into vmware-tanzu:master Jan 13, 2022
@antgamdia antgamdia deleted the kubeappsapisQPS branch January 13, 2022 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants