Skip to content

Commit eb73b38

Browse files
sarvodaya-kumar-26sarvodaya-kumar-26
committed
fix: prevent spurious continue token when limit equals total event count
Request one extra record beyond the limit from etcd to reliably detect whether additional results exist. This avoids returning a continue token that leads to an empty result set when the number of records exactly matches the requested limit. Resolves sensu/sensu-enterprise-go#2609 Signed-off-by: Sarvodaya Kumar <sarvodaya.kumar.ctr@sumologic.com>
1 parent 43105d3 commit eb73b38

2 files changed

Lines changed: 100 additions & 6 deletions

File tree

‎backend/store/etcd/event_store.go‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,15 @@ func (s *Store) DeleteEventByEntityCheck(ctx context.Context, entityName, checkN
9696
// GetEvents returns the events for an (optional) namespace. If namespace is the
9797
// empty string, GetEvents returns all events for all namespaces.
9898
func (s *Store) GetEvents(ctx context.Context, pred *store.SelectionPredicate) ([]*corev2.Event, error) {
99+
// Request one extra record beyond the limit to reliably detect whether
100+
// more results exist, avoiding spurious continue tokens on the last page.
101+
queryLimit := pred.Limit
102+
if queryLimit > 0 {
103+
queryLimit++
104+
}
105+
99106
opts := []clientv3.OpOption{
100-
clientv3.WithLimit(pred.Limit),
107+
clientv3.WithLimit(queryLimit),
101108
}
102109

103110
keyPrefix := GetEventsPath(ctx, "")
@@ -126,8 +133,15 @@ func (s *Store) GetEvents(ctx context.Context, pred *store.SelectionPredicate) (
126133
return []*corev2.Event{}, nil
127134
}
128135

136+
// Determine if there are more results beyond the requested limit
137+
hasMore := pred.Limit > 0 && int64(len(resp.Kvs)) > pred.Limit
138+
kvs := resp.Kvs
139+
if hasMore {
140+
kvs = kvs[:pred.Limit]
141+
}
142+
129143
events := []*corev2.Event{}
130-
for _, kv := range resp.Kvs {
144+
for _, kv := range kvs {
131145
event := &corev2.Event{}
132146
if err := unmarshal(kv.Value, event); err != nil {
133147
return nil, &store.ErrDecode{Err: err}
@@ -143,7 +157,7 @@ func (s *Store) GetEvents(ctx context.Context, pred *store.SelectionPredicate) (
143157
events = append(events, event)
144158
}
145159

146-
if pred.Limit != 0 && resp.Count > pred.Limit {
160+
if hasMore {
147161
pred.Continue = ComputeContinueToken(ctx, events[len(events)-1])
148162
} else {
149163
pred.Continue = ""
@@ -158,8 +172,15 @@ func (s *Store) GetEventsByEntity(ctx context.Context, entityName string, pred *
158172
return nil, &store.ErrNotValid{Err: errors.New("must specify entity name")}
159173
}
160174

175+
// Request one extra record beyond the limit to reliably detect whether
176+
// more results exist, avoiding spurious continue tokens on the last page.
177+
queryLimit := pred.Limit
178+
if queryLimit > 0 {
179+
queryLimit++
180+
}
181+
161182
opts := []clientv3.OpOption{
162-
clientv3.WithLimit(pred.Limit),
183+
clientv3.WithLimit(queryLimit),
163184
}
164185

165186
keyPrefix := GetEventsPath(ctx, entityName)
@@ -179,8 +200,15 @@ func (s *Store) GetEventsByEntity(ctx context.Context, entityName string, pred *
179200
return nil, nil
180201
}
181202

203+
// Determine if there are more results beyond the requested limit
204+
hasMore := pred.Limit > 0 && int64(len(resp.Kvs)) > pred.Limit
205+
kvs := resp.Kvs
206+
if hasMore {
207+
kvs = kvs[:pred.Limit]
208+
}
209+
182210
events := []*corev2.Event{}
183-
for _, kv := range resp.Kvs {
211+
for _, kv := range kvs {
184212
event := &corev2.Event{}
185213
if err := unmarshal(kv.Value, event); err != nil {
186214
return nil, &store.ErrDecode{Err: err}
@@ -196,7 +224,7 @@ func (s *Store) GetEventsByEntity(ctx context.Context, entityName string, pred *
196224
events = append(events, event)
197225
}
198226

199-
if pred.Limit != 0 && resp.Count > pred.Limit {
227+
if hasMore {
200228
lastEvent := events[len(events)-1]
201229
pred.Continue = lastEvent.Check.Name + "\x00"
202230
} else {

‎backend/store/etcd/event_store_integration_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,72 @@ func testGetEventsByEntityPagination(t *testing.T, ctx context.Context, etcd sto
649649
}
650650
}
651651

652+
func TestGetEventsPaginationNoContinueTokenOnExactLimit(t *testing.T) {
653+
testWithEtcd(t, func(s store.Store) {
654+
ctx := context.WithValue(context.Background(), corev2.NamespaceKey, "default")
655+
656+
// Create exactly 5 events
657+
for i := 1; i <= 5; i++ {
658+
event := corev2.FixtureEvent(fmt.Sprintf("entity%.2d", i), fmt.Sprintf("check%.2d", i))
659+
_, _, err := s.UpdateEvent(ctx, event)
660+
require.NoError(t, err)
661+
}
662+
663+
// Query with limit equal to the total count — should NOT produce a continue token
664+
pred := &store.SelectionPredicate{Limit: 5}
665+
events, err := s.GetEvents(ctx, pred)
666+
require.NoError(t, err)
667+
assert.Equal(t, 5, len(events))
668+
assert.Empty(t, pred.Continue, "should not have a continue token when limit equals total count")
669+
670+
// Query with limit less than total — should produce a continue token
671+
pred = &store.SelectionPredicate{Limit: 3}
672+
events, err = s.GetEvents(ctx, pred)
673+
require.NoError(t, err)
674+
assert.Equal(t, 3, len(events))
675+
assert.NotEmpty(t, pred.Continue, "should have a continue token when more events exist")
676+
677+
// Follow the continue token — should get remaining 2 events with no further token
678+
events, err = s.GetEvents(ctx, pred)
679+
require.NoError(t, err)
680+
assert.Equal(t, 2, len(events))
681+
assert.Empty(t, pred.Continue, "should not have a continue token on the final page")
682+
})
683+
}
684+
685+
func TestGetEventsByEntityPaginationNoContinueTokenOnExactLimit(t *testing.T) {
686+
testWithEtcd(t, func(s store.Store) {
687+
ctx := context.WithValue(context.Background(), corev2.NamespaceKey, "default")
688+
689+
// Create exactly 5 checks for the same entity
690+
for i := 1; i <= 5; i++ {
691+
event := corev2.FixtureEvent("testentity", fmt.Sprintf("check%.2d", i))
692+
_, _, err := s.UpdateEvent(ctx, event)
693+
require.NoError(t, err)
694+
}
695+
696+
// Query with limit equal to the total count — should NOT produce a continue token
697+
pred := &store.SelectionPredicate{Limit: 5}
698+
events, err := s.GetEventsByEntity(ctx, "testentity", pred)
699+
require.NoError(t, err)
700+
assert.Equal(t, 5, len(events))
701+
assert.Empty(t, pred.Continue, "should not have a continue token when limit equals total count")
702+
703+
// Query with limit less than total — should produce a continue token
704+
pred = &store.SelectionPredicate{Limit: 3}
705+
events, err = s.GetEventsByEntity(ctx, "testentity", pred)
706+
require.NoError(t, err)
707+
assert.Equal(t, 3, len(events))
708+
assert.NotEmpty(t, pred.Continue, "should have a continue token when more events exist")
709+
710+
// Follow the continue token — should get remaining 2 events with no further token
711+
events, err = s.GetEventsByEntity(ctx, "testentity", pred)
712+
require.NoError(t, err)
713+
assert.Equal(t, 2, len(events))
714+
assert.Empty(t, pred.Continue, "should not have a continue token on the final page")
715+
})
716+
}
717+
652718
func TestHandleExpireOnResolveEntries(t *testing.T) {
653719
expireOnResolve := func(s *corev2.Silenced) *corev2.Silenced {
654720
s.ExpireOnResolve = true

0 commit comments

Comments
 (0)