From 25034865ab7ef969ae26afc6dc2446fb164bf866 Mon Sep 17 00:00:00 2001 From: Bilal Akhtar Date: Mon, 27 Mar 2023 17:23:10 +0000 Subject: [PATCH] storage: CheckSSTConflicts: fix instance of iterator mismatch Previously, in one case, we'd let the sst iterator advance ahead of the engine iterator, which violates an invariant in this function. This change fixes that case. Fixes #99566. Fixes #99010. Epic: none Release note: None --- pkg/kv/kvserver/batcheval/cmd_add_sstable_test.go | 6 ++++++ pkg/storage/sst.go | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/pkg/kv/kvserver/batcheval/cmd_add_sstable_test.go b/pkg/kv/kvserver/batcheval/cmd_add_sstable_test.go index 97bcb23ed507..813305964774 100644 --- a/pkg/kv/kvserver/batcheval/cmd_add_sstable_test.go +++ b/pkg/kv/kvserver/batcheval/cmd_add_sstable_test.go @@ -1083,6 +1083,12 @@ func TestEvalAddSSTable(t *testing.T) { sst: kvs{pointKV("oe", 11, "foo"), pointKV("oih", 12, "foo"), rangeKV("ods", "ogvh", 10, ""), rangeKV("ogvh", "ohl", 10, ""), rangeKV("ogvh", "ohl", 9, "")}, ignoreExpect: true, }, + "DisallowConflict maintains ext iter ahead of sst iter": { + noConflict: true, + data: kvs{pointKV("c", 6, "foo"), rangeKV("c", "e", 5, "")}, + sst: kvs{rangeKV("a", "b", 10, ""), pointKV("d", 9, "foo")}, + ignoreExpect: true, + }, } testutils.RunTrueAndFalse(t, "IngestAsWrites", func(t *testing.T, ingestAsWrites bool) { testutils.RunValues(t, "RewriteConcurrency", []interface{}{0, 8}, func(t *testing.T, c interface{}) { diff --git a/pkg/storage/sst.go b/pkg/storage/sst.go index abdcd1a84275..9089eed401a4 100644 --- a/pkg/storage/sst.go +++ b/pkg/storage/sst.go @@ -1137,6 +1137,10 @@ func CheckSSTConflicts( if sstChangedKeys && !extChangedKeys { sstIter.SeekGE(MVCCKey{Key: extIter.UnsafeKey().Key}) sstOK, sstErr = sstIter.Valid() + if sstOK && extIter.UnsafeKey().Key.Compare(sstIter.UnsafeKey().Key) < 0 { + extIter.SeekGE(MVCCKey{Key: sstIter.UnsafeKey().Key}) + extOK, extErr = extIter.Valid() + } } // Re-seek the ext iterator if the ext iterator changed keys and: // 1) the SST iterator did not change keys, and we need to bring the ext