From 3ea0462864cc337bdd7bd2dcd06cc56d01b6c204 Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Thu, 4 Jul 2024 14:35:51 +0800 Subject: [PATCH] *: keep tombstone if revision == compactAtRev Before this patch, the tombstone can be deleted if its revision is equal compacted revision. It causes that the watch subscriber won't get this DELETE event. Based on Compact API[1], we should keep tombstone revision if it's not less than the compaction revision. > CompactionRequest compacts the key-value store up to a given revision. > All superseded keys with a revision less than the compaction revision > will be removed. [1]: https://etcd.io/docs/latest/dev-guide/api_reference_v3/ Signed-off-by: Wei Fu --- server/storage/mvcc/hash.go | 13 + server/storage/mvcc/index_test.go | 505 +++++++++++++++++++++----- server/storage/mvcc/key_index.go | 17 +- server/storage/mvcc/key_index_test.go | 51 ++- 4 files changed, 489 insertions(+), 97 deletions(-) diff --git a/server/storage/mvcc/hash.go b/server/storage/mvcc/hash.go index 56416ba48478..6dff2ff9b336 100644 --- a/server/storage/mvcc/hash.go +++ b/server/storage/mvcc/hash.go @@ -63,6 +63,9 @@ func (h *kvHasher) WriteKeyValue(k, v []byte) { if !upper.GreaterThan(kr) { return } + + isTombstone := BytesToBucketKey(k).tombstone + lower := Revision{Main: h.compactRevision + 1} // skip revisions that are scheduled for deletion // due to compacting; don't skip if there isn't one. @@ -71,6 +74,16 @@ func (h *kvHasher) WriteKeyValue(k, v []byte) { return } } + + // NOTE: If the previous compacted revision was tombstone, existing + // releases already deleted that key(s). However, new releases won't + // delete the key(s) on compacted revision, no matter what type key it + // is. So, we should skip previous compacted revision to keep consistent + // with existing releases. + if kr.Main == h.compactRevision && isTombstone { + return + } + h.hash.Write(k) h.hash.Write(v) } diff --git a/server/storage/mvcc/index_test.go b/server/storage/mvcc/index_test.go index 7ac27c9608ed..95d7b3bd3d38 100644 --- a/server/storage/mvcc/index_test.go +++ b/server/storage/mvcc/index_test.go @@ -18,7 +18,7 @@ import ( "reflect" "testing" - "github.com/google/btree" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" ) @@ -235,101 +235,444 @@ func TestIndexRevision(t *testing.T) { func TestIndexCompactAndKeep(t *testing.T) { maxRev := int64(20) - tests := []struct { - key []byte - remove bool - rev Revision - created Revision - ver int64 - }{ - {[]byte("foo"), false, Revision{Main: 1}, Revision{Main: 1}, 1}, - {[]byte("foo1"), false, Revision{Main: 2}, Revision{Main: 2}, 1}, - {[]byte("foo2"), false, Revision{Main: 3}, Revision{Main: 3}, 1}, - {[]byte("foo2"), false, Revision{Main: 4}, Revision{Main: 3}, 2}, - {[]byte("foo"), false, Revision{Main: 5}, Revision{Main: 1}, 2}, - {[]byte("foo1"), false, Revision{Main: 6}, Revision{Main: 2}, 2}, - {[]byte("foo1"), true, Revision{Main: 7}, Revision{}, 0}, - {[]byte("foo2"), true, Revision{Main: 8}, Revision{}, 0}, - {[]byte("foo"), true, Revision{Main: 9}, Revision{}, 0}, - {[]byte("foo"), false, Revision{Main: 10}, Revision{Main: 10}, 1}, - {[]byte("foo1"), false, Revision{Main: 10, Sub: 1}, Revision{Main: 10, Sub: 1}, 1}, + + // key: "foo" + // modified: 10 + // generations: + // {{10, 0}} + // {{1, 0}, {5, 0}, {9, 0}(t)} + // + // key: "foo1" + // modified: 10, 1 + // generations: + // {{10, 1}} + // {{2, 0}, {6, 0}, {7, 0}(t)} + // + // key: "foo2" + // modified: 8 + // generations: + // {empty} + // {{3, 0}, {4, 0}, {8, 0}(t)} + // + buildTreeIndex := func() index { + ti := newTreeIndex(zaptest.NewLogger(t)) + + ti.Put([]byte("foo"), Revision{Main: 1}) + ti.Put([]byte("foo1"), Revision{Main: 2}) + ti.Put([]byte("foo2"), Revision{Main: 3}) + ti.Put([]byte("foo2"), Revision{Main: 4}) + ti.Put([]byte("foo"), Revision{Main: 5}) + ti.Put([]byte("foo1"), Revision{Main: 6}) + require.NoError(t, ti.Tombstone([]byte("foo1"), Revision{Main: 7})) + require.NoError(t, ti.Tombstone([]byte("foo2"), Revision{Main: 8})) + require.NoError(t, ti.Tombstone([]byte("foo"), Revision{Main: 9})) + ti.Put([]byte("foo"), Revision{Main: 10}) + ti.Put([]byte("foo1"), Revision{Main: 10, Sub: 1}) + return ti } - // Continuous Compact and Keep - ti := newTreeIndex(zaptest.NewLogger(t)) - for _, tt := range tests { - if tt.remove { - ti.Tombstone(tt.key, tt.rev) - } else { - ti.Put(tt.key, tt.rev) - } + afterCompacts := []struct { + atRev int + keyIndexes []keyIndex + keep map[Revision]struct{} + compacted map[Revision]struct{} + }{ + { + atRev: 1, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 1}, Revision{Main: 5}, Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 3, created: Revision{Main: 2}, revs: []Revision{Revision{Main: 2}, Revision{Main: 6}, Revision{Main: 7}}}, + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + { + key: []byte("foo2"), + modified: Revision{Main: 8}, + generations: []generation{ + {ver: 3, created: Revision{Main: 3}, revs: []Revision{Revision{Main: 3}, Revision{Main: 4}, Revision{Main: 8}}}, + {}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 1}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 1}: {}, + }, + }, + { + atRev: 2, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 1}, Revision{Main: 5}, Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 3, created: Revision{Main: 2}, revs: []Revision{Revision{Main: 2}, Revision{Main: 6}, Revision{Main: 7}}}, + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + { + key: []byte("foo2"), + modified: Revision{Main: 8}, + generations: []generation{ + {ver: 3, created: Revision{Main: 3}, revs: []Revision{Revision{Main: 3}, Revision{Main: 4}, Revision{Main: 8}}}, + {}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 1}: {}, + Revision{Main: 2}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 1}: {}, + Revision{Main: 2}: {}, + }, + }, + { + atRev: 3, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 1}, Revision{Main: 5}, Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 3, created: Revision{Main: 2}, revs: []Revision{Revision{Main: 2}, Revision{Main: 6}, Revision{Main: 7}}}, + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + { + key: []byte("foo2"), + modified: Revision{Main: 8}, + generations: []generation{ + {ver: 3, created: Revision{Main: 3}, revs: []Revision{Revision{Main: 3}, Revision{Main: 4}, Revision{Main: 8}}}, + {}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 1}: {}, + Revision{Main: 2}: {}, + Revision{Main: 3}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 1}: {}, + Revision{Main: 2}: {}, + Revision{Main: 3}: {}, + }, + }, + { + atRev: 4, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 1}, Revision{Main: 5}, Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 3, created: Revision{Main: 2}, revs: []Revision{Revision{Main: 2}, Revision{Main: 6}, Revision{Main: 7}}}, + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + { + key: []byte("foo2"), + modified: Revision{Main: 8}, + generations: []generation{ + {ver: 3, created: Revision{Main: 3}, revs: []Revision{Revision{Main: 4}, Revision{Main: 8}}}, + {}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 1}: {}, + Revision{Main: 2}: {}, + Revision{Main: 4}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 1}: {}, + Revision{Main: 2}: {}, + Revision{Main: 4}: {}, + }, + }, + { + atRev: 5, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 5}, Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 3, created: Revision{Main: 2}, revs: []Revision{Revision{Main: 2}, Revision{Main: 6}, Revision{Main: 7}}}, + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + { + key: []byte("foo2"), + modified: Revision{Main: 8}, + generations: []generation{ + {ver: 3, created: Revision{Main: 3}, revs: []Revision{Revision{Main: 4}, Revision{Main: 8}}}, + {}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 2}: {}, + Revision{Main: 4}: {}, + Revision{Main: 5}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 2}: {}, + Revision{Main: 4}: {}, + Revision{Main: 5}: {}, + }, + }, + { + atRev: 6, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 5}, Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 3, created: Revision{Main: 2}, revs: []Revision{Revision{Main: 6}, Revision{Main: 7}}}, + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + { + key: []byte("foo2"), + modified: Revision{Main: 8}, + generations: []generation{ + {ver: 3, created: Revision{Main: 3}, revs: []Revision{Revision{Main: 4}, Revision{Main: 8}}}, + {}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 6}: {}, + Revision{Main: 4}: {}, + Revision{Main: 5}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 6}: {}, + Revision{Main: 4}: {}, + Revision{Main: 5}: {}, + }, + }, + { + atRev: 7, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 5}, Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 3, created: Revision{Main: 2}, revs: []Revision{Revision{Main: 7}}}, + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + { + key: []byte("foo2"), + modified: Revision{Main: 8}, + generations: []generation{ + {ver: 3, created: Revision{Main: 3}, revs: []Revision{Revision{Main: 4}, Revision{Main: 8}}}, + {}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 4}: {}, + Revision{Main: 5}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 7}: {}, + Revision{Main: 4}: {}, + Revision{Main: 5}: {}, + }, + }, + { + atRev: 8, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 5}, Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + { + key: []byte("foo2"), + modified: Revision{Main: 8}, + generations: []generation{ + {ver: 3, created: Revision{Main: 3}, revs: []Revision{Revision{Main: 8}}}, + {}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 5}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 8}: {}, + Revision{Main: 5}: {}, + }, + }, + { + atRev: 9, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 3, created: Revision{Main: 1}, revs: []Revision{Revision{Main: 9}}}, + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + }, + keep: map[Revision]struct{}{}, + compacted: map[Revision]struct{}{ + Revision{Main: 9}: {}, + }, + }, + { + atRev: 10, + keyIndexes: []keyIndex{ + { + key: []byte("foo"), + modified: Revision{Main: 10}, + generations: []generation{ + {ver: 1, created: Revision{Main: 10}, revs: []Revision{Revision{Main: 10}}}, + }, + }, + { + key: []byte("foo1"), + modified: Revision{Main: 10, Sub: 1}, + generations: []generation{ + {ver: 1, created: Revision{Main: 10, Sub: 1}, revs: []Revision{Revision{Main: 10, Sub: 1}}}, + }, + }, + }, + keep: map[Revision]struct{}{ + Revision{Main: 10}: {}, + Revision{Main: 10, Sub: 1}: {}, + }, + compacted: map[Revision]struct{}{ + Revision{Main: 10}: {}, + Revision{Main: 10, Sub: 1}: {}, + }, + }, } + + ti := buildTreeIndex() for i := int64(1); i < maxRev; i++ { + j := i - 1 + if i >= int64(len(afterCompacts)) { + j = int64(len(afterCompacts)) - 1 + } + am := ti.Compact(i) + require.Equal(t, afterCompacts[j].compacted, am, "#%d: compact(%d) != expected", i, i) + keep := ti.Keep(i) - if !(reflect.DeepEqual(am, keep)) { - t.Errorf("#%d: compact keep %v != Keep keep %v", i, am, keep) - } - wti := &treeIndex{tree: btree.NewG(32, func(aki *keyIndex, bki *keyIndex) bool { - return aki.Less(bki) - })} - for _, tt := range tests { - if _, ok := am[tt.rev]; ok || tt.rev.GreaterThan(Revision{Main: i}) { - if tt.remove { - wti.Tombstone(tt.key, tt.rev) - } else { - restore(wti, tt.key, tt.created, tt.rev, tt.ver) - } - } - } - if !ti.Equal(wti) { - t.Errorf("#%d: not equal ti", i) + require.Equal(t, afterCompacts[j].keep, keep, "#%d: keep(%d) != expected", i, i) + + nti := newTreeIndex(zaptest.NewLogger(t)).(*treeIndex) + for k := range afterCompacts[j].keyIndexes { + ki := afterCompacts[j].keyIndexes[k] + nti.tree.ReplaceOrInsert(&ki) } + require.True(t, ti.Equal(nti), "#%d: not equal ti", i) } // Once Compact and Keep for i := int64(1); i < maxRev; i++ { - ti := newTreeIndex(zaptest.NewLogger(t)) - for _, tt := range tests { - if tt.remove { - ti.Tombstone(tt.key, tt.rev) - } else { - ti.Put(tt.key, tt.rev) - } + ti := buildTreeIndex() + + j := i - 1 + if i >= int64(len(afterCompacts)) { + j = int64(len(afterCompacts)) - 1 } + am := ti.Compact(i) + require.Equal(t, afterCompacts[j].compacted, am, "#%d: compact(%d) != expected", i, i) + keep := ti.Keep(i) - if !(reflect.DeepEqual(am, keep)) { - t.Errorf("#%d: compact keep %v != Keep keep %v", i, am, keep) - } - wti := &treeIndex{tree: btree.NewG(32, func(aki *keyIndex, bki *keyIndex) bool { - return aki.Less(bki) - })} - for _, tt := range tests { - if _, ok := am[tt.rev]; ok || tt.rev.GreaterThan(Revision{Main: i}) { - if tt.remove { - wti.Tombstone(tt.key, tt.rev) - } else { - restore(wti, tt.key, tt.created, tt.rev, tt.ver) - } - } - } - if !ti.Equal(wti) { - t.Errorf("#%d: not equal ti", i) - } - } -} + require.Equal(t, afterCompacts[j].keep, keep, "#%d: keep(%d) != expected", i, i) -func restore(ti *treeIndex, key []byte, created, modified Revision, ver int64) { - keyi := &keyIndex{key: key} + nti := newTreeIndex(zaptest.NewLogger(t)).(*treeIndex) + for k := range afterCompacts[j].keyIndexes { + ki := afterCompacts[j].keyIndexes[k] + nti.tree.ReplaceOrInsert(&ki) + } - ti.Lock() - defer ti.Unlock() - okeyi, _ := ti.tree.Get(keyi) - if okeyi == nil { - keyi.restore(ti.lg, created, modified, ver) - ti.tree.ReplaceOrInsert(keyi) - return + require.True(t, ti.Equal(nti), "#%d: not equal ti", i) } - okeyi.put(ti.lg, modified.Main, modified.Sub) } diff --git a/server/storage/mvcc/key_index.go b/server/storage/mvcc/key_index.go index 07ad930c18a1..c9ae23d51e83 100644 --- a/server/storage/mvcc/key_index.go +++ b/server/storage/mvcc/key_index.go @@ -65,7 +65,8 @@ var ( // compact(5): // generations: // -// {empty} -> key SHOULD be removed. +// {empty} +// {5.0(t)} // // compact(6): // generations: @@ -202,8 +203,7 @@ func (ki *keyIndex) since(lg *zap.Logger, rev int64) []Revision { } // compact compacts a keyIndex by removing the versions with smaller or equal -// revision than the given atRev except the largest one (If the largest one is -// a tombstone, it will not be kept). +// revision than the given atRev except the largest one. // If a generation becomes empty during compaction, it will be removed. func (ki *keyIndex) compact(lg *zap.Logger, atRev int64, available map[Revision]struct{}) { if ki.isEmpty() { @@ -221,11 +221,6 @@ func (ki *keyIndex) compact(lg *zap.Logger, atRev int64, available map[Revision] if revIndex != -1 { g.revs = g.revs[revIndex:] } - // remove any tombstone - if len(g.revs) == 1 && genIdx != len(ki.generations)-1 { - delete(available, g.revs[0]) - genIdx++ - } } // remove the previous generations. @@ -242,6 +237,10 @@ func (ki *keyIndex) keep(atRev int64, available map[Revision]struct{}) { g := &ki.generations[genIdx] if !g.isEmpty() { // remove any tombstone + // + // NOTE: It's different from compact function which keeps + // tombstone in available, because we need to keep consistent + // with existing releases. if revIndex == len(g.revs)-1 && genIdx != len(ki.generations)-1 { delete(available, g.revs[revIndex]) } @@ -262,7 +261,7 @@ func (ki *keyIndex) doCompact(atRev int64, available map[Revision]struct{}) (gen genIdx, g := 0, &ki.generations[0] // find first generation includes atRev or created after atRev for genIdx < len(ki.generations)-1 { - if tomb := g.revs[len(g.revs)-1].Main; tomb > atRev { + if tomb := g.revs[len(g.revs)-1].Main; tomb >= atRev { break } genIdx++ diff --git a/server/storage/mvcc/key_index_test.go b/server/storage/mvcc/key_index_test.go index 6a8adb91143c..82ba8c474771 100644 --- a/server/storage/mvcc/key_index_test.go +++ b/server/storage/mvcc/key_index_test.go @@ -18,6 +18,7 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" "go.uber.org/zap" "go.uber.org/zap/zaptest" ) @@ -308,12 +309,15 @@ func TestKeyIndexCompactAndKeep(t *testing.T) { key: []byte("foo"), modified: Revision{Main: 16}, generations: []generation{ + {created: Revision{Main: 2}, ver: 3, revs: []Revision{Revision{Main: 6}}}, {created: Revision{Main: 8}, ver: 3, revs: []Revision{Revision{Main: 8}, Revision{Main: 10}, Revision{Main: 12}}}, {created: Revision{Main: 14}, ver: 3, revs: []Revision{Revision{Main: 14}, Revision{Main: 15, Sub: 1}, Revision{Main: 16}}}, {}, }, }, - map[Revision]struct{}{}, + map[Revision]struct{}{ + Revision{Main: 6}: {}, + }, }, { 7, @@ -394,11 +398,14 @@ func TestKeyIndexCompactAndKeep(t *testing.T) { key: []byte("foo"), modified: Revision{Main: 16}, generations: []generation{ + {created: Revision{Main: 8}, ver: 3, revs: []Revision{Revision{Main: 12}}}, {created: Revision{Main: 14}, ver: 3, revs: []Revision{Revision{Main: 14}, Revision{Main: 15, Sub: 1}, Revision{Main: 16}}}, {}, }, }, - map[Revision]struct{}{}, + map[Revision]struct{}{ + Revision{Main: 12}: {}, + }, }, { 13, @@ -442,6 +449,20 @@ func TestKeyIndexCompactAndKeep(t *testing.T) { }, { 16, + &keyIndex{ + key: []byte("foo"), + modified: Revision{Main: 16}, + generations: []generation{ + {created: Revision{Main: 14}, ver: 3, revs: []Revision{Revision{Main: 16}}}, + {}, + }, + }, + map[Revision]struct{}{ + Revision{Main: 16}: {}, + }, + }, + { + 17, &keyIndex{ key: []byte("foo"), modified: Revision{Main: 16}, @@ -453,6 +474,12 @@ func TestKeyIndexCompactAndKeep(t *testing.T) { }, } + tombstones := map[int64]struct{}{ + 6: {}, + 12: {}, + 16: {}, + } + // Continuous Compaction and finding Keep ki := newTestKeyIndex(zaptest.NewLogger(t)) for i, tt := range tests { @@ -462,9 +489,14 @@ func TestKeyIndexCompactAndKeep(t *testing.T) { if !reflect.DeepEqual(ki, kiclone) { t.Errorf("#%d: ki = %+v, want %+v", i, ki, kiclone) } - if !reflect.DeepEqual(am, tt.wam) { - t.Errorf("#%d: am = %+v, want %+v", i, am, tt.wam) + + if _, ok := tombstones[tt.compact]; ok { + assert.Equal(t, len(am), 0, "#%d: ki = %d, keep result wants empty because tombstone", i, ki) + } else { + assert.Equal(t, tt.wam, am, + "#%d: ki = %d, compact keep should be equal to keep keep if it's not tombstone", i, ki) } + am = make(map[Revision]struct{}) ki.compact(zaptest.NewLogger(t), tt.compact, am) if !reflect.DeepEqual(ki, tt.wki) { @@ -478,7 +510,7 @@ func TestKeyIndexCompactAndKeep(t *testing.T) { // Jump Compaction and finding Keep ki = newTestKeyIndex(zaptest.NewLogger(t)) for i, tt := range tests { - if (i%2 == 0 && i < 6) || (i%2 == 1 && i > 6) { + if _, ok := tombstones[tt.compact]; !ok { am := make(map[Revision]struct{}) kiclone := cloneKeyIndex(ki) ki.keep(tt.compact, am) @@ -508,9 +540,14 @@ func TestKeyIndexCompactAndKeep(t *testing.T) { if !reflect.DeepEqual(ki, kiClone) { t.Errorf("#%d: ki = %+v, want %+v", i, ki, kiClone) } - if !reflect.DeepEqual(am, tt.wam) { - t.Errorf("#%d: am = %+v, want %+v", i, am, tt.wam) + + if _, ok := tombstones[tt.compact]; ok { + assert.Equal(t, len(am), 0, "#%d: ki = %d, keep result wants empty because tombstone", i, ki) + } else { + assert.Equal(t, tt.wam, am, + "#%d: ki = %d, compact keep should be equal to keep keep if it's not tombstone", i, ki) } + am = make(map[Revision]struct{}) ki.compact(zaptest.NewLogger(t), tt.compact, am) if !reflect.DeepEqual(ki, tt.wki) {