Przeglądaj źródła

Merge pull request #3368 from yichengq/storage-test

add unit tests for storage
Yicheng Qin 10 lat temu
rodzic
commit
59a5a7e309

+ 1 - 2
storage/index.go

@@ -115,8 +115,7 @@ func (ti *treeIndex) Tombstone(key []byte, rev revision) error {
 	}
 
 	ki := item.(*keyIndex)
-	ki.tombstone(rev.main, rev.sub)
-	return nil
+	return ki.tombstone(rev.main, rev.sub)
 }
 
 func (ti *treeIndex) Compact(rev int64) map[revision]struct{} {

+ 167 - 136
storage/index_test.go

@@ -5,35 +5,55 @@ import (
 	"testing"
 )
 
-func TestIndexPutAndGet(t *testing.T) {
-	index := newTestTreeIndex()
-
-	tests := []T{
-		{[]byte("foo"), 0, ErrRevisionNotFound, 0},
-		{[]byte("foo"), 1, nil, 1},
-		{[]byte("foo"), 3, nil, 1},
-		{[]byte("foo"), 5, nil, 5},
-		{[]byte("foo"), 6, nil, 5},
-
-		{[]byte("foo1"), 0, ErrRevisionNotFound, 0},
-		{[]byte("foo1"), 1, ErrRevisionNotFound, 0},
-		{[]byte("foo1"), 2, nil, 2},
-		{[]byte("foo1"), 5, nil, 2},
-		{[]byte("foo1"), 6, nil, 6},
-
-		{[]byte("foo2"), 0, ErrRevisionNotFound, 0},
-		{[]byte("foo2"), 1, ErrRevisionNotFound, 0},
-		{[]byte("foo2"), 3, nil, 3},
-		{[]byte("foo2"), 4, nil, 4},
-		{[]byte("foo2"), 6, nil, 4},
+func TestIndexGet(t *testing.T) {
+	index := newTreeIndex()
+	index.Put([]byte("foo"), revision{main: 2})
+	index.Put([]byte("foo"), revision{main: 4})
+	index.Tombstone([]byte("foo"), revision{main: 6})
+
+	tests := []struct {
+		rev int64
+
+		wrev     revision
+		wcreated revision
+		wver     int64
+		werr     error
+	}{
+		{0, revision{}, revision{}, 0, ErrRevisionNotFound},
+		{1, revision{}, revision{}, 0, ErrRevisionNotFound},
+		{2, revision{main: 2}, revision{main: 2}, 1, nil},
+		{3, revision{main: 2}, revision{main: 2}, 1, nil},
+		{4, revision{main: 4}, revision{main: 2}, 2, nil},
+		{5, revision{main: 4}, revision{main: 2}, 2, nil},
+		{6, revision{}, revision{}, 0, ErrRevisionNotFound},
+	}
+	for i, tt := range tests {
+		rev, created, ver, err := index.Get([]byte("foo"), tt.rev)
+		if err != tt.werr {
+			t.Errorf("#%d: err = %v, want %v", i, err, tt.werr)
+		}
+		if rev != tt.wrev {
+			t.Errorf("#%d: rev = %+v, want %+v", i, rev, tt.wrev)
+		}
+		if created != tt.wcreated {
+			t.Errorf("#%d: created = %+v, want %+v", i, created, tt.wcreated)
+		}
+		if ver != tt.wver {
+			t.Errorf("#%d: ver = %d, want %d", i, ver, tt.wver)
+		}
 	}
-	verify(t, index, tests)
 }
 
 func TestIndexRange(t *testing.T) {
-	atRev := int64(3)
 	allKeys := [][]byte{[]byte("foo"), []byte("foo1"), []byte("foo2")}
 	allRevs := []revision{{main: 1}, {main: 2}, {main: 3}}
+
+	index := newTreeIndex()
+	for i := range allKeys {
+		index.Put(allKeys[i], allRevs[i])
+	}
+
+	atRev := int64(3)
 	tests := []struct {
 		key, end []byte
 		wkeys    [][]byte
@@ -73,7 +93,6 @@ func TestIndexRange(t *testing.T) {
 		},
 	}
 	for i, tt := range tests {
-		index := newTestTreeIndex()
 		keys, revs := index.Range(tt.key, tt.end, atRev)
 		if !reflect.DeepEqual(keys, tt.wkeys) {
 			t.Errorf("#%d: keys = %+v, want %+v", i, keys, tt.wkeys)
@@ -85,139 +104,151 @@ func TestIndexRange(t *testing.T) {
 }
 
 func TestIndexTombstone(t *testing.T) {
-	index := newTestTreeIndex()
+	index := newTreeIndex()
+	index.Put([]byte("foo"), revision{main: 1})
 
-	err := index.Tombstone([]byte("foo"), revision{main: 7})
+	err := index.Tombstone([]byte("foo"), revision{main: 2})
 	if err != nil {
 		t.Errorf("tombstone error = %v, want nil", err)
 	}
-	_, _, _, err = index.Get([]byte("foo"), 7)
+
+	_, _, _, err = index.Get([]byte("foo"), 2)
 	if err != ErrRevisionNotFound {
 		t.Errorf("get error = %v, want nil", err)
 	}
-}
-
-func TestContinuousCompact(t *testing.T) {
-	index := newTestTreeIndex()
-
-	tests := []T{
-		{[]byte("foo"), 0, ErrRevisionNotFound, 0},
-		{[]byte("foo"), 1, nil, 1},
-		{[]byte("foo"), 3, nil, 1},
-		{[]byte("foo"), 5, nil, 5},
-		{[]byte("foo"), 6, nil, 5},
-
-		{[]byte("foo1"), 0, ErrRevisionNotFound, 0},
-		{[]byte("foo1"), 1, ErrRevisionNotFound, 0},
-		{[]byte("foo1"), 2, nil, 2},
-		{[]byte("foo1"), 5, nil, 2},
-		{[]byte("foo1"), 6, nil, 6},
-
-		{[]byte("foo2"), 0, ErrRevisionNotFound, 0},
-		{[]byte("foo2"), 1, ErrRevisionNotFound, 0},
-		{[]byte("foo2"), 3, nil, 3},
-		{[]byte("foo2"), 4, nil, 4},
-		{[]byte("foo2"), 6, nil, 4},
-	}
-	wa := map[revision]struct{}{
-		revision{main: 1}: {},
-	}
-	ga := index.Compact(1)
-	if !reflect.DeepEqual(ga, wa) {
-		t.Errorf("a = %v, want %v", ga, wa)
+	err = index.Tombstone([]byte("foo"), revision{main: 3})
+	if err != ErrRevisionNotFound {
+		t.Errorf("tombstone error = %v, want %v", err, ErrRevisionNotFound)
 	}
-	verify(t, index, tests)
+}
 
-	wa = map[revision]struct{}{
-		revision{main: 1}: {},
-		revision{main: 2}: {},
+func TestIndexCompact(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{10, 0}, revision{10, 0}, 1},
+		{[]byte("foo1"), false, revision{10, 1}, revision{10, 1}, 1},
+	}
+
+	// Continuous Compact
+	index := newTreeIndex()
+	for _, tt := range tests {
+		if tt.remove {
+			index.Tombstone(tt.key, tt.rev)
+		} else {
+			index.Put(tt.key, tt.rev)
+		}
 	}
-	ga = index.Compact(2)
-	if !reflect.DeepEqual(ga, wa) {
-		t.Errorf("a = %v, want %v", ga, wa)
+	for i := int64(1); i < maxRev; i++ {
+		am := index.Compact(i)
+
+		windex := newTreeIndex()
+		for _, tt := range tests {
+			if _, ok := am[tt.rev]; ok || tt.rev.GreaterThan(revision{main: i}) {
+				if tt.remove {
+					windex.Tombstone(tt.key, tt.rev)
+				} else {
+					windex.Restore(tt.key, tt.created, tt.rev, tt.ver)
+				}
+			}
+		}
+		if !index.Equal(windex) {
+			t.Errorf("#%d: not equal index", i)
+		}
 	}
-	verify(t, index, tests)
 
-	wa = map[revision]struct{}{
-		revision{main: 1}: {},
-		revision{main: 2}: {},
-		revision{main: 3}: {},
-	}
-	ga = index.Compact(3)
-	if !reflect.DeepEqual(ga, wa) {
-		t.Errorf("a = %v, want %v", ga, wa)
+	// Once Compact
+	for i := int64(1); i < maxRev; i++ {
+		index := newTreeIndex()
+		for _, tt := range tests {
+			if tt.remove {
+				index.Tombstone(tt.key, tt.rev)
+			} else {
+				index.Put(tt.key, tt.rev)
+			}
+		}
+		am := index.Compact(i)
+
+		windex := newTreeIndex()
+		for _, tt := range tests {
+			if _, ok := am[tt.rev]; ok || tt.rev.GreaterThan(revision{main: i}) {
+				if tt.remove {
+					windex.Tombstone(tt.key, tt.rev)
+				} else {
+					windex.Restore(tt.key, tt.created, tt.rev, tt.ver)
+				}
+			}
+		}
+		if !index.Equal(windex) {
+			t.Errorf("#%d: not equal index", i)
+		}
 	}
-	verify(t, index, tests)
+}
 
-	wa = map[revision]struct{}{
-		revision{main: 1}: {},
-		revision{main: 2}: {},
-		revision{main: 4}: {},
-	}
-	ga = index.Compact(4)
-	delete(wa, revision{main: 3})
-	tests[12] = T{[]byte("foo2"), 3, ErrRevisionNotFound, 0}
-	if !reflect.DeepEqual(wa, ga) {
-		t.Errorf("a = %v, want %v", ga, wa)
-	}
-	verify(t, index, tests)
+func TestIndexRestore(t *testing.T) {
+	key := []byte("foo")
 
-	wa = map[revision]struct{}{
-		revision{main: 2}: {},
-		revision{main: 4}: {},
-		revision{main: 5}: {},
-	}
-	ga = index.Compact(5)
-	delete(wa, revision{main: 1})
-	if !reflect.DeepEqual(ga, wa) {
-		t.Errorf("a = %v, want %v", ga, wa)
-	}
-	tests[1] = T{[]byte("foo"), 1, ErrRevisionNotFound, 0}
-	tests[2] = T{[]byte("foo"), 3, ErrRevisionNotFound, 0}
-	verify(t, index, tests)
-
-	wa = map[revision]struct{}{
-		revision{main: 4}: {},
-		revision{main: 5}: {},
-		revision{main: 6}: {},
-	}
-	ga = index.Compact(6)
-	delete(wa, revision{main: 2})
-	if !reflect.DeepEqual(ga, wa) {
-		t.Errorf("a = %v, want %v", ga, wa)
+	tests := []struct {
+		created  revision
+		modified revision
+		ver      int64
+	}{
+		{revision{1, 0}, revision{1, 0}, 1},
+		{revision{1, 0}, revision{1, 1}, 2},
+		{revision{1, 0}, revision{2, 0}, 3},
 	}
-	tests[7] = T{[]byte("foo1"), 2, ErrRevisionNotFound, 0}
-	tests[8] = T{[]byte("foo1"), 5, ErrRevisionNotFound, 0}
-	verify(t, index, tests)
-}
 
-func verify(t *testing.T, index index, tests []T) {
+	// Continuous Restore
+	index := newTreeIndex()
 	for i, tt := range tests {
-		h, _, _, err := index.Get(tt.key, tt.rev)
-		if err != tt.werr {
-			t.Errorf("#%d: err = %v, want %v", i, err, tt.werr)
+		index.Restore(key, tt.created, tt.modified, tt.ver)
+
+		modified, created, ver, err := index.Get(key, tt.modified.main)
+		if modified != tt.modified {
+			t.Errorf("#%d: modified = %v, want %v", i, modified, tt.modified)
+		}
+		if created != tt.created {
+			t.Errorf("#%d: created = %v, want %v", i, created, tt.created)
 		}
-		if h.main != tt.wrev {
-			t.Errorf("#%d: rev = %d, want %d", i, h.main, tt.wrev)
+		if ver != tt.ver {
+			t.Errorf("#%d: ver = %d, want %d", i, ver, tt.ver)
+		}
+		if err != nil {
+			t.Errorf("#%d: err = %v, want nil", i, err)
 		}
 	}
-}
 
-type T struct {
-	key []byte
-	rev int64
-
-	werr error
-	wrev int64
-}
+	// Once Restore
+	for i, tt := range tests {
+		index := newTreeIndex()
+		index.Restore(key, tt.created, tt.modified, tt.ver)
 
-func newTestTreeIndex() index {
-	index := newTreeIndex()
-	index.Put([]byte("foo"), revision{main: 1})
-	index.Put([]byte("foo1"), revision{main: 2})
-	index.Put([]byte("foo2"), revision{main: 3})
-	index.Put([]byte("foo2"), revision{main: 4})
-	index.Put([]byte("foo"), revision{main: 5})
-	index.Put([]byte("foo1"), revision{main: 6})
-	return index
+		modified, created, ver, err := index.Get(key, tt.modified.main)
+		if modified != tt.modified {
+			t.Errorf("#%d: modified = %v, want %v", i, modified, tt.modified)
+		}
+		if created != tt.created {
+			t.Errorf("#%d: created = %v, want %v", i, created, tt.created)
+		}
+		if ver != tt.ver {
+			t.Errorf("#%d: ver = %d, want %d", i, ver, tt.ver)
+		}
+		if err != nil {
+			t.Errorf("#%d: err = %v, want nil", i, err)
+		}
+	}
 }

+ 7 - 2
storage/key_index.go

@@ -90,12 +90,17 @@ func (ki *keyIndex) restore(created, modified revision, ver int64) {
 
 // tombstone puts a revision, pointing to a tombstone, to the keyIndex.
 // It also creates a new empty generation in the keyIndex.
-func (ki *keyIndex) tombstone(main int64, sub int64) {
+// It returns ErrRevisionNotFound when tombstone on an empty generation.
+func (ki *keyIndex) tombstone(main int64, sub int64) error {
 	if ki.isEmpty() {
 		log.Panicf("store.keyindex: unexpected tombstone on empty keyIndex %s", string(ki.key))
 	}
+	if ki.generations[len(ki.generations)-1].isEmpty() {
+		return ErrRevisionNotFound
+	}
 	ki.put(main, sub)
 	ki.generations = append(ki.generations, generation{})
+	return nil
 }
 
 // get gets the modified, created revision and version of the key that satisfies the given atRev.
@@ -240,7 +245,7 @@ type generation struct {
 
 func (g *generation) isEmpty() bool { return g == nil || len(g.revs) == 0 }
 
-// walk walks through the revisions in the generation in ascending order.
+// walk walks through the revisions in the generation in descending order.
 // It passes the revision to the given function.
 // walk returns until: 1. it finishs walking all pairs 2. the function returns false.
 // walk returns the position at where it stopped. If it stopped after

+ 185 - 7
storage/key_index_test.go

@@ -22,20 +22,25 @@ func TestKeyIndexGet(t *testing.T) {
 		werr error
 	}{
 		{13, 0, ErrRevisionNotFound},
-		{13, 0, ErrRevisionNotFound},
+		{12, 0, ErrRevisionNotFound},
 
 		// get on generation 2
-		{12, 0, ErrRevisionNotFound},
 		{11, 10, nil},
 		{10, 10, nil},
 		{9, 8, nil},
 		{8, 8, nil},
+
 		{7, 0, ErrRevisionNotFound},
+		{6, 0, ErrRevisionNotFound},
 
 		// get on generation 1
-		{6, 0, ErrRevisionNotFound},
 		{5, 4, nil},
 		{4, 4, nil},
+
+		{3, 0, ErrRevisionNotFound},
+		{2, 0, ErrRevisionNotFound},
+		{1, 0, ErrRevisionNotFound},
+		{0, 0, ErrRevisionNotFound},
 	}
 
 	for i, tt := range tests {
@@ -74,11 +79,28 @@ func TestKeyIndexPut(t *testing.T) {
 	}
 }
 
+func TestKeyIndexRestore(t *testing.T) {
+	ki := &keyIndex{key: []byte("foo")}
+	ki.restore(revision{5, 0}, revision{7, 0}, 2)
+
+	wki := &keyIndex{
+		key:         []byte("foo"),
+		modified:    revision{7, 0},
+		generations: []generation{{created: revision{5, 0}, ver: 2, revs: []revision{{main: 7}}}},
+	}
+	if !reflect.DeepEqual(ki, wki) {
+		t.Errorf("ki = %+v, want %+v", ki, wki)
+	}
+}
+
 func TestKeyIndexTombstone(t *testing.T) {
 	ki := &keyIndex{key: []byte("foo")}
 	ki.put(5, 0)
 
-	ki.tombstone(7, 0)
+	err := ki.tombstone(7, 0)
+	if err != nil {
+		t.Errorf("unexpected tombstone error: %v", err)
+	}
 
 	wki := &keyIndex{
 		key:         []byte("foo"),
@@ -91,7 +113,10 @@ func TestKeyIndexTombstone(t *testing.T) {
 
 	ki.put(8, 0)
 	ki.put(9, 0)
-	ki.tombstone(15, 0)
+	err = ki.tombstone(15, 0)
+	if err != nil {
+		t.Errorf("unexpected tombstone error: %v", err)
+	}
 
 	wki = &keyIndex{
 		key:      []byte("foo"),
@@ -105,6 +130,11 @@ func TestKeyIndexTombstone(t *testing.T) {
 	if !reflect.DeepEqual(ki, wki) {
 		t.Errorf("ki = %+v, want %+v", ki, wki)
 	}
+
+	err = ki.tombstone(16, 0)
+	if err != ErrRevisionNotFound {
+		t.Errorf("tombstone error = %v, want %v", err, ErrRevisionNotFound)
+	}
 }
 
 func TestKeyIndexCompact(t *testing.T) {
@@ -291,8 +321,8 @@ func TestKeyIndexCompact(t *testing.T) {
 		}
 	}
 
-	ki = newTestKeyIndex()
 	// Jump Compaction
+	ki = newTestKeyIndex()
 	for i, tt := range tests {
 		if (i%2 == 0 && i < 6) || (i%2 == 1 && i > 6) {
 			am := make(map[revision]struct{})
@@ -306,7 +336,7 @@ func TestKeyIndexCompact(t *testing.T) {
 		}
 	}
 
-	// OnceCompaction
+	// Once Compaction
 	for i, tt := range tests {
 		ki := newTestKeyIndex()
 		am := make(map[revision]struct{})
@@ -320,6 +350,154 @@ func TestKeyIndexCompact(t *testing.T) {
 	}
 }
 
+// test that compact on version that higher than last modified version works well
+func TestKeyIndexCompactOnFurtherRev(t *testing.T) {
+	ki := &keyIndex{key: []byte("foo")}
+	ki.put(1, 0)
+	ki.put(2, 0)
+	am := make(map[revision]struct{})
+	ki.compact(3, am)
+
+	wki := &keyIndex{
+		key:      []byte("foo"),
+		modified: revision{2, 0},
+		generations: []generation{
+			{created: revision{1, 0}, ver: 2, revs: []revision{{main: 2}}},
+		},
+	}
+	wam := map[revision]struct{}{
+		revision{main: 2}: {},
+	}
+	if !reflect.DeepEqual(ki, wki) {
+		t.Errorf("ki = %+v, want %+v", ki, wki)
+	}
+	if !reflect.DeepEqual(am, wam) {
+		t.Errorf("am = %+v, want %+v", am, wam)
+	}
+}
+
+func TestKeyIndexIsEmpty(t *testing.T) {
+	tests := []struct {
+		ki *keyIndex
+		w  bool
+	}{
+		{
+			&keyIndex{
+				key:         []byte("foo"),
+				generations: []generation{{}},
+			},
+			true,
+		},
+		{
+			&keyIndex{
+				key:      []byte("foo"),
+				modified: revision{2, 0},
+				generations: []generation{
+					{created: revision{1, 0}, ver: 2, revs: []revision{{main: 2}}},
+				},
+			},
+			false,
+		},
+	}
+	for i, tt := range tests {
+		g := tt.ki.isEmpty()
+		if g != tt.w {
+			t.Errorf("#%d: isEmpty = %v, want %v", i, g, tt.w)
+		}
+	}
+}
+
+func TestKeyIndexFindGeneration(t *testing.T) {
+	ki := newTestKeyIndex()
+
+	tests := []struct {
+		rev int64
+		wg  *generation
+	}{
+		{0, nil},
+		{1, nil},
+		{2, &ki.generations[0]},
+		{3, &ki.generations[0]},
+		{4, &ki.generations[0]},
+		{5, &ki.generations[0]},
+		{6, nil},
+		{7, nil},
+		{8, &ki.generations[1]},
+		{9, &ki.generations[1]},
+		{10, &ki.generations[1]},
+		{11, &ki.generations[1]},
+		{12, nil},
+		{13, nil},
+	}
+	for i, tt := range tests {
+		g := ki.findGeneration(tt.rev)
+		if g != tt.wg {
+			t.Errorf("#%d: generation = %+v, want %+v", i, g, tt.wg)
+		}
+	}
+}
+
+func TestKeyIndexLess(t *testing.T) {
+	ki := &keyIndex{key: []byte("foo")}
+
+	tests := []struct {
+		ki *keyIndex
+		w  bool
+	}{
+		{&keyIndex{key: []byte("doo")}, false},
+		{&keyIndex{key: []byte("foo")}, false},
+		{&keyIndex{key: []byte("goo")}, true},
+	}
+	for i, tt := range tests {
+		g := ki.Less(tt.ki)
+		if g != tt.w {
+			t.Errorf("#%d: Less = %v, want %v", i, g, tt.w)
+		}
+	}
+}
+
+func TestGenerationIsEmpty(t *testing.T) {
+	tests := []struct {
+		g *generation
+		w bool
+	}{
+		{nil, true},
+		{&generation{}, true},
+		{&generation{revs: []revision{{main: 1}}}, false},
+	}
+	for i, tt := range tests {
+		g := tt.g.isEmpty()
+		if g != tt.w {
+			t.Errorf("#%d: isEmpty = %v, want %v", i, g, tt.w)
+		}
+	}
+}
+
+func TestGenerationWalk(t *testing.T) {
+	g := &generation{
+		ver:     3,
+		created: revision{2, 0},
+		revs:    []revision{{main: 2}, {main: 4}, {main: 6}},
+	}
+	tests := []struct {
+		f  func(rev revision) bool
+		wi int
+	}{
+		{func(rev revision) bool { return rev.main >= 7 }, 2},
+		{func(rev revision) bool { return rev.main >= 6 }, 1},
+		{func(rev revision) bool { return rev.main >= 5 }, 1},
+		{func(rev revision) bool { return rev.main >= 4 }, 0},
+		{func(rev revision) bool { return rev.main >= 3 }, 0},
+		{func(rev revision) bool { return rev.main >= 2 }, -1},
+	}
+	for i, tt := range tests {
+		idx := g.walk(tt.f)
+		if idx != tt.wi {
+			t.Errorf("#%d: index = %d, want %d", i, idx, tt.wi)
+		}
+	}
+}
+
 func newTestKeyIndex() *keyIndex {
 	// key:     "foo"
 	// rev: 12

+ 44 - 31
storage/kv_test.go

@@ -1,7 +1,10 @@
 package storage
 
 import (
+	"io/ioutil"
+	"log"
 	"os"
+	"path"
 	"reflect"
 	"testing"
 	"time"
@@ -56,14 +59,24 @@ var (
 		}
 		return n, rev
 	}
+
+	tmpPath string
 )
 
+func init() {
+	tmpDir, err := ioutil.TempDir(os.TempDir(), "etcd_test_storage")
+	if err != nil {
+		log.Fatal(err)
+	}
+	tmpPath = path.Join(tmpDir, "database")
+}
+
 func TestKVRange(t *testing.T)    { testKVRange(t, normalRangeFunc) }
 func TestKVTxnRange(t *testing.T) { testKVRange(t, txnRangeFunc) }
 
 func testKVRange(t *testing.T, f rangeFunc) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar"))
 	s.Put([]byte("foo1"), []byte("bar1"))
@@ -129,8 +142,8 @@ func TestKVRangeRev(t *testing.T)    { testKVRangeRev(t, normalRangeFunc) }
 func TestKVTxnRangeRev(t *testing.T) { testKVRangeRev(t, normalRangeFunc) }
 
 func testKVRangeRev(t *testing.T, f rangeFunc) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar"))
 	s.Put([]byte("foo1"), []byte("bar1"))
@@ -171,8 +184,8 @@ func TestKVRangeBadRev(t *testing.T)    { testKVRangeBadRev(t, normalRangeFunc)
 func TestKVTxnRangeBadRev(t *testing.T) { testKVRangeBadRev(t, normalRangeFunc) }
 
 func testKVRangeBadRev(t *testing.T, f rangeFunc) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar"))
 	s.Put([]byte("foo1"), []byte("bar1"))
@@ -203,8 +216,8 @@ func TestKVRangeLimit(t *testing.T)    { testKVRangeLimit(t, normalRangeFunc) }
 func TestKVTxnRangeLimit(t *testing.T) { testKVRangeLimit(t, txnRangeFunc) }
 
 func testKVRangeLimit(t *testing.T, f rangeFunc) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar"))
 	s.Put([]byte("foo1"), []byte("bar1"))
@@ -247,8 +260,8 @@ func TestKVPutMultipleTimes(t *testing.T)    { testKVPutMultipleTimes(t, normalP
 func TestKVTxnPutMultipleTimes(t *testing.T) { testKVPutMultipleTimes(t, txnPutFunc) }
 
 func testKVPutMultipleTimes(t *testing.T, f putFunc) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	for i := 0; i < 10; i++ {
 		base := int64(i + 1)
@@ -308,7 +321,7 @@ func testKVDeleteRange(t *testing.T, f deleteRangeFunc) {
 	}
 
 	for i, tt := range tests {
-		s := New("test")
+		s := New(tmpPath)
 
 		s.Put([]byte("foo"), []byte("bar"))
 		s.Put([]byte("foo1"), []byte("bar1"))
@@ -319,7 +332,7 @@ func testKVDeleteRange(t *testing.T, f deleteRangeFunc) {
 			t.Errorf("#%d: n = %d, rev = %d, want (%d, %d)", i, n, rev, tt.wN, tt.wrev)
 		}
 
-		cleanup(s, "test")
+		cleanup(s, tmpPath)
 	}
 }
 
@@ -327,8 +340,8 @@ func TestKVDeleteMultipleTimes(t *testing.T)    { testKVDeleteMultipleTimes(t, n
 func TestKVTxnDeleteMultipleTimes(t *testing.T) { testKVDeleteMultipleTimes(t, txnDeleteRangeFunc) }
 
 func testKVDeleteMultipleTimes(t *testing.T, f deleteRangeFunc) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar"))
 
@@ -347,8 +360,8 @@ func testKVDeleteMultipleTimes(t *testing.T, f deleteRangeFunc) {
 
 // test that range, put, delete on single key in sequence repeatedly works correctly.
 func TestKVOperationInSequence(t *testing.T) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	for i := 0; i < 10; i++ {
 		base := int64(i * 2)
@@ -393,8 +406,8 @@ func TestKVOperationInSequence(t *testing.T) {
 }
 
 func TestKVTxnBlockNonTnxOperations(t *testing.T) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	tests := []func(){
 		func() { s.Range([]byte("foo"), nil, 0, 0) },
@@ -424,8 +437,8 @@ func TestKVTxnBlockNonTnxOperations(t *testing.T) {
 }
 
 func TestKVTxnWrongID(t *testing.T) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	id := s.TxnBegin()
 	wrongid := id + 1
@@ -460,8 +473,8 @@ func TestKVTxnWrongID(t *testing.T) {
 
 // test that txn range, put, delete on single key in sequence repeatedly works correctly.
 func TestKVTnxOperationInSequence(t *testing.T) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	for i := 0; i < 10; i++ {
 		id := s.TxnBegin()
@@ -515,8 +528,8 @@ func TestKVTnxOperationInSequence(t *testing.T) {
 }
 
 func TestKVCompactReserveLastValue(t *testing.T) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar0"))
 	s.Put([]byte("foo"), []byte("bar1"))
@@ -568,8 +581,8 @@ func TestKVCompactReserveLastValue(t *testing.T) {
 }
 
 func TestKVCompactBad(t *testing.T) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar0"))
 	s.Put([]byte("foo"), []byte("bar1"))
@@ -614,7 +627,7 @@ func TestKVRestore(t *testing.T) {
 		},
 	}
 	for i, tt := range tests {
-		s := New("test")
+		s := New(tmpPath)
 		tt(s)
 		var kvss [][]storagepb.KeyValue
 		for k := int64(0); k < 10; k++ {
@@ -623,7 +636,7 @@ func TestKVRestore(t *testing.T) {
 		}
 		s.Close()
 
-		ns := New("test")
+		ns := New(tmpPath)
 		ns.Restore()
 		// wait for possible compaction to finish
 		testutil.WaitSchedule()
@@ -632,7 +645,7 @@ func TestKVRestore(t *testing.T) {
 			nkvs, _, _ := ns.Range([]byte("a"), []byte("z"), 0, k)
 			nkvss = append(nkvss, nkvs)
 		}
-		cleanup(ns, "test")
+		cleanup(ns, tmpPath)
 
 		if !reflect.DeepEqual(nkvss, kvss) {
 			t.Errorf("#%d: kvs history = %+v, want %+v", i, nkvss, kvss)
@@ -641,8 +654,8 @@ func TestKVRestore(t *testing.T) {
 }
 
 func TestKVSnapshot(t *testing.T) {
-	s := New("test")
-	defer cleanup(s, "test")
+	s := New(tmpPath)
+	defer cleanup(s, tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar"))
 	s.Put([]byte("foo1"), []byte("bar1"))

+ 79 - 0
storage/kvstore_compaction_test.go

@@ -0,0 +1,79 @@
+package storage
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestScheduleCompaction(t *testing.T) {
+	revs := []revision{{1, 0}, {2, 0}, {3, 0}}
+
+	tests := []struct {
+		rev   int64
+		keep  map[revision]struct{}
+		wrevs []revision
+	}{
+		// compact at 1 and discard all history
+		{
+			1,
+			nil,
+			revs[1:],
+		},
+		// compact at 3 and discard all history
+		{
+			3,
+			nil,
+			nil,
+		},
+		// compact at 1 and keeps history one step earlier
+		{
+			1,
+			map[revision]struct{}{
+				revision{main: 1}: {},
+			},
+			revs,
+		},
+		// compact at 1 and keeps history two steps earlier
+		{
+			3,
+			map[revision]struct{}{
+				revision{main: 2}: {},
+				revision{main: 3}: {},
+			},
+			revs[1:],
+		},
+	}
+	for i, tt := range tests {
+		s := newStore(tmpPath)
+		tx := s.b.BatchTx()
+
+		tx.Lock()
+		ibytes := newRevBytes()
+		for _, rev := range revs {
+			revToBytes(rev, ibytes)
+			tx.UnsafePut(keyBucketName, ibytes, []byte("bar"))
+		}
+		tx.Unlock()
+		// call `s.wg.Add(1)` to match the `s.wg.Done()` call in scheduleCompaction
+		// to avoid panic from wait group
+		s.wg.Add(1)
+		s.scheduleCompaction(tt.rev, tt.keep)
+
+		tx.Lock()
+		for _, rev := range tt.wrevs {
+			revToBytes(rev, ibytes)
+			keys, _ := tx.UnsafeRange(keyBucketName, ibytes, nil, 0)
+			if len(keys) != 1 {
+				t.Errorf("#%d: range on %v = %d, want 1", i, rev, len(keys))
+			}
+		}
+		_, vals := tx.UnsafeRange(metaBucketName, finishedCompactKeyName, nil, 0)
+		revToBytes(revision{main: tt.rev}, ibytes)
+		if w := [][]byte{ibytes}; !reflect.DeepEqual(vals, w) {
+			t.Errorf("#%d: vals on %v = %+v, want %+v", i, finishedCompactKeyName, vals, w)
+		}
+		tx.Unlock()
+
+		cleanup(s, tmpPath)
+	}
+}

+ 7 - 7
storage/kvstore_test.go

@@ -12,8 +12,8 @@ import (
 
 // TODO: improve to a unit test
 func TestRangeLimitWhenKeyDeleted(t *testing.T) {
-	s := newStore("test")
-	defer os.Remove("test")
+	s := newStore(tmpPath)
+	defer os.Remove(tmpPath)
 
 	s.Put([]byte("foo"), []byte("bar"))
 	s.Put([]byte("foo1"), []byte("bar1"))
@@ -46,8 +46,8 @@ func TestRangeLimitWhenKeyDeleted(t *testing.T) {
 }
 
 func TestRestoreContinueUnfinishedCompaction(t *testing.T) {
-	s0 := newStore("test")
-	defer os.Remove("test")
+	s0 := newStore(tmpPath)
+	defer os.Remove(tmpPath)
 
 	s0.Put([]byte("foo"), []byte("bar"))
 	s0.Put([]byte("foo"), []byte("bar1"))
@@ -63,7 +63,7 @@ func TestRestoreContinueUnfinishedCompaction(t *testing.T) {
 
 	s0.Close()
 
-	s1 := newStore("test")
+	s1 := newStore(tmpPath)
 	s1.Restore()
 
 	// wait for scheduled compaction to be finished
@@ -86,8 +86,8 @@ func TestRestoreContinueUnfinishedCompaction(t *testing.T) {
 }
 
 func BenchmarkStorePut(b *testing.B) {
-	s := newStore("test")
-	defer os.Remove("test")
+	s := newStore(tmpPath)
+	defer os.Remove(tmpPath)
 
 	// prepare keys
 	keys := make([][]byte, b.N)