db: allow loose excised table bounds when using DB.Excise()

In the future we can easily change the policy for IngestAndExcise as
well if we decide to.

Fixes #4417

Author: RaduBerinde

compaction.go

@@ -1345,10 +1345,11 @@ func (d *DB) runIngestFlush(c *compaction) (*manifest.VersionEdit, error) {
 		levelMetrics.TablesIngested++
 	}
 	if ingestFlushable.exciseSpan.Valid() {
+		exciseBounds := ingestFlushable.exciseSpan.UserKeyBounds()
 		// Iterate through all levels and find files that intersect with exciseSpan.
 		for layer, ls := range c.version.AllLevelsAndSublevels() {
 			for m := range ls.Overlaps(d.cmp, ingestFlushable.exciseSpan.UserKeyBounds()).All() {
-				leftTable, rightTable, err := d.exciseTable(context.TODO(), ingestFlushable.exciseSpan.UserKeyBounds(), m, layer.Level())
+				leftTable, rightTable, err := d.exciseTable(context.TODO(), exciseBounds, m, layer.Level(), tightExciseBounds)
 				if err != nil {
 					return nil, err
 				}
@@ -2750,7 +2751,8 @@ func (d *DB) applyHintOnFile(
 	}
 
 	levelMetrics.TablesExcised++
-	leftTable, rightTable, err := d.exciseTable(context.TODO(), base.UserKeyBoundsEndExclusive(h.start, h.end), f, level)
+	exciseBounds := base.UserKeyBoundsEndExclusive(h.start, h.end)
+	leftTable, rightTable, err := d.exciseTable(context.TODO(), exciseBounds, f, level, tightExciseBounds)
 	if err != nil {
 		return nil, errors.Wrap(err, "error when running excise for delete-only compaction")
 	}

excise.go

@@ -12,6 +12,7 @@ import (
 	"github.com/cockroachdb/pebble/internal/base"
 	"github.com/cockroachdb/pebble/internal/invariants"
 	"github.com/cockroachdb/pebble/internal/manifest"
+	"github.com/cockroachdb/pebble/objstorage"
 )
 
 // Excise atomically deletes all data overlapping with the provided span. All
@@ -40,43 +41,79 @@ func (d *DB) Excise(ctx context.Context, span KeyRange) error {
 			v, FormatVirtualSSTables,
 		)
 	}
-	_, err := d.ingest(ctx, nil, nil, span, nil)
+	_, err := d.ingest(ctx, ingestArgs{ExciseSpan: span, ExciseBoundsPolicy: tightExciseBoundsIfLocal})
 	return err
 }
 
+// exciseBoundsPolicy controls whether we open excised files to obtain tight
+// bounds for the remaining file(s).
+type exciseBoundsPolicy uint8
+
+const (
+	// tightExciseBounds means that we will always open the file to find the exact
+	// bounds of the remaining file(s).
+	tightExciseBounds exciseBoundsPolicy = iota
+	// looseExciseBounds means that we will not open the file and will assign bounds
+	// pessimistically.
+	looseExciseBounds
+	// tightExciseBoundsLocalOnly means that we will only open the file if it is
+	// local; otherwise we will assign loose bounds to the remaining file(s).
+	tightExciseBoundsIfLocal
+)
+
 // exciseTable initializes up to two virtual tables for what is left over after
 // excising the given span from the table.
 //
-// Returns the left and/or right tables, if they exist.
+// Returns the left and/or right tables, if they exist. The boundsPolicy controls
+// whether we create iterators for m to determine tight bounds. Note that if the
+// exciseBounds are end-inclusive, tight bounds will be used regardless of the
+// policy.
 //
 // The file bounds must overlap with the excise span.
 //
 // This method is agnostic to whether d.mu is held or not. Some cases call it with
 // the db mutex held (eg. ingest-time excises), while in the case of compactions
 // the mutex is not held.
 func (d *DB) exciseTable(
-	ctx context.Context, exciseSpan base.UserKeyBounds, m *tableMetadata, level int,
+	ctx context.Context,
+	exciseBounds base.UserKeyBounds,
+	m *tableMetadata,
+	level int,
+	boundsPolicy exciseBoundsPolicy,
 ) (leftTable, rightTable *tableMetadata, _ error) {
 	// Check if there's actually an overlap between m and exciseSpan.
 	mBounds := m.UserKeyBounds()
-	if !exciseSpan.Overlaps(d.cmp, &mBounds) {
+	if !exciseBounds.Overlaps(d.cmp, &mBounds) {
 		return nil, nil, base.AssertionFailedf("excise span does not overlap table")
 	}
 	// Fast path: m sits entirely within the exciseSpan, so just delete it.
-	if exciseSpan.ContainsInternalKey(d.cmp, m.Smallest) && exciseSpan.ContainsInternalKey(d.cmp, m.Largest) {
+	if exciseBounds.ContainsInternalKey(d.cmp, m.Smallest) && exciseBounds.ContainsInternalKey(d.cmp, m.Largest) {
 		return nil, nil, nil
 	}
 
-	// The file partially overlaps the excise span; we will need to open it to
-	// determine tight bounds for the left-over table(s).
-	iters, err := d.newIters(ctx, m, &IterOptions{
-		Category: categoryIngest,
-		layer:    manifest.Level(level),
-	}, internalIterOpts{}, iterPointKeys|iterRangeDeletions|iterRangeKeys)
-	if err != nil {
-		return nil, nil, err
+	looseBounds := boundsPolicy == looseExciseBounds ||
+		(boundsPolicy == tightExciseBoundsIfLocal && !objstorage.IsLocalTable(d.objProvider, m.FileBacking.DiskFileNum))
+
+	if exciseBounds.End.Kind == base.Inclusive {
+		// Loose bounds are not allowed with end-inclusive bounds. This can only
+		// happen for ingest splits.
+		looseBounds = false
+	}
+
+	// The file partially overlaps the excise span; unless looseBounds is true, we
+	// will need to open it to determine tight bounds for the left-over table(s).
+	var iters iterSet
+	if !looseBounds {
+		var err error
+		iters, err = d.newIters(ctx, m, &IterOptions{
+			Category: categoryIngest,
+			layer:    manifest.Level(level),
+		}, internalIterOpts{}, iterPointKeys|iterRangeDeletions|iterRangeKeys)
+		if err != nil {
+			return nil, nil, err
+		}
+		defer func() { _ = iters.CloseAll() }()
 	}
-	defer func() { _ = iters.CloseAll() }()
 
 	// Create a file to the left of the excise span, if necessary.
 	// The bounds of this file will be [m.Smallest, lastKeyBefore(exciseSpan.Start)].
@@ -100,7 +137,7 @@ func (d *DB) exciseTable(
 	// files, then grab the lock again and recalculate for just the files that
 	// have changed since our previous calculation. Do this optimiaztino as part of
 	// https://github.com/cockroachdb/pebble/issues/2112 .
-	if d.cmp(m.Smallest.UserKey, exciseSpan.Start) < 0 {
+	if d.cmp(m.Smallest.UserKey, exciseBounds.Start) < 0 {
 		leftTable = &tableMetadata{
 			Virtual: true,
 			FileNum: d.mu.versions.getNextFileNum(),
@@ -111,7 +148,9 @@ func (d *DB) exciseTable(
 			LargestSeqNumAbsolute:    m.LargestSeqNumAbsolute,
 			SyntheticPrefixAndSuffix: m.SyntheticPrefixAndSuffix,
 		}
-		if err := determineLeftTableBounds(d.cmp, m, leftTable, exciseSpan.Start, iters); err != nil {
+		if looseBounds {
+			looseLeftTableBounds(d.cmp, m, leftTable, exciseBounds.Start)
+		} else if err := determineLeftTableBounds(d.cmp, m, leftTable, exciseBounds.Start, iters); err != nil {
 			return nil, nil, err
 		}
 
@@ -129,7 +168,7 @@ func (d *DB) exciseTable(
 		}
 	}
 	// Create a file to the right, if necessary.
-	if !exciseSpan.End.IsUpperBoundForInternalKey(d.cmp, m.Largest) {
+	if !exciseBounds.End.IsUpperBoundForInternalKey(d.cmp, m.Largest) {
 		// Create a new file, rightFile, between [firstKeyAfter(exciseSpan.End), m.Largest].
 		//
 		// See comment before the definition of leftFile for the motivation behind
@@ -144,7 +183,10 @@ func (d *DB) exciseTable(
 			LargestSeqNumAbsolute:    m.LargestSeqNumAbsolute,
 			SyntheticPrefixAndSuffix: m.SyntheticPrefixAndSuffix,
 		}
-		if err := determineRightTableBounds(d.cmp, m, rightTable, exciseSpan.End, iters); err != nil {
+		if looseBounds {
+			// We already checked that the end bound is exclusive.
+			looseRightTableBounds(d.cmp, m, rightTable, exciseBounds.End.Key)
+		} else if err := determineRightTableBounds(d.cmp, m, rightTable, exciseBounds.End, iters); err != nil {
 			return nil, nil, err
 		}
 		if rightTable.HasRangeKeys || rightTable.HasPointKeys {
@@ -207,8 +249,62 @@ func exciseOverlapBounds(
 	return extended
 }
 
+// looseLeftTableBounds initializes the bounds for the table that remains to the
+// left of the excise span after excising originalTable, without consulting the
+// contents of originalTable. The resulting bounds are loose.
+//
+// Sets the smallest and largest keys, as well as HasPointKeys/HasRangeKeys in
+// the leftFile.
+func looseLeftTableBounds(
+	cmp Compare, originalTable, leftTable *tableMetadata, exciseSpanStart []byte,
+) {
+	if originalTable.HasPointKeys {
+		largestPointKey := originalTable.LargestPointKey
+		if largestPointKey.IsUpperBoundFor(cmp, exciseSpanStart) {
+			largestPointKey = base.MakeRangeDeleteSentinelKey(exciseSpanStart)
+		}
+		leftTable.ExtendPointKeyBounds(cmp, originalTable.SmallestPointKey, largestPointKey)
+	}
+	if originalTable.HasRangeKeys {
+		largestRangeKey := originalTable.LargestRangeKey
+		if largestRangeKey.IsUpperBoundFor(cmp, exciseSpanStart) {
+			largestRangeKey = base.MakeExclusiveSentinelKey(InternalKeyKindRangeKeyMin, exciseSpanStart)
+		}
+		leftTable.ExtendRangeKeyBounds(cmp, originalTable.SmallestRangeKey, largestRangeKey)
+	}
+}
+
+// looseRightTableBounds initializes the bounds for the table that remains to the
+// right of the excise span after excising originalTable, without consulting the
+// contents of originalTable. The resulting bounds are loose.
+//
+// Sets the smallest and largest keys, as well as HasPointKeys/HasRangeKeys in
+// the rightFile.
+//
+// The excise span end bound is assumed to be exclusive; this function cannot be
+// used with an inclusive end bound.
+func looseRightTableBounds(
+	cmp Compare, originalTable, rightTable *tableMetadata, exciseSpanEnd []byte,
+) {
+	if originalTable.HasPointKeys {
+		smallestPointKey := originalTable.SmallestPointKey
+		if !smallestPointKey.IsUpperBoundFor(cmp, exciseSpanEnd) {
+			smallestPointKey = base.MakeInternalKey(exciseSpanEnd, 0, base.InternalKeyKindMaxForSSTable)
+		}
+		rightTable.ExtendPointKeyBounds(cmp, smallestPointKey, originalTable.LargestPointKey)
+	}
+	if originalTable.HasRangeKeys {
+		smallestRangeKey := originalTable.SmallestRangeKey
+		if !smallestRangeKey.IsUpperBoundFor(cmp, exciseSpanEnd) {
+			smallestRangeKey = base.MakeInternalKey(exciseSpanEnd, 0, base.InternalKeyKindRangeKeyMax)
+		}
+		rightTable.ExtendRangeKeyBounds(cmp, smallestRangeKey, originalTable.LargestRangeKey)
+	}
+}
+
 // determineLeftTableBounds calculates the bounds for the table that remains to
-// the left of the excise span after excising originalFile.
+// the left of the excise span after excising originalTable. The bounds around
+// the excise span are determined precisely by looking inside the file.
 //
 // Sets the smallest and largest keys, as well as HasPointKeys/HasRangeKeys in
 // the leftFile.
@@ -256,7 +352,8 @@ func determineLeftTableBounds(
 }
 
 // determineRightTableBounds calculates the bounds for the table that remains to
-// the right of the excise span after excising originalFile.
+// the right of the excise span after excising originalTable. The bounds around
+// the excise span are determined precisely by looking inside the file.
 //
 // Sets the smallest and largest keys, as well as HasPointKeys/HasRangeKeys in
 // the right.

excise_test.go

@@ -50,6 +50,7 @@ func TestExcise(t *testing.T) {
 		flushed = false
 	}
 
+	var remoteStorage remote.Storage
 	var opts *Options
 	reset := func(blockSize int) {
 		for _, e := range efos {
@@ -62,6 +63,7 @@ func TestExcise(t *testing.T) {
 
 		mem = vfs.NewMem()
 		require.NoError(t, mem.MkdirAll("ext", 0755))
+		remoteStorage = remote.NewInMem()
 		opts = &Options{
 			BlockPropertyCollectors: []func() BlockPropertyCollector{
 				sstable.NewTestKeysBlockPropertyCollector,
@@ -80,6 +82,10 @@ func TestExcise(t *testing.T) {
 		if blockSize != 0 {
 			opts.Levels = append(opts.Levels, LevelOptions{BlockSize: blockSize, IndexBlockSize: 32 << 10})
 		}
+		opts.Experimental.RemoteStorage = remote.MakeSimpleFactory(map[remote.Locator]remote.Storage{
+			"external-locator": remoteStorage,
+		})
+		opts.Experimental.CreateOnShared = remote.CreateOnSharedNone
 		// Disable automatic compactions because otherwise we'll race with
 		// delete-only compactions triggered by ingesting range tombstones.
 		opts.DisableAutomaticCompactions = true
@@ -128,6 +134,11 @@ func TestExcise(t *testing.T) {
 				return err.Error()
 			}
 			return ""
+		case "build-remote":
+			if err := runBuildRemoteCmd(td, d, remoteStorage); err != nil {
+				return err.Error()
+			}
+			return ""
 		case "flush":
 			if err := d.Flush(); err != nil {
 				return err.Error()
@@ -193,7 +204,12 @@ func TestExcise(t *testing.T) {
 				}
 				return "memtable flushed"
 			}
-			return ""
+
+		case "ingest-external":
+			if err := runIngestExternalCmd(t, td, d, remoteStorage, "external-locator"); err != nil {
+				return err.Error()
+			}
+
 		case "file-only-snapshot":
 			if len(td.CmdArgs) != 1 {
 				panic("insufficient args for file-only-snapshot command")
@@ -281,10 +297,11 @@ func TestExcise(t *testing.T) {
 			d.mu.Unlock()
 			current := d.mu.versions.currentVersion()
 
+			exciseBounds := exciseSpan.UserKeyBounds()
 			for l, ls := range current.AllLevelsAndSublevels() {
 				iter := ls.Iter()
 				for m := iter.SeekGE(d.cmp, exciseSpan.Start); m != nil && d.cmp(m.Smallest.UserKey, exciseSpan.End) < 0; m = iter.Next() {
-					leftTable, rightTable, err := d.exciseTable(context.Background(), exciseSpan.UserKeyBounds(), m, l.Level())
+					leftTable, rightTable, err := d.exciseTable(context.Background(), exciseBounds, m, l.Level(), tightExciseBounds)
 					if err != nil {
 						td.Fatalf(t, "error when excising %s: %s", m.FileNum, err.Error())
 					}
@@ -335,10 +352,11 @@ func TestExcise(t *testing.T) {
 			if err != nil {
 				return err.Error()
 			}
-			return ""
+
 		default:
-			return fmt.Sprintf("unknown command: %s", td.Cmd)
+			td.Fatalf(t, "unknown command: %s", td.Cmd)
 		}
+		return ""
 	})
 }
 
@@ -661,10 +679,11 @@ func TestConcurrentExcise(t *testing.T) {
 			d.mu.versions.logLock()
 			d.mu.Unlock()
 			current := d.mu.versions.currentVersion()
+			exciseBounds := exciseSpan.UserKeyBounds()
 			for level := range current.Levels {
 				iter := current.Levels[level].Iter()
 				for m := iter.SeekGE(d.cmp, exciseSpan.Start); m != nil && d.cmp(m.Smallest.UserKey, exciseSpan.End) < 0; m = iter.Next() {
-					leftTable, rightTable, err := d.exciseTable(context.Background(), exciseSpan.UserKeyBounds(), m, level)
+					leftTable, rightTable, err := d.exciseTable(context.Background(), exciseBounds, m, level, tightExciseBounds)
 					if err != nil {
 						d.mu.Lock()
 						d.mu.versions.logUnlock()
@@ -835,6 +854,9 @@ func TestExciseBounds(t *testing.T) {
 				var t manifest.TableMetadata
 				checkErr(determineLeftTableBounds(cmp, m, &t, exciseSpan.Start, iters))
 				printBounds("Left table bounds", &t)
+				t = manifest.TableMetadata{}
+				looseLeftTableBounds(cmp, m, &t, exciseSpan.Start)
+				printBounds("Left table bounds (loose)", &t)
 			}
 
 			if !exciseSpan.End.IsUpperBoundForInternalKey(cmp, m.Largest) {
@@ -858,6 +880,11 @@ func TestExciseBounds(t *testing.T) {
 				} else {
 					printBounds("Right table bounds", &t)
 				}
+				if exciseSpan.End.Kind == base.Exclusive {
+					t = manifest.TableMetadata{}
+					looseRightTableBounds(cmp, m, &t, exciseSpan.End.Key)
+					printBounds("Right table bounds (loose)", &t)
+				}
 			}
 			checkErr(iters.CloseAll())