Skip to content

Commit

Permalink
Merge #114820
Browse files Browse the repository at this point in the history
114820: storage: assert local single delete safety r=sumeerbhola a=jbowens

During intent resolution, examine internal keys when intending to delete an
intent using a single delete tombstone to validate that it is indeed safe
within the local Engine. This is a safety measure to validate that we do not
violate the invariants that make singe deletion deterministic and cause replica
divergence. False negatives are possible, since this safety mechanism only
examines the local engine state and only the state visible to the open lock
table iterator at the time of proposal evaluation.

Release note: none
Epic: none
Informs #114492.
Informs #114421.

Co-authored-by: Jackson Owens <jackson@cockroachlabs.com>
  • Loading branch information
craig[bot] and jbowens committed Nov 22, 2023
2 parents 4314ad5 + 36a2f4c commit 4bb5fd1
Show file tree
Hide file tree
Showing 7 changed files with 193 additions and 53 deletions.
6 changes: 6 additions & 0 deletions pkg/kv/kvserver/spanset/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,12 @@ func (i *EngineIterator) Stats() storage.IteratorStats {
return i.i.Stats()
}

// CanDeterministicallySingleDelete is part of the storage.EngineIterator
// interface.
func (i *EngineIterator) CanDeterministicallySingleDelete() (bool, error) {
return i.i.CanDeterministicallySingleDelete()
}

type spanSetReader struct {
r storage.Reader
spans *SpanSet
Expand Down
16 changes: 16 additions & 0 deletions pkg/storage/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,22 @@ type EngineIterator interface {
PrevEngineKeyWithLimit(limit roachpb.Key) (state pebble.IterValidityState, err error)
// Stats returns statistics about the iterator.
Stats() IteratorStats
// CanDeterministicallySingleDelete is a specific purpose-built method for
// determining whether the current key (UnsafeRawEngineKey()/EngineKey())
// may be deterministically deleted through a single delete key on the local
// engine state. The determination is completely to local to the Engine, and
// a true return value does not mean that clearing the key with a single
// delete will be deterministic on other replicas on other Engines.
//
// CanDeterministicallySingleDelete does not change the iterator position
// (all subsequent iterator operations should behave as if
// CanDeterministicallySingleDelete was never invoked), although it DOES
// invalidate the memory associated with the current iterator position's
// value.
//
// CanDeterministicallySingleDelete may only be called when oriented in the
// forward direction and only once at a given iterator position.
CanDeterministicallySingleDelete() (ok bool, err error)
}

// CloneContext is an opaque type encapsulating sufficient context to construct
Expand Down
5 changes: 5 additions & 0 deletions pkg/storage/lock_table_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,11 @@ func (i *LockTableIterator) Stats() IteratorStats {
return i.iter.Stats()
}

// CanDeterministicallySingleDelete implements the EngineIterator interface.
func (i *LockTableIterator) CanDeterministicallySingleDelete() (ok bool, err error) {
return i.iter.CanDeterministicallySingleDelete()
}

//gcassert:inline
func isLockTableKey(key roachpb.Key) bool {
return bytes.HasPrefix(key, keys.LocalRangeLockTablePrefix)
Expand Down
Loading

0 comments on commit 4bb5fd1

Please sign in to comment.