Repository: cockroachdb/pebble

Issue:
make tombstone compaction threshold configurable
On some workloads, tombstone compactions can lead to increased resource usage with no visible benefit on latencies. We need the threshold to be configurable so we can easily turn it down or off in these cases.

Jira issue: PEBBLE-1225

Hints from issue comments:
@ever0de: If I'm understanding this correctly, I'd like to try implementing it. Could you let me know which threshold you're referring to?

1.  `TombstoneDenseCompactionThreshold` (default: 0.10) - [Already configurable](https://github.com/cockroachdb/pebble/blob/fcb941e0785b472a77a4076b32ee958e8c89cc14/options.go#L627-L637) since(https://github.com/cockroachdb/pebble/commit/28840262)

2.  `maxOverlappingRatio` (default: 40.0) - [Currently hardcoded](https://github.com/cockroachdb/pebble/blob/3dd8e6f3304f35209142eade45dde1da99a3bdad/compaction_picker.go#L1838-L1844) since ([fcb941e0](https://github.com/cockroachdb/pebble/commit/fcb941e0))

I suspect you mean 2 based on your description.@RaduBerinde: I was thinking of TombstoneDenseCompactionThreshold. I should have said *dynamically* reconfigurable (making it a function).

Solution patch:
diff --git a/compaction_picker.go b/compaction_picker.go
index 3f551164..9df6b293 100644
--- a/compaction_picker.go
+++ b/compaction_picker.go
@@ -1827,7 +1827,8 @@ func (p *compactionPickerByScore) pickBlobFileRewriteCandidate(
 func (p *compactionPickerByScore) pickTombstoneDensityCompaction(
 	env compactionEnv,
 ) (pc *pickedTableCompaction) {
-	if p.opts.Experimental.TombstoneDenseCompactionThreshold <= 0 {
+	threshold := p.opts.Experimental.TombstoneDenseCompactionThreshold()
+	if threshold <= 0 {
 		// Tombstone density compactions are disabled.
 		return nil
 	}
@@ -1855,7 +1856,7 @@ func (p *compactionPickerByScore) pickTombstoneDensityCompaction(
 			if !propsValid {
 				continue
 			}
-			if props.TombstoneDenseBlocksRatio < p.opts.Experimental.TombstoneDenseCompactionThreshold {
+			if props.TombstoneDenseBlocksRatio < threshold {
 				continue
 			}
 			overlaps := p.vers.Overlaps(lastNonEmptyLevel, f.UserKeyBounds())
diff --git a/options.go b/options.go
index e01fb6f1..0f4a3daf 100644
--- a/options.go
+++ b/options.go
@@ -688,17 +688,19 @@ type Options struct {
 		// sstable writers. The default value is 0.5.
 		DeletionSizeRatioThreshold float32
 
-		// TombstoneDenseCompactionThreshold is the minimum percent of data
-		// blocks in a table that must be tombstone-dense for that table to be
-		// eligible for a tombstone density compaction. It should be defined as a
-		// ratio out of 1. The default value is 0.10.
+		// TombstoneDenseCompactionThreshold is a function that returns the minimum
+		// percent of data blocks in a table that must be tombstone-dense for that
+		// table to be eligible for a tombstone density compaction. The value should
+		// be defined as a ratio out of 1. The default value is 0.10.
 		//
 		// If multiple tables are eligible for a tombstone density compaction, then
 		// tables with a higher percent of tombstone-dense blocks are still
 		// prioritized for compaction.
 		//
-		// A zero or negative value disables tombstone density compactions.
-		TombstoneDenseCompactionThreshold float64
+		// Using a function allows for dynamic reconfiguration of the threshold based
+		// on workload characteristics. A zero or negative value disables tombstone
+		// density compactions.
+		TombstoneDenseCompactionThreshold func() float64
 
 		// FileCacheShards is the number of shards per file cache.
 		// Reducing the value can reduce the number of idle goroutines per DB
@@ -1641,8 +1643,8 @@ func (o *Options) EnsureDefaults() {
 	if o.Experimental.DeletionSizeRatioThreshold == 0 {
 		o.Experimental.DeletionSizeRatioThreshold = sstable.DefaultDeletionSizeRatioThreshold
 	}
-	if o.Experimental.TombstoneDenseCompactionThreshold == 0 {
-		o.Experimental.TombstoneDenseCompactionThreshold = 0.10
+	if o.Experimental.TombstoneDenseCompactionThreshold == nil {
+		o.Experimental.TombstoneDenseCompactionThreshold = func() float64 { return 0.10 }
 	}
 	if o.Experimental.FileCacheShards <= 0 {
 		o.Experimental.FileCacheShards = runtime.GOMAXPROCS(0)
@@ -1787,7 +1789,7 @@ func (o *Options) String() string {
 	fmt.Fprintf(&buf, "  read_sampling_multiplier=%d\n", o.Experimental.ReadSamplingMultiplier)
 	fmt.Fprintf(&buf, "  num_deletions_threshold=%d\n", o.Experimental.NumDeletionsThreshold)
 	fmt.Fprintf(&buf, "  deletion_size_ratio_threshold=%f\n", o.Experimental.DeletionSizeRatioThreshold)
-	fmt.Fprintf(&buf, "  tombstone_dense_compaction_threshold=%f\n", o.Experimental.TombstoneDenseCompactionThreshold)
+	fmt.Fprintf(&buf, "  tombstone_dense_compaction_threshold=%f\n", o.Experimental.TombstoneDenseCompactionThreshold())
 	// We no longer care about strict_wal_tail, but set it to true in case an
 	// older version reads the options.
 	fmt.Fprintf(&buf, "  strict_wal_tail=%t\n", true)
@@ -2216,7 +2218,11 @@ func (o *Options) Parse(s string, hooks *ParseHooks) error {
 				o.Experimental.DeletionSizeRatioThreshold = float32(val)
 				err = parseErr
 			case "tombstone_dense_compaction_threshold":
-				o.Experimental.TombstoneDenseCompactionThreshold, err = strconv.ParseFloat(value, 64)
+				var threshold float64
+				threshold, err = strconv.ParseFloat(value, 64)
+				if err == nil {
+					o.Experimental.TombstoneDenseCompactionThreshold = func() float64 { return threshold }
+				}
 			case "table_cache_shards":
 				o.Experimental.FileCacheShards, err = strconv.Atoi(value)
 			case "table_format":



Test patch:
diff --git a/compaction_test.go b/compaction_test.go
index c9db1257..0dd4c0eb 100644
--- a/compaction_test.go
+++ b/compaction_test.go
@@ -3506,7 +3506,7 @@ func TestTombstoneDensityCompactionMoveOptimization(t *testing.T) {
 	)
 
 	opts := DefaultOptions()
-	opts.Experimental.TombstoneDenseCompactionThreshold = 0.5 // Lower for test
+	opts.Experimental.TombstoneDenseCompactionThreshold = func() float64 { return 0.5 } // Lower for test
 	opts.Experimental.NumDeletionsThreshold = 1
 	opts.Experimental.CompactionScheduler = func() CompactionScheduler {
 		return NewConcurrencyLimitSchedulerWithNoPeriodicGrantingForTest()
@@ -3608,7 +3608,7 @@ func TestTombstoneDensityCompactionMoveOptimization_NoMoveWithOverlap(t *testing
 	)
 
 	opts := DefaultOptions()
-	opts.Experimental.TombstoneDenseCompactionThreshold = 0.5 // Lower for test
+	opts.Experimental.TombstoneDenseCompactionThreshold = func() float64 { return 0.5 } // Lower for test
 	opts.Experimental.NumDeletionsThreshold = 1
 	opts.Experimental.CompactionScheduler = func() CompactionScheduler {
 		return NewConcurrencyLimitSchedulerWithNoPeriodicGrantingForTest()
@@ -3692,7 +3692,7 @@ func TestTombstoneDensityCompactionMoveOptimization_GrandparentOverlapTooLarge(t
 	)
 
 	opts := DefaultOptions()
-	opts.Experimental.TombstoneDenseCompactionThreshold = 0.5
+	opts.Experimental.TombstoneDenseCompactionThreshold = func() float64 { return 0.5 }
 	opts.Experimental.NumDeletionsThreshold = 1
 	opts.Experimental.CompactionScheduler = func() CompactionScheduler {
 		return NewConcurrencyLimitSchedulerWithNoPeriodicGrantingForTest()
@@ -3760,7 +3760,7 @@ func TestTombstoneDensityCompactionMoveOptimization_BelowDensityThreshold(t *tes
 	)
 
 	opts := DefaultOptions()
-	opts.Experimental.TombstoneDenseCompactionThreshold = 0.9 // Set high threshold
+	opts.Experimental.TombstoneDenseCompactionThreshold = func() float64 { return 0.9 } // Set high threshold
 	opts.Experimental.NumDeletionsThreshold = 1
 	opts.Experimental.CompactionScheduler = func() CompactionScheduler {
 		return NewConcurrencyLimitSchedulerWithNoPeriodicGrantingForTest()
@@ -3812,7 +3812,7 @@ func TestTombstoneDensityCompactionMoveOptimization_InvalidStats(t *testing.T) {
 	)
 
 	opts := DefaultOptions()
-	opts.Experimental.TombstoneDenseCompactionThreshold = 0.5
+	opts.Experimental.TombstoneDenseCompactionThreshold = func() float64 { return 0.5 }
 	opts.Experimental.NumDeletionsThreshold = 1
 	opts.Experimental.CompactionScheduler = func() CompactionScheduler {
 		return NewConcurrencyLimitSchedulerWithNoPeriodicGrantingForTest()
diff --git a/metamorphic/options_test.go b/metamorphic/options_test.go
index 73d6817a..e935e4b4 100644
--- a/metamorphic/options_test.go
+++ b/metamorphic/options_test.go
@@ -85,6 +85,7 @@ func TestOptionsRoundtrip(t *testing.T) {
 		"Experimental.RemoteStorage:",
 		"Experimental.SingleDeleteInvariantViolationCallback:",
 		"Experimental.EnableDeleteOnlyCompactionExcises:",
+		"Experimental.TombstoneDenseCompactionThreshold:",
 		"Experimental.ValueSeparationPolicy:",
 		"Levels[0].Compression:",
 		"Levels[1].Compression:",
@@ -116,6 +117,7 @@ func TestOptionsRoundtrip(t *testing.T) {
 		expectEqualFn(t, o.Opts.Experimental.DisableIngestAsFlushable, parsed.Opts.Experimental.DisableIngestAsFlushable)
 		expectEqualFn(t, o.Opts.Experimental.IngestSplit, parsed.Opts.Experimental.IngestSplit)
 		expectEqualFn(t, o.Opts.Experimental.CompactionGarbageFractionForMaxConcurrency, parsed.Opts.Experimental.CompactionGarbageFractionForMaxConcurrency)
+		expectEqualFn(t, o.Opts.Experimental.TombstoneDenseCompactionThreshold, parsed.Opts.Experimental.TombstoneDenseCompactionThreshold)
 		expectEqualFn(t, o.Opts.Experimental.ValueSeparationPolicy, parsed.Opts.Experimental.ValueSeparationPolicy)
 		expectEqualFn(t, o.Opts.TargetByteDeletionRate, parsed.Opts.TargetByteDeletionRate)
 
diff --git a/options_test.go b/options_test.go
index 11d7427e..f8f555ff 100644
--- a/options_test.go
+++ b/options_test.go
@@ -406,7 +406,7 @@ func TestOptionsParse(t *testing.T) {
 			opts.Experimental.ReadSamplingMultiplier = 400
 			opts.Experimental.NumDeletionsThreshold = 500
 			opts.Experimental.DeletionSizeRatioThreshold = 0.7
-			opts.Experimental.TombstoneDenseCompactionThreshold = 0.2
+			opts.Experimental.TombstoneDenseCompactionThreshold = func() float64 { return 0.2 }
 			opts.Experimental.FileCacheShards = 500
 			opts.Experimental.SecondaryCacheSizeBytes = 1024
 			opts.Experimental.ValueSeparationPolicy = func() ValueSeparationPolicy {