-
Notifications
You must be signed in to change notification settings - Fork 446
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
feat(pageserver): add k-merge layer iterator with lazy loading #8053
Conversation
3067 tests run: 2952 passed, 0 failed, 115 skipped (full report)Flaky tests (2)Postgres 16
Postgres 14
Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
d9e0e29 at 2024-07-09T20:44:56.287Z :recycle: |
0e752b5
to
05fd319
Compare
b1c8ff6
to
62786da
Compare
Signed-off-by: Alex Chi Z <chi@neon.tech>
05fd319
to
b4eb0b3
Compare
Signed-off-by: Alex Chi Z <chi@neon.tech>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aplogogies for the delayed review!
Why aren't we implementing futures::Stream
so we can use peekable
instead of the home-grown thing added in this PR?
(Not asking for it to happen in this PR, just generally => maybe let's discuss on Slack)
Requesting some changes, if you disagree let's discuss on Slack . Re-request review once address.
I want to avoid using |
What I mean is No opaque types anywhere. But, anyway, not asking for it in this PR. |
Signed-off-by: Alex Chi Z <chi@neon.tech>
273ee1c
to
6bf5532
Compare
Signed-off-by: Alex Chi Z <chi@neon.tech>
Co-authored-by: Christian Schwarz <christian@neon.tech>
Signed-off-by: Alex Chi Z <chi@neon.tech>
Part of #8002. This pull request adds a k-merge iterator for bottom-most compaction. ## Summary of changes * Added back lsn_range / key_range in delta layer inner. This was removed due to #8050, but added back because iterators need that information to process lazy loading. * Added lazy-loading k-merge iterator. * Added iterator wrapper as a unified iterator type for image+delta iterator. The current status and test should cover the use case for L0 compaction so that the L0 compaction process can bypass page cache and have a fixed amount of memory usage. The next step is to integrate this with the new bottom-most compaction. --------- Signed-off-by: Alex Chi Z <chi@neon.tech> Co-authored-by: Christian Schwarz <christian@neon.tech>
Problem
Part of #8002. This pull request adds a k-merge iterator for bottom-most compaction.
Summary of changes
The current status and test should cover the use case for L0 compaction so that the L0 compaction process can bypass page cache and have a fixed amount of memory usage. The next step is to integrate this with the new bottom-most compaction.
Checklist before requesting a review
Checklist before merging