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

Removes write version from tiered storage #33566

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Oct 6, 2023

Problem

"Write version" is a relic when we appended to AppendVecs and supported multiple AppendVecs per slot. The write version was used to identify the correct version of each account to use (i.e. pick the version with the highest write version).

With the new tiered storage, we'll always perform writing the full file in one shot, and appends will not be allowed. This means there will only ever be one account per file. Additionally, we only allow one accounts storage file per slot, so later slots hold more recent account information.

This means the write version is not needed for the new tiered storage format.

Summary of Changes

Remove write version from tiered storage.

@brooksprumo brooksprumo self-assigned this Oct 6, 2023
@brooksprumo brooksprumo marked this pull request as ready for review October 6, 2023 17:33
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #33566 (7df960a) into master (ecb1f8a) will decrease coverage by 0.1%.
The diff coverage is 96.6%.

@@            Coverage Diff            @@
##           master   #33566     +/-   ##
=========================================
- Coverage    81.7%    81.7%   -0.1%     
=========================================
  Files         805      805             
  Lines      218246   218180     -66     
=========================================
- Hits       178436   178358     -78     
- Misses      39810    39822     +12     

@brooksprumo
Copy link
Contributor Author

I'll rebase this one once #33544 is merged. I'm guessing there will be merge conflicts I'll need to resolve—I don't want to break the build!

yhchiang-sol
yhchiang-sol previously approved these changes Oct 6, 2023
Copy link
Contributor

@yhchiang-sol yhchiang-sol left a comment

Choose a reason for hiding this comment

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

Looks good! Glad that we have more than one contributor on tiered-storage now!

Thanks for working on this!

accounts-db/src/tiered_storage/byte_block.rs Show resolved Hide resolved
@brooksprumo
Copy link
Contributor Author

Force-pushed due to rebase.

Copy link
Contributor

@yhchiang-sol yhchiang-sol left a comment

Choose a reason for hiding this comment

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

LG!

@brooksprumo brooksprumo merged commit bb27bd8 into solana-labs:master Oct 6, 2023
16 checks passed
@brooksprumo brooksprumo deleted the tiered-storage/remove-write-version branch October 6, 2023 20:19
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.

2 participants