-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: Adapter has high qps to apiserver #2922
fix: Adapter has high qps to apiserver #2922
Conversation
Looking good, could you please also pass the same client to the Also, have you been able to test this setup to see if it really helped with the issue? |
will do later
I test the image locally, only the first request will hit the apiserver, after there are only list from informer cache(guaranteed by list-watch) |
Cool :) Then please also fix DCO and update the Changelog, that's a great improvement! |
337f0ab
to
3b034e6
Compare
@zroubalik Can I just skip the test case in this PR? Since this one only mitigate the burden to Apiserver, will not affect the performance/logic in Keda side |
@bamboo12366 yeah, no need for test in this case. |
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.
LGTM
Awesome improvement! ❤️
Only 2 small nits inline
/run-e2e |
/run-e2e |
/run-e2e |
I'm not sure how, but it seems that these changes are breaking e2e tests. They have failed 3 times in a row when in |
/run-e2e |
/run-e2e mysql* |
/run-e2e argo* |
Hey @bamboo12366 |
@JorTurFer I update CHANLOG and resolve conflicts accordingly, could you please help me review again to see if everything fit? |
/run-e2e Failure is expected, but only graphite scaler |
I'm not sure about what commit breaks the DCO check @bamboo12366, but at least 1 commit is not signed. |
sorry I am not that familiar with git. I already merge the rebase and merge the main into current branch, I don't know how to squash commit now, could you advise more? @JorTurFer |
Sure, you can do this: git checkout yourBranch
git reset $(git merge-base main $(git branch --show-current))
git add -A
git commit -sm "one commit on yourBranch" Squash the branch is not mandatory, but it's the easiest way to ensure all commits are signed (because only 1 needs to be signed xD) |
Signed-off-by: Xinzhu Zhou <[email protected]>
f861892
to
d5e7d7f
Compare
Thanks A Lot!! You truly save my live! |
/run-e2e |
Provide a description of what has been changed
Checklist
Fixes #2914