Skip to content

Commit

Permalink
metamorphic: don't panic on SingleDeleteInvariantViolationCallback
Browse files Browse the repository at this point in the history
The code comment motivating this change is copied from
cockroachdb/cockroach#116889.

Fixes #3183
  • Loading branch information
sumeerbhola committed Jan 4, 2024
1 parent f0f5f40 commit d4a90ce
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 7 deletions.
7 changes: 0 additions & 7 deletions metamorphic/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,6 @@ func defaultOptions() *pebble.Options {
}},
BlockPropertyCollectors: blockPropertyCollectorConstructors,
}
// TODO(sumeer): add IneffectualSingleDeleteCallback that panics by
// supporting a test option that does not generate ineffectual single
// deletes.
opts.Experimental.SingleDeleteInvariantViolationCallback = func(
userKey []byte) {
panic(errors.AssertionFailedf("single del invariant violations on key %q", userKey))
}
return opts
}

Expand Down
58 changes: 58 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,64 @@ type Options struct {
// on shared storage in bytes. If it is 0, no cache is used.
SecondaryCacheSizeBytes int64

// NB: DO NOT crash on SingleDeleteInvariantViolationCallback or
// IneffectualSingleDeleteCallback, since these can be false positives
// even if SingleDel has been used correctly.
//
// Pebble's delete-only compactions can cause a recent RANGEDEL to peek
// below an older SINGLEDEL and delete an arbitrary subset of data below
// that SINGLEDEL. When that SINGLEDEL gets compacted (without the
// RANGEDEL), any of these callbacks can happen, without it being a real
// correctness problem.
//
// Example 1:
// RANGEDEL [a, c)#10 in L0
// SINGLEDEL b#5 in L1
// SET b#3 in L6
//
// If the L6 file containing the SET is narrow and the L1 file containing
// the SINGLEDEL is wide, a delete-only compaction can remove the file in
// L2 before the SINGLEDEL is compacted down. Then when the SINGLEDEL is
// compacted down, it will not find any SET to delete, resulting in the
// ineffectual callback.
//
// Example 2:
// RANGEDEL [a, z)#60 in L0
// SINGLEDEL g#50 in L1
// SET g#40 in L2
// RANGEDEL [g,h)#30 in L3
// SET g#20 in L6
//
// In this example, the two SETs represent the same user write, and the
// RANGEDELs are caused by the CockroachDB range being dropped. That is,
// the user wrote to g once, range was dropped, then added back, which
// caused the SET again, then at some point g was validly deleted using a
// SINGLEDEL, and then the range was dropped again. The older RANGEDEL can
// get fragmented due to compactions it has been part of. Say this L3 file
// containing the RANGEDEL is very narrow, while the L1, L2, L6 files are
// wider than the RANGEDEL in L0. Then the RANGEDEL in L3 can be dropped
// using a delete-only compaction, resulting in an LSM with state:
//
// RANGEDEL [a, z)#60 in L0
// SINGLEDEL g#50 in L1
// SET g#40 in L2
// SET g#20 in L6
//
// A multi-level compaction involving L1, L2, L6 will cause the invariant
// violation callback. This example doesn't need multi-level compactions:
// say there was a Pebble snapshot at g#21 preventing g#20 from being
// dropped when it meets g#40 in a compaction. That snapshot will not save
// RANGEDEL [g,h)#30, so we can have:
//
// SINGLEDEL g#50 in L1
// SET g#40, SET g#20 in L6
//
// And say the snapshot is removed and then the L1 and L6 compaction
// happens, resulting in the invariant violation callback.
//
// TODO(sumeer): rename SingleDeleteInvariantViolationCallback to remove
// the word "invariant".

// IneffectualPointDeleteCallback is called in compactions/flushes if any
// single delete is being elided without deleting a point set/merge.
IneffectualSingleDeleteCallback func(userKey []byte)
Expand Down

0 comments on commit d4a90ce

Please sign in to comment.