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

ingest: verify ingestion sstables have legal boundary range keys #3829

Open
itsbilal opened this issue Aug 6, 2024 · 0 comments · May be fixed by #4126
Open

ingest: verify ingestion sstables have legal boundary range keys #3829

itsbilal opened this issue Aug 6, 2024 · 0 comments · May be fixed by #4126
Assignees
Labels

Comments

@itsbilal
Copy link
Member

itsbilal commented Aug 6, 2024

Currently, we don't allow range keys to have suffixed keys as bounds at any read interface point with Cockroach; the start,end of every range key fragment must be a suffixless key. On the write side, this is enforced in batch.go, but there's no such enforcement of this invariant in the ingestion path at all.

While reading every range key during ingestion is likely too expensive to be worthwhile, it's probably appropriate to verify (at ingestLoad time) that the smallest/largest range keys of an sstable at least have bounds that are suffixless, or if the bounds do contain suffixes, that these range keys perfectly defragment with range keys in adjacent ingestion sstables, such that a read post-ingestion would only surface suffixless keys and not panic in the read path.

Motivated from a conversation around cockroachdb/cockroach#127997 and how it can fragment sstables at suffixed keys.

Jira issue: PEBBLE-234

@itsbilal itsbilal added the C-enhancement New feature or request label Aug 6, 2024
@itsbilal itsbilal self-assigned this Aug 13, 2024
itsbilal added a commit to itsbilal/pebble that referenced this issue Oct 31, 2024
We now allow sstables to have range key bounds that have suffixes
in them, during ingestion. This is to allow for Cockroach-side
defragmentation of range keys. This change ensures that such
suffixed range keys are adjacent and defrag cleanly with rangekeys
in other ingested sstables, to not cause panics later on in the read
path.

Fixes cockroachdb#3829.
itsbilal added a commit to itsbilal/pebble that referenced this issue Oct 31, 2024
We now allow sstables to have range key bounds that have suffixes
in them, during ingestion. This is to allow for Cockroach-side
defragmentation of range keys. This change ensures that such
suffixed range keys are adjacent and defrag cleanly with rangekeys
in other ingested sstables, to not cause panics later on in the read
path.

Fixes cockroachdb#3829.
itsbilal added a commit to itsbilal/pebble that referenced this issue Oct 31, 2024
We now allow sstables to have range key bounds that have suffixes
in them, during ingestion. This is to allow for Cockroach-side
defragmentation of range keys. This change ensures that such
suffixed range keys are adjacent and defrag cleanly with rangekeys
in other ingested sstables, to not cause panics later on in the read
path.

Fixes cockroachdb#3829.
itsbilal added a commit to itsbilal/pebble that referenced this issue Nov 1, 2024
We now allow sstables to have range key bounds that have suffixes
in them, during ingestion. This is to allow for Cockroach-side
defragmentation of range keys. This change ensures that such
suffixed range keys are adjacent and defrag cleanly with rangekeys
in other ingested sstables, to not cause panics later on in the read
path.

Fixes cockroachdb#3829.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress (this milestone)
Development

Successfully merging a pull request may close this issue.

1 participant