From 406899360b55b3a0e28970f369288a2bb8a804fe Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Wed, 21 Jun 2023 14:02:46 +0200 Subject: [PATCH] Refactor shouldDelegateList/shouldListFromStorage to better explain decisions --- .../k8s.io/apiserver/pkg/storage/cacher/cacher.go | 14 +++++++------- .../flowcontrol/request/list_work_estimator.go | 8 +++++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go index 5e5aa572d33..a350faf6a50 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go @@ -721,17 +721,17 @@ func shouldDelegateList(opts storage.ListOptions) bool { pred := opts.Predicate match := opts.ResourceVersionMatch pagingEnabled := utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) + + // Serve consistent reads from storage + consistentReadFromStorage := resourceVersion == "" + // Watch cache doesn't support continuations, so serve them from etcd. hasContinuation := pagingEnabled && len(pred.Continue) > 0 + // Serve paginated requests about revision "0" from watch cache to avoid overwhelming etcd. hasLimit := pagingEnabled && pred.Limit > 0 && resourceVersion != "0" + // Watch cache only supports ResourceVersionMatchNotOlderThan (default). unsupportedMatch := match != "" && match != metav1.ResourceVersionMatchNotOlderThan - // If resourceVersion is not specified, serve it from underlying - // storage (for backward compatibility). If a continuation is - // requested, serve it from the underlying storage as well. - // Limits are only sent to storage when resourceVersion is non-zero - // since the watch cache isn't able to perform continuations, and - // limits are ignored when resource version is zero - return resourceVersion == "" || hasContinuation || hasLimit || unsupportedMatch + return consistentReadFromStorage || hasContinuation || hasLimit || unsupportedMatch } func (c *Cacher) listItems(ctx context.Context, listRV uint64, key string, pred storage.SelectionPredicate, recursive bool) ([]interface{}, uint64, string, error) { diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go index 130746a411e..22f556d2506 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/request/list_work_estimator.go @@ -149,9 +149,15 @@ func shouldListFromStorage(query url.Values, opts *metav1.ListOptions) bool { resourceVersion := opts.ResourceVersion match := opts.ResourceVersionMatch pagingEnabled := utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) + + // Serve consistent reads from storage + consistentReadFromStorage := resourceVersion == "" + // Watch cache doesn't support continuations, so serve them from etcd. hasContinuation := pagingEnabled && len(opts.Continue) > 0 + // Serve paginated requests about revision "0" from watch cache to avoid overwhelming etcd. hasLimit := pagingEnabled && opts.Limit > 0 && resourceVersion != "0" + // Watch cache only supports ResourceVersionMatchNotOlderThan (default). unsupportedMatch := match != "" && match != metav1.ResourceVersionMatchNotOlderThan - return resourceVersion == "" || hasContinuation || hasLimit || unsupportedMatch + return consistentReadFromStorage || hasContinuation || hasLimit || unsupportedMatch }