Skip to content

Commit 168c338

Browse files
committed
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.
1 parent 3d9fcb7 commit 168c338

File tree

5 files changed

+21
-44
lines changed

5 files changed

+21
-44
lines changed

staging/src/k8s.io/apiserver/pkg/storage/cacher/store.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ type storeIndexer interface {
7373
}
7474

7575
type orderedLister interface {
76-
ListPrefix(prefix, continueKey string, limit int) (items []interface{}, hasMore bool)
76+
ListPrefix(prefix, continueKey string) []interface{}
7777
Count(prefix, continueKey string) (count int)
7878
Clone() orderedLister
7979
}

staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree.go

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package cacher
1818

1919
import (
2020
"fmt"
21-
"math"
2221
"strings"
2322
"sync"
2423

@@ -100,10 +99,10 @@ func (si *threadedStoreIndexer) List() []interface{} {
10099
return si.store.List()
101100
}
102101

103-
func (si *threadedStoreIndexer) ListPrefix(prefix, continueKey string, limit int) ([]interface{}, bool) {
102+
func (si *threadedStoreIndexer) ListPrefix(prefix, continueKey string) []interface{} {
104103
si.lock.RLock()
105104
defer si.lock.RUnlock()
106-
return si.store.ListPrefix(prefix, continueKey, limit)
105+
return si.store.ListPrefix(prefix, continueKey)
107106
}
108107

109108
func (si *threadedStoreIndexer) ListKeys() []string {
@@ -254,31 +253,19 @@ func (s *btreeStore) getByKey(key string) (item interface{}, exists bool, err er
254253
return item, exists, nil
255254
}
256255

257-
func (s *btreeStore) ListPrefix(prefix, continueKey string, limit int) ([]interface{}, bool) {
258-
if limit < 0 {
259-
return nil, false
260-
}
256+
func (s *btreeStore) ListPrefix(prefix, continueKey string) []interface{} {
261257
if continueKey == "" {
262258
continueKey = prefix
263259
}
264260
var result []interface{}
265-
var hasMore bool
266-
if limit == 0 {
267-
limit = math.MaxInt
268-
}
269261
s.tree.AscendGreaterOrEqual(&storeElement{Key: continueKey}, func(item *storeElement) bool {
270262
if !strings.HasPrefix(item.Key, prefix) {
271263
return false
272264
}
273-
// TODO: Might be worth to lookup one more item to provide more accurate HasMore.
274-
if len(result) >= limit {
275-
hasMore = true
276-
return false
277-
}
278265
result = append(result, item)
279266
return true
280267
})
281-
return result, hasMore
268+
return result
282269
}
283270

284271
func (s *btreeStore) Count(prefix, continueKey string) (count int) {

staging/src/k8s.io/apiserver/pkg/storage/cacher/store_btree_test.go

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,40 +41,30 @@ func TestStoreListPrefix(t *testing.T) {
4141
assert.NoError(t, store.Add(testStorageElement("foo2", "bar1", 3)))
4242
assert.NoError(t, store.Add(testStorageElement("bar", "baz", 4)))
4343

44-
items, hasMore := store.ListPrefix("foo", "", 0)
45-
assert.False(t, hasMore)
44+
items := store.ListPrefix("foo", "")
4645
assert.Equal(t, []interface{}{
4746
testStorageElement("foo1", "bar2", 2),
4847
testStorageElement("foo2", "bar1", 3),
4948
testStorageElement("foo3", "bar3", 1),
5049
}, items)
5150

52-
items, hasMore = store.ListPrefix("foo2", "", 0)
53-
assert.False(t, hasMore)
51+
items = store.ListPrefix("foo2", "")
5452
assert.Equal(t, []interface{}{
5553
testStorageElement("foo2", "bar1", 3),
5654
}, items)
5755

58-
items, hasMore = store.ListPrefix("foo", "", 1)
59-
assert.True(t, hasMore)
60-
assert.Equal(t, []interface{}{
61-
testStorageElement("foo1", "bar2", 2),
62-
}, items)
63-
64-
items, hasMore = store.ListPrefix("foo", "foo1\x00", 1)
65-
assert.True(t, hasMore)
56+
items = store.ListPrefix("foo", "foo1\x00")
6657
assert.Equal(t, []interface{}{
6758
testStorageElement("foo2", "bar1", 3),
59+
testStorageElement("foo3", "bar3", 1),
6860
}, items)
6961

70-
items, hasMore = store.ListPrefix("foo", "foo2\x00", 1)
71-
assert.False(t, hasMore)
62+
items = store.ListPrefix("foo", "foo2\x00")
7263
assert.Equal(t, []interface{}{
7364
testStorageElement("foo3", "bar3", 1),
7465
}, items)
7566

76-
items, hasMore = store.ListPrefix("bar", "", 0)
77-
assert.False(t, hasMore)
67+
items = store.ListPrefix("bar", "")
7868
assert.Equal(t, []interface{}{
7969
testStorageElement("bar", "baz", 4),
8070
}, items)
@@ -143,7 +133,7 @@ func (f fakeOrderedLister) Add(obj interface{}) error { return nil }
143133
func (f fakeOrderedLister) Update(obj interface{}) error { return nil }
144134
func (f fakeOrderedLister) Delete(obj interface{}) error { return nil }
145135
func (f fakeOrderedLister) Clone() orderedLister { return f }
146-
func (f fakeOrderedLister) ListPrefix(prefixKey, continueKey string, limit int) ([]interface{}, bool) {
147-
return nil, false
136+
func (f fakeOrderedLister) ListPrefix(prefixKey, continueKey string) []interface{} {
137+
return nil
148138
}
149139
func (f fakeOrderedLister) Count(prefixKey, continueKey string) int { return 0 }

staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ func (w *watchCache) WaitUntilFreshAndList(ctx context.Context, resourceVersion
523523
}
524524
}
525525
if store, ok := w.store.(orderedLister); ok {
526-
result, _ := store.ListPrefix(key, "", 0)
526+
result := store.ListPrefix(key, "")
527527
return listResp{
528528
Items: result,
529529
ResourceVersion: w.resourceVersion,

staging/src/k8s.io/apiserver/pkg/storage/cacher/watch_cache_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,7 +1318,7 @@ func TestCacheSnapshots(t *testing.T) {
13181318
assert.False(t, found, "Expected store to not include rev 99")
13191319
lister, found := store.snapshots.GetLessOrEqual(100)
13201320
assert.True(t, found, "Expected store to not include rev 100")
1321-
elements, _ := lister.ListPrefix("", "", 0)
1321+
elements := lister.ListPrefix("", "")
13221322
assert.Len(t, elements, 1)
13231323
assert.Equal(t, makeTestPod("foo", 100), elements[0].(*storeElement).Object)
13241324

@@ -1330,20 +1330,20 @@ func TestCacheSnapshots(t *testing.T) {
13301330
t.Log("Test cache on rev 200")
13311331
lister, found = store.snapshots.GetLessOrEqual(200)
13321332
assert.True(t, found, "Expected store to still keep rev 200")
1333-
elements, _ = lister.ListPrefix("", "", 0)
1333+
elements = lister.ListPrefix("", "")
13341334
assert.Len(t, elements, 1)
13351335
assert.Equal(t, makeTestPod("foo", 200), elements[0].(*storeElement).Object)
13361336

13371337
t.Log("Test cache on rev 300")
13381338
lister, found = store.snapshots.GetLessOrEqual(300)
13391339
assert.True(t, found, "Expected store to still keep rev 300")
1340-
elements, _ = lister.ListPrefix("", "", 0)
1340+
elements = lister.ListPrefix("", "")
13411341
assert.Empty(t, elements)
13421342

13431343
t.Log("Test cache on rev 400")
13441344
lister, found = store.snapshots.GetLessOrEqual(400)
13451345
assert.True(t, found, "Expected store to still keep rev 400")
1346-
elements, _ = lister.ListPrefix("", "", 0)
1346+
elements = lister.ListPrefix("", "")
13471347
assert.Len(t, elements, 1)
13481348
assert.Equal(t, makeTestPod("foo", 400), elements[0].(*storeElement).Object)
13491349

@@ -1359,7 +1359,7 @@ func TestCacheSnapshots(t *testing.T) {
13591359
t.Log("Test cache on rev 500")
13601360
lister, found = store.snapshots.GetLessOrEqual(500)
13611361
assert.True(t, found, "Expected store to still keep rev 500")
1362-
elements, _ = lister.ListPrefix("", "", 0)
1362+
elements = lister.ListPrefix("", "")
13631363
assert.Len(t, elements, 1)
13641364
assert.Equal(t, makeTestPod("foo", 500), elements[0].(*storeElement).Object)
13651365

@@ -1371,7 +1371,7 @@ func TestCacheSnapshots(t *testing.T) {
13711371
t.Log("Test cache on rev 600")
13721372
lister, found = store.snapshots.GetLessOrEqual(600)
13731373
assert.True(t, found, "Expected replace to be snapshotted")
1374-
elements, _ = lister.ListPrefix("", "", 0)
1374+
elements = lister.ListPrefix("", "")
13751375
assert.Len(t, elements, 1)
13761376
assert.Equal(t, makeTestPod("foo", 600), elements[0].(*storeElement).Object)
13771377

@@ -1388,7 +1388,7 @@ func TestCacheSnapshots(t *testing.T) {
13881388
t.Log("Test cache on rev 700")
13891389
lister, found = store.snapshots.GetLessOrEqual(700)
13901390
assert.True(t, found, "Expected replace to be snapshotted")
1391-
elements, _ = lister.ListPrefix("", "", 0)
1391+
elements = lister.ListPrefix("", "")
13921392
assert.Len(t, elements, 1)
13931393
assert.Equal(t, makeTestPod("foo", 600), elements[0].(*storeElement).Object)
13941394
}

0 commit comments

Comments
 (0)