diff --git a/pkg/storage/intent_interleaving_iter.go b/pkg/storage/intent_interleaving_iter.go index f2acc81583c5..e7e6d8549b2e 100644 --- a/pkg/storage/intent_interleaving_iter.go +++ b/pkg/storage/intent_interleaving_iter.go @@ -737,12 +737,21 @@ func (i *intentInterleavingIter) SeekLT(key MVCCKey) { return } if i.constraint != notConstrained { - i.checkConstraint(key.Key, true) - if i.constraint == constrainedToLocal && bytes.Equal(key.Key, keys.LocalMax) { + // If the seek key of SeekLT is the boundary between the local and global + // keyspaces, iterators constrained in either direction are permitted. + // Iterators constrained to the local keyspace may be scanning from their + // upper bound. Iterators constrained to the global keyspace may have found + // a key on the boundary and may now be scanning before the key, using the + // boundary as an exclusive upper bound. + localMax := bytes.Equal(key.Key, keys.LocalMax) + if !localMax { + i.checkConstraint(key.Key, true) + } + if localMax && i.constraint == constrainedToLocal { // Move it down to below the lock table so can iterate down cleanly into // the local key space. Note that we disallow anyone using a seek key - // that is a local key above the lock table, and there should no keys in - // the engine there either (at least not keys that we need to see using + // that is a local key above the lock table, and there should be no keys + // in the engine there either (at least not keys that we need to see using // an MVCCIterator). key.Key = keys.LocalRangeLockTablePrefix } diff --git a/pkg/storage/intent_interleaving_iter_test.go b/pkg/storage/intent_interleaving_iter_test.go index 52be98904e4f..cd586706b286 100644 --- a/pkg/storage/intent_interleaving_iter_test.go +++ b/pkg/storage/intent_interleaving_iter_test.go @@ -413,7 +413,7 @@ func TestIntentInterleavingIterBoundaries(t *testing.T) { defer iter.Close() iter.SeekLT(MVCCKey{Key: keys.MaxKey}) }) - // Boundary cases for constrainedToGlobal + // Boundary cases for constrainedToGlobal. func() { opts := IterOptions{LowerBound: keys.LocalMax} iter := newIntentInterleavingIterator(eng, opts).(*intentInterleavingIter) @@ -427,13 +427,13 @@ func TestIntentInterleavingIterBoundaries(t *testing.T) { require.Equal(t, constrainedToGlobal, iter.constraint) iter.SetUpperBound(keys.LocalMax) }) - require.Panics(t, func() { + func() { opts := IterOptions{LowerBound: keys.LocalMax} iter := newIntentInterleavingIterator(eng, opts).(*intentInterleavingIter) defer iter.Close() require.Equal(t, constrainedToGlobal, iter.constraint) iter.SeekLT(MVCCKey{Key: keys.LocalMax}) - }) + }() // Panics for using a local key that is above the lock table. require.Panics(t, func() { opts := IterOptions{UpperBound: keys.LocalMax} diff --git a/pkg/storage/pebble_mvcc_scanner.go b/pkg/storage/pebble_mvcc_scanner.go index b9f714db37f3..54b2ccd2c7c7 100644 --- a/pkg/storage/pebble_mvcc_scanner.go +++ b/pkg/storage/pebble_mvcc_scanner.go @@ -29,7 +29,7 @@ import ( ) const ( - maxItersBeforeSeek = 10 + maxItersBeforeSeek = 0 // Key value lengths take up 8 bytes (2 x Uint32). kvLenSize = 8 @@ -356,7 +356,7 @@ type pebbleMVCCScanner struct { // Stores any error returned. If non-nil, iteration short circuits. err error // Number of iterations to try before we do a Seek/SeekReverse. Stays within - // [1, maxItersBeforeSeek] and defaults to maxItersBeforeSeek/2 . + // [0, maxItersBeforeSeek] and defaults to maxItersBeforeSeek/2 . itersBeforeSeek int } @@ -483,8 +483,8 @@ func (p *pebbleMVCCScanner) incrementItersBeforeSeek() { // Decrements itersBeforeSeek while ensuring it stays positive. func (p *pebbleMVCCScanner) decrementItersBeforeSeek() { p.itersBeforeSeek-- - if p.itersBeforeSeek < 1 { - p.itersBeforeSeek = 1 + if p.itersBeforeSeek < 0 { + p.itersBeforeSeek = 0 } } @@ -971,6 +971,13 @@ func (p *pebbleMVCCScanner) addAndAdvance( func (p *pebbleMVCCScanner) seekVersion( ctx context.Context, seekTS hlc.Timestamp, uncertaintyCheck bool, ) bool { + if seekTS.IsEmpty() { + // If the seek timestamp is empty, we've already seen all versions of this + // key, so seek to the next key. Seeking to version zero of the current key + // would be incorrect, as version zero is stored before all other versions. + return p.advanceKey() + } + seekKey := MVCCKey{Key: p.curUnsafeKey.Key, Timestamp: seekTS} p.keyBuf = EncodeMVCCKeyToBuf(p.keyBuf[:0], seekKey) origKey := p.keyBuf[:len(p.curUnsafeKey.Key)] diff --git a/pkg/storage/testdata/mvcc_histories/uncertainty_interval_with_local_uncertainty_limit b/pkg/storage/testdata/mvcc_histories/uncertainty_interval_with_local_uncertainty_limit index 41f854262a02..5f1252ea8085 100644 --- a/pkg/storage/testdata/mvcc_histories/uncertainty_interval_with_local_uncertainty_limit +++ b/pkg/storage/testdata/mvcc_histories/uncertainty_interval_with_local_uncertainty_limit @@ -200,17 +200,15 @@ scan t=txn1 k=k1 localUncertaintyLimit=5,0 ---- scan: "k1"-"k1\x00" -> -run error +run ok get t=txn1 k=k2 localUncertaintyLimit=5,0 ---- get: "k2" -> -error: (*roachpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 5.000000000,0 encountered previous write with future timestamp 10.000000000,0? within uncertainty interval `t <= (local=5.000000000,0, global=10.000000000,0)`; observed timestamps: [] -run error +run ok scan t=txn1 k=k2 localUncertaintyLimit=5,0 ---- scan: "k2"-"k2\x00" -> -error: (*roachpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 5.000000000,0 encountered previous write with future timestamp 10.000000000,0? within uncertainty interval `t <= (local=5.000000000,0, global=10.000000000,0)`; observed timestamps: [] run ok get t=txn1 k=k3 localUncertaintyLimit=5,0 @@ -222,17 +220,15 @@ scan t=txn1 k=k3 localUncertaintyLimit=5,0 ---- scan: "k3"-"k3\x00" -> -run error +run ok get t=txn1 k=k4 localUncertaintyLimit=5,0 ---- get: "k4" -> -error: (*roachpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 5.000000000,0 encountered previous write with future timestamp 10.000000000,0? within uncertainty interval `t <= (local=5.000000000,0, global=10.000000000,0)`; observed timestamps: [] -run error +run ok scan t=txn1 k=k4 localUncertaintyLimit=5,0 ---- scan: "k4"-"k4\x00" -> -error: (*roachpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 5.000000000,0 encountered previous write with future timestamp 10.000000000,0? within uncertainty interval `t <= (local=5.000000000,0, global=10.000000000,0)`; observed timestamps: [] run ok get t=txn1 k=k5 localUncertaintyLimit=5,0 @@ -244,17 +240,15 @@ scan t=txn1 k=k5 localUncertaintyLimit=5,0 ---- scan: "k5"-"k5\x00" -> -run error +run ok get t=txn1 k=k6 localUncertaintyLimit=5,0 ---- get: "k6" -> -error: (*roachpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 5.000000000,0 encountered previous write with future timestamp 10.000000000,0? within uncertainty interval `t <= (local=5.000000000,0, global=10.000000000,0)`; observed timestamps: [] -run error +run ok scan t=txn1 k=k6 localUncertaintyLimit=5,0 ---- scan: "k6"-"k6\x00" -> -error: (*roachpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 5.000000000,0 encountered previous write with future timestamp 10.000000000,0? within uncertainty interval `t <= (local=5.000000000,0, global=10.000000000,0)`; observed timestamps: [] run ok get t=txn1 k=k7 localUncertaintyLimit=5,0 @@ -266,17 +260,15 @@ scan t=txn1 k=k7 localUncertaintyLimit=5,0 ---- scan: "k7"-"k7\x00" -> -run error +run ok get t=txn1 k=k8 localUncertaintyLimit=5,0 ---- get: "k8" -> -error: (*roachpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 5.000000000,0 encountered previous write with future timestamp 10.000000000,0? within uncertainty interval `t <= (local=5.000000000,0, global=10.000000000,0)`; observed timestamps: [] -run error +run ok scan t=txn1 k=k8 localUncertaintyLimit=5,0 ---- scan: "k8"-"k8\x00" -> -error: (*roachpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 5.000000000,0 encountered previous write with future timestamp 10.000000000,0? within uncertainty interval `t <= (local=5.000000000,0, global=10.000000000,0)`; observed timestamps: [] run ok