db: consolidate datadriven DB opts parsing; fix block-size bug

This commit cleans up and consolidates a couple places where we parsed
datadriven command args for configuring pebble.Options. It fixes a bug where
the block-size command arg would be ignored in one parse site.

data_test.go

@@ -13,6 +13,7 @@ import (
 	"math"
 	"math/rand/v2"
 	"regexp"
+	"slices"
 	"strconv"
 	"strings"
 	"testing"
@@ -791,20 +792,14 @@ func runDBDefineCmd(td *datadriven.TestData, opts *Options) (*DB, error) {
 // the caller to have set an appropriate FS already.
 func runDBDefineCmdReuseFS(td *datadriven.TestData, opts *Options) (*DB, error) {
 	opts = opts.EnsureDefaults()
+	if err := parseDBOptionsArgs(opts, td.CmdArgs); err != nil {
+		return nil, err
+	}
 
 	var snapshots []base.SeqNum
 	var levelMaxBytes map[int]int64
 	for _, arg := range td.CmdArgs {
 		switch arg.Key {
-		case "target-file-sizes":
-			opts.Levels = make([]LevelOptions, len(arg.Vals))
-			for i := range arg.Vals {
-				size, err := strconv.ParseInt(arg.Vals[i], 10, 64)
-				if err != nil {
-					return nil, err
-				}
-				opts.Levels[i].TargetFileSize = size
-			}
 		case "snapshots":
 			snapshots = make([]base.SeqNum, len(arg.Vals))
 			for i := range arg.Vals {
@@ -813,12 +808,6 @@ func runDBDefineCmdReuseFS(td *datadriven.TestData, opts *Options) (*DB, error)
 					return nil, errors.New("Snapshots must be in ascending order")
 				}
 			}
-		case "lbase-max-bytes":
-			lbaseMaxBytes, err := strconv.ParseInt(arg.Vals[0], 10, 64)
-			if err != nil {
-				return nil, err
-			}
-			opts.LBaseMaxBytes = lbaseMaxBytes
 		case "level-max-bytes":
 			levelMaxBytes = map[int]int64{}
 			for i := range arg.Vals {
@@ -834,89 +823,32 @@ func runDBDefineCmdReuseFS(td *datadriven.TestData, opts *Options) (*DB, error)
 				}
 				levelMaxBytes[level] = size
 			}
-		case "memtable-size":
-			memTableSize, err := strconv.ParseUint(arg.Vals[0], 10, 64)
-			if err != nil {
-				return nil, err
-			}
-			opts.MemTableSize = memTableSize
-		case "auto-compactions":
-			switch arg.Vals[0] {
-			case "off":
-				opts.DisableAutomaticCompactions = true
-			case "on":
-				opts.DisableAutomaticCompactions = false
-			default:
-				return nil, errors.Errorf("Unrecognized %q %q arg value: %q", td.Cmd, arg.Key, arg.Vals[0])
-			}
-		case "enable-table-stats":
-			enable, err := strconv.ParseBool(arg.Vals[0])
-			if err != nil {
-				return nil, errors.Errorf("%s: could not parse %q as bool: %s", td.Cmd, arg.Vals[0], err)
-			}
-			opts.DisableTableStats = !enable
-		case "block-size":
-			size, err := strconv.Atoi(arg.Vals[0])
-			if err != nil {
-				return nil, err
-			}
-			for _, levelOpts := range opts.Levels {
-				levelOpts.BlockSize = size
-			}
-		case "bloom-bits-per-key":
-			v, err := strconv.Atoi(arg.Vals[0])
-			if err != nil {
-				return nil, err
-			}
-			fp := bloom.FilterPolicy(v)
-			opts.Filters = map[string]FilterPolicy{fp.Name(): fp}
-			for i := range opts.Levels {
-				opts.Levels[i].FilterPolicy = fp
-			}
-		case "format-major-version":
-			fmv, err := strconv.Atoi(arg.Vals[0])
-			if err != nil {
-				return nil, err
-			}
-			opts.FormatMajorVersion = FormatMajorVersion(fmv)
-		case "disable-multi-level":
-			opts.Experimental.MultiLevelCompactionHeuristic = NoMultiLevel{}
 		}
 	}
 
-	// This is placed after the argument parsing above, because the arguments
-	// to define should be parsed even if td.Input is empty.
-	if td.Input == "" {
-		// Empty LSM.
-		d, err := Open("", opts)
-		if err != nil {
-			return nil, err
-		}
-		d.mu.Lock()
-		for i := range snapshots {
-			s := &Snapshot{db: d}
-			s.seqNum = snapshots[i]
-			d.mu.snapshots.pushBack(s)
-		}
-		for l, maxBytes := range levelMaxBytes {
-			d.mu.versions.picker.(*compactionPickerByScore).levelMaxBytes[l] = maxBytes
-		}
-		d.mu.Unlock()
-		return d, nil
-	}
-
 	d, err := Open("", opts)
 	if err != nil {
 		return nil, err
 	}
 	d.mu.Lock()
-	d.mu.versions.dynamicBaseLevel = false
+	defer d.mu.Unlock()
 	for i := range snapshots {
 		s := &Snapshot{db: d}
 		s.seqNum = snapshots[i]
 		d.mu.snapshots.pushBack(s)
 	}
-	defer d.mu.Unlock()
+	// Set the level max bytes only right before we exit; the body of this
+	// function expects it to be unset.
+	defer func() {
+		for l, maxBytes := range levelMaxBytes {
+			d.mu.versions.picker.(*compactionPickerByScore).levelMaxBytes[l] = maxBytes
+		}
+	}()
+	if td.Input == "" {
+		// Empty LSM.
+		return d, nil
+	}
+	d.mu.versions.dynamicBaseLevel = false
 
 	var mem *memTable
 	var start, end *base.InternalKey
@@ -1119,10 +1051,6 @@ func runDBDefineCmdReuseFS(td *datadriven.TestData, opts *Options) (*DB, error)
 		d.updateTableStatsLocked(ve.NewFiles)
 	}
 
-	for l, maxBytes := range levelMaxBytes {
-		d.mu.versions.picker.(*compactionPickerByScore).levelMaxBytes[l] = maxBytes
-	}
-
 	return d, nil
 }
 
@@ -1466,36 +1394,23 @@ func parseDBOptionsArgs(opts *Options, args []datadriven.CmdArg) error {
 			default:
 				return errors.Errorf("Unrecognized %q arg value: %q", cmdArg.Key, cmdArg.Vals[0])
 			}
-		case "inject-errors":
-			injs := make([]errorfs.Injector, len(cmdArg.Vals))
-			for i := 0; i < len(cmdArg.Vals); i++ {
-				inj, err := errorfs.ParseDSL(cmdArg.Vals[i])
-				if err != nil {
-					return err
-				}
-				injs[i] = inj
-			}
-			opts.FS = errorfs.Wrap(opts.FS, errorfs.Any(injs...))
-		case "enable-table-stats":
-			enable, err := strconv.ParseBool(cmdArg.Vals[0])
-			if err != nil {
-				return errors.Errorf("%s: could not parse %q as bool: %s", cmdArg.Key, cmdArg.Vals[0], err)
-			}
-			opts.DisableTableStats = !enable
-		case "format-major-version":
+		case "block-size":
 			v, err := strconv.Atoi(cmdArg.Vals[0])
 			if err != nil {
 				return err
 			}
-			// Override the DB version.
-			opts.FormatMajorVersion = FormatMajorVersion(v)
-		case "block-size":
+			for i := range opts.Levels {
+				opts.Levels[i].BlockSize = v
+			}
+		case "bloom-bits-per-key":
 			v, err := strconv.Atoi(cmdArg.Vals[0])
 			if err != nil {
 				return err
 			}
+			fp := bloom.FilterPolicy(v)
+			opts.Filters = map[string]FilterPolicy{fp.Name(): fp}
 			for i := range opts.Levels {
-				opts.Levels[i].BlockSize = v
+				opts.Levels[i].FilterPolicy = fp
 			}
 		case "cache-size":
 			if opts.Cache != nil {
@@ -1507,39 +1422,68 @@ func parseDBOptionsArgs(opts *Options, args []datadriven.CmdArg) error {
 				return err
 			}
 			opts.Cache = NewCache(size)
-		case "index-block-size":
+		case "disable-multi-level":
+			opts.Experimental.MultiLevelCompactionHeuristic = NoMultiLevel{}
+		case "enable-table-stats":
+			enable, err := strconv.ParseBool(cmdArg.Vals[0])
+			if err != nil {
+				return errors.Errorf("%s: could not parse %q as bool: %s", cmdArg.Key, cmdArg.Vals[0], err)
+			}
+			opts.DisableTableStats = !enable
+		case "format-major-version":
 			v, err := strconv.Atoi(cmdArg.Vals[0])
 			if err != nil {
 				return err
 			}
-			for i := range opts.Levels {
-				opts.Levels[i].IndexBlockSize = v
-			}
-		case "target-file-size":
+			opts.FormatMajorVersion = FormatMajorVersion(v)
+		case "index-block-size":
 			v, err := strconv.Atoi(cmdArg.Vals[0])
 			if err != nil {
 				return err
 			}
 			for i := range opts.Levels {
-				opts.Levels[i].TargetFileSize = int64(v)
+				opts.Levels[i].IndexBlockSize = v
 			}
-		case "bloom-bits-per-key":
-			v, err := strconv.Atoi(cmdArg.Vals[0])
+		case "inject-errors":
+			injs := make([]errorfs.Injector, len(cmdArg.Vals))
+			for i := 0; i < len(cmdArg.Vals); i++ {
+				inj, err := errorfs.ParseDSL(cmdArg.Vals[i])
+				if err != nil {
+					return err
+				}
+				injs[i] = inj
+			}
+			opts.FS = errorfs.Wrap(opts.FS, errorfs.Any(injs...))
+		case "lbase-max-bytes":
+			lbaseMaxBytes, err := strconv.ParseInt(cmdArg.Vals[0], 10, 64)
 			if err != nil {
 				return err
 			}
-			fp := bloom.FilterPolicy(v)
-			opts.Filters = map[string]FilterPolicy{fp.Name(): fp}
-			for i := range opts.Levels {
-				opts.Levels[i].FilterPolicy = fp
+			opts.LBaseMaxBytes = lbaseMaxBytes
+		case "memtable-size":
+			memTableSize, err := strconv.ParseUint(cmdArg.Vals[0], 10, 64)
+			if err != nil {
+				return err
 			}
+			opts.MemTableSize = memTableSize
 		case "merger":
 			switch cmdArg.Vals[0] {
 			case "appender":
 				opts.Merger = base.DefaultMerger
 			default:
 				return errors.Newf("unrecognized Merger %q\n", cmdArg.Vals[0])
 			}
+		case "target-file-sizes":
+			if len(opts.Levels) < len(cmdArg.Vals) {
+				opts.Levels = slices.Grow(opts.Levels, len(cmdArg.Vals)-len(opts.Levels))[0:len(cmdArg.Vals)]
+			}
+			for i := range cmdArg.Vals {
+				size, err := strconv.ParseInt(cmdArg.Vals[i], 10, 64)
+				if err != nil {
+					return err
+				}
+				opts.Levels[i].TargetFileSize = size
+			}
 		}
 	}
 	return nil

testdata/iter_histories/next_prefix

@@ -84,7 +84,7 @@ p@10: (p@10, .)
 p@1: (p@1, .)
 q@100: (q@100, .)
 
-reset target-file-size=1
+reset target-file-sizes=(1)
 ----
 
 populate keylen=1 timestamps=(1, 10, 100)

testdata/iter_histories/prefix_iteration

@@ -68,7 +68,7 @@ d@7: (., [d-"d\x00") @5=foo UPDATED)
 # Test a LSM with range keys fragmented within a prefix.
 # This is a regression test for cockroachdb/cockroach#86102.
 
-reset target-file-size=1
+reset target-file-sizes=(1)
 ----
 
 batch commit

testdata/table_stats

@@ -209,7 +209,7 @@ num-entries: 4
 num-deletions: 2
 num-range-key-sets: 0
 point-deletions-bytes-estimate: 0
-range-deletions-bytes-estimate: 68
+range-deletions-bytes-estimate: 78
 
 # Hints that partially overlap tables in lower levels only count blocks that are
 # contained within the hint.
@@ -247,7 +247,7 @@ num-entries: 1
 num-deletions: 1
 num-range-key-sets: 0
 point-deletions-bytes-estimate: 0
-range-deletions-bytes-estimate: 33
+range-deletions-bytes-estimate: 52
 
 # Table 000005 deletes the second block in table 000006 (containing 'd') and the
 # first block in table 000007 (containing 'e').
@@ -258,7 +258,7 @@ num-entries: 1
 num-deletions: 1
 num-range-key-sets: 0
 point-deletions-bytes-estimate: 0
-range-deletions-bytes-estimate: 66
+range-deletions-bytes-estimate: 78
 
 # Test the interaction between point and range key deletions.
 
@@ -491,7 +491,7 @@ range-deletions-bytes-estimate: 26
 
 # Test point tombstone compensation that uses DELSIZED keys.
 
-define format-major-version=15
+define format-major-version=15 block-size=32768
 L6
   bar.SET.0:<rand-bytes=10>
   bax.SET.0:<rand-bytes=10>