Skip to content

[runtime/utils/buffer] [storage/journal/fixed] buffer pool managed read caching for blobs #1157

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 27 commits into from
Jul 8, 2025
Merged

[runtime/utils/buffer] [storage/journal/fixed] buffer pool managed read caching for blobs #1157

merged 27 commits into from
Jul 8, 2025

Conversation

roberto-bayardo
Copy link
Collaborator

@roberto-bayardo roberto-bayardo commented Jun 25, 2025 •
edited
Loading

  • pool.rs: Adds a buffer pool that can be shared among cache-enabled blobs.
    • Uses the Clock cache replacement policy for lightweight LRU approximation.
    • Prevents thundering herds by limiting concurrent fetches of the same page from the underlying blob to a single task.
  • append.rs: Adds the Append blob wrapper, providing random-access read caching and write caching for blobs that can be modified only through appends.
  • tip.rs: Moves the "Buffer" from write.rs to its own module now that it's shared between Write and Append.
  • fixed.rs:
    • Changed to use Append wrappers for all blobs.
    • "Tail" blob is now maintained separately from "historical" blobs since they are subject to different invariants.
    • Added a few additional blob consistency checks in initialization.
    • Changed prune() to no longer return the actual pruning position.

@patrick-ogrady
Copy link
Contributor

patrick-ogrady commented Jun 26, 2025 •
edited
Loading

I am getting the sense that "invalidating" pages if we wrap mutable buffers may not be that bad. Maybe I'm being optimistic.

This could allow us to just add Write buffers under this thing, we could just call this Read, and rename the Read to Replay.

@roberto-bayardo
Copy link
Collaborator Author

I am getting the sense that "invalidating" pages if we wrap mutable buffers may not be that bad. Maybe I'm being optimistic.

I agree, just need to potentially maintain a dirty list of pages or something like that.

This could allow us to just add Write buffers under this thing, we could just call this Read, and rename the Read to Replay.

Yes if things work out these can be combined in interesting ways.

@roberto-bayardo roberto-bayardo force-pushed the buffer-pool-work branch 12 times, most recently from ae2095b to fde1ac4 Compare July 2, 2025 01:59
@roberto-bayardo
Copy link
Collaborator Author

roberto-bayardo commented Jul 2, 2025 •
edited
Loading

I am getting the sense that "invalidating" pages if we wrap mutable buffers may not be that bad. Maybe I'm being optimistic.

This could allow us to just add Write buffers under this thing, we could just call this Read, and rename the Read to Replay.

I have added a cached blob type that allows writes, but only if they are appends. It's quite lightweight, so using that even when you could use an Immutable wrapper should be fine (not have much performance impact.). This way the buffer pool need only manage read-only data, which keeps things simple & fast.

@roberto-bayardo roberto-bayardo changed the title [WiP] Buffer pool work [storage/journal/fixed & runtime/utils/buffer] buffer pool managed read caching for blobs Jul 2, 2025
@roberto-bayardo roberto-bayardo force-pushed the buffer-pool-work branch 13 times, most recently from b3b69f9 to 06a15ca Compare July 2, 2025 22:32
@roberto-bayardo
remove Immutable wrapper
12aedf5
@roberto-bayardo
move the Buffer class to a submodule called tip
7f80d2b
@roberto-bayardo
thundering herd protection
7351d4a
@roberto-bayardo
clean up handling of removing blobs from storage by fixed journal
d33e1ed
@roberto-bayardo
move some code from fixed journal init to a helper function
fac0b01
@roberto-bayardo
tests for buffer pool
36ebcc6
@roberto-bayardo
tip::Buffer append and resize tests
906870d
@roberto-bayardo
buffer::Append append and read tests
a8b4b64
@roberto-bayardo
introduce a type alias for the ugly Current db type used in tests
503ab14
@roberto-bayardo
Future -> Fut
94a1155
@roberto-bayardo
warn->debug for duplciate page update
aa0bb38
@roberto-bayardo
eliminate same-page-fetch race condition
8dee312
@roberto-bayardo
make PAGE_SIZE a runtime param instead of a generic parameter
064729a
@roberto-bayardo
fixed journal tweaks
c3c4ffc
@roberto-bayardo
restore WRITE_BUFFER const
51046d2
@roberto-bayardo
address comments
3da882e
@roberto-bayardo
fix consistency issue with rewind
f4c48e7
@roberto-bayardo
simplify size handling
922d2c7
@roberto-bayardo
Remove reset_buffer
80f7bd6
@roberto-bayardo
remove println
05e16ed
@roberto-bayardo
fix buggy pruning position return value
4cc0314
@roberto-bayardo roberto-bayardo mentioned this pull request Jul 8, 2025
Closed
@patrick-ogrady patrick-ogrady merged commit 926b82b into main Jul 8, 2025
31 checks passed
@patrick-ogrady patrick-ogrady deleted the buffer-pool-work branch July 8, 2025 17:38
@codecov
Copy link

codecov bot commented Jul 8, 2025

Codecov Report

Attention: Patch coverage is 98.52476% with 14 lines in your changes missing coverage. Please review.

Project coverage is 91.45%. Comparing base (04cc0e4) to head (83161c6).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
runtime/src/utils/buffer/pool.rs 95.12% 10 Missing ⚠️
storage/src/journal/fixed.rs 98.65% 4 Missing ⚠️ 
@@            Coverage Diff             @@
##             main    #1157      +/-   ##
==========================================
+ Coverage   91.39%   91.45%   +0.06%     
==========================================
  Files         218      221       +3     
  Lines       59455    59938     +483     
==========================================
+ Hits        54338    54817     +479     
- Misses       5117     5121       +4
Files with missing lines Coverage Δ
runtime/src/utils/buffer/append.rs 100.00% <100.00%> (ø)
runtime/src/utils/buffer/mod.rs 99.87% <ø> (ø)
runtime/src/utils/buffer/tip.rs 100.00% <100.00%> (ø)
runtime/src/utils/buffer/write.rs 98.79% <ø> (+5.08%) ⬆️
storage/src/adb/any.rs 99.09% <100.00%> (-0.02%) ⬇️
storage/src/adb/current.rs 96.72% <100.00%> (-0.16%) ⬇️
storage/src/index/storage.rs 99.33% <100.00%> (+<0.01%) ⬆️
storage/src/mmr/journaled.rs 98.06% <100.00%> (+<0.01%) ⬆️
storage/src/journal/fixed.rs 97.84% <98.65%> (+0.16%) ⬆️
runtime/src/utils/buffer/pool.rs 95.12% <95.12%> (ø)

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04cc0e4...83161c6. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danlaine danlaine danlaine left review comments

Copilot code review Copilot Copilot left review comments

@patrick-ogrady patrick-ogrady patrick-ogrady approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v0.0.55

Development

Successfully merging this pull request may close these issues.

3 participants

@roberto-bayardo @patrick-ogrady @danlaine