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

Conversation

kvoli
Copy link
Collaborator

@kvoli kvoli commented May 30, 2023

It was possible that a load based split was suggested for meta1, which
would call MVCCFirstSplitKey and panic as the meta1 start key
\Min is local, whilst the meta1 end key is global 0x02 0xff 0xff.

Add a check that the start key is greater than the meta1 end key before
processing in MVCCFirstSplitKey to prevent the panic.

Note meta1 would never be split regardless, as
storage.IsValidSplitKey would fail after finding a split key.

Also note that if the desired split key is a local key, the same problem
doesn't exist as the minimum split key would be used to seek the first
split key instead.

Fixes: #104007

Release note: None

@kvoli kvoli self-assigned this May 30, 2023
@cockroach-teamcity
Copy link
Member

This change is Reviewable

It was possible that a load based split was suggested for `meta1`, which
would call `MVCCFirstSplitKey` and panic as the `meta1` start key
`\Min` is local, whilst the `meta1` end key is global `0x02 0xff 0xff`.

Add a check that the start key is greater than the `meta1` end key before
processing in `MVCCFirstSplitKey` to prevent the panic.

Note `meta1` would never be split regardless, as
`storage.IsValidSplitKey` would fail after finding a split key.

Also note that if the desired split key is a local key, the same problem
doesn't exist as the minimum split key would be used to seek the first
split key instead.

Fixes: cockroachdb#104007

Release note: None
@kvoli kvoli force-pushed the 230530.split-first-panic branch from 2b4d2b9 to 30caa00 Compare May 30, 2023 14:24
@kvoli kvoli changed the title storage: prevent iter on meta1 first split key storage: prevent iter panic on meta1 split key May 30, 2023
@kvoli kvoli marked this pull request as ready for review May 30, 2023 15:01
@kvoli kvoli requested a review from a team as a code owner May 30, 2023 15:01
Copy link
Member

@itsbilal itsbilal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kvoli
Copy link
Collaborator Author

kvoli commented May 30, 2023

TYFTR

bors r=itsbilal

@craig
Copy link
Contributor

craig bot commented May 30, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented May 30, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kvserver: iterator panic during split
3 participants