From 168c338f7cb44a08f3d1a7e1d0e72cac241e9a29 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Tue, 25 Feb 2025 19:17:30 +0100 Subject: [PATCH] Remove limit support from btree store We cannot use limit as it would apply it before filtering, which is done in cacher. Limit is not currently used, but let's remove it to be save, until filtering is implemented in store. --- .../apiserver/pkg/storage/cacher/store.go | 2 +- .../pkg/storage/cacher/store_btree.go | 21 +++------------ .../pkg/storage/cacher/store_btree_test.go | 26 ++++++------------- .../pkg/storage/cacher/watch_cache.go | 2 +- .../pkg/storage/cacher/watch_cache_test.go | 14 +++++----- 5 files changed, 21 insertions(+), 44 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/store.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/store.go index 3b9d9decd03..9ec654e7d1a 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/store.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/store.go @@ -73,7 +73,7 @@ type storeIndexer interface { } type orderedLister interface { - ListPrefix(prefix, continueKey string, limit int) (items []interface{}, hasMore bool) + ListPrefix(prefix, continueKey string) []interface{} Count(prefix, continueKey string) (count int) Clone() orderedLister } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree.go index 3b5d57a9755..021a3b12f10 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree.go @@ -18,7 +18,6 @@ package cacher import ( "fmt" - "math" "strings" "sync" @@ -100,10 +99,10 @@ func (si *threadedStoreIndexer) List() []interface{} { return si.store.List() } -func (si *threadedStoreIndexer) ListPrefix(prefix, continueKey string, limit int) ([]interface{}, bool) { +func (si *threadedStoreIndexer) ListPrefix(prefix, continueKey string) []interface{} { si.lock.RLock() defer si.lock.RUnlock() - return si.store.ListPrefix(prefix, continueKey, limit) + return si.store.ListPrefix(prefix, continueKey) } func (si *threadedStoreIndexer) ListKeys() []string { @@ -254,31 +253,19 @@ func (s *btreeStore) getByKey(key string) (item interface{}, exists bool, err er return item, exists, nil } -func (s *btreeStore) ListPrefix(prefix, continueKey string, limit int) ([]interface{}, bool) { - if limit < 0 { - return nil, false - } +func (s *btreeStore) ListPrefix(prefix, continueKey string) []interface{} { if continueKey == "" { continueKey = prefix } var result []interface{} - var hasMore bool - if limit == 0 { - limit = math.MaxInt - } s.tree.AscendGreaterOrEqual(&storeElement{Key: continueKey}, func(item *storeElement) bool { if !strings.HasPrefix(item.Key, prefix) { return false } - // TODO: Might be worth to lookup one more item to provide more accurate HasMore. - if len(result) >= limit { - hasMore = true - return false - } result = append(result, item) return true }) - return result, hasMore + return result } func (s *btreeStore) Count(prefix, continueKey string) (count int) { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree_test.go index a7e7071d7f3..185c433956c 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree_test.go @@ -41,40 +41,30 @@ func TestStoreListPrefix(t *testing.T) { assert.NoError(t, store.Add(testStorageElement("foo2", "bar1", 3))) assert.NoError(t, store.Add(testStorageElement("bar", "baz", 4))) - items, hasMore := store.ListPrefix("foo", "", 0) - assert.False(t, hasMore) + items := store.ListPrefix("foo", "") assert.Equal(t, []interface{}{ testStorageElement("foo1", "bar2", 2), testStorageElement("foo2", "bar1", 3), testStorageElement("foo3", "bar3", 1), }, items) - items, hasMore = store.ListPrefix("foo2", "", 0) - assert.False(t, hasMore) + items = store.ListPrefix("foo2", "") assert.Equal(t, []interface{}{ testStorageElement("foo2", "bar1", 3), }, items) - items, hasMore = store.ListPrefix("foo", "", 1) - assert.True(t, hasMore) - assert.Equal(t, []interface{}{ - testStorageElement("foo1", "bar2", 2), - }, items) - - items, hasMore = store.ListPrefix("foo", "foo1\x00", 1) - assert.True(t, hasMore) + items = store.ListPrefix("foo", "foo1\x00") assert.Equal(t, []interface{}{ testStorageElement("foo2", "bar1", 3), + testStorageElement("foo3", "bar3", 1), }, items) - items, hasMore = store.ListPrefix("foo", "foo2\x00", 1) - assert.False(t, hasMore) + items = store.ListPrefix("foo", "foo2\x00") assert.Equal(t, []interface{}{ testStorageElement("foo3", "bar3", 1), }, items) - items, hasMore = store.ListPrefix("bar", "", 0) - assert.False(t, hasMore) + items = store.ListPrefix("bar", "") assert.Equal(t, []interface{}{ testStorageElement("bar", "baz", 4), }, items) @@ -143,7 +133,7 @@ func (f fakeOrderedLister) Add(obj interface{}) error { return nil } func (f fakeOrderedLister) Update(obj interface{}) error { return nil } func (f fakeOrderedLister) Delete(obj interface{}) error { return nil } func (f fakeOrderedLister) Clone() orderedLister { return f } -func (f fakeOrderedLister) ListPrefix(prefixKey, continueKey string, limit int) ([]interface{}, bool) { - return nil, false +func (f fakeOrderedLister) ListPrefix(prefixKey, continueKey string) []interface{} { + return nil } func (f fakeOrderedLister) Count(prefixKey, continueKey string) int { return 0 } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache.go index 835840ba7aa..7db92d7a37a 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache.go @@ -523,7 +523,7 @@ func (w *watchCache) WaitUntilFreshAndList(ctx context.Context, resourceVersion } } if store, ok := w.store.(orderedLister); ok { - result, _ := store.ListPrefix(key, "", 0) + result := store.ListPrefix(key, "") return listResp{ Items: result, ResourceVersion: w.resourceVersion, diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache_test.go index a6a59ed1873..623e2c7afd5 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache_test.go @@ -1318,7 +1318,7 @@ func TestCacheSnapshots(t *testing.T) { assert.False(t, found, "Expected store to not include rev 99") lister, found := store.snapshots.GetLessOrEqual(100) assert.True(t, found, "Expected store to not include rev 100") - elements, _ := lister.ListPrefix("", "", 0) + elements := lister.ListPrefix("", "") assert.Len(t, elements, 1) assert.Equal(t, makeTestPod("foo", 100), elements[0].(*storeElement).Object) @@ -1330,20 +1330,20 @@ func TestCacheSnapshots(t *testing.T) { t.Log("Test cache on rev 200") lister, found = store.snapshots.GetLessOrEqual(200) assert.True(t, found, "Expected store to still keep rev 200") - elements, _ = lister.ListPrefix("", "", 0) + elements = lister.ListPrefix("", "") assert.Len(t, elements, 1) assert.Equal(t, makeTestPod("foo", 200), elements[0].(*storeElement).Object) t.Log("Test cache on rev 300") lister, found = store.snapshots.GetLessOrEqual(300) assert.True(t, found, "Expected store to still keep rev 300") - elements, _ = lister.ListPrefix("", "", 0) + elements = lister.ListPrefix("", "") assert.Empty(t, elements) t.Log("Test cache on rev 400") lister, found = store.snapshots.GetLessOrEqual(400) assert.True(t, found, "Expected store to still keep rev 400") - elements, _ = lister.ListPrefix("", "", 0) + elements = lister.ListPrefix("", "") assert.Len(t, elements, 1) assert.Equal(t, makeTestPod("foo", 400), elements[0].(*storeElement).Object) @@ -1359,7 +1359,7 @@ func TestCacheSnapshots(t *testing.T) { t.Log("Test cache on rev 500") lister, found = store.snapshots.GetLessOrEqual(500) assert.True(t, found, "Expected store to still keep rev 500") - elements, _ = lister.ListPrefix("", "", 0) + elements = lister.ListPrefix("", "") assert.Len(t, elements, 1) assert.Equal(t, makeTestPod("foo", 500), elements[0].(*storeElement).Object) @@ -1371,7 +1371,7 @@ func TestCacheSnapshots(t *testing.T) { t.Log("Test cache on rev 600") lister, found = store.snapshots.GetLessOrEqual(600) assert.True(t, found, "Expected replace to be snapshotted") - elements, _ = lister.ListPrefix("", "", 0) + elements = lister.ListPrefix("", "") assert.Len(t, elements, 1) assert.Equal(t, makeTestPod("foo", 600), elements[0].(*storeElement).Object) @@ -1388,7 +1388,7 @@ func TestCacheSnapshots(t *testing.T) { t.Log("Test cache on rev 700") lister, found = store.snapshots.GetLessOrEqual(700) assert.True(t, found, "Expected replace to be snapshotted") - elements, _ = lister.ListPrefix("", "", 0) + elements = lister.ListPrefix("", "") assert.Len(t, elements, 1) assert.Equal(t, makeTestPod("foo", 600), elements[0].(*storeElement).Object) }