db: support virtual sstables in block checksum validation

Supports validation of virtual sstable block checksums during ingestion.
It was easier to deduplicate the virtual sstables by file backing to
ensure that the checksums for each file are only validated once. We
could also implement a VirtualReader.ValidateVirtual function which only
validates blocks which fall within virtual sstable bounds, but that
seems like too much work for a feature which is off by default and was
never turned on.

This pr also prepares us for
#2885. The metamorphic test
might set `ValidateOnIngest` to true in which case we'll hit a panic.

ingest.go

@@ -2021,28 +2021,31 @@ func (d *DB) ingestApply(
 	// The ingestion may have pushed a level over the threshold for compaction,
 	// so check to see if one is necessary and schedule it.
 	d.maybeScheduleCompaction()
-	d.maybeValidateSSTablesLocked(ve.NewFiles)
+	var toValidate []manifest.NewFileEntry
+	dedup := make(map[base.DiskFileNum]struct{})
+	for _, entry := range ve.NewFiles {
+		if _, ok := dedup[entry.Meta.FileBacking.DiskFileNum]; !ok {
+			toValidate = append(toValidate, entry)
+			dedup[entry.Meta.FileBacking.DiskFileNum] = struct{}{}
+		}
+	}
+	d.maybeValidateSSTablesLocked(toValidate)
 	return ve, nil
 }
 
 // maybeValidateSSTablesLocked adds the slice of newFileEntrys to the pending
 // queue of files to be validated, when the feature is enabled.
-// DB.mu must be locked when calling.
 //
-// TODO(bananabrick): Make sure that the ingestion step only passes in the
-// physical sstables for validation here.
+// Note that if two entries with the same backing file are added twice, then the
+// block checksums for the backing file will be validated twice.
+//
+// DB.mu must be locked when calling.
 func (d *DB) maybeValidateSSTablesLocked(newFiles []newFileEntry) {
 	// Only add to the validation queue when the feature is enabled.
 	if !d.opts.Experimental.ValidateOnIngest {
 		return
 	}
 
-	for _, f := range newFiles {
-		if f.Meta.Virtual {
-			panic("pebble: invalid call to maybeValidateSSTablesLocked")
-		}
-	}
-
 	d.mu.tableValidation.pending = append(d.mu.tableValidation.pending, newFiles...)
 	if d.shouldValidateSSTablesLocked() {
 		go d.validateSSTables()
@@ -2102,10 +2105,19 @@ func (d *DB) validateSSTables() {
 			}
 		}
 
-		err := d.tableCache.withReader(
-			f.Meta.PhysicalMeta(), func(r *sstable.Reader) error {
-				return r.ValidateBlockChecksums()
-			})
+		var err error
+		if f.Meta.Virtual {
+			err = d.tableCache.withVirtualReader(
+				f.Meta.VirtualMeta(), func(v sstable.VirtualReader) error {
+					return v.ValidateBlockChecksumsOnBacking()
+				})
+		} else {
+			err = d.tableCache.withReader(
+				f.Meta.PhysicalMeta(), func(r *sstable.Reader) error {
+					return r.ValidateBlockChecksums()
+				})
+		}
+
 		if err != nil {
 			// TODO(travers): Hook into the corruption reporting pipeline, once
 			// available. See pebble#1192.

ingest_test.go

@@ -606,6 +606,9 @@ func TestExcise(t *testing.T) {
 		// Disable automatic compactions because otherwise we'll race with
 		// delete-only compactions triggered by ingesting range tombstones.
 		opts.DisableAutomaticCompactions = true
+		// Set this to true to add some testing for the virtual sstable validation
+		// code paths.
+		opts.Experimental.ValidateOnIngest = true
 
 		var err error
 		d, err = Open("", opts)

sstable/reader_virtual.go

@@ -89,6 +89,12 @@ func (v *VirtualReader) NewIterWithBlockPropertyFiltersAndContextEtc(
 	)
 }
 
+// ValidateBlockChecksumsOnBacking will call ValidateBlockChecksumsOnBacking on the underlying reader.
+// Note that block checksum validation is NOT restricted to virtual sstable bounds.
+func (v *VirtualReader) ValidateBlockChecksumsOnBacking() error {
+	return v.reader.ValidateBlockChecksums()
+}
+
 // NewRawRangeDelIter wraps Reader.NewRawRangeDelIter.
 func (v *VirtualReader) NewRawRangeDelIter() (keyspan.FragmentIterator, error) {
 	iter, err := v.reader.NewRawRangeDelIter()