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

internal/manifest: efficient range-key fileMetadata seeks #1742

Closed
jbowens opened this issue Jun 7, 2022 · 1 comment · Fixed by #1771
Closed

internal/manifest: efficient range-key fileMetadata seeks #1742

jbowens opened this issue Jun 7, 2022 · 1 comment · Fixed by #1771
Assignees

Comments

@jbowens
Copy link
Collaborator

jbowens commented Jun 7, 2022

In designing range keys, we decided to store range keys in the same sstables as point keys to preserve the LSM invariant relationship between the two. However this intermingling poses a performance obstacle.

The range key and point key iterator stacks are distinct. A seek on a combined iterator needs to seek both iterators, which twice need to seek through a level's metadata to find the correct file. Because the two iterator stacks are separate, these two iterators care about separate but overlapping sets of sstables (eg, only those that hold point keys and only those that hold range keys)

For range keys which are expected to be rare, wading through many point key-only sstables adds unnecessary key comparisons. Additionally, some levels may not contain any range keys at all, and it would be preferable to exclude those levels from the range key iterator stack altogether.

A couple of potential solutions:

  1. Use a b-tree annotation to annotate subtrees of a level that contain range keys. When annotations are up to date, this allows O(1) testing whether a level contains range keys during iterator construction. With extensions to manifest.LevelIterator and its Filter method, these annotations could also be used to skip subtrees that contain no range keys. The details of this approach might be tricky.
  2. Maintain a parallel , range-key [NumLevels]LevelMetadata on manifest.Version. This parallel LSM would be a projection of the main LSM, holding only sstables that contain range keys. It would be incrementally updated alongside the combined LSM.
@jbowens
Copy link
Collaborator Author

jbowens commented Jun 8, 2022

I'm leaning towards the second option. I worry the first won't end up being performant enough.

@itsbilal itsbilal self-assigned this Jun 15, 2022
itsbilal added a commit to itsbilal/pebble that referenced this issue Jun 16, 2022
This change creates a new LevelMetadata btree just for
sstables containing range keys. This btree is incrementally
updated with sstable additions/deletions that contain range
keys. This should have a relatively light overhead, as
range keys and files containing range keys are expected to be
rare. This also has the benefit of reducing the time it takes
to determine whether files contain range keys.

Fixes cockroachdb#1742.
itsbilal added a commit to itsbilal/pebble that referenced this issue Jun 16, 2022
This change creates a new LevelMetadata btree just for
sstables containing range keys. This btree is incrementally
updated with sstable additions/deletions that contain range
keys. This should have a relatively light overhead, as
range keys and files containing range keys are expected to be
rare. This also has the benefit of reducing the time it takes
to determine whether files contain range keys.

Fixes cockroachdb#1742.
itsbilal added a commit that referenced this issue Jun 16, 2022
This change creates a new LevelMetadata btree just for
sstables containing range keys. This btree is incrementally
updated with sstable additions/deletions that contain range
keys. This should have a relatively light overhead, as
range keys and files containing range keys are expected to be
rare. This also has the benefit of reducing the time it takes
to determine whether files contain range keys.

Fixes #1742.
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 a pull request may close this issue.

2 participants