Add a test for split+delete and dirty part tracking

yandex-cloud · Dec 19, 2023 · f010609 · f010609
1 parent c1972a4
commit f010609
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 4 deletions.
diff --git a/internal/buffer_list.go b/internal/buffer_list.go
@@ -258,7 +258,9 @@ func (l *BufferList) unqueue(b *FileBuffer) {
 			p := l.dirtyParts[i]
 			if p != nil {
 				p.refcnt--
-				if p.refcnt == 0 {
+				if p.refcnt < 0 {
+					panic("BUG: dirty buffer count of part < 0")
+				} else if p.refcnt == 0 {
 					l.dirtyQueue.Delete(p.queueId)
 					delete(l.dirtyParts, i)
 				}
@@ -285,7 +287,7 @@ func (l *BufferList) referenceDirtyPart(partNum uint64) {
 
 func (l *BufferList) queue(b *FileBuffer) {
 	if b.length == 0 {
-		panic("buffer length should never be 0")
+		panic("BUG: buffer length should never be 0")
 	}
 	if b.state == BUF_DIRTY {
 		l.dirtyCount++
@@ -304,7 +306,7 @@ func (l *BufferList) queue(b *FileBuffer) {
 
 func (l *BufferList) requeueSplit(left *FileBuffer) {
 	if left.length == 0 {
-		panic("buffer length should never be 0")
+		panic("BUG: buffer length should never be 0")
 	}
 	if left.state == BUF_DIRTY {
 		l.dirtyCount++
@@ -421,7 +423,7 @@ func (l *BufferList) insertOrAppend(offset uint64, data []byte, state BufferStat
 	})
 	if prev != nil && prev.offset+prev.length == end {
 		panic(fmt.Sprintf(
-			"Tried to insert %x+%x (s%v) but already have %x+%x (s%v)",
+			"BUG: Tried to insert %x+%x (s%v) but already have %x+%x (s%v)",
 			offset, len(data), state, prev.offset, prev.length, prev.state,
 		))
 	}

diff --git a/internal/buffer_list_test.go b/internal/buffer_list_test.go
@@ -65,3 +65,39 @@ func (s *BufferListTest) TestGetHoles(t *C) {
 	t.Assert(loading, Equals, false)
 	t.Assert(flcl, Equals, false)
 }
+
+// Test targets the requeueSplit() function
+func (s *BufferListTest) TestSplitDirtyQueue(t *C) {
+	l := BufferList{
+		helpers: &TestBLHelpers{},
+	}
+	zeroed, allocated := l.ZeroRange(0, 100*1024)
+	t.Assert(zeroed, Equals, true)
+	t.Assert(allocated, Equals, int64(0))
+	// 6*1024 and 12*1024 isn't part boundary, refcnt of part 2 and 3 = 2, 1 and others = 1
+	t.Assert(l.Add(0, make([]byte, 6*1024), BUF_DIRTY, false), Equals, int64(6*1024))
+	t.Assert(l.Add(6*1024, make([]byte, 6*1024), BUF_DIRTY, false), Equals, int64(6*1024))
+	data, ids, err := l.GetData(12*1024, (100-12)*1024, true)
+	t.Assert(err, IsNil)
+	t.Assert(len(data), Equals, 1)
+	t.Assert(len(data[0]), Equals, (100-12)*1024)
+	t.Assert(ids, DeepEquals, map[uint64]bool{
+		4: true,
+	})
+	l.SetState(12*1024, (100-12)*1024, ids, BUF_CLEAN)
+	data, ids, err = l.GetData(0, 12*1024, true)
+	t.Assert(err, IsNil)
+	t.Assert(len(data), Equals, 2)
+	t.Assert(len(data[0]), Equals, 6*1024)
+	t.Assert(len(data[1]), Equals, 6*1024)
+	t.Assert(ids, DeepEquals, map[uint64]bool{
+		3: true,
+		5: true,
+	})
+	l.SetState(0, 12*1024, ids, BUF_CLEAN)
+	// Now check dirty list - it should be empty
+	// With incorrect refcounting it would either be non-empty or the code would panic()
+	numDirty := 0
+	l.IterateDirtyParts(func(partNum uint64) bool { numDirty++; return true; })
+	t.Assert(numDirty, Equals, 0)
+}