scorch zap TestIndexRollback fixes

The TestIndexRollback unit test was failing more often than ever
(perhaps raciness?), so this commit tries to remove avenues of
raciness in the test...

- The Scorch.Open() method is refactored into an Scorch.openBolt()
  helper method in order to allow unit tests to control which
  background goroutines are started.

- TestIndexRollback() doesn't start the merger goroutine, to simulate
  a really slow merger that never gets around to merging old segments.

- TestIndexRollback() creates a long-lived reader after the first
  batch, so that the first index snapshot isn't removed due to the
  long-lived reader's ref-count.

- TestIndexRollback() temporarily bumps NumSnapshotsToKeep to a large
  number so the persister isn't tempted to removeOldData() that we're
  trying to rollback to.

index/scorch/scorch.go

@@ -114,6 +114,25 @@ func (s *Scorch) fireAsyncError(err error) {
 }
 
 func (s *Scorch) Open() error {
+	err := s.openBolt()
+	if err != nil {
+		return err
+	}
+
+	s.asyncTasks.Add(1)
+	go s.mainLoop()
+
+	if !s.readOnly && s.path != "" {
+		s.asyncTasks.Add(1)
+		go s.persisterLoop()
+		s.asyncTasks.Add(1)
+		go s.mergerLoop()
+	}
+
+	return nil
+}
+
+func (s *Scorch) openBolt() error {
 	var ok bool
 	s.path, ok = s.config["path"].(string)
 	if !ok {
@@ -136,6 +155,7 @@ func (s *Scorch) Open() error {
 			}
 		}
 	}
+
 	rootBoltPath := s.path + string(os.PathSeparator) + "root.bolt"
 	var err error
 	if s.path != "" {
@@ -166,16 +186,6 @@ func (s *Scorch) Open() error {
 		}
 	}
 
-	s.asyncTasks.Add(1)
-	go s.mainLoop()
-
-	if !s.readOnly && s.path != "" {
-		s.asyncTasks.Add(1)
-		go s.persisterLoop()
-		s.asyncTasks.Add(1)
-		go s.mergerLoop()
-	}
-
 	return nil
 }
 

index/scorch/snapshot_rollback_test.go

@@ -22,7 +22,12 @@ import (
 )
 
 func TestIndexRollback(t *testing.T) {
+	numSnapshotsToKeepOrig := NumSnapshotsToKeep
+	NumSnapshotsToKeep = 1000
+
 	defer func() {
+		NumSnapshotsToKeep = numSnapshotsToKeepOrig
+
 		err := DestroyTest()
 		if err != nil {
 			t.Fatal(err)
@@ -34,10 +39,6 @@ func TestIndexRollback(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = idx.Open()
-	if err != nil {
-		t.Fatalf("error opening index: %v", err)
-	}
 	defer func() {
 		err := idx.Close()
 		if err != nil {
@@ -45,6 +46,22 @@ func TestIndexRollback(t *testing.T) {
 		}
 	}()
 
+	sh, ok := idx.(*Scorch)
+	if !ok {
+		t.Fatalf("Not a scorch index?")
+	}
+
+	err = sh.openBolt()
+	if err != nil {
+		t.Fatalf("error opening index: %v", err)
+	}
+
+	// start background goroutines except for the merger, which
+	// simulates a super slow merger
+	sh.asyncTasks.Add(2)
+	go sh.mainLoop()
+	go sh.persisterLoop()
+
 	// create a batch, insert 2 new documents
 	batch := index.NewBatch()
 	doc := document.NewDocument("1")
@@ -59,14 +76,17 @@ func TestIndexRollback(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	sh, ok := idx.(*Scorch)
-	if !ok {
-		t.Fatalf("Not a scorch index?")
+	readerSlow, err := idx.Reader() // keep snapshot around so it's not cleaned up
+	if err != nil {
+		t.Fatal(err)
 	}
+	defer func() {
+		_ = readerSlow.Close()
+	}()
 
 	// fetch rollback points available as of here
 	rollbackPoints, err := sh.RollbackPoints()
-	if err != nil || len(rollbackPoints) == 0 {
+	if err != nil || len(rollbackPoints) != 1 {
 		t.Fatal(err, len(rollbackPoints))
 	}
 
@@ -88,6 +108,21 @@ func TestIndexRollback(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	rollbackPointsB, err := sh.RollbackPoints()
+	if err != nil || len(rollbackPointsB) != 3 {
+		t.Fatal(err, len(rollbackPointsB))
+	}
+
+	found := false
+	for _, p := range rollbackPointsB {
+		if rollbackPoint.epoch == p.epoch {
+			found = true
+		}
+	}
+	if !found {
+		t.Fatalf("expected rollbackPoint epoch to still be available")
+	}
+
 	reader, err := idx.Reader()
 	if err != nil {
 		t.Fatal(err)