Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: prevent iter panic on meta1 split key #104082

Merged
merged 1 commit into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion pkg/storage/mvcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -5941,10 +5941,16 @@ func mvccMinSplitKey(it MVCCIterator, startKey roachpb.Key) (roachpb.Key, error)
// 4. Not in between the start and end of a row for table ranges.
//
// The returned split key is NOT guaranteed to be outside a no-split span, such
// as Meta1, Meta2Max or Node Liveness.
// as Meta2Max or Node Liveness.
func MVCCFirstSplitKey(
_ context.Context, reader Reader, desiredSplitKey, startKey, endKey roachpb.RKey,
) (roachpb.Key, error) {
// If the start key of the range is within the meta1 key space, the range
// cannot be split.
if startKey.Less(roachpb.RKey(keys.LocalMax)) {
return nil, nil
}

it := reader.NewMVCCIterator(MVCCKeyAndIntentsIterKind, IterOptions{UpperBound: endKey.AsRawKey()})
defer it.Close()

Expand Down
20 changes: 20 additions & 0 deletions pkg/storage/mvcc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4562,6 +4562,25 @@ func TestMVCCFirstSplitKey(t *testing.T) {
{desired: roachpb.Key("z"), expected: nil},
},
},
{
// meta1 cannot be split. Previously, this test would cause a panic in
// mvccMinSplitKey, called by MVCCFirstSplitKey. The iterator is
// initialized with a global key constraint from the endKey
// ("\x02\xff\xff"), but we seekGE the start key (MinKey="") which is
// local because it is before LocalMax (0x02).
keys: []roachpb.Key{
roachpb.Key("\x02"),
roachpb.Key("\x02\x00"),
roachpb.Key("\x02\xff"),
},
startKey: keys.MinKey,
endKey: keys.Meta1KeyMax,
splits: []splitExpect{
{desired: keys.MinKey, expected: nil},
{desired: roachpb.Key("\x02"), expected: nil},
{desired: roachpb.Key("\x02\x00"), expected: nil},
},
},
{
// All keys are outside the range, no keys to spit at so expect no
// splits.
Expand Down Expand Up @@ -4613,6 +4632,7 @@ func TestMVCCFirstSplitKey(t *testing.T) {
{desired: roachpb.Key("0"), expected: roachpb.Key("c")},
{desired: roachpb.Key("b"), expected: roachpb.Key("c")},
{desired: roachpb.Key("c"), expected: roachpb.Key("c")},
{desired: keys.MinKey, expected: roachpb.Key("c")},
// Desired split key is after the last key in the range (c), shouldn't
// split.
{desired: roachpb.Key("d"), expected: nil},
Expand Down