From 2d49f18737ed19db8dc6f534bbe1ee7c952442c2 Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Wed, 25 Sep 2024 10:56:32 +0200 Subject: [PATCH 1/2] storage: test combined failOnMoreRecent and uncertainty error Epic: none Release note: None --- .../testdata/mvcc_histories/range_tombstone_scans | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/storage/testdata/mvcc_histories/range_tombstone_scans b/pkg/storage/testdata/mvcc_histories/range_tombstone_scans index b71e1858b7b6..2922ec48e4e0 100644 --- a/pkg/storage/testdata/mvcc_histories/range_tombstone_scans +++ b/pkg/storage/testdata/mvcc_histories/range_tombstone_scans @@ -477,6 +477,18 @@ scan k=b end=d ts=3 globalUncertaintyLimit=4 scan: "b"-"d" -> error: (*kvpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 3.000000000,0 encountered previous write with future timestamp 4.000000000,0 within uncertainty interval `t <= (local=0,0, global=0,0)`; observed timestamps: [] +run error +scan k=b end=d ts=3 globalUncertaintyLimit=4 failOnMoreRecent +---- +scan: "b"-"d" -> +error: (*kvpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 3.000000000,0 encountered previous write with future timestamp 4.000000000,0 within uncertainty interval `t <= (local=0,0, global=0,0)`; observed timestamps: [] + +run error +get k=c ts=3 globalUncertaintyLimit=4 failOnMoreRecent +---- +get: "c" -> +error: (*kvpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 3.000000000,0 encountered previous write with future timestamp 4.000000000,0 within uncertainty interval `t <= (local=0,0, global=0,0)`; observed timestamps: [] + run ok scan k=b end=d ts=4 globalUncertaintyLimit=5 ---- From 10664ba1e34fe859ced742c26ff4a9c4050976bc Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Fri, 20 Sep 2024 16:18:07 +0200 Subject: [PATCH 2/2] storage: disable checkUncertainty on failOnMoreRecent in scanner It was possible for reads with failOnMoreRecent to hit a ReadWithinUncertaintyIntervalError instead of the desired WriteTooOldError. This commit disables uncertainty checks when failOnMoreRecent is active, as the latter is a stronger check anyway. Fixes #119681. Fixes #131005. Epic: none Release note: None --- pkg/storage/pebble_mvcc_scanner.go | 8 +++++++- pkg/storage/testdata/mvcc_histories/range_tombstone_scans | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/storage/pebble_mvcc_scanner.go b/pkg/storage/pebble_mvcc_scanner.go index 10732c8116f9..3711c3428b80 100644 --- a/pkg/storage/pebble_mvcc_scanner.go +++ b/pkg/storage/pebble_mvcc_scanner.go @@ -551,7 +551,13 @@ func (p *pebbleMVCCScanner) init( // because the local uncertainty limit cannot be applied to values with // future-time timestamps with earlier local timestamps. We are only able // to skip uncertainty checks if p.ts >= global_uncertainty_limit. - p.checkUncertainty = p.ts.Less(p.uncertainty.GlobalLimit) + // + // We disable checkUncertainty when the scanner is configured with failOnMoreRecent. + // This avoids cases in which a scan would have failed with a WriteTooOldError + // but instead gets an unexpected ReadWithinUncertaintyIntervalError + // See: + // https://github.com/cockroachdb/cockroach/issues/119681 + p.checkUncertainty = p.ts.Less(p.uncertainty.GlobalLimit) && !p.failOnMoreRecent } // get seeks to the start key exactly once and adds one KV to the result set. diff --git a/pkg/storage/testdata/mvcc_histories/range_tombstone_scans b/pkg/storage/testdata/mvcc_histories/range_tombstone_scans index 2922ec48e4e0..d8037e8202e9 100644 --- a/pkg/storage/testdata/mvcc_histories/range_tombstone_scans +++ b/pkg/storage/testdata/mvcc_histories/range_tombstone_scans @@ -481,13 +481,13 @@ run error scan k=b end=d ts=3 globalUncertaintyLimit=4 failOnMoreRecent ---- scan: "b"-"d" -> -error: (*kvpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 3.000000000,0 encountered previous write with future timestamp 4.000000000,0 within uncertainty interval `t <= (local=0,0, global=0,0)`; observed timestamps: [] +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 get k=c ts=3 globalUncertaintyLimit=4 failOnMoreRecent ---- get: "c" -> -error: (*kvpb.ReadWithinUncertaintyIntervalError:) ReadWithinUncertaintyIntervalError: read at time 3.000000000,0 encountered previous write with future timestamp 4.000000000,0 within uncertainty interval `t <= (local=0,0, global=0,0)`; observed timestamps: [] +error: (*kvpb.WriteTooOldError:) WriteTooOldError: write for key "c" at timestamp 3.000000000,0 too old; must write at or above 4.000000000,1 run ok scan k=b end=d ts=4 globalUncertaintyLimit=5