db: only create one CreatedBackingTables entry per sstable

Problem explained in detail in: #2947 (comment)

We were adding too many CreatedBackingTables entries to the manifest. Such an
entry should only be added when an sstable is first virtualized. This is
mentioned as an invariant above VersionEdit.CreatedBackingTables.

Note that the excise function will create backing tables and add them to the
version edit. But these backing tables should only be added to the version edit,
if the sstable being virtualized is NOT virtual. If we're further virtualizing
an already virtual sstable, then a backing table entry associated with the file
backing will already be present in the manifest as part of an older version edit.

ingest.go

@@ -1608,7 +1608,7 @@ func (d *DB) excise(
 	var iter internalIterator
 	var rangeDelIter keyspan.FragmentIterator
 	var rangeKeyIter keyspan.FragmentIterator
-	backingTableCreated := false
+	needsBacking := false
 	// Create a file to the left of the excise span, if necessary.
 	// The bounds of this file will be [m.Smallest, lastKeyBefore(exciseSpan.Start)].
 	//
@@ -1722,14 +1722,20 @@ func (d *DB) excise(
 			leftFile.ValidateVirtual(m)
 			d.checkVirtualBounds(leftFile)
 			ve.NewFiles = append(ve.NewFiles, newFileEntry{Level: level, Meta: leftFile})
-			ve.CreatedBackingTables = append(ve.CreatedBackingTables, leftFile.FileBacking)
-			backingTableCreated = true
+			needsBacking = true
 			numCreatedFiles++
 		}
 	}
 	// Create a file to the right, if necessary.
 	if exciseSpan.Contains(d.cmp, m.Largest) {
 		// No key exists to the right of the excise span in this file.
+		if needsBacking && !m.Virtual {
+			// If m is virtual, then its file backing is already known to the manifest.
+			// We don't need to create another file backing. Note that there must be
+			// only one CreatedBackingTables entry per backing sstable. This is
+			// indicated by the VersionEdit.CreatedBackingTables invariant.
+			ve.CreatedBackingTables = append(ve.CreatedBackingTables, m.FileBacking)
+		}
 		return ve.NewFiles[len(ve.NewFiles)-numCreatedFiles:], nil
 	}
 	// Create a new file, rightFile, between [firstKeyAfter(exciseSpan.End), m.Largest].
@@ -1832,13 +1838,18 @@ func (d *DB) excise(
 		rightFile.ValidateVirtual(m)
 		d.checkVirtualBounds(rightFile)
 		ve.NewFiles = append(ve.NewFiles, newFileEntry{Level: level, Meta: rightFile})
-		if !backingTableCreated {
-			ve.CreatedBackingTables = append(ve.CreatedBackingTables, rightFile.FileBacking)
-			backingTableCreated = true
-		}
+		needsBacking = true
 		numCreatedFiles++
 	}
 
+	if needsBacking && !m.Virtual {
+		// If m is virtual, then its file backing is already known to the manifest.
+		// We don't need to create another file backing. Note that there must be
+		// only one CreatedBackingTables entry per backing sstable. This is
+		// indicated by the VersionEdit.CreatedBackingTables invariant.
+		ve.CreatedBackingTables = append(ve.CreatedBackingTables, m.FileBacking)
+	}
+
 	if err := rightFile.Validate(d.cmp, d.opts.Comparer.FormatKey); err != nil {
 		return nil, err
 	}

ingest_test.go

@@ -586,14 +586,15 @@ func TestExcise(t *testing.T) {
 		require.NoError(t, d.Close())
 	}()
 
+	var opts *Options
 	reset := func() {
 		if d != nil {
 			require.NoError(t, d.Close())
 		}
 
 		mem = vfs.NewMem()
 		require.NoError(t, mem.MkdirAll("ext", 0755))
-		opts := &Options{
+		opts = &Options{
 			FS:                    mem,
 			L0CompactionThreshold: 100,
 			L0StopWritesThreshold: 100,
@@ -621,6 +622,13 @@ func TestExcise(t *testing.T) {
 		switch td.Cmd {
 		case "reset":
 			reset()
+			return ""
+		case "reopen":
+			require.NoError(t, d.Close())
+			var err error
+			d, err = Open("", opts)
+			require.NoError(t, err)
+
 			return ""
 		case "batch":
 			b := d.NewIndexedBatch()
@@ -731,6 +739,36 @@ func TestExcise(t *testing.T) {
 			d.mu.Unlock()
 			return fmt.Sprintf("would excise %d files, use ingest-and-excise to excise.\n%s", len(ve.DeletedFiles), ve.DebugString(base.DefaultFormatter))
 
+		case "confirm-backing":
+			// Confirms that the files have the same FileBacking.
+			fileNums := make(map[base.FileNum]struct{})
+			for i := range td.CmdArgs {
+				fNum, err := strconv.Atoi(td.CmdArgs[i].Key)
+				if err != nil {
+					panic("invalid file number")
+				}
+				fileNums[base.FileNum(fNum)] = struct{}{}
+			}
+			d.mu.Lock()
+			currVersion := d.mu.versions.currentVersion()
+			var ptr *manifest.FileBacking
+			for _, level := range currVersion.Levels {
+				lIter := level.Iter()
+				for f := lIter.First(); f != nil; f = lIter.Next() {
+					if _, ok := fileNums[f.FileNum]; ok {
+						if ptr == nil {
+							ptr = f.FileBacking
+							continue
+						}
+						if f.FileBacking != ptr {
+							d.mu.Unlock()
+							return "file backings are not the same"
+						}
+					}
+				}
+			}
+			d.mu.Unlock()
+			return "file backings are the same"
 		case "compact":
 			if len(td.CmdArgs) != 2 {
 				panic("insufficient args for compact command")

testdata/excise

@@ -212,3 +212,128 @@ lsm
   000008:[d#12,SET-ee#inf,RANGEKEYSET]
 6:
   000006:[z#14,SET-z#14,SET]
+
+# Regression test for https://github.com/cockroachdb/pebble/issues/2947.
+reset
+----
+
+batch
+set a a
+set b b
+set c c
+set d d
+set e e
+set f f
+set g g
+set h h
+set i i
+set j j
+----
+
+flush
+----
+
+lsm
+----
+0.0:
+  000005:[a#10,SET-j#19,SET]
+
+build ext2
+set z z
+----
+
+ingest-and-excise ext2 excise=d-e
+----
+
+lsm
+----
+0.0:
+  000007:[a#10,SET-c#12,SET]
+  000008:[e#14,SET-j#19,SET]
+6:
+  000006:[z#20,SET-z#20,SET]
+
+build ext3
+set zz zz
+----
+
+ingest-and-excise ext3 excise=g-h
+----
+
+# 7, 10, 11 should have the same file backing struct.
+lsm
+----
+0.0:
+  000007:[a#10,SET-c#12,SET]
+  000010:[e#14,SET-f#15,SET]
+  000011:[h#17,SET-j#19,SET]
+6:
+  000006:[z#20,SET-z#20,SET]
+  000009:[zz#21,SET-zz#21,SET]
+
+confirm-backing 7 10 11
+----
+file backings are the same
+
+reopen
+----
+
+# 7, 10, 11 should still have the same file backing struct even after manifest
+# replay.
+lsm
+----
+0.0:
+  000007:[a#10,SET-c#12,SET]
+  000010:[e#14,SET-f#15,SET]
+  000011:[h#17,SET-j#19,SET]
+6:
+  000006:[z#20,SET-z#20,SET]
+  000009:[zz#21,SET-zz#21,SET]
+
+confirm-backing 7 10 11
+----
+file backings are the same
+
+# Excise one boundary, the file backing should still be set.
+reset
+----
+
+batch
+set a a
+set b b
+set c c
+set d d
+set e e
+----
+
+flush
+----
+
+lsm
+----
+0.0:
+  000005:[a#10,SET-e#14,SET]
+
+build ext2
+set z z
+----
+
+ingest-and-excise ext2 excise=d-f
+----
+
+lsm
+----
+0.0:
+  000007:[a#10,SET-c#12,SET]
+6:
+  000006:[z#15,SET-z#15,SET]
+
+reopen
+----
+
+lsm
+----
+0.0:
+  000007:[a#10,SET-c#12,SET]
+6:
+  000006:[z#15,SET-z#15,SET]