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

add ancient append vec test #33762

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Oct 19, 2023

Problem

When we disable rewrites, we will accumulate old append vecs & slots. We use ancient append vec packing to combine those. There exist many accounts which will remain in their dead state indefinitely.
As a result, if we don't distinguish different categories of accounts where ref_count > 1, we'll have many slots which can't be packed and moved.

Summary of Changes

Add a test to prepare to distinguish accounts with refcount > 1 where there is an alive index entry with a higher slot #. Soon, accounts will be treated differently if refcount > 1 depending on if this slot's index entry for the pubkey is higher or lower than other alive entries.

This new test is very similar to the existing test_calc_accounts_to_combine_opposite. In the next pr, that test will change to test the case where we have 2 refs, and 1 is from an older slot.

Fixes #

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #33762 (6200490) into master (0fcc0a0) will increase coverage by 0.0%.
Report is 7 commits behind head on master.
The diff coverage is 100.0%.

@@           Coverage Diff            @@
##           master   #33762    +/-   ##
========================================
  Coverage    81.8%    81.8%            
========================================
  Files         806      806            
  Lines      217905   217988    +83     
========================================
+ Hits       178278   178380   +102     
+ Misses      39627    39608    -19     

Co-authored-by: Brooks <brooks@prumo.org>
accounts_shrunk_same_slot.first().unwrap().pubkey(),
pk_with_2_refs
);
assert_eq!(
Copy link
Contributor

Choose a reason for hiding this comment

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

So account_with_1_ref shouldn't exist in the new_storage, right?
Can we add an assert for this?

Copy link
Contributor

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm

@jeffwashington jeffwashington merged commit f13c78b into solana-labs:master Oct 19, 2023
16 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