-
Notifications
You must be signed in to change notification settings - Fork 473
Allocate pages using a mult-tenant lock-free pool #2587
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
base: master
Are you sure you want to change the base?
Conversation
crates/core/src/db/datastore/locking_tx_datastore/committed_state.rs
Outdated
Show resolved
Hide resolved
9bf77a5
to
925547e
Compare
@@ -433,6 +433,7 @@ impl<P: BlobProvider> SnapshotFetcher<P> { | |||
} | |||
|
|||
fn verify_page(&self, expected_hash: blake3::Hash, buf: &[u8]) -> Result<()> { | |||
// TODO(centril, kim): consider whether we want to use the page pool here. |
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.
cc @kim
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.
Minor nits. Looks good! Would love some benchmark results and a bot test.
crates/core/src/db/datastore/locking_tx_datastore/committed_state.rs
Outdated
Show resolved
Hide resolved
benchmarks please |
Criterion benchmark resultsError when comparing benchmarks: Couldn't find AWS credentials in environment, credentials file, or IAM role. Caused by: |
Callgrind benchmark in progress... |
1 similar comment
Callgrind benchmark in progress... |
crates/core/src/db/datastore/locking_tx_datastore/committed_state.rs
Outdated
Show resolved
Hide resolved
Criterion benchmark resultsError when comparing benchmarks: Couldn't find AWS credentials in environment, credentials file, or IAM role. Caused by: |
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.
This looks good. My main comment is about adding metrics to gauge how useful this is.
1e09780
to
37d500f
Compare
Description of Changes
Fixes #2016.
This PR implements page allocation through a lock-free multi-tenant pool,
using
crossbeam_queue::ArrayQueue
as the basis of the pool.This lock-free queue is bounded. And this PR sets that bound to to 32 GiB. After that, pages will be dropped.
API and ABI breaking changes
None
Expected complexity level and risk
2
Testing
Most of the testing is done by adjusting existing tests.