Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

ancient: add many_refs_this_is_newest_alive #33741

Merged
merged 8 commits into from
Oct 20, 2023

Conversation

jeffwashington
Copy link
Contributor

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.

Summary of Changes

Allow accounts which are alive and alive at the newest slot to still get packed together and move slots.

Fixes #

@jeffwashington jeffwashington force-pushed the oc10 branch 2 times, most recently from 478921b to f056c58 Compare October 19, 2023 01:11
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #33741 (0f7209c) into master (c98c24b) will decrease coverage by 0.1%.
Report is 2 commits behind head on master.
The diff coverage is 95.4%.

@@            Coverage Diff            @@
##           master   #33741     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         806      806             
  Lines      218058   218015     -43     
=========================================
- Hits       178415   178367     -48     
- Misses      39643    39648      +5     

}
fn with_capacity(capacity: usize, slot: Slot) -> Self {
Self {
one_ref: AliveAccounts::with_capacity(capacity, slot),
many_refs: AliveAccounts::with_capacity(capacity, slot),
many_refs_this_is_newest_alive: AliveAccounts::with_capacity(0, slot),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

expectation is there are few accounts with ref_count > 1, so set capacity to 0.

@jeffwashington jeffwashington marked this pull request as ready for review October 19, 2023 18:16
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.

Looks good overall. Some comments/questions/nits. Also, can the PR's title be updated since it's not longer a work in progress (I assume)?

accounts-db/src/ancient_append_vecs.rs Outdated Show resolved Hide resolved
accounts-db/src/ancient_append_vecs.rs Outdated Show resolved Hide resolved
accounts-db/src/ancient_append_vecs.rs Show resolved Hide resolved
accounts-db/src/ancient_append_vecs.rs Outdated Show resolved Hide resolved
@jeffwashington jeffwashington changed the title wip: add many_refs_this_is_newest_alive ancient: add many_refs_this_is_newest_alive Oct 20, 2023
@jeffwashington
Copy link
Contributor Author

rebased to fix merge errors with the AccountsHash change.

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

@jeffwashington jeffwashington merged commit e137561 into solana-labs:master Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants