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

Performance enhancements related to caching #269

Merged
merged 14 commits into from Nov 24, 2022
Merged

Performance enhancements related to caching #269

merged 14 commits into from Nov 24, 2022

Conversation

slawlor
Copy link
Contributor

@slawlor slawlor commented Nov 22, 2022

This PR adds support for

  1. dashmap which is a more performant multi-access cache when there are multiple readers and writers
  2. Removing tokio::sync::RwLock<bool> and replacing it with an AtomicBool which is also more performant under high-parallel load
  3. Adding support for memory pressure processing to limit the cache size based on the memory pressure on the system
  4. A complete rewrite of the storage layer to a Database trait and StorageManager struct which manages the specified database. This greatly simplifies what a db needs to implement and abstracts away caching & transaction management to a shared logic space.

Update: closes #217

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 22, 2022
…ing and transactions away from the underlying database implementation so the implementer won't care.
@slawlor
Copy link
Contributor Author

slawlor commented Nov 23, 2022

A note to reviewers, it's probably better to review this commit-by-commit. It'll make more sense and break-down the changes to more manageable amounts

@slawlor slawlor marked this pull request as ready for review November 23, 2022 23:55
Copy link
Contributor

@kevinlewi kevinlewi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me. The separating of the cache logic from the db is nice to have for sure!

akd/src/storage/cache/mod.rs Outdated Show resolved Hide resolved
@slawlor slawlor merged commit 9e2ef7e into main Nov 24, 2022
@slawlor slawlor deleted the slawlor/perf branch November 24, 2022 14:21
slawlor added a commit that referenced this pull request Nov 24, 2022
* Adding support for dashmap and the ability to measure the size of the cache

* Adding support for memory pressure processing, such that we can protect out-of-memory (OOM) crashes

* AKD rewrite with new storage model. This abstracts the notion of caching and transactions away from the underlying database implementation so the implementer won't care.

* Refactoring the other libraries to support the new storage model

* Removing features and moving to always enabled functionality

* cargo fmt

* Rename Storage trait to Database

* Fix type annotation in transaction.rs so clippy can run properly

* Small bug in setting cached values on a db batch set write

* removing old feature flag

* Resolving #217

* Exposing cache clean frequency option

* logical ordering of transaction v. cache mgmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix faulty azks polling test
4 participants