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_test.go

@@ -1530,18 +1530,17 @@ 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 == UseDefaultValueStorage {
+							policy.ValueStoragePolicy.PolicyAdjustment = OverrideValueStorage
 						}
-						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 {

options.go

@@ -1299,22 +1299,17 @@ func (p SpanPolicy) String() string {
 	}
 	switch p.ValueStoragePolicy.PolicyAdjustment {
 	case NoValueSeparation:
-		sb.WriteString("no-value-separation,")
+		sb.WriteString("no-blob-value-separation,")
 	case OverrideValueStorage:
-		sb.WriteString("override,")
+		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 OverrideValueStorage.
+
 	// MinimumSize is the minimum size of the value.
 	MinimumSize int
 }
@@ -1333,11 +1335,11 @@ type ValueStoragePolicyAdjustmentType uint8
 
 const (
 	// UseDefaultValueStorage is the default value; Pebble will respect global
-	// configuration for value separation.
+	// configuration for value separation into blob files.
 	UseDefaultValueStorage ValueStoragePolicyAdjustmentType = iota
 
-	// NoValueSeparation indicates Pebble should prefer storing values
-	// in-place.
+	// NoValueSeparation indicates Pebble should prefer storing values in
+	// the same sstable (either in-place or in a value block).
 	NoValueSeparation
 
 	// OverrideValueStorage indicates that value separation thresholds (see
@@ -1360,7 +1362,8 @@ var ValueStorageLatencyTolerant = ValueStoragePolicyAdjustment{
 // ValueStorageLowReadLatency is the suggested ValueStoragePolicyAdjustment
 // to use for key ranges that require low value retrieval latency.
 var ValueStorageLowReadLatency = ValueStoragePolicyAdjustment{
-	PolicyAdjustment: NoValueSeparation,
+	PolicyAdjustment:          NoValueSeparation,
+	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,147 @@ 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 disable-separation-by-suffix
+----
+
+
+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
+----
+
+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:770
+
+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
+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
+
+
+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