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

Refactor for threadsafe BucketListDB #4172

Closed
wants to merge 3 commits into from

Conversation

SirTyson
Copy link
Contributor

@SirTyson SirTyson commented Jan 30, 2024

Description

First step towards #4127

This PR refactors BucketListDB code as a first step towards parallel BucketListDB access. This should have no observable changes.

Currently, BucketListDB is thread safe with the exception of Bucket file streams. This change refactors all BucketListDB code out of BucketList and Bucket and into SearchableBucketListSnapshot and SearchableBucketSnapshot. SearchableBucketListSnapshot is a lightweight wrapper around BucketList that stores SearchableBucketSnapshot. SearchableBucketSnapshot is another wrapper class that maintains a pointer to the underlying Bucket as well as a stream to read from the Bucket. In order to facilitate multi thread reads, all BucketList loads must go through these snapshot classes that maintain their own local stream object.

In addition to the refactor, a few mutexes have been added around adding entries to the BucketList and modifying ledger header pointers. While these locks are not currently necessary, further work will build on this and actually enable parallel access.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

@SirTyson SirTyson mentioned this pull request Jan 30, 2024
6 tasks
@SirTyson SirTyson mentioned this pull request Feb 13, 2024
6 tasks
@SirTyson SirTyson requested a review from dmkozh February 13, 2024 18:40
@SirTyson
Copy link
Contributor Author

Closing, we should just merge #4176 instead.

@SirTyson SirTyson closed this Feb 26, 2024
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.

1 participant