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

Move cleaned pages into read cache instead of discarding them. #809

Merged
merged 2 commits into from
May 12, 2024
Merged

Move cleaned pages into read cache instead of discarding them. #809

merged 2 commits into from
May 12, 2024

Conversation

adamreichold
Copy link
Contributor

Just after dirty pages have been written out to disk, they are currently discard and need to be read in again when these pages are accessed in the future.

This changes moves them into the read cache instead thereby keeping the clean data available and cache hot in memory.

It turned out that this was also what was making direct I/O slower in #808, i.e. this was not that noticeable when buffered I/O is use because the pages were read back in from the kernel page cache most of the time but it should be a win in both scenarios.

@adamreichold
Copy link
Contributor Author

The intermediate fuzzer failure was me trying to be clever and avoid reference count bump by Option::takeing the buffers out of the write cache but that was not so clever if one of the intermediate writes would fail, I would never call clear and leave the empty buffers in place for the next flush or another call to write to assert on them. Should be fixed now.

@adamreichold
Copy link
Contributor Author

The intermediate fuzzer failure was me trying to be clever and avoid reference count bump by Option::takeing the buffers out of the write cache but that was not so clever if one of the intermediate writes would fail, I would never call clear and leave the empty buffers in place for the next flush or another call to write to assert on them. Should be fixed now.

Turns out I can avoid that atomic reference count bump, but I think it is safer to just do this in two passes in any case.

src/tree_store/page_store/cached_file.rs Outdated Show resolved Hide resolved
src/tree_store/page_store/cached_file.rs Outdated Show resolved Hide resolved
Just after dirty pages have been written out to disk, they are currently discard
and need to be read in again when these pages are accessed in the future.

This changes moves them into the read cache instead thereby keeping the clean
data available and cache hot in memory.
@cberner cberner merged commit 015ed10 into cberner:master May 12, 2024
3 checks passed
@cberner
Copy link
Owner

cberner commented May 12, 2024

Merged. Thanks!

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.

2 participants