db: run score-based compactions before manual compactions

Score-based compactions are important for maintaining LSM health.

Fixes #4571

Author: sumeerbhola

compaction.go

@@ -1807,11 +1807,20 @@ func (d *DB) makeCompactionEnvLocked() *compactionEnv {
 
 // pickAnyCompaction tries to pick a manual or automatic compaction.
 func (d *DB) pickAnyCompaction(env compactionEnv) (pc *pickedCompaction) {
-	pc = d.pickManualCompaction(env)
-	if pc == nil && !d.opts.DisableAutomaticCompactions {
-		pc = d.mu.versions.picker.pickAuto(env)
+	// Pick a score-based compaction first, since a misshapen LSM is bad.
+	if !d.opts.DisableAutomaticCompactions {
+		if pc = d.mu.versions.picker.pickAutoScore(env); pc != nil {
+			return pc
+		}
+	}
+	// Pick a manual compaction, if any.
+	if pc = d.pickManualCompaction(env); pc != nil {
+		return pc
 	}
-	return pc
+	if !d.opts.DisableAutomaticCompactions {
+		return d.mu.versions.picker.pickAutoNonScore(env)
+	}
+	return nil
 }
 
 // runPickedCompaction kicks off the provided pickedCompaction. In case the

compaction_picker.go

@@ -50,10 +50,8 @@ type compactionPicker interface {
 	getScores([]compactionInfo) [numLevels]float64
 	getBaseLevel() int
 	estimatedCompactionDebt(l0ExtraSize uint64) uint64
-	pickAuto(env compactionEnv) (pc *pickedCompaction)
-	pickElisionOnlyCompaction(env compactionEnv) (pc *pickedCompaction)
-	pickRewriteCompaction(env compactionEnv) (pc *pickedCompaction)
-	pickReadTriggeredCompaction(env compactionEnv) (pc *pickedCompaction)
+	pickAutoScore(env compactionEnv) (pc *pickedCompaction)
+	pickAutoNonScore(env compactionEnv) (pc *pickedCompaction)
 	forceBaseLevel1()
 }
 
@@ -1244,70 +1242,71 @@ func (p *compactionPickerByScore) getCompactionConcurrency() int {
 	return max(min(maxConcurrentCompactions, max(l0ReadAmpCompactions, compactionDebtCompactions)), 1)
 }
 
-// pickAuto picks the best compaction, if any.
-//
-// On each call, pickAuto computes per-level size adjustments based on
-// in-progress compactions, and computes a per-level score. The levels are
-// iterated over in decreasing score order trying to find a valid compaction
-// anchored at that level.
-//
-// If a score-based compaction cannot be found, pickAuto falls back to looking
-// for an elision-only compaction to remove obsolete keys.
-func (p *compactionPickerByScore) pickAuto(env compactionEnv) (pc *pickedCompaction) {
-	scores := p.calculateLevelScores(env.inProgressCompactions)
+// TODO(sumeer): remove unless someone actually finds this useful.
+func (p *compactionPickerByScore) logCompactionForTesting(
+	env compactionEnv, scores [numLevels]candidateLevelInfo, pc *pickedCompaction,
+) {
+	var buf bytes.Buffer
+	for i := 0; i < numLevels; i++ {
+		if i != 0 && i < p.baseLevel {
+			continue
+		}
 
-	// TODO(bananabrick): Either remove, or change this into an event sent to the
-	// EventListener.
-	logCompaction := func(pc *pickedCompaction) {
-		var buf bytes.Buffer
-		for i := 0; i < numLevels; i++ {
-			if i != 0 && i < p.baseLevel {
-				continue
+		var info *candidateLevelInfo
+		for j := range scores {
+			if scores[j].level == i {
+				info = &scores[j]
+				break
 			}
+		}
 
-			var info *candidateLevelInfo
-			for j := range scores {
-				if scores[j].level == i {
-					info = &scores[j]
-					break
-				}
-			}
+		marker := " "
+		if pc.startLevel.level == info.level {
+			marker = "*"
+		}
+		fmt.Fprintf(&buf, "  %sL%d: %5.1f  %5.1f  %5.1f  %5.1f %8s  %8s",
+			marker, info.level, info.compensatedScoreRatio, info.compensatedScore,
+			info.uncompensatedScoreRatio, info.uncompensatedScore,
+			humanize.Bytes.Int64(int64(totalCompensatedSize(
+				p.vers.Levels[info.level].All(),
+			))),
+			humanize.Bytes.Int64(p.levelMaxBytes[info.level]),
+		)
 
-			marker := " "
-			if pc.startLevel.level == info.level {
-				marker = "*"
-			}
-			fmt.Fprintf(&buf, "  %sL%d: %5.1f  %5.1f  %5.1f  %5.1f %8s  %8s",
-				marker, info.level, info.compensatedScoreRatio, info.compensatedScore,
-				info.uncompensatedScoreRatio, info.uncompensatedScore,
-				humanize.Bytes.Int64(int64(totalCompensatedSize(
-					p.vers.Levels[info.level].All(),
-				))),
-				humanize.Bytes.Int64(p.levelMaxBytes[info.level]),
-			)
-
-			count := 0
-			for i := range env.inProgressCompactions {
-				c := &env.inProgressCompactions[i]
-				if c.inputs[0].level != info.level {
-					continue
-				}
-				count++
-				if count == 1 {
-					fmt.Fprintf(&buf, "  [")
-				} else {
-					fmt.Fprintf(&buf, " ")
-				}
-				fmt.Fprintf(&buf, "L%d->L%d", c.inputs[0].level, c.outputLevel)
+		count := 0
+		for i := range env.inProgressCompactions {
+			c := &env.inProgressCompactions[i]
+			if c.inputs[0].level != info.level {
+				continue
 			}
-			if count > 0 {
-				fmt.Fprintf(&buf, "]")
+			count++
+			if count == 1 {
+				fmt.Fprintf(&buf, "  [")
+			} else {
+				fmt.Fprintf(&buf, " ")
 			}
-			fmt.Fprintf(&buf, "\n")
+			fmt.Fprintf(&buf, "L%d->L%d", c.inputs[0].level, c.outputLevel)
 		}
-		p.opts.Logger.Infof("pickAuto: L%d->L%d\n%s",
-			pc.startLevel.level, pc.outputLevel.level, buf.String())
+		if count > 0 {
+			fmt.Fprintf(&buf, "]")
+		}
+		fmt.Fprintf(&buf, "\n")
 	}
+	p.opts.Logger.Infof("pickAuto: L%d->L%d\n%s",
+		pc.startLevel.level, pc.outputLevel.level, buf.String())
+}
+
+// pickAutoScore picks the best score-based compaction, if any.
+//
+// On each call, pickAutoScore computes per-level size adjustments based on
+// in-progress compactions, and computes a per-level score. The levels are
+// iterated over in decreasing score order trying to find a valid compaction
+// anchored at that level.
+//
+// If a score-based compaction cannot be found, pickAuto falls back to looking
+// for an elision-only compaction to remove obsolete keys.
+func (p *compactionPickerByScore) pickAutoScore(env compactionEnv) (pc *pickedCompaction) {
+	scores := p.calculateLevelScores(env.inProgressCompactions)
 
 	// Check for a score-based compaction. candidateLevelInfos are first sorted
 	// by whether they should be compacted, so if we find a level which shouldn't
@@ -1328,9 +1327,8 @@ func (p *compactionPickerByScore) pickAuto(env compactionEnv) (pc *pickedCompact
 			if pc != nil && !inputRangeAlreadyCompacting(env, pc) {
 				p.addScoresToPickedCompactionMetrics(pc, scores)
 				pc.score = info.compensatedScoreRatio
-				// TODO(bananabrick): Create an EventListener for logCompaction.
 				if false {
-					logCompaction(pc)
+					p.logCompactionForTesting(env, scores, pc)
 				}
 				return pc
 			}
@@ -1349,14 +1347,17 @@ func (p *compactionPickerByScore) pickAuto(env compactionEnv) (pc *pickedCompact
 		if pc != nil && !inputRangeAlreadyCompacting(env, pc) {
 			p.addScoresToPickedCompactionMetrics(pc, scores)
 			pc.score = info.compensatedScoreRatio
-			// TODO(bananabrick): Create an EventListener for logCompaction.
 			if false {
-				logCompaction(pc)
+				p.logCompactionForTesting(env, scores, pc)
 			}
 			return pc
 		}
 	}
+	return nil
+}
 
+// pickAutoNonScore picks the best non-score-based compaction, if any.
+func (p *compactionPickerByScore) pickAutoNonScore(env compactionEnv) (pc *pickedCompaction) {
 	// Check for files which contain excessive point tombstones that could slow
 	// down reads. Unlike elision-only compactions, these compactions may select
 	// a file at any level rather than only the lowest level.

compaction_picker_test.go

@@ -198,7 +198,7 @@ func TestCompactionPickerTargetLevel(t *testing.T) {
 		if inProgressCompactions >= allowedCompactions {
 			return nil
 		}
-		return pickerByScore.pickAuto(env)
+		return pickerByScore.pickAutoScore(env)
 	}
 
 	datadriven.RunTest(t, "testdata/compaction_picker_target_level",
@@ -572,11 +572,20 @@ func TestCompactionPickerL0(t *testing.T) {
 			td.MaybeScanArgs(t, "l0_compaction_threshold", &opts.L0CompactionThreshold)
 			td.MaybeScanArgs(t, "l0_compaction_file_threshold", &opts.L0CompactionFileThreshold)
 
-			pc = picker.pickAuto(compactionEnv{
+			score := true
+			if td.HasArg("non-score") {
+				score = false
+			}
+			env := compactionEnv{
 				diskAvailBytes:          math.MaxUint64,
 				earliestUnflushedSeqNum: math.MaxUint64,
 				inProgressCompactions:   inProgressCompactions,
-			})
+			}
+			if score {
+				pc = picker.pickAutoScore(env)
+			} else {
+				pc = picker.pickAutoNonScore(env)
+			}
 			var result strings.Builder
 			if pc != nil {
 				checkClone(t, pc)
@@ -804,7 +813,7 @@ func TestCompactionPickerConcurrency(t *testing.T) {
 			allowedCompactions := picker.getCompactionConcurrency()
 			var pc *pickedCompaction
 			if inProgressCount < allowedCompactions {
-				pc = picker.pickAuto(env)
+				pc = picker.pickAutoScore(env)
 			}
 			var result strings.Builder
 			fmt.Fprintf(&result, "picker.getCompactionConcurrency: %d\n", allowedCompactions)
@@ -946,14 +955,20 @@ func TestCompactionPickerPickReadTriggered(t *testing.T) {
 			return sb.String()
 
 		case "pick-auto":
-			pc := picker.pickAuto(compactionEnv{
+			var result strings.Builder
+			var pc *pickedCompaction
+			env := compactionEnv{
 				earliestUnflushedSeqNum: math.MaxUint64,
 				readCompactionEnv: readCompactionEnv{
 					readCompactions: &rcList,
 					flushing:        false,
 				},
-			})
-			var result strings.Builder
+			}
+			if pc = picker.pickAutoScore(env); pc != nil {
+				fmt.Fprintf(&result, "picked score-based compaction:\n")
+			} else if pc = picker.pickAutoNonScore(env); pc != nil {
+				fmt.Fprintf(&result, "picked non-score-based compaction:\n")
+			}
 			if pc != nil {
 				fmt.Fprintf(&result, "L%d -> L%d\n", pc.startLevel.level, pc.outputLevel.level)
 				fmt.Fprintf(&result, "L%d: %s\n", pc.startLevel.level, fileNums(pc.startLevel.files))
@@ -1334,7 +1349,7 @@ func TestCompactionOutputFileSize(t *testing.T) {
 			return buf.String()
 
 		case "pick-auto":
-			pc := picker.pickAuto(compactionEnv{
+			pc := picker.pickAutoNonScore(compactionEnv{
 				earliestUnflushedSeqNum: math.MaxUint64,
 				earliestSnapshotSeqNum:  math.MaxUint64,
 			})

compaction_scheduler.go

@@ -125,18 +125,20 @@ var scheduledCompactionMap map[compactionKind]compactionOptionalAndPriority
 var manualCompactionPriority int
 
 func init() {
-	// Manual compactions have the highest priority since DB.pickAnyCompaction
-	// first picks a manual compaction, before calling
-	// compactionPickerByScore.pickAuto.
-	manualCompactionPriority = 100
+	// Manual compactions have priority just below the score-rebased
+	// compactions, since DB.pickAnyCompaction first picks score-based
+	// compactions, and then manual compactions.
+	manualCompactionPriority = 70
 	scheduledCompactionMap = map[compactionKind]compactionOptionalAndPriority{}
+	// Score-based-compactions have priorities {100, 90, 80}.
+	//
 	// We don't actually know if it is a compactionKindMove or
 	// compactionKindCopy until a compactionKindDefault is turned from a
 	// pickedCompaction into a compaction struct. So we will never see those
 	// values here, but for completeness we include them.
-	scheduledCompactionMap[compactionKindMove] = compactionOptionalAndPriority{priority: 90}
-	scheduledCompactionMap[compactionKindCopy] = compactionOptionalAndPriority{priority: 80}
-	scheduledCompactionMap[compactionKindDefault] = compactionOptionalAndPriority{priority: 70}
+	scheduledCompactionMap[compactionKindMove] = compactionOptionalAndPriority{priority: 100}
+	scheduledCompactionMap[compactionKindCopy] = compactionOptionalAndPriority{priority: 90}
+	scheduledCompactionMap[compactionKindDefault] = compactionOptionalAndPriority{priority: 80}
 	scheduledCompactionMap[compactionKindTombstoneDensity] =
 		compactionOptionalAndPriority{optional: true, priority: 60}
 	scheduledCompactionMap[compactionKindElisionOnly] =

compaction_test.go

@@ -86,7 +86,7 @@ func (p *compactionPickerForTesting) estimatedCompactionDebt(l0ExtraSize uint64)
 
 func (p *compactionPickerForTesting) forceBaseLevel1() {}
 
-func (p *compactionPickerForTesting) pickAuto(env compactionEnv) (pc *pickedCompaction) {
+func (p *compactionPickerForTesting) pickAutoScore(env compactionEnv) (pc *pickedCompaction) {
 	if p.score < 1 {
 		return nil
 	}
@@ -107,21 +107,7 @@ func (p *compactionPickerForTesting) pickAuto(env compactionEnv) (pc *pickedComp
 	return pickAutoLPositive(env, p.opts, p.vers, p.l0Organizer, cInfo, p.baseLevel)
 }
 
-func (p *compactionPickerForTesting) pickElisionOnlyCompaction(
-	env compactionEnv,
-) (pc *pickedCompaction) {
-	return nil
-}
-
-func (p *compactionPickerForTesting) pickRewriteCompaction(
-	env compactionEnv,
-) (pc *pickedCompaction) {
-	return nil
-}
-
-func (p *compactionPickerForTesting) pickReadTriggeredCompaction(
-	env compactionEnv,
-) (pc *pickedCompaction) {
+func (p *compactionPickerForTesting) pickAutoNonScore(env compactionEnv) (pc *pickedCompaction) {
 	return nil
 }
 
@@ -528,7 +514,7 @@ func TestPickCompaction(t *testing.T) {
 		vs.versions.Init(nil)
 		vs.append(tc.picker.vers)
 		vs.picker = &tc.picker
-		pc, got := vs.picker.pickAuto(compactionEnv{diskAvailBytes: math.MaxUint64}), ""
+		pc, got := vs.picker.pickAutoScore(compactionEnv{diskAvailBytes: math.MaxUint64}), ""
 		if pc != nil {
 			c := newCompaction(pc, opts, time.Now(), nil /* provider */, noopGrantHandle{}, neverSeparateValues)
 
@@ -1075,6 +1061,14 @@ func TestCompaction(t *testing.T) {
 				d.mu.Unlock()
 				return describeLSM(d, verbose)
 
+			case "set-disable-auto-compact":
+				var v bool
+				td.ScanArgs(t, "v", &v)
+				d.mu.Lock()
+				d.opts.DisableAutomaticCompactions = v
+				d.mu.Unlock()
+				return ""
+
 			case "async-compact":
 				var s string
 				ch := make(chan error, 1)
@@ -1236,6 +1230,12 @@ func TestCompaction(t *testing.T) {
 			maxVersion: FormatExperimentalValueSeparation,
 			verbose:    true,
 		},
+		"score_compaction_picked_before_manual": {
+			// Run at a specific version, so that a single sstable format is used,
+			// since the test prints the compaction log which includes file sizes.
+			minVersion: FormatExperimentalValueSeparation,
+			maxVersion: FormatExperimentalValueSeparation,
+		},
 	}
 	datadriven.Walk(t, "testdata/compaction", func(t *testing.T, path string) {
 		filename := filepath.Base(path)