blob: deduplicate values during rewrite

Because of the existence of virtual sstables and the lack of accurate blob
value liveness for virtual sstables, a blob file rewrite may observe multiple
references to the same value. Blob file rewriting must deduplicate these value
IDs to preserve the correct indexing of values within the virtual block.

Note that it's not sufficient to deduplicate referencing sstables by backing
sstable. Consider a value V contained within virtual sstable s1, backed by b1.
Imagine there's another virtual sstable s2 also backed by b1. If s1 is
compacted, the reference to value V will be carried forward to a new physical
sstable b2 which will encode the reference in its blob value liveness index.
However the original backing file b1 is still in use, and its blob value
liveness index continues to be in-use.

Ultimately, we will fix this issue with accurate virtual sstable blob value
liveness (#4915). For now, we deduplicate the valueIDs at rewrite time.

Author: jbowens

compaction_test.go

@@ -1022,6 +1022,12 @@ func TestCompaction(t *testing.T) {
 				}
 				return s
 
+			case "excise":
+				if err := runExciseCmd(td, d); err != nil {
+					return err.Error()
+				}
+				return describeLSM(d, verbose)
+
 			case "excise-dryrun":
 				ve, err := runExciseDryRunCmd(td, d)
 				if err != nil {
@@ -1081,6 +1087,31 @@ func TestCompaction(t *testing.T) {
 				require.NoError(t, b.Commit(nil))
 				return fmt.Sprintf("wrote %d keys\n", count)
 
+			case "run-blob-rewrite-compaction":
+				err := func() error {
+					d.mu.Lock()
+					defer d.mu.Unlock()
+					d.mu.versions.logLock()
+					env := d.makeCompactionEnvLocked()
+					require.NotNil(t, env)
+					picker := d.mu.versions.picker.(*compactionPickerByScore)
+					pc := picker.pickBlobFileRewriteCompaction(*env)
+					if pc == nil {
+						d.mu.versions.logUnlock()
+						return errors.New("no blob file rewrite compaction")
+					}
+					d.mu.versions.logUnlock()
+					d.runPickedCompaction(pc, noopGrantHandle{})
+					for d.mu.compact.compactingCount > 0 {
+						d.mu.compact.cond.Wait()
+					}
+					return nil
+				}()
+				if err != nil {
+					return err.Error()
+				}
+				return describeLSM(d, verbose)
+
 			case "auto-compact":
 				expectedCount := int64(1)
 				td.MaybeScanArgs(t, "count", &expectedCount)

sstable/blob/rewrite.go

@@ -69,6 +69,14 @@ func (rw *FileRewriter) CopyBlock(
 
 	previousValueID := -1
 	for _, valueID := range valueIDs {
+		// Subsequent logic depends on the valueIDs being unique.
+		// TODO(jackson): This is a workaround because we don't have per-sstable
+		// liveness data. See https://github.com/cockroachdb/pebble/issues/4915.
+		// If we had per-sstable liveness data, we should be able to make this
+		// an assertion failure.
+		if previousValueID == valueID {
+			continue
+		}
 		// If there is a gap in the referenced Value IDs within this block, we
 		// need to represent this sparseness as empty values within the block.
 		// We can represent sparseness at the tail of a block or between blocks
@@ -84,6 +92,10 @@ func (rw *FileRewriter) CopyBlock(
 		if err != nil {
 			return err
 		}
+		// We don't know the value size, but we know it must not be empty.
+		if len(value) == 0 {
+			return errors.AssertionFailedf("value is empty")
+		}
 		rw.w.stats.ValueCount++
 		rw.w.stats.UncompressedValueBytes += uint64(len(value))
 		rw.w.valuesEncoder.AddValue(value)

testdata/blob_rewrite

@@ -76,4 +76,4 @@ rewrite-blob 000002 000001 000003
 Successfully rewrote blob file 000002 to 000004
 Input SSTables: [000001 000003]
 SSTables with blob references: 2
-{BlockCount: 1, ValueCount: 4, UncompressedValueBytes: 30, FileLen: 123}
+{BlockCount: 1, ValueCount: 2, UncompressedValueBytes: 15, FileLen: 106}

testdata/compaction/value_separation

@@ -392,6 +392,44 @@ Filter utility: 0.0%
 Ingestions: 0  as flushable: 0 (0B in 0 tables)
 Cgo memory usage: 0B  block cache: 0B (data: 0B, maps: 0B, entries: 0B)  memtables: 0B
 
+# Test a blob file rewrite compaction with virtual sstable references.
+
+define value-separation=(true,1,10,0s,0.01)
+----
+
+batch
+set a apple
+set b banana
+set c coconut
+----
+
+compact a-b
+----
+L6:
+  000005:[a#10,SET-c#12,SET] seqnums:[10-12] points:[a#10,SET-c#12,SET] size:862 blobrefs:[(B000006: 18); depth:1]
+Blob files:
+  B000006 physical:{000006 size:[109 (109B)] vals:[18 (18B)]}
+
+excise b ba
+----
+L6:
+  000007(000005):[a#10,SET-a#10,SET] seqnums:[10-12] points:[a#10,SET-a#10,SET] size:104(862) blobrefs:[(B000006: 2); depth:1]
+  000008(000005):[c#12,SET-c#12,SET] seqnums:[10-12] points:[c#12,SET-c#12,SET] size:104(862) blobrefs:[(B000006: 2); depth:1]
+Blob files:
+  B000006 physical:{000006 size:[109 (109B)] vals:[18 (18B)]}
+
+# Run a blob-rewrite compaction. It'll rewrite the blob file, but it won't
+# actually reclaim disk space. This is a known limitation due to the lack of
+# accurate blob value liveness for virtual sstables. See
+# https://github.com/cockroachdb/pebble/issues/4915.
+
+run-blob-rewrite-compaction
+----
+L6:
+  000007(000005):[a#10,SET-a#10,SET] seqnums:[10-12] points:[a#10,SET-a#10,SET] size:104(862) blobrefs:[(B000006: 2); depth:1]
+  000008(000005):[c#12,SET-c#12,SET] seqnums:[10-12] points:[c#12,SET-c#12,SET] size:104(862) blobrefs:[(B000006: 2); depth:1]
+Blob files:
+  B000006 physical:{000009 size:[110 (110B)] vals:[18 (18B)]}
 
 define value-separation=(true,5,5,0s,1.0) l0-compaction-threshold=1
 ----