db: disable writing to value blocks if DisableSeparationBySuffix is true

Make sure this field, which has been moved from SpanPolicy to
ValueStoragePolicy, is read for the DisableValueBlocks option in the sst writer.

Author: xinhaoz

compaction.go

@@ -3480,11 +3480,11 @@ func (d *DB) compactAndWrite(
 		}
 		vSep := valueSeparation
 		switch spanPolicy.ValueStoragePolicy.PolicyAdjustment {
-		case UseDefaultValueStorage:
+		case UseDefaultValueSeparation:
 			// No change to value separation.
-		case NoValueSeparation:
+		case NoBlobHandles:
 			vSep = valsep.NeverSeparateValues{}
-		case OverrideValueStorage:
+		case OverrideValueSeparation:
 			// This span of keyspace is more tolerant of latency, so set a more
 			// aggressive value separation policy for this output.
 			vSep.SetNextOutputConfig(valsep.ValueSeparationOutputConfig{

compaction_test.go

@@ -1530,24 +1530,23 @@ func runCompactionTest(
 						if len(parts) != 1 {
 							td.Fatalf(t, "expected disable-separation-by-suffix with no value, got: %s", arg)
 						}
-						if policy.ValueStoragePolicy.PolicyAdjustment == NoValueSeparation {
-							td.Fatalf(t, "conflicting value storage policies for span: %s", line)
+						if policy.ValueStoragePolicy.PolicyAdjustment == UseDefaultValueSeparation {
+							policy.ValueStoragePolicy.PolicyAdjustment = OverrideValueSeparation
 						}
-						policy.ValueStoragePolicy.PolicyAdjustment = OverrideValueStorage
 						policy.ValueStoragePolicy.DisableSeparationBySuffix = true
 					case "val-sep-minimum-size":
 						if len(parts) != 2 {
 							td.Fatalf(t, "expected val-sep-minimum-size=<size>, got: %s", arg)
 						}
 						size, err := strconv.ParseUint(parts[1], 10, 64)
 						if err != nil {
-							td.Fatalf(t, "parsing value-minimum-size: %s", err)
+							td.Fatalf(t, "parsing val-sep-minimum-size: %s", err)
 						}
 						policy.ValueStoragePolicy.MinimumSize = int(size)
 						if size == 0 {
-							policy.ValueStoragePolicy.PolicyAdjustment = NoValueSeparation
+							policy.ValueStoragePolicy.PolicyAdjustment = NoBlobHandles
 						} else if int(size) != d.opts.Experimental.ValueSeparationPolicy().MinimumSize {
-							policy.ValueStoragePolicy.PolicyAdjustment = OverrideValueStorage
+							policy.ValueStoragePolicy.PolicyAdjustment = OverrideValueSeparation
 						}
 					default:
 						td.Fatalf(t, "unknown span policy arg: %s", arg)

compaction_value_separation.go

@@ -138,9 +138,9 @@ func shouldWriteBlobFiles(
 					return true, 0
 				}
 				switch spanPolicy.ValueStoragePolicy.PolicyAdjustment {
-				case UseDefaultValueStorage:
+				case UseDefaultValueSeparation:
 					// Use the global policy.
-				case OverrideValueStorage:
+				case OverrideValueSeparation:
 					expectedMinSize = spanPolicy.ValueStoragePolicy.MinimumSize
 					if expectedMinSize == 0 {
 						// A 0 minimum value size on the span policy indicates the field
@@ -149,7 +149,7 @@ func shouldWriteBlobFiles(
 						expectedMinSize = policy.MinimumSize
 					}
 					expectedValSepBySuffixDisabled = spanPolicy.ValueStoragePolicy.DisableSeparationBySuffix
-				case NoValueSeparation:
+				case NoBlobHandles:
 					expectedMinSize = 0
 				}
 			}

options.go

@@ -1298,23 +1298,18 @@ func (p SpanPolicy) String() string {
 		sb.WriteString("disable-value-separation-by-suffix,")
 	}
 	switch p.ValueStoragePolicy.PolicyAdjustment {
-	case NoValueSeparation:
-		sb.WriteString("no-value-separation,")
-	case OverrideValueStorage:
-		sb.WriteString("override,")
+	case NoBlobHandles:
+		sb.WriteString("no-blob-value-separation,")
+	case OverrideValueSeparation:
+		sb.WriteString("override-value-separation,")
 	}
 	return strings.TrimSuffix(sb.String(), ",")
 }
 
 // ValueStoragePolicyAdjustment is used to determine where to store the values for
-// KVs. If the PolicyAdjustment specified is OverrideValueStorage, the remaining fields
-// are used to override the global configuration for value separation.
+// KVs, overriding global policies. Values can be configured to be stored in-place,
+// in value blocks, or in blob files.
 type ValueStoragePolicyAdjustment struct {
-	// PolicyAdjustment specifies the policy adjustment to apply.
-	PolicyAdjustment ValueStoragePolicyAdjustmentType
-
-	// Remaining fields are ignored, unless the PolicyAdjustment is OverrideValueStorage.
-
 	// DisableSeparationBySuffix disables discriminating KVs depending on
 	// suffix.
 	//
@@ -1323,6 +1318,13 @@ type ValueStoragePolicyAdjustment struct {
 	// keys (where the smallest suffix is the latest version), but should be
 	// disabled for keys where the suffix does not correspond to a version.
 	DisableSeparationBySuffix bool
+
+	// PolicyAdjustment specifies the policy adjustment to apply for value separation
+	// into blob files.
+	PolicyAdjustment ValueStoragePolicyAdjustmentType
+
+	// Remaining fields are ignored, unless the PolicyAdjustment is OverrideValueSeparation.
+
 	// MinimumSize is the minimum size of the value.
 	MinimumSize int
 }
@@ -1332,35 +1334,36 @@ type ValueStoragePolicyAdjustment struct {
 type ValueStoragePolicyAdjustmentType uint8
 
 const (
-	// UseDefaultValueStorage is the default value; Pebble will respect global
-	// configuration for value separation.
-	UseDefaultValueStorage ValueStoragePolicyAdjustmentType = iota
+	// UseDefaultValueSeparation is the default value; Pebble will respect global
+	// configuration for value separation into blob files.
+	UseDefaultValueSeparation ValueStoragePolicyAdjustmentType = iota
 
-	// NoValueSeparation indicates Pebble should prefer storing values
-	// in-place.
-	NoValueSeparation
+	// NoBlobHandles indicates Pebble should prefer storing values in
+	// the same sstable (either in-place or in a value block).
+	NoBlobHandles
 
-	// OverrideValueStorage indicates that value separation thresholds (see
+	// OverrideValueSeparation indicates that value separation thresholds (see
 	// valsep.ValueSeparationOutputConfig) for this key range are being
 	// overridden from a SpanPolicy. If the global Options enable value
-	// separation, Pebble will separate values under the OverrideValueStorage
+	// separation, Pebble will separate values under the OverrideValueSeparation
 	// policy even if they do not meet the minimum size threshold of the
 	// global Options' ValueSeparationPolicy.
-	OverrideValueStorage
+	OverrideValueSeparation
 )
 
 // ValueStorageLatencyTolerant is the suggested ValueStoragePolicyAdjustment
 // to use for key ranges that can tolerate higher value retrieval
 // latency.
 var ValueStorageLatencyTolerant = ValueStoragePolicyAdjustment{
-	PolicyAdjustment: OverrideValueStorage,
+	PolicyAdjustment: OverrideValueSeparation,
 	MinimumSize:      10,
 }
 
 // ValueStorageLowReadLatency is the suggested ValueStoragePolicyAdjustment
 // to use for key ranges that require low value retrieval latency.
 var ValueStorageLowReadLatency = ValueStoragePolicyAdjustment{
-	PolicyAdjustment: NoValueSeparation,
+	PolicyAdjustment:          NoBlobHandles,
+	DisableSeparationBySuffix: true,
 }
 
 // SpanPolicyFunc is used to determine the SpanPolicy for a key region.

sstable/properties.go

@@ -125,8 +125,9 @@ type Properties struct {
 	// The minimum size a value must be to be separated into a blob file during writing.
 	ValueSeparationMinSize uint64 `prop:"pebble.value-separation.min-size"`
 	// ValueSeparationBySuffixDisabled indicates if special value separation rules were
-	// applied based on the KV suffix when writing. Pebble attempts to optimize writing of
-	// MVCC garbage values into blob files, which are recognized by the key suffix.
+	// applied based on the KV suffix when writing blob files. Pebble attempts to optimize
+	// separating MVCC garbage, which are recognized by the key suffix. Note that this
+	// value corresponds only for blob file writing and not value blocks.
 	ValueSeparationBySuffixDisabled bool `prop:"pebble.value-separation.by-suffix.disabled"`
 	// User collected properties. Currently, we only use them to store block
 	// properties aggregated at the table level.

testdata/compaction/mvcc_garbage_blob

@@ -67,7 +67,7 @@ flush-log
 
 # Another test with the same value separation config, but this time we set a span
 # policy for the span y-zz that disables value separation by suffix. This should
-# prevent MVCC garbage from being written to blob files.
+# prevent MVCC garbage from being separeted (both into blob files and value blocks).
 
 define value-separation=(enabled, min-size=5, max-ref-depth=3, garbage-ratios=1.0:1.0)
 ----
@@ -127,3 +127,99 @@ pebble.compression_stats: None:234
 pebble.value-separation.min-size: 5
 pebble.value-separation.by-suffix.disabled: true
 obsolete-key: hex:00
+
+define value-separation=(enabled, min-size=5, max-ref-depth=3, garbage-ratios=1.0:1.0)
+----
+
+set-span-policies
+a,c val-sep-minimum-size=0
+----
+
+
+batch
+set a@2 a2
+set b@5 b5
+del b@4
+set b@3 bat3
+set b@2 bat2
+del b@1
+set b@1 bat1
+set b@0 bat0
+----
+
+# Although we've disabled value separation into blob files, writing to 
+# value blocks should still be enabled.
+flush
+----
+L0.0:
+  000005:[a@2#10,SET-b@0#17,SET] seqnums:[10-17] points:[a@2#10,SET-b@0#17,SET] size:880
+
+sstable-properties file=000005
+----
+rocksdb.num.entries: 7
+rocksdb.raw.key.size: 77
+rocksdb.raw.value.size: 20
+pebble.raw.point-tombstone.key.size: 3
+rocksdb.deleted.keys: 1
+rocksdb.num.range-deletions: 0
+pebble.value-blocks.size: 25
+rocksdb.num.data.blocks: 1
+rocksdb.comparator: pebble.internal.testkeys
+rocksdb.data.size: 175
+rocksdb.filter.size: 0
+rocksdb.index.size: 36
+rocksdb.block.based.table.index.type: 0
+pebble.colblk.schema: DefaultKeySchema(pebble.internal.testkeys,16)
+rocksdb.merge.operator: pebble.concatenate
+rocksdb.merge.operands: 0
+pebble.num.value-blocks: 1
+pebble.num.values.in.value-blocks: 3
+rocksdb.property.collectors: [obsolete-key]
+rocksdb.compression: Snappy
+pebble.compression_stats: None:221
+obsolete-key: hex:00
+
+batch
+set a@2 a2
+set b@5 b5
+del b@4
+set b@3 bat3
+set b@2 bat2
+del b@1
+set b@1 bat1
+set b@0 bat0
+----
+
+# Disabling value separation by suffix should also disable writing to value blocks.
+set-span-policies
+a,c val-sep-minimum-size=0 disable-separation-by-suffix
+----
+
+flush
+----
+L0.1:
+  000007:[a@2#18,SET-b@0#25,SET] seqnums:[18-25] points:[a@2#18,SET-b@0#25,SET] size:770
+L0.0:
+  000005:[a@2#10,SET-b@0#17,SET] seqnums:[10-17] points:[a@2#10,SET-b@0#17,SET] size:880
+
+sstable-properties file=000007
+----
+rocksdb.num.entries: 7
+rocksdb.raw.key.size: 77
+rocksdb.raw.value.size: 20
+pebble.raw.point-tombstone.key.size: 3
+rocksdb.deleted.keys: 1
+rocksdb.num.range-deletions: 0
+rocksdb.num.data.blocks: 1
+rocksdb.comparator: pebble.internal.testkeys
+rocksdb.data.size: 157
+rocksdb.filter.size: 0
+rocksdb.index.size: 36
+rocksdb.block.based.table.index.type: 0
+pebble.colblk.schema: DefaultKeySchema(pebble.internal.testkeys,16)
+rocksdb.merge.operator: pebble.concatenate
+rocksdb.merge.operands: 0
+rocksdb.property.collectors: [obsolete-key]
+rocksdb.compression: Snappy
+pebble.compression_stats: None:188
+obsolete-key: hex:00

testdata/static_span_policy_func

@@ -6,8 +6,8 @@ a b c d e f g
 a ->  until d
 b ->  until d
 c ->  until d
-d -> no-value-separation until f
-e -> no-value-separation until f
+d -> disable-value-separation-by-suffix,no-blob-value-separation until f
+e -> disable-value-separation-by-suffix,no-blob-value-separation until f
 f -> none
 g -> none
 
@@ -17,11 +17,11 @@ g -> none
 test a-z:lowlatency
 a b c d e z
 ----
-a -> no-value-separation until z
-b -> no-value-separation until z
-c -> no-value-separation until z
-d -> no-value-separation until z
-e -> no-value-separation until z
+a -> disable-value-separation-by-suffix,no-blob-value-separation until z
+b -> disable-value-separation-by-suffix,no-blob-value-separation until z
+c -> disable-value-separation-by-suffix,no-blob-value-separation until z
+d -> disable-value-separation-by-suffix,no-blob-value-separation until z
+e -> disable-value-separation-by-suffix,no-blob-value-separation until z
 z -> none
 
 # Abutting policies.
@@ -30,12 +30,12 @@ test b-d:lowlatency d-f:latencytolerant f-h:lowlatency
 a b c d e f g h i z
 ----
 a ->  until b
-b -> no-value-separation until d
-c -> no-value-separation until d
-d -> override until f
-e -> override until f
-f -> no-value-separation until h
-g -> no-value-separation until h
+b -> disable-value-separation-by-suffix,no-blob-value-separation until d
+c -> disable-value-separation-by-suffix,no-blob-value-separation until d
+d -> override-value-separation until f
+e -> override-value-separation until f
+f -> disable-value-separation-by-suffix,no-blob-value-separation until h
+g -> disable-value-separation-by-suffix,no-blob-value-separation until h
 h -> none
 i -> none
 z -> none
@@ -46,14 +46,14 @@ test b-d:lowlatency h-j:latencytolerant
 a b c d e f g h i j k l
 ----
 a ->  until b
-b -> no-value-separation until d
-c -> no-value-separation until d
+b -> disable-value-separation-by-suffix,no-blob-value-separation until d
+c -> disable-value-separation-by-suffix,no-blob-value-separation until d
 d ->  until h
 e ->  until h
 f ->  until h
 g ->  until h
-h -> override until j
-i -> override until j
+h -> override-value-separation until j
+i -> override-value-separation until j
 j -> none
 k -> none
 l -> none