-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
set WatchListPageSize
when WatchError
#13461
Labels
Comments
fyp711
changed the title
Always set ResourceVersion="" may make etcd overload
【discuss】Always set ResourceVersion="" may make etcd overload
Aug 13, 2024
fyp711
changed the title
【discuss】Always set ResourceVersion="" may make etcd overload
[discuss] Always set ResourceVersion="" may make etcd overload
Aug 13, 2024
agilgur5
changed the title
[discuss] Always set ResourceVersion="" may make etcd overload
[discuss] Setting Aug 13, 2024
ResourceVersion=""
may overload etcd
Could someone help review this? #13466 Thanks! |
fyp711
changed the title
[discuss] Setting
fix(controller): set Sep 18, 2024
ResourceVersion=""
may overload etcdWatchListPageSize
when WatchError
Setting Below code is a part of ...
switch {
case r.WatchListPageSize != 0:
pager.PageSize = r.WatchListPageSize
case r.paginatedResult:
// We got a paginated result initially. Assume this resource and server honor
// paging requests (i.e. watch cache is probably disabled) and leave the default
// pager size set.
case options.ResourceVersion != "" && options.ResourceVersion != "0":
// User didn't explicitly request pagination.
//
// With ResourceVersion != "", we have a possibility to list from watch cache,
// but we do that (for ResourceVersion != "0") only if Limit is unset.
// To avoid thundering herd on etcd (e.g. on master upgrades), we explicitly
// switch off pagination to force listing from watch cache (if enabled).
// With the existing semantic of RV (result is at least as fresh as provided RV),
// this is correct and doesn't lead to going back in time.
//
// We also don't turn off pagination for ResourceVersion="0", since watch cache
// is ignoring Limit in that case anyway, and if watch cache is not enabled
// we don't introduce regression.
pager.PageSize = 0
}
list, paginatedResult, err = pager.List(context.Background(), options)
... |
agilgur5
changed the title
fix(controller): set
set Sep 21, 2024
WatchListPageSize
when WatchError
WatchListPageSize
when WatchError
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Pre-requisites
:latest
image tag (i.e.quay.io/argoproj/workflow-controller:latest
) and can confirm the issue still exists on:latest
. If not, I have explained why, in detail, in my description below.What happened? What did you expect to happen?
This submission is an optimization based on this #11343
Optimize two key points:
Version(s)
3.4.9
Paste a minimal workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.
None
Logs from the workflow controller
Logs from in your workflow's wait container
The text was updated successfully, but these errors were encountered: