[wip] Make generate_index 2-pass for dead slots cleanup#8337
[wip] Make generate_index 2-pass for dead slots cleanup#8337ryoqun wants to merge 6 commits intosolana-labs:masterfrom
Conversation
| } | ||
|
|
||
| let mut reclaims: Vec<(u64, AccountInfo)> = vec![]; | ||
| accounts_index.clear(); |
There was a problem hiding this comment.
We start over the generation of index here. :)
| } else { | ||
| trace!("id: {} clearing count", id); | ||
| store.count_and_status.write().unwrap().0 = 0; | ||
| let mut c = store.count_and_status.write().unwrap(); |
There was a problem hiding this comment.
This part is the main dish in this PR; In this condition, we're pretty sure we can remove those Storages (=AppendVecs). And majority of thoese storages are under this category.
| AccountsDB::merge(&mut account_maps, &maps); | ||
| } | ||
| for (pubkey, (_, account_info)) in account_maps.iter() { | ||
| accounts_index.insert(*alive_slot, pubkey, account_info.clone(), &mut reclaims); |
There was a problem hiding this comment.
We had to be conservative here; purge_zero_lamport_account didn't work since the introduction of #8218 after generate_index and before verify_snapshot().
Corresponding test is this https://github.com/solana-labs/solana/pull/8337/files#diff-2099c5256db4eb5975c8834af38f6456R2307
There was a problem hiding this comment.
convervative means previously we only kept the last entry for each given pubkey in account index. That's very aggressive in a way. That's because this is updating the index after marking the updated slot as root before that.
ryoqun
force-pushed
the
2-pass-generate-index
branch
from
e6fc62d to
549ee14
Compare
| let mut account_maps = accumulator.pop().unwrap(); | ||
| let mut account_maps = std::collections::BTreeMap::new(); | ||
| while let Some(maps) = accumulator.pop() { | ||
| AccountsDB::merge(&mut account_maps, &maps); |
There was a problem hiding this comment.
we can't merge here; we must faithfully recalculate the store count here.
| @@ -1431,6 +1522,12 @@ impl AccountsDB { | |||
|
|
|||
| //d b g!(&reclaims); | |||
| self.handle_reclaims(&reclaims); | |||
There was a problem hiding this comment.
move this inner loop
| accounts.add_root(current_slot); | ||
|
|
||
| current_slot += 1; | ||
| accounts.store(current_slot, &[(&dummy_pubkey, &dummy_account)]); |
There was a problem hiding this comment.
Originally, this workaround is added here: https://github.com/solana-labs/solana/pull/8176/files#r376724066
To add next artificial slot, in addition of just add_root, I also had to store a dummy account otherwise missing bank hash error occurred.
| dead_slots | ||
| } | ||
|
|
||
| // The caller must ensure given slots are actually dead (= older than the root) |
Codecov Report
@@ Coverage Diff @@
## master #8337 +/- ##
=======================================
Coverage 80.3% 80.4%
=======================================
Files 254 254
Lines 55683 55883 +200
=======================================
+ Hits 44765 44965 +200
Misses 10918 10918 |
|
Phew, CI's back to green!!! |
|
This PR still contains the something to be merged. I'll work on this today. |
|
To be clear, the very cleaning logic is still not landed on the master... |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
stale
Bot
added
the
stale
Conservative ones got landed at #8811 |
stale
Bot
removed
the
stale
1 participant
Problem
#8168
Summary of Changes
This is still rough; but works.
When backporting, do so in tandem with #8148. Only backporting with one of the two introduces bugs.
Fixes #8168