Skip to content

Commit

Permalink
CBG-3945: Fix for panic in _clip method on skipped sequence slice (#6842
Browse files Browse the repository at this point in the history
)
  • Loading branch information
gregns1 committed Jun 27, 2024
1 parent a77eb22 commit e2784db
Showing 2 changed files with 51 additions and 4 deletions.
5 changes: 3 additions & 2 deletions db/skipped_sequence.go
Original file line number Diff line number Diff line change
@@ -187,10 +187,11 @@ func (s *SkippedSequenceSlice) _clip(ctx context.Context) {
threshold := 2.5 * float64(len(s.list))

if cap(s.list) > int(threshold) {
newCap := len(s.list) + s.ClipCapacityHeadroom
// check if we can safely clip without an out of bound errors
if (len(s.list) + s.ClipCapacityHeadroom) < cap(s.list) {
if newCap < cap(s.list) {
base.DebugfCtx(ctx, base.KeyCache, "clipping skipped list capacity")
s.list = s.list[:len(s.list):s.ClipCapacityHeadroom]
s.list = s.list[:len(s.list):newCap]
}
}
}
50 changes: 48 additions & 2 deletions db/skipped_sequence_test.go
Original file line number Diff line number Diff line change
@@ -604,11 +604,11 @@ func TestCompactSkippedListClipHandling(t *testing.T) {
}{
{
name: "single_items",
expectedCap: 100,
expectedCap: 101,
},
{
name: "range_items",
expectedCap: 100,
expectedCap: 101,
},
}
for _, testCase := range testCases {
@@ -844,6 +844,52 @@ func TestRemoveSequenceRange(t *testing.T) {

}

// TestClipSafety:
// - Test case 1: (CBG-3945) calling clip on slice that should be clipped, asserting that the new capacity doesn't cause
// out of bounds error. CBG-3945 was hit due to clip attempting to clip capacity to a value below the
// current length of the slice
// - Test case 2: calling clip on a slice that meets the threshold to be clipped but shouldn't as it would cause out of
// bounds error (length of list + headroom > the current capacity)
// - Test case 3: calling clip on slice that doesn't meet clip threshold, assert that it doesn't get clipped
func TestClipSafety(t *testing.T) {
testCases := []struct {
name string
inputList []*SkippedSequenceListEntry
expectedLen int
expectedCap int
}{
{
name: "larger_length_than_headroom",
inputList: make([]*SkippedSequenceListEntry, 66291, 10000000),
expectedCap: 67291,
expectedLen: 66291,
},
{
name: "above_threshold_capacity_new_cap_out_of_bounds",
inputList: make([]*SkippedSequenceListEntry, 1, 100),
expectedCap: 100,
expectedLen: 1,
},
{
name: "threshold_not_met_for_clip",
inputList: make([]*SkippedSequenceListEntry, 100, 101),
expectedCap: 101,
expectedLen: 100,
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
skippedSlice := NewSkippedSequenceSlice(DefaultClipCapacityHeadroom)
ctx := base.TestCtx(t)
skippedSlice.list = testCase.inputList

skippedSlice._clip(ctx)
assert.Equal(t, testCase.expectedLen, len(skippedSlice.list))
assert.Equal(t, testCase.expectedCap, cap(skippedSlice.list))
})
}
}

func TestProcessUnusedSequenceRangeAtSkipped(t *testing.T) {
testCases := []struct {
name string

0 comments on commit e2784db

Please sign in to comment.