From 49886c205c4bed5c7e43ccb66e0ecd183fbcf273 Mon Sep 17 00:00:00 2001 From: Siyuan Zhang Date: Fri, 20 Oct 2023 12:06:46 -0700 Subject: [PATCH 1/2] k8s.io/apiserver/storage/etcd: refactor etcd GetList. reduce redundant update of withRev after request. Signed-off-by: Siyuan Zhang Kubernetes-commit: 84ec5e2eccbc07b17f3b3e3e00dc3996105e0346 --- pkg/storage/etcd3/store.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/pkg/storage/etcd3/store.go b/pkg/storage/etcd3/store.go index e96efccce..480c371d4 100644 --- a/pkg/storage/etcd3/store.go +++ b/pkg/storage/etcd3/store.go @@ -627,6 +627,11 @@ func (s *store) GetList(ctx context.Context, key string, opts storage.ListOption limitOption = &options[len(options)-1] } + if opts.Recursive { + rangeEnd := clientv3.GetPrefixRangeEnd(keyPrefix) + options = append(options, clientv3.WithRange(rangeEnd)) + } + newItemFunc := getNewItemFunc(listObj, v) var fromRV *uint64 @@ -675,10 +680,6 @@ func (s *store) GetList(ctx context.Context, key string, opts storage.ListOption } } - if opts.Recursive { - rangeEnd := clientv3.GetPrefixRangeEnd(keyPrefix) - options = append(options, clientv3.WithRange(rangeEnd)) - } if withRev != 0 { options = append(options, clientv3.WithRev(withRev)) } @@ -717,6 +718,11 @@ func (s *store) GetList(ctx context.Context, key string, opts storage.ListOption if len(getResp.Kvs) == 0 && getResp.More { return fmt.Errorf("no results were found, but etcd indicated there were more values remaining") } + // indicate to the client which resource version was returned, and use the same resource version for subsequent requests. + if withRev == 0 { + withRev = getResp.Header.Revision + options = append(options, clientv3.WithRev(withRev)) + } // avoid small allocations for the result slice, since this can be called in many // different contexts and we don't know how significantly the result will be filtered @@ -776,14 +782,6 @@ func (s *store) GetList(ctx context.Context, key string, opts storage.ListOption *limitOption = clientv3.WithLimit(limit) } preparedKey = string(lastKey) + "\x00" - if withRev == 0 { - withRev = getResp.Header.Revision - options = append(options, clientv3.WithRev(withRev)) - } - } - // indicate to the client which resource version was returned - if withRev == 0 { - withRev = getResp.Header.Revision } if v.IsNil() { From ef409f941bd8badd123c9cf17656c0dfc8defd29 Mon Sep 17 00:00:00 2001 From: Siyuan Zhang Date: Fri, 20 Oct 2023 12:39:51 -0700 Subject: [PATCH 2/2] k8s.io/apiserver/storage/etcd: refactor etcd GetList. Reorder some code. Signed-off-by: Siyuan Zhang Kubernetes-commit: a968f51fa2f87ed57f9e48ba436e11421c403b27 --- pkg/storage/etcd3/store.go | 39 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/pkg/storage/etcd3/store.go b/pkg/storage/etcd3/store.go index 480c371d4..2b3f28c7d 100644 --- a/pkg/storage/etcd3/store.go +++ b/pkg/storage/etcd3/store.go @@ -634,15 +634,6 @@ func (s *store) GetList(ctx context.Context, key string, opts storage.ListOption newItemFunc := getNewItemFunc(listObj, v) - var fromRV *uint64 - if len(opts.ResourceVersion) > 0 { - parsedRV, err := s.versioner.ParseResourceVersion(opts.ResourceVersion) - if err != nil { - return apierrors.NewBadRequest(fmt.Sprintf("invalid resource version: %v", err)) - } - fromRV = &parsedRV - } - var continueRV, withRev int64 var continueKey string switch { @@ -662,21 +653,23 @@ func (s *store) GetList(ctx context.Context, key string, opts storage.ListOption if continueRV > 0 { withRev = continueRV } - default: - if fromRV != nil { - switch opts.ResourceVersionMatch { - case metav1.ResourceVersionMatchNotOlderThan: - // The not older than constraint is checked after we get a response from etcd, - // and returnedRV is then set to the revision we get from the etcd response. - case metav1.ResourceVersionMatchExact: - withRev = int64(*fromRV) - case "": // legacy case - if opts.Recursive && opts.Predicate.Limit > 0 && *fromRV > 0 { - withRev = int64(*fromRV) - } - default: - return fmt.Errorf("unknown ResourceVersionMatch value: %v", opts.ResourceVersionMatch) + case len(opts.ResourceVersion) > 0: + parsedRV, err := s.versioner.ParseResourceVersion(opts.ResourceVersion) + if err != nil { + return apierrors.NewBadRequest(fmt.Sprintf("invalid resource version: %v", err)) + } + switch opts.ResourceVersionMatch { + case metav1.ResourceVersionMatchNotOlderThan: + // The not older than constraint is checked after we get a response from etcd, + // and returnedRV is then set to the revision we get from the etcd response. + case metav1.ResourceVersionMatchExact: + withRev = int64(parsedRV) + case "": // legacy case + if opts.Recursive && opts.Predicate.Limit > 0 && parsedRV > 0 { + withRev = int64(parsedRV) } + default: + return fmt.Errorf("unknown ResourceVersionMatch value: %v", opts.ResourceVersionMatch) } }