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

[TieredStorage] Add AccountsFile::TieredStorage #72

Merged
merged 1 commit into from
Mar 24, 2024

Conversation

yhchiang-sol
Copy link

@yhchiang-sol yhchiang-sol commented Mar 4, 2024

Problem

AccountsFile currently doesn't have an implementation for TieredStorage.
To enable AccountsDB tests for the TieredStorage, we need AccountsFile
to support TieredStorage.

Summary of Changes

This PR implements a AccountsFile::TieredStorage, a thin wrapper between
AccountsFile and TieredStorage.

@yhchiang-sol
Copy link
Author

Putting it back to draft to understand more about the test failure.

@brooksprumo brooksprumo removed their request for review March 5, 2024 20:18
@brooksprumo
Copy link

Please readd me as a reviewer when the PR is ready. Thanks!

@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2024

Codecov Report

Attention: Patch coverage is 21.42857% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 81.8%. Comparing base (b6d2237) to head (5b85d59).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master      #72     +/-   ##
=========================================
- Coverage    81.9%    81.8%   -0.1%     
=========================================
  Files         840      840             
  Lines      228058   228105     +47     
=========================================
+ Hits       186795   186815     +20     
- Misses      41263    41290     +27     

@yhchiang-sol yhchiang-sol force-pushed the ts-pr-accounts-file-hot branch 4 times, most recently from 37f4515 to 80b251e Compare March 15, 2024 17:36
@yhchiang-sol yhchiang-sol force-pushed the ts-pr-accounts-file-hot branch from 80b251e to b5b3309 Compare March 20, 2024 17:51
@yhchiang-sol yhchiang-sol marked this pull request as ready for review March 20, 2024 17:51
@yhchiang-sol yhchiang-sol changed the title [TieredStorage] Add AccountsFile::TieredHot [TieredStorage] Add AccountsFile::TieredStorage Mar 20, 2024
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/ancient_append_vecs.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Show resolved Hide resolved
@yhchiang-sol
Copy link
Author

Rebase to address conflicts.

@yhchiang-sol yhchiang-sol force-pushed the ts-pr-accounts-file-hot branch 2 times, most recently from 9db160d to 40030e1 Compare March 22, 2024 08:45
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Show resolved Hide resolved
accounts-db/src/accounts_file.rs Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
@yhchiang-sol yhchiang-sol force-pushed the ts-pr-accounts-file-hot branch from 732964a to 717476f Compare March 22, 2024 20:51
@yhchiang-sol
Copy link
Author

Rebased and addressed comments.

@yhchiang-sol
Copy link
Author

Rebased on top of master with #400

@yhchiang-sol yhchiang-sol force-pushed the ts-pr-accounts-file-hot branch from 0c7053d to 5f71d03 Compare March 22, 2024 22:05
@yhchiang-sol
Copy link
Author

Rebased on top of #401 that includes the capacity() API.

accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_file.rs Outdated Show resolved Hide resolved
@yhchiang-sol yhchiang-sol force-pushed the ts-pr-accounts-file-hot branch from 5f71d03 to 8441131 Compare March 23, 2024 00:40
@yhchiang-sol
Copy link
Author

Rebased on top of #401

@yhchiang-sol yhchiang-sol force-pushed the ts-pr-accounts-file-hot branch from a33f52f to 6a825f1 Compare March 24, 2024 05:16
@yhchiang-sol yhchiang-sol force-pushed the ts-pr-accounts-file-hot branch from 6a825f1 to 5b85d59 Compare March 24, 2024 05:17
@yhchiang-sol
Copy link
Author

Rebased on top of master with #401

Copy link

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

:shipit:

@yhchiang-sol yhchiang-sol merged commit a916edb into anza-xyz:master Mar 24, 2024
37 checks passed
yhchiang-sol added a commit that referenced this pull request Mar 25, 2024
…ed-storage (#418)

#### Problem
As #72 introduced AccountsFile::TieredStorage, it also performs
file-type check when opening an accounts-file to determine whether
it is a tiered-storage or an append-vec.  But before tiered-storage is
enabled, this opening check is unnecessary.

#### Summary of Changes
Remove the accounts-file type check code and simply assume everything
is append-vec on AccountsFile::new_from_file().
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.

3 participants