From c294f5ee4226afe59e56be4baa9505d190997412 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Mon, 23 Sep 2024 16:52:48 +0000 Subject: [PATCH] storage: set correct key in WriteTooOld error from pebbleMVCCScanner Noticed while reviewing #131093. This commit fixes pebbleMVCCScanner to set the correct key for the mostRecentKey field when a locking read encounters multiple keys with more recent timestamps. These keys were added to WriteTooOld error in 52c6b1b5, which had previously only been carrying the maximum conflicting timestamp. They are currently only used for debugging. Before this fix, we would always return the first conflicting key, even though we correctly returned the maximum conflicting timestamp. Epic: None Release note: None --- pkg/storage/pebble_mvcc_scanner.go | 13 +++++------- .../mvcc_histories/range_tombstone_scans | 2 +- .../mvcc_histories/read_fail_on_more_recent | 4 ++-- .../testdata/mvcc_histories/skip_locked | 20 +++++++++---------- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/pkg/storage/pebble_mvcc_scanner.go b/pkg/storage/pebble_mvcc_scanner.go index e57021b0d91e..113cfbfac39e 100644 --- a/pkg/storage/pebble_mvcc_scanner.go +++ b/pkg/storage/pebble_mvcc_scanner.go @@ -850,9 +850,8 @@ func (p *pebbleMVCCScanner) getOne(ctx context.Context) (ok, added bool) { // timestamp with the maximum timestamp we've seen so we know to // return an error, but then keep scanning so that we can return // the largest possible time. - p.mostRecentTS.Forward(p.curUnsafeKey.Timestamp) - if len(p.mostRecentKey) == 0 { - p.mostRecentKey = append(p.mostRecentKey, p.curUnsafeKey.Key...) + if p.mostRecentTS.Forward(p.curUnsafeKey.Timestamp) { + p.mostRecentKey = append(p.mostRecentKey[:0], p.curUnsafeKey.Key...) } return true /* ok */, false } @@ -883,9 +882,8 @@ func (p *pebbleMVCCScanner) getOne(ctx context.Context) (ok, added bool) { // timestamp with the maximum timestamp we've seen so we know to // return an error, but then keep scanning so that we can return // the largest possible time. - p.mostRecentTS.Forward(p.curUnsafeKey.Timestamp) - if len(p.mostRecentKey) == 0 { - p.mostRecentKey = append(p.mostRecentKey, p.curUnsafeKey.Key...) + if p.mostRecentTS.Forward(p.curUnsafeKey.Timestamp) { + p.mostRecentKey = append(p.mostRecentKey[:0], p.curUnsafeKey.Key...) } return true /* ok */, false } @@ -1521,8 +1519,7 @@ func (p *pebbleMVCCScanner) processRangeKeys(seeked bool, reverse bool) bool { if p.failOnMoreRecent { if key := p.parent.UnsafeKey(); !hasPoint || !key.Timestamp.IsEmpty() { if newest := p.curRangeKeys.Newest(); p.ts.LessEq(newest) { - p.mostRecentTS.Forward(newest) - if len(p.mostRecentKey) == 0 { + if p.mostRecentTS.Forward(newest) { p.mostRecentKey = append(p.mostRecentKey[:0], key.Key...) } } diff --git a/pkg/storage/testdata/mvcc_histories/range_tombstone_scans b/pkg/storage/testdata/mvcc_histories/range_tombstone_scans index d8fe0756ec37..b71e1858b7b6 100644 --- a/pkg/storage/testdata/mvcc_histories/range_tombstone_scans +++ b/pkg/storage/testdata/mvcc_histories/range_tombstone_scans @@ -384,7 +384,7 @@ run error scan k=a end=d ts=3 failOnMoreRecent ---- scan: "a"-"d" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "a" at timestamp 3.000000000,0 too old; must write at or above 4.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "c" at timestamp 3.000000000,0 too old; must write at or above 4.000000000,1 run error scan k=a end=d ts=4 failOnMoreRecent diff --git a/pkg/storage/testdata/mvcc_histories/read_fail_on_more_recent b/pkg/storage/testdata/mvcc_histories/read_fail_on_more_recent index 2a0760a710e3..48c3f03bd755 100644 --- a/pkg/storage/testdata/mvcc_histories/read_fail_on_more_recent +++ b/pkg/storage/testdata/mvcc_histories/read_fail_on_more_recent @@ -242,13 +242,13 @@ run error scan k=a end=b_next ts=9,0 failOnMoreRecent ---- scan: "a"-"b_next" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "a" at timestamp 9.000000000,0 too old; must write at or above 13.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "b" at timestamp 9.000000000,0 too old; must write at or above 13.000000000,1 run error scan k=a end=c_next ts=9,0 failOnMoreRecent ---- scan: "a"-"c_next" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "a" at timestamp 9.000000000,0 too old; must write at or above 13.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "b" at timestamp 9.000000000,0 too old; must write at or above 13.000000000,1 run error scan k=b end=c_next ts=9,0 failOnMoreRecent diff --git a/pkg/storage/testdata/mvcc_histories/skip_locked b/pkg/storage/testdata/mvcc_histories/skip_locked index 561e88bfecd9..ec3477b6212e 100644 --- a/pkg/storage/testdata/mvcc_histories/skip_locked +++ b/pkg/storage/testdata/mvcc_histories/skip_locked @@ -764,13 +764,13 @@ run error scan ts=10 k=k1 end=k9 skipLocked failOnMoreRecent str=shared ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k1" at timestamp 10.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 10.000000000,0 too old; must write at or above 17.000000000,1 run error scan ts=10 k=k1 end=k9 reverse skipLocked failOnMoreRecent str=shared ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k6" at timestamp 10.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 10.000000000,0 too old; must write at or above 17.000000000,1 run error get ts=11 k=k1 skipLocked failOnMoreRecent str=shared @@ -812,13 +812,13 @@ run error scan ts=11 k=k1 end=k9 skipLocked failOnMoreRecent str=shared ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k1" at timestamp 11.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 11.000000000,0 too old; must write at or above 17.000000000,1 run error scan ts=11 k=k1 end=k9 reverse skipLocked failOnMoreRecent str=shared ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k6" at timestamp 11.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 11.000000000,0 too old; must write at or above 17.000000000,1 run ok get ts=12 k=k1 skipLocked failOnMoreRecent str=shared @@ -865,7 +865,7 @@ run error scan ts=12 k=k1 end=k9 reverse skipLocked failOnMoreRecent str=shared ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k6" at timestamp 12.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 12.000000000,0 too old; must write at or above 17.000000000,1 run ok get ts=12 t=A k=k1 skipLocked failOnMoreRecent str=shared @@ -912,7 +912,7 @@ run error scan ts=12 t=A k=k1 end=k9 reverse skipLocked failOnMoreRecent str=shared ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k6" at timestamp 12.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 12.000000000,0 too old; must write at or above 17.000000000,1 run ok get ts=13 k=k1 skipLocked failOnMoreRecent str=shared @@ -959,7 +959,7 @@ run error scan ts=13 k=k1 end=k9 reverse skipLocked failOnMoreRecent str=shared ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k6" at timestamp 13.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 13.000000000,0 too old; must write at or above 17.000000000,1 run ok get ts=13 t=B k=k1 skipLocked failOnMoreRecent str=shared @@ -1005,7 +1005,7 @@ run error scan ts=13 t=B k=k1 end=k9 reverse skipLocked failOnMoreRecent str=shared ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k6" at timestamp 13.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 13.000000000,0 too old; must write at or above 17.000000000,1 run ok get ts=14 k=k1 skipLocked failOnMoreRecent str=shared @@ -1424,7 +1424,7 @@ run error scan ts=10 k=k1 end=k9 skipLocked failOnMoreRecent str=exclusive ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k1" at timestamp 10.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 10.000000000,0 too old; must write at or above 17.000000000,1 run error scan ts=10 k=k1 end=k9 reverse skipLocked failOnMoreRecent str=exclusive @@ -1472,7 +1472,7 @@ run error scan ts=11 k=k1 end=k9 skipLocked failOnMoreRecent str=exclusive ---- scan: "k1"-"k9" -> -error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k1" at timestamp 11.000000000,0 too old; must write at or above 17.000000000,1 +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "k5" at timestamp 11.000000000,0 too old; must write at or above 17.000000000,1 run error scan ts=11 k=k1 end=k9 reverse skipLocked failOnMoreRecent str=exclusive