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

remaining_bytes aligns len since all writes will align first #34171

Merged
merged 4 commits into from
Nov 21, 2023

Conversation

jeffwashington
Copy link
Contributor

Problem

ancient append vec appending relies on a reliable count of remaining bytes. It turns out that the append vec's len() is up to the last written byte. This can be non-u64 aligned. The result is if we are to append more to the file, the first operation will be a u64_align!. This means that remaining_bytes was reporting too many bytes. Not all those bytes could be written to, thanks to the bytes wasted for alignment. The result was that writes could fail, causing an attempt at creating a second append vec for the slot. This is no longer allowed and will fail with an assert.

Summary of Changes

Align len() so that remaining bytes returns an accurate number.

Fixes #

Copy link

codecov bot commented Nov 19, 2023

Codecov Report

Merging #34171 (2f84b89) into master (3368579) will decrease coverage by 0.1%.
Report is 2 commits behind head on master.
The diff coverage is 95.6%.

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #34171     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         819      819             
  Lines      220168   220189     +21     
=========================================
- Hits       180441   180428     -13     
- Misses      39727    39761     +34     

@HaoranYi HaoranYi self-assigned this Nov 20, 2023
@@ -5752,7 +5752,7 @@ impl AccountsDb {
fn has_space_available(&self, slot: Slot, size: u64) -> bool {
let store = self.storage.get_slot_storage_entry(slot).unwrap();
if store.status() == AccountStorageStatus::Available
&& (store.accounts.capacity() - store.accounts.len() as u64) > size
&& store.accounts.remaining_bytes() > size
Copy link
Contributor

@HaoranYi HaoranYi Nov 20, 2023

Choose a reason for hiding this comment

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

Since we are fixing remaining bytes, has_space_available should be fixed too, stay consistent. right?

@HaoranYi HaoranYi marked this pull request as ready for review November 20, 2023 15:00
Copy link
Contributor

@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.

lgtm

@HaoranYi HaoranYi merged commit 481c357 into solana-labs:master Nov 21, 2023
32 checks passed
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