db: fix elision-only compaction max output size

Elision-only compactions mistakenly passed L6 as the base level when constructing a picked compaction. This caused these compactions to be initialized with a target file size of L1. Fix cockroachdb#1097.
sumeerbhola · May 3, 2021 · 1387689 · 1387689
1 parent f8e879e
commit 1387689
Show file tree

Hide file tree

Showing 3 changed files with 178 additions and 28 deletions.
diff --git a/compaction_picker.go b/compaction_picker.go
@@ -1083,7 +1083,7 @@ func (p *compactionPickerByScore) pickElisionOnlyCompaction(
 
 	// Construct a picked compaction of the elision candidate's atomic
 	// compaction unit.
-	pc = newPickedCompaction(p.opts, p.vers, numLevels-1, numLevels-1)
+	pc = newPickedCompaction(p.opts, p.vers, numLevels-1, p.baseLevel)
 	var isCompacting bool
 	pc.startLevel.files, isCompacting = expandToAtomicUnit(p.opts.Comparer.Compare, lf.Slice(), false /* disableIsCompacting */)
 	if isCompacting {

diff --git a/compaction_picker_test.go b/compaction_picker_test.go
@@ -433,15 +433,6 @@ func TestCompactionPickerIntraL0(t *testing.T) {
 }
 
 func TestCompactionPickerL0(t *testing.T) {
-	fileNums := func(files manifest.LevelSlice) string {
-		var ss []string
-		files.Each(func(f *fileMetadata) {
-			ss = append(ss, f.FileNum.String())
-		})
-		sort.Strings(ss)
-		return strings.Join(ss, ",")
-	}
-
 	parseMeta := func(s string) (*fileMetadata, error) {
 		parts := strings.Split(s, ":")
 		fileNum, err := strconv.Atoi(parts[0])
@@ -654,15 +645,6 @@ func TestCompactionPickerL0(t *testing.T) {
 }
 
 func TestCompactionPickerConcurrency(t *testing.T) {
-	fileNums := func(files manifest.LevelSlice) string {
-		var ss []string
-		files.Each(func(f *fileMetadata) {
-			ss = append(ss, f.FileNum.String())
-		})
-		sort.Strings(ss)
-		return strings.Join(ss, ",")
-	}
-
 	parseMeta := func(s string) (*fileMetadata, error) {
 		parts := strings.Split(s, ":")
 		fileNum, err := strconv.Atoi(parts[0])
@@ -881,15 +863,6 @@ func TestCompactionPickerPickReadTriggered(t *testing.T) {
 	var rcList []readCompaction
 	var vers *version
 
-	fileNums := func(files manifest.LevelSlice) string {
-		var ss []string
-		files.Each(func(f *fileMetadata) {
-			ss = append(ss, f.FileNum.String())
-		})
-		sort.Strings(ss)
-		return strings.Join(ss, ",")
-	}
-
 	parseMeta := func(s string) (*fileMetadata, error) {
 		parts := strings.Split(s, ":")
 		fileNum, err := strconv.Atoi(parts[0])
@@ -1204,3 +1177,125 @@ func TestPickedCompactionExpandInputs(t *testing.T) {
 			}
 		})
 }
+
+func TestCompactionOutputFileSize(t *testing.T) {
+	opts := (*Options)(nil).EnsureDefaults()
+	var picker *compactionPickerByScore
+	var vers *version
+
+	parseMeta := func(s string) (*fileMetadata, error) {
+		parts := strings.Split(s, ":")
+		fileNum, err := strconv.Atoi(parts[0])
+		if err != nil {
+			return nil, err
+		}
+		fields := strings.Fields(parts[1])
+		parts = strings.Split(fields[0], "-")
+		if len(parts) != 2 {
+			return nil, errors.Errorf("malformed table spec: %s. usage: <file-num>:start.SET.1-end.SET.2", s)
+		}
+		m := &fileMetadata{
+			FileNum:  base.FileNum(fileNum),
+			Size:     1028,
+			Smallest: base.ParseInternalKey(strings.TrimSpace(parts[0])),
+			Largest:  base.ParseInternalKey(strings.TrimSpace(parts[1])),
+		}
+		for _, p := range fields[1:] {
+			if strings.HasPrefix(p, "size=") {
+				v, err := strconv.Atoi(strings.TrimPrefix(p, "size="))
+				if err != nil {
+					return nil, err
+				}
+				m.Size = uint64(v)
+			}
+			if strings.HasPrefix(p, "range-deletions-bytes-estimate=") {
+				v, err := strconv.Atoi(strings.TrimPrefix(p, "range-deletions-bytes-estimate="))
+				if err != nil {
+					return nil, err
+				}
+				m.Stats.Valid = true
+				m.Stats.RangeDeletionsBytesEstimate = uint64(v)
+			}
+		}
+		m.SmallestSeqNum = m.Smallest.SeqNum()
+		m.LargestSeqNum = m.Largest.SeqNum()
+		return m, nil
+	}
+
+	datadriven.RunTest(t, "testdata/compaction_output_file_size", func(td *datadriven.TestData) string {
+		switch td.Cmd {
+		case "define":
+			fileMetas := [manifest.NumLevels][]*fileMetadata{}
+			level := 0
+			var err error
+			lines := strings.Split(td.Input, "\n")
+
+			for len(lines) > 0 {
+				data := strings.TrimSpace(lines[0])
+				lines = lines[1:]
+				switch data {
+				case "L0", "L1", "L2", "L3", "L4", "L5", "L6":
+					level, err = strconv.Atoi(data[1:])
+					if err != nil {
+						return err.Error()
+					}
+				default:
+					meta, err := parseMeta(data)
+					if err != nil {
+						return err.Error()
+					}
+					fileMetas[level] = append(fileMetas[level], meta)
+				}
+			}
+
+			vers = newVersion(opts, fileMetas)
+			vs := &versionSet{
+				opts:    opts,
+				cmp:     DefaultComparer.Compare,
+				cmpName: DefaultComparer.Name,
+			}
+			vs.versions.Init(nil)
+			vs.append(vers)
+			var sizes [numLevels]int64
+			for l := 0; l < len(sizes); l++ {
+				slice := vers.Levels[l].Slice()
+				sizes[l] = int64(slice.SizeSum())
+			}
+			var inProgressCompactions []compactionInfo
+			picker = newCompactionPicker(vers, opts, inProgressCompactions, sizes).(*compactionPickerByScore)
+			vs.picker = picker
+
+			var buf bytes.Buffer
+			fmt.Fprint(&buf, vers.DebugString(base.DefaultFormatter))
+			return buf.String()
+
+		case "pick-auto":
+			pc := picker.pickAuto(compactionEnv{
+				bytesCompacted:          new(uint64),
+				earliestUnflushedSeqNum: math.MaxUint64,
+				earliestSnapshotSeqNum:  math.MaxUint64,
+			})
+			var buf bytes.Buffer
+			if pc != nil {
+				fmt.Fprintf(&buf, "L%d -> L%d\n", pc.startLevel.level, pc.outputLevel.level)
+				fmt.Fprintf(&buf, "L%d: %s\n", pc.startLevel.level, fileNums(pc.startLevel.files))
+				fmt.Fprintf(&buf, "maxOutputFileSize: %d\n", pc.maxOutputFileSize)
+			} else {
+				return "nil"
+			}
+			return buf.String()
+
+		default:
+			return fmt.Sprintf("unrecognized command: %s", td.Cmd)
+		}
+	})
+}
+
+func fileNums(files manifest.LevelSlice) string {
+	var ss []string
+	files.Each(func(f *fileMetadata) {
+		ss = append(ss, f.FileNum.String())
+	})
+	sort.Strings(ss)
+	return strings.Join(ss, ",")
+}
diff --git a/testdata/compaction_output_file_size b/testdata/compaction_output_file_size
@@ -0,0 +1,55 @@
+define
+L3
+  010001:a.SET.1111-f.SET.1112 size=10
+  010002:g.SET.1111-l.SET.1112 size=10
+L4
+  001001:a.SET.111-f.SET.112 size=100
+  001002:g.SET.111-l.SET.112 size=100
+L5
+  000101:a.SET.11-f.SET.12 size=1000
+  000102:g.SET.11-l.SET.12 size=1000
+L6
+  000010:a.SET.1-f.SET.2 size=128000000
+  000011:g.SET.1-l.SET.2 size=128000000 range-deletions-bytes-estimate=28000000
+----
+3:
+  010001:[a#1111,SET-f#1112,SET]
+  010002:[g#1111,SET-l#1112,SET]
+4:
+  001001:[a#111,SET-f#112,SET]
+  001002:[g#111,SET-l#112,SET]
+5:
+  000101:[a#11,SET-f#12,SET]
+  000102:[g#11,SET-l#12,SET]
+6:
+  000010:[a#1,SET-f#2,SET]
+  000011:[g#1,SET-l#2,SET]
+
+# Max output file size should be 32MiB because Lbase is L3.
+pick-auto
+----
+L6 -> L6
+L6: 000011
+maxOutputFileSize: 33554432
+
+define
+L5
+  000101:a.SET.11-f.SET.12 size=1000
+  000102:g.SET.11-l.SET.12 size=1000
+L6
+  000010:a.SET.1-f.SET.2 size=128000000
+  000011:g.SET.1-l.SET.2 size=128000000 range-deletions-bytes-estimate=28000000
+----
+5:
+  000101:[a#11,SET-f#12,SET]
+  000102:[g#11,SET-l#12,SET]
+6:
+  000010:[a#1,SET-f#2,SET]
+  000011:[g#1,SET-l#2,SET]
+
+# Max output file size should be 8MiB because Lbase is L5.
+pick-auto
+----
+L6 -> L6
+L6: 000011
+maxOutputFileSize: 8388608