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

fix(server): pass through burst and qps for auth.kubeclient #12575

Merged
merged 10 commits into from
Feb 9, 2024

Conversation

shuangkun
Copy link
Member

@shuangkun shuangkun commented Jan 25, 2024

When I submit or access a large number of workflows at the same time in argo-server URL,I encountered many client-side throttling which let my request slow, I find the root cause is the client in workflow_server.go 's burst and qps is 0. So I want to increase it. After I added it, it was fine. There is no throttling.

wfClient := auth.GetWfClient(ctx)
kubeClient := auth.GetKubeClient(ctx)

some log in argo server

I0123 08:01:37.531986       1 request.go:601] Waited for 1.196145076s due to client-side throttling, not priority and fairness, request: DELETE:https://192.168.1.138:6443/api/v1/namespaces/default/pods/steps-bszqb-whalesay-600069335
I0123 08:01:47.731411       1 request.go:601] Waited for 11.379013988s due to client-side throttling, not priority and fairness, request: DELETE:https://192.168.1.138:6443/api/v1/namespaces/default/pods/steps-bszqb-whalesay-4291585860
I0123 08:01:57.930750       1 request.go:601] Waited for 21.53704195s due to client-side throttling, not priority and fairness, request: DELETE:https://192.168.1.138:6443/api/v1/namespaces/default/pods/steps-bszqb-whalesay-4209244865
I0123 08:02:07.930913       1 request.go:601] Waited for 31.536611828s due to client-side throttling, not priority and fairness, request: DELETE:https://192.168.1.138:6443/api/v1/namespaces/default/pods/steps-bszqb-whalesay-163250805

Signed-off-by: shuangkun <tsk2013uestc@163.com>
@shuangkun shuangkun marked this pull request as draft January 25, 2024 14:42
Signed-off-by: shuangkun <tsk2013uestc@163.com>
@shuangkun shuangkun marked this pull request as ready for review January 25, 2024 14:49
@agilgur5
Copy link
Member

agilgur5 commented Jan 26, 2024

For reference for other readers, this PR takes the burst and QPS from the Server's configuration in the CLI and passes it through to the auth kubeClient of the Server. Notably the defaults there are 20 QPS, 30 burst, so this does also implicitly increase the defaults of the auth kubeClient

I think that makes sense to do, but it wasn't clear from the PR where the restConfig values come from

server/auth/gatekeeper.go Outdated Show resolved Hide resolved
server/auth/gatekeeper.go Outdated Show resolved Hide resolved
@agilgur5 agilgur5 self-assigned this Jan 26, 2024
@agilgur5
Copy link
Member

this PR takes the burst and QPS from the Server's configuration in the CLI and passes it through to the auth kubeClient of the Server

If this is just pass through, this might make sense as a fix rather than a feat, since one would think it would pass through already

@shuangkun
Copy link
Member Author

shuangkun commented Jan 27, 2024

https://github.com/argoproj/argo-workflows/blob/main/cmd/argo/commands/server.go

config.Burst = kubeAPIBurst
config.QPS = kubeAPIQPS

@shuangkun
Copy link
Member Author

this PR takes the burst and QPS from the Server's configuration in the CLI and passes it through to the auth kubeClient of the Server

If this is just pass through, this might make sense as a fix rather than a feat, since one would think it would pass through already

OK i will change it

@shuangkun shuangkun changed the title feat: set burst and qps for auth.kubeclient fix: set burst and qps for auth.kubeclient Jan 27, 2024
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
server/auth/gatekeeper.go Outdated Show resolved Hide resolved
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: shuangkun <tsk2013uestc@163.com>
@shuangkun shuangkun closed this Feb 4, 2024
@shuangkun shuangkun reopened this Feb 4, 2024
@shuangkun shuangkun closed this Feb 4, 2024
@shuangkun shuangkun reopened this Feb 4, 2024
@shuangkun shuangkun added the prioritized-review For members of the Sustainability Effort label Feb 7, 2024
server/auth/gatekeeper.go Show resolved Hide resolved
server/auth/gatekeeper.go Show resolved Hide resolved
Signed-off-by: shuangkun <tsk2013uestc@163.com>
@agilgur5 agilgur5 changed the title fix: set burst and qps for auth.kubeclient fix: pass through burst and qps for auth.kubeclient Feb 9, 2024
@agilgur5 agilgur5 enabled auto-merge (squash) February 9, 2024 03:42
@agilgur5 agilgur5 merged commit ae0973a into argoproj:main Feb 9, 2024
28 checks passed
@shuangkun
Copy link
Member Author

Thanks!

isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request Feb 20, 2024
Signed-off-by: shuangkun <tsk2013uestc@163.com>
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request Feb 27, 2024
Signed-off-by: shuangkun <tsk2013uestc@163.com>
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request Feb 28, 2024
Signed-off-by: shuangkun <tsk2013uestc@163.com>
Signed-off-by: Isitha Subasinghe <isubasinghe@student.unimelb.edu.au>
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request Mar 12, 2024
Signed-off-by: shuangkun <tsk2013uestc@163.com>
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request May 6, 2024
Signed-off-by: shuangkun <tsk2013uestc@163.com>
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request May 7, 2024
Signed-off-by: shuangkun <tsk2013uestc@163.com>
@agilgur5 agilgur5 changed the title fix: pass through burst and qps for auth.kubeclient fix(server): pass through burst and qps for auth.kubeclient May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/server prioritized-review For members of the Sustainability Effort
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants