From eb42ad1955a2302d3d3b77f6f7ce59163eab2c52 Mon Sep 17 00:00:00 2001 From: Arul Ajmani Date: Wed, 27 Sep 2023 19:29:57 -0500 Subject: [PATCH] rditer: add a filter for just lock table spans This patch adds a new `ReplicatedSpansFilter` called `ReplicatedSpansLocksOnly`, which can be used to select just lock table spans for a given range. While here, I also wrapped some comments over 80 chars that were bothering me. Epic: none Release note: None --- pkg/kv/kvserver/rditer/select.go | 39 +++++++++++++------ pkg/kv/kvserver/rditer/select_test.go | 8 ++++ .../rditer/testdata/TestSelect/r2_locksonly | 18 +++++++++ 3 files changed, 53 insertions(+), 12 deletions(-) create mode 100644 pkg/kv/kvserver/rditer/testdata/TestSelect/r2_locksonly diff --git a/pkg/kv/kvserver/rditer/select.go b/pkg/kv/kvserver/rditer/select.go index 3d1fb96873ff..f255307cf799 100644 --- a/pkg/kv/kvserver/rditer/select.go +++ b/pkg/kv/kvserver/rditer/select.go @@ -15,20 +15,30 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" ) +// ReplicatedSpansFilter is used to declare filters when selecting replicated +// spans. +// +// TODO(arul): we could consider using a bitset here instead. Note that a lot of +// the fields here are mutually exclusive (e.g. ReplicatedSpansExcludeLocks and +// ReplicatedSpansLocksOnly), so we'd need some sort of validation here. type ReplicatedSpansFilter int const ( // ReplicatedSpansAll includes all replicated spans, including user keys, // range descriptors, and lock keys. ReplicatedSpansAll ReplicatedSpansFilter = iota - // ReplicatedSpansExcludeUser includes all replicated spans except for user keys. + // ReplicatedSpansExcludeUser includes all replicated spans except for user + // keys. ReplicatedSpansExcludeUser // ReplicatedSpansUserOnly includes just user keys, and no other replicated // spans. ReplicatedSpansUserOnly - // ReplicatedSpansExcludeLockTable includes all replicated spans except for the + // ReplicatedSpansExcludeLocks includes all replicated spans except for the // lock table. ReplicatedSpansExcludeLocks + // ReplicatedSpansLocksOnly includes just spans for the lock table, and no + // other replicated spans. + ReplicatedSpansLocksOnly ) // SelectOpts configures which spans for a Replica to return from Select. @@ -70,18 +80,20 @@ func Select(rangeID roachpb.RangeID, opts SelectOpts) []roachpb.Span { } if !opts.ReplicatedBySpan.Equal(roachpb.RSpan{}) { - // r1 "really" only starts at LocalMax. But because we use a StartKey of RKeyMin - // for r1, we actually do anchor range descriptors (and their locks and txn records) - // at RKeyMin as well. On the other hand, the "user key space" cannot start at RKeyMin - // because then it encompasses the special-cased prefix \x02... (/Local/). - // So awkwardly for key-based local keyspace we must not call KeySpan, for - // user keys we have to. + // r1 "really" only starts at LocalMax. But because we use a StartKey of + // RKeyMin for r1, we actually do anchor range descriptors (and their locks + // and txn records) at RKeyMin as well. On the other hand, the "user key + // space" cannot start at RKeyMin because then it encompasses the + // special-cased prefix \x02... (/Local/). So awkwardly for key-based local + // keyspace we must not call KeySpan, for user keys we have to. // // See also the comment on KeySpan. in := opts.ReplicatedBySpan adjustedIn := in.KeySpan() if opts.ReplicatedSpansFilter != ReplicatedSpansUserOnly { - sl = append(sl, makeRangeLocalKeySpan(in)) + if opts.ReplicatedSpansFilter != ReplicatedSpansLocksOnly { + sl = append(sl, makeRangeLocalKeySpan(in)) + } // Lock table. if opts.ReplicatedSpansFilter != ReplicatedSpansExcludeLocks { @@ -89,7 +101,8 @@ func Select(rangeID roachpb.RangeID, opts SelectOpts) []roachpb.Span { // is a range local key that can have a replicated lock acquired on it. startRangeLocal, _ := keys.LockTableSingleKey(keys.MakeRangeKeyPrefix(in.Key), nil) endRangeLocal, _ := keys.LockTableSingleKey(keys.MakeRangeKeyPrefix(in.EndKey), nil) - // Need adjusted start key to avoid overlapping with the local lock span right above. + // Need adjusted start key to avoid overlapping with the local lock span + // right above. startGlobal, _ := keys.LockTableSingleKey(adjustedIn.Key.AsRawKey(), nil) endGlobal, _ := keys.LockTableSingleKey(adjustedIn.EndKey.AsRawKey(), nil) sl = append(sl, roachpb.Span{ @@ -101,8 +114,10 @@ func Select(rangeID roachpb.RangeID, opts SelectOpts) []roachpb.Span { }) } } - if opts.ReplicatedSpansFilter != ReplicatedSpansExcludeUser { - // Adjusted span because r1's "normal" keyspace starts only at LocalMax, not RKeyMin. + if opts.ReplicatedSpansFilter != ReplicatedSpansExcludeUser && + opts.ReplicatedSpansFilter != ReplicatedSpansLocksOnly { + // Adjusted span because r1's "normal" keyspace starts only at LocalMax, + // not RKeyMin. sl = append(sl, adjustedIn.AsRawSpanWithNoLocals()) } } diff --git a/pkg/kv/kvserver/rditer/select_test.go b/pkg/kv/kvserver/rditer/select_test.go index c9cda164b7f6..080c543aea2b 100644 --- a/pkg/kv/kvserver/rditer/select_test.go +++ b/pkg/kv/kvserver/rditer/select_test.go @@ -76,6 +76,14 @@ func TestSelect(t *testing.T) { }, filter: ReplicatedSpansExcludeLocks, }, + { + name: "r2_locksonly", + sp: roachpb.RSpan{ + Key: roachpb.RKey("a"), + EndKey: roachpb.RKey("c"), + }, + filter: ReplicatedSpansLocksOnly, + }, { name: "r3", sp: roachpb.RSpan{ diff --git a/pkg/kv/kvserver/rditer/testdata/TestSelect/r2_locksonly b/pkg/kv/kvserver/rditer/testdata/TestSelect/r2_locksonly new file mode 100644 index 000000000000..44ed12b9daf0 --- /dev/null +++ b/pkg/kv/kvserver/rditer/testdata/TestSelect/r2_locksonly @@ -0,0 +1,18 @@ +echo +---- +Select({ReplicatedBySpan:{a-c} ReplicatedSpansFilter:4 ReplicatedByRangeID:false UnreplicatedByRangeID:false}): + /Local/Lock/Local/Range"{a"-c"} + /Local/Lock"{a"-c"} +Select({ReplicatedBySpan:{a-c} ReplicatedSpansFilter:4 ReplicatedByRangeID:false UnreplicatedByRangeID:true}): + /Local/RangeID/123/{u""-v""} + /Local/Lock/Local/Range"{a"-c"} + /Local/Lock"{a"-c"} +Select({ReplicatedBySpan:{a-c} ReplicatedSpansFilter:4 ReplicatedByRangeID:true UnreplicatedByRangeID:false}): + /Local/RangeID/123/{r""-s""} + /Local/Lock/Local/Range"{a"-c"} + /Local/Lock"{a"-c"} +Select({ReplicatedBySpan:{a-c} ReplicatedSpansFilter:4 ReplicatedByRangeID:true UnreplicatedByRangeID:true}): + /Local/RangeID/123/{r""-s""} + /Local/RangeID/123/{u""-v""} + /Local/Lock/Local/Range"{a"-c"} + /Local/Lock"{a"-c"}