From aada2e7333600a95695050b9a71725e5cdaa4505 Mon Sep 17 00:00:00 2001 From: Patrick Mezard Date: Tue, 20 Oct 2015 19:05:53 +0200 Subject: [PATCH 1/4] store_test: test RangeIterator.Seek on goleveldb --- index/store/goleveldb/store_test.go | 6 ++ index/store/test/iterator.go | 97 +++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/index/store/goleveldb/store_test.go b/index/store/goleveldb/store_test.go index e643fd4c..6fceee78 100644 --- a/index/store/goleveldb/store_test.go +++ b/index/store/goleveldb/store_test.go @@ -75,6 +75,12 @@ func TestGoLevelDBRangeIterator(t *testing.T) { test.CommonTestRangeIterator(t, s) } +func TestGoLevelDBRangeIteratorSeek(t *testing.T) { + s := open(t, nil) + defer cleanup(t, s) + test.CommonTestRangeIteratorSeek(t, s) +} + func TestGoLevelDBMerge(t *testing.T) { s := open(t, &test.TestMergeCounter{}) defer cleanup(t, s) diff --git a/index/store/test/iterator.go b/index/store/test/iterator.go index ff5be9e7..aaf6149f 100644 --- a/index/store/test/iterator.go +++ b/index/store/test/iterator.go @@ -3,6 +3,7 @@ package test import ( "bytes" "reflect" + "strings" "testing" "github.com/blevesearch/bleve/index/store" @@ -275,3 +276,99 @@ func CommonTestRangeIterator(t *testing.T, s store.KVStore) { t.Fatal(err) } } + +func CommonTestRangeIteratorSeek(t *testing.T, s store.KVStore) { + + data := []struct { + key []byte + val []byte + }{ + {[]byte("a1"), []byte("val")}, + {[]byte("b1"), []byte("val")}, + {[]byte("c1"), []byte("val")}, + {[]byte("d1"), []byte("val")}, + {[]byte("e1"), []byte("val")}, + } + + // open a writer + writer, err := s.Writer() + if err != nil { + t.Fatal(err) + } + + // write the data + batch := writer.NewBatch() + for _, row := range data { + batch.Set(row.key, row.val) + } + err = writer.ExecuteBatch(batch) + if err != nil { + t.Fatal(err) + } + + // close the writer + err = writer.Close() + if err != nil { + t.Fatal(err) + } + + // open a reader + reader, err := s.Reader() + if err != nil { + t.Fatal(err) + } + + // get an iterator on a central subset of the data + start := []byte("b1") + end := []byte("d1") + iter := reader.RangeIterator(start, end) + + // seek before, at and after every possible key + targets := [][]byte{} + for _, row := range data { + prefix := string(row.key[:1]) + targets = append(targets, []byte(prefix+"0")) + targets = append(targets, []byte(prefix+"1")) + targets = append(targets, []byte(prefix+"2")) + } + for _, target := range targets { + found := []string{} + for iter.Seek(target); iter.Valid(); iter.Next() { + found = append(found, string(iter.Key())) + if len(found) > len(data) { + t.Fatalf("enumerated more than data keys after seeking to %s", + string(target)) + } + } + wanted := []string{} + for _, row := range data { + if bytes.Compare(row.key, start) < 0 || + bytes.Compare(row.key, target) < 0 || + bytes.Compare(row.key, end) >= 0 { + continue + } + wanted = append(wanted, string(row.key)) + } + fs := strings.Join(found, ", ") + ws := strings.Join(wanted, ", ") + if fs != ws { + t.Fatalf("iterating from %s returned [%s] instead of [%s]", + string(target), fs, ws) + } + } + + err = iter.Close() + if err != nil { + t.Fatal(err) + } + + if err != nil { + t.Fatal(err) + } + + // close the reader + err = reader.Close() + if err != nil { + t.Fatal(err) + } +} From 5d7628ba3b94d5d99d3315090ea4a41080c76ea8 Mon Sep 17 00:00:00 2001 From: Patrick Mezard Date: Tue, 20 Oct 2015 19:01:29 +0200 Subject: [PATCH 2/4] boltdb: fix RangeIterator outside of range seeks Two issues: - Seeking before i.start and iterating returned keys before i.start - Seeking after the store last key did not invalidate the iterator and could cause infinite loops. --- index/store/boltdb/iterator.go | 13 +++++++++---- index/store/boltdb/store_test.go | 6 ++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/index/store/boltdb/iterator.go b/index/store/boltdb/iterator.go index bccfeaee..63bde9f5 100644 --- a/index/store/boltdb/iterator.go +++ b/index/store/boltdb/iterator.go @@ -29,14 +29,19 @@ type Iterator struct { func (i *Iterator) updateValid() { i.valid = (i.key != nil) - if i.valid && i.prefix != nil { - i.valid = bytes.HasPrefix(i.key, i.prefix) - } else if i.end != nil { - i.valid = bytes.Compare(i.key, i.end) < 0 + if i.valid { + if i.prefix != nil { + i.valid = bytes.HasPrefix(i.key, i.prefix) + } else if i.end != nil { + i.valid = bytes.Compare(i.key, i.end) < 0 + } } } func (i *Iterator) Seek(k []byte) { + if bytes.Compare(k, i.start) < 0 { + k = i.start + } i.key, i.val = i.cursor.Seek(k) i.updateValid() } diff --git a/index/store/boltdb/store_test.go b/index/store/boltdb/store_test.go index 0380a92c..4abde11b 100644 --- a/index/store/boltdb/store_test.go +++ b/index/store/boltdb/store_test.go @@ -72,6 +72,12 @@ func TestBoltDBRangeIterator(t *testing.T) { test.CommonTestRangeIterator(t, s) } +func TestBoltDBRangeIteratorSeek(t *testing.T) { + s := open(t, nil) + defer cleanup(t, s) + test.CommonTestRangeIteratorSeek(t, s) +} + func TestBoltDBMerge(t *testing.T) { s := open(t, &test.TestMergeCounter{}) defer cleanup(t, s) From 873f4838041c2899c178703e92c50f438f7c9130 Mon Sep 17 00:00:00 2001 From: Patrick Mezard Date: Tue, 20 Oct 2015 18:44:00 +0200 Subject: [PATCH 3/4] gtreap: RangeIterator.Seek should not move before start --- index/store/gtreap/iterator.go | 4 ++++ index/store/gtreap/reader.go | 5 +++-- index/store/gtreap/store_test.go | 6 ++++++ index/store/metrics/store_test.go | 6 ++++++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/index/store/gtreap/iterator.go b/index/store/gtreap/iterator.go index d64dce55..bc56eb34 100644 --- a/index/store/gtreap/iterator.go +++ b/index/store/gtreap/iterator.go @@ -31,10 +31,14 @@ type Iterator struct { currOk bool prefix []byte + start []byte end []byte } func (w *Iterator) Seek(k []byte) { + if bytes.Compare(k, w.start) < 0 { + k = w.start + } w.restart(&Item{k: k}) } diff --git a/index/store/gtreap/reader.go b/index/store/gtreap/reader.go index a67671e9..6f92a751 100644 --- a/index/store/gtreap/reader.go +++ b/index/store/gtreap/reader.go @@ -46,8 +46,9 @@ func (w *Reader) PrefixIterator(k []byte) store.KVIterator { func (w *Reader) RangeIterator(start, end []byte) store.KVIterator { rv := Iterator{ - t: w.t, - end: end, + t: w.t, + start: start, + end: end, } rv.restart(&Item{k: start}) return &rv diff --git a/index/store/gtreap/store_test.go b/index/store/gtreap/store_test.go index 82e12cf6..b7686ee6 100644 --- a/index/store/gtreap/store_test.go +++ b/index/store/gtreap/store_test.go @@ -70,6 +70,12 @@ func TestGTreapRangeIterator(t *testing.T) { test.CommonTestRangeIterator(t, s) } +func TestGTreapRangeIteratorSeek(t *testing.T) { + s := open(t, nil) + defer cleanup(t, s) + test.CommonTestRangeIteratorSeek(t, s) +} + func TestGTreapMerge(t *testing.T) { s := open(t, &test.TestMergeCounter{}) defer cleanup(t, s) diff --git a/index/store/metrics/store_test.go b/index/store/metrics/store_test.go index d3c65f79..f51d6d61 100644 --- a/index/store/metrics/store_test.go +++ b/index/store/metrics/store_test.go @@ -59,6 +59,12 @@ func TestMetricsRangeIterator(t *testing.T) { test.CommonTestRangeIterator(t, s) } +func TestMetricsRangeIteratorSeek(t *testing.T) { + s := open(t, nil) + defer cleanup(t, s) + test.CommonTestRangeIteratorSeek(t, s) +} + func TestMetricsMerge(t *testing.T) { s := open(t, &test.TestMergeCounter{}) defer cleanup(t, s) From da72d0c2b9ac20bdd5e3784fe184d745edf40d3e Mon Sep 17 00:00:00 2001 From: Patrick Mezard Date: Tue, 20 Oct 2015 19:21:01 +0200 Subject: [PATCH 4/4] store_test: deduplicate store initialization --- index/store/test/iterator.go | 102 +++++++++++++---------------------- 1 file changed, 36 insertions(+), 66 deletions(-) diff --git a/index/store/test/iterator.go b/index/store/test/iterator.go index aaf6149f..0d7d8992 100644 --- a/index/store/test/iterator.go +++ b/index/store/test/iterator.go @@ -11,12 +11,39 @@ import ( // tests around the correct behavior of iterators +type testRow struct { + key []byte + val []byte +} + +func batchWriteRows(s store.KVStore, rows []testRow) error { + // open a writer + writer, err := s.Writer() + if err != nil { + return err + } + + // write the data + batch := writer.NewBatch() + for _, row := range rows { + batch.Set(row.key, row.val) + } + err = writer.ExecuteBatch(batch) + if err != nil { + return err + } + + // close the writer + err = writer.Close() + if err != nil { + return err + } + return nil +} + func CommonTestPrefixIterator(t *testing.T, s store.KVStore) { - data := []struct { - key []byte - val []byte - }{ + data := []testRow{ {[]byte("apple"), []byte("val")}, {[]byte("cat1"), []byte("val")}, {[]byte("cat2"), []byte("val")}, @@ -40,24 +67,7 @@ func CommonTestPrefixIterator(t *testing.T, s store.KVStore) { []byte("dog4"), } - // open a writer - writer, err := s.Writer() - if err != nil { - t.Fatal(err) - } - - // write the data - batch := writer.NewBatch() - for _, row := range data { - batch.Set(row.key, row.val) - } - err = writer.ExecuteBatch(batch) - if err != nil { - t.Fatal(err) - } - - // close the writer - err = writer.Close() + err := batchWriteRows(s, data) if err != nil { t.Fatal(err) } @@ -123,10 +133,7 @@ func CommonTestPrefixIterator(t *testing.T, s store.KVStore) { func CommonTestRangeIterator(t *testing.T, s store.KVStore) { - data := []struct { - key []byte - val []byte - }{ + data := []testRow{ {[]byte("a1"), []byte("val")}, {[]byte("b1"), []byte("val")}, {[]byte("b2"), []byte("val")}, @@ -154,24 +161,7 @@ func CommonTestRangeIterator(t *testing.T, s store.KVStore) { } } - // open a writer - writer, err := s.Writer() - if err != nil { - t.Fatal(err) - } - - // write the data - batch := writer.NewBatch() - for _, row := range data { - batch.Set(row.key, row.val) - } - err = writer.ExecuteBatch(batch) - if err != nil { - t.Fatal(err) - } - - // close the writer - err = writer.Close() + err := batchWriteRows(s, data) if err != nil { t.Fatal(err) } @@ -279,10 +269,7 @@ func CommonTestRangeIterator(t *testing.T, s store.KVStore) { func CommonTestRangeIteratorSeek(t *testing.T, s store.KVStore) { - data := []struct { - key []byte - val []byte - }{ + data := []testRow{ {[]byte("a1"), []byte("val")}, {[]byte("b1"), []byte("val")}, {[]byte("c1"), []byte("val")}, @@ -290,24 +277,7 @@ func CommonTestRangeIteratorSeek(t *testing.T, s store.KVStore) { {[]byte("e1"), []byte("val")}, } - // open a writer - writer, err := s.Writer() - if err != nil { - t.Fatal(err) - } - - // write the data - batch := writer.NewBatch() - for _, row := range data { - batch.Set(row.key, row.val) - } - err = writer.ExecuteBatch(batch) - if err != nil { - t.Fatal(err) - } - - // close the writer - err = writer.Close() + err := batchWriteRows(s, data) if err != nil { t.Fatal(err) }