diff --git a/ingest.go b/ingest.go index da34783442..fa6304a6bd 100644 --- a/ingest.go +++ b/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 } diff --git a/ingest_test.go b/ingest_test.go index 4272127c47..fd0465fba4 100644 --- a/ingest_test.go +++ b/ingest_test.go @@ -586,6 +586,7 @@ func TestExcise(t *testing.T) { require.NoError(t, d.Close()) }() + var opts *Options reset := func() { if d != nil { require.NoError(t, d.Close()) @@ -593,7 +594,7 @@ func TestExcise(t *testing.T) { 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") diff --git a/testdata/excise b/testdata/excise index 0935147b40..4102e1ebd0 100644 --- a/testdata/excise +++ b/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]