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 c1e2607f..d41a3633 100644
--- a/compaction_picker.go
+++ b/compaction_picker.go
@@ -1522,7 +1522,8 @@ func (p *compactionPickerByScore) pickRewriteCompaction(env compactionEnv) (pc *
 func (p *compactionPickerByScore) pickTombstoneDensityCompaction(
 	env compactionEnv,
 ) (pc *pickedCompaction) {
-	if p.opts.Experimental.TombstoneDenseCompactionThreshold <= 0 {
+	threshold := p.opts.Experimental.TombstoneDenseCompactionThreshold()
+	if threshold <= 0 {
 		// Tombstone density compactions are disabled.
 		return nil
 	}
@@ -1545,7 +1546,7 @@ func (p *compactionPickerByScore) pickTombstoneDensityCompaction(
 			if f.IsCompacting() || !f.StatsValid() || f.Size == 0 {
 				continue
 			}
-			if f.Stats.TombstoneDenseBlocksRatio < p.opts.Experimental.TombstoneDenseCompactionThreshold {
+			if f.Stats.TombstoneDenseBlocksRatio < threshold {
 				continue
 			}
 			overlaps := p.vers.Overlaps(lastNonEmptyLevel, f.UserKeyBounds())
diff --git a/options.go b/options.go
index b45d24fc..2655ac47 100644
--- a/options.go
+++ b/options.go
@@ -625,17 +625,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
 
 		// TableCacheShards is the number of shards per table cache.
 		// Reducing the value can reduce the number of idle goroutines per DB
@@ -1357,8 +1359,8 @@ func (o *Options) EnsureDefaults() *Options {
 	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.TableCacheShards <= 0 {
 		o.Experimental.TableCacheShards = runtime.GOMAXPROCS(0)
@@ -1495,7 +1497,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)
@@ -1899,7 +1901,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.TableCacheShards, err = strconv.Atoi(value)
 			case "table_format":



Test patch:
diff --git a/metamorphic/options_test.go b/metamorphic/options_test.go
index 5847c7d0..54541df4 100644
--- a/metamorphic/options_test.go
+++ b/metamorphic/options_test.go
@@ -80,6 +80,7 @@ func TestOptionsRoundtrip(t *testing.T) {
 		"Experimental.RemoteStorage:",
 		"Experimental.SingleDeleteInvariantViolationCallback:",
 		"Experimental.EnableDeleteOnlyCompactionExcises:",
+		"Experimental.TombstoneDenseCompactionThreshold:",
 		"Levels[0].Compression:",
 		"Levels[1].Compression:",
 		"Levels[2].Compression:",
@@ -125,6 +126,7 @@ func TestOptionsRoundtrip(t *testing.T) {
 		if o.Opts.Experimental.IngestSplit != nil && o.Opts.Experimental.IngestSplit() {
 			require.Equal(t, o.Opts.Experimental.IngestSplit(), parsed.Opts.Experimental.IngestSplit())
 		}
+		require.Equal(t, o.Opts.Experimental.TombstoneDenseCompactionThreshold(), parsed.Opts.Experimental.TombstoneDenseCompactionThreshold())
 
 		expBaseline, expUpper := o.Opts.CompactionConcurrencyRange()
 		parsedBaseline, parsedUpper := parsed.Opts.CompactionConcurrencyRange()
diff --git a/options_test.go b/options_test.go
index 928943ef..8df5ca0a 100644
--- a/options_test.go
+++ b/options_test.go
@@ -303,7 +303,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.TableCacheShards = 500
 			opts.Experimental.MaxWriterConcurrency = 1
 			opts.Experimental.ForceWriterParallelism = true