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

Fix accounts_db store counts in purging accounts logic #8218

Merged
merged 2 commits into from
Feb 12, 2020
Merged

Fix accounts_db store counts in purging accounts logic #8218

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5485fd6
Show insufficient purge_zero_lamport_account logic
ryoqun Feb 8, 2020
c567f11
Add another pass to detect non-deleted values and increment the count
sakridge Feb 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions runtime/src/accounts_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,21 @@ impl AccountsDB {
}
}

for account_infos in purges.values() {
let mut no_delete = false;
for (_slot_id, account_info) in account_infos {
if *store_counts.get(&account_info.store_id).unwrap() != 0 {
no_delete = true;
break;
}
}
if no_delete {
for (_slot_id, account_info) in account_infos {
Copy link
Contributor

Choose a reason for hiding this comment

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

I carefully re-read the purge_zero_lamport_accounts. And I think this new logic works as intended. Very nice and thank you so much! It's sad for me not to notice nor write this... ;)

My previous perception was wrong... Especially the DAG thing. I wonder why I was urged to think so. Maybe, too tired...

And this makes purge_zero_lamport_accounts yet more conservative. So, regardless a hostile or normal situation, I think accumulated zero lamport accounts impose us no-op wasted CPU time by bunch of loops for each snapshot_slot_interval to some extent.

Especially, this loop exhibits some contaminating/cascading behavior, easily approaching to the worst cost of O(N) where N is the total number of all updates to every accounts which became 0 lamport in the past at least once. That's because making no_delete be true for vast of unrelated pubkeys can easily be done by triggering the avalanche counting just with a single zero account with many updates.

In near future, I think we'd need some compaction mechanism as implied by #8168...

*store_counts.get_mut(&account_info.store_id).unwrap() += 1;
}
}
}

// Only keep purges where the entire history of the account in the root set
// can be purged. All AppendVecs for those updates are dead.
purges.retain(|_pubkey, account_infos| {
Expand Down Expand Up @@ -2126,6 +2141,57 @@ pub mod tests {
assert_load_account(&accounts, current_slot, pubkey, zero_lamport);
}

#[test]
fn test_accounts_purge_chained() {
solana_logger::setup();

let some_lamport = 223;
let zero_lamport = 0;
let dummy_lamport = 999;
let no_data = 0;
let owner = Account::default().owner;

let account = Account::new(some_lamport, no_data, &owner);
let account2 = Account::new(some_lamport + 100_001, no_data, &owner);
let account3 = Account::new(some_lamport + 100_002, no_data, &owner);
let zero_lamport_account = Account::new(zero_lamport, no_data, &owner);

let pubkey = Pubkey::new_rand();
let purged_pubkey1 = Pubkey::new_rand();
let purged_pubkey2 = Pubkey::new_rand();

let dummy_account = Account::new(dummy_lamport, no_data, &owner);
let dummy_pubkey = Pubkey::default();

let accounts = AccountsDB::new_single();

let mut current_slot = 1;
accounts.store(current_slot, &[(&pubkey, &account)]);
accounts.store(current_slot, &[(&purged_pubkey1, &account2)]);
accounts.add_root(current_slot);

current_slot += 1;
accounts.store(current_slot, &[(&purged_pubkey1, &zero_lamport_account)]);
accounts.store(current_slot, &[(&purged_pubkey2, &account3)]);
accounts.add_root(current_slot);

current_slot += 1;
accounts.store(current_slot, &[(&purged_pubkey2, &zero_lamport_account)]);
accounts.add_root(current_slot);

current_slot += 1;
accounts.store(current_slot, &[(&dummy_pubkey, &dummy_account)]);
accounts.add_root(current_slot);

purge_zero_lamport_accounts(&accounts, current_slot);
let accounts = reconstruct_accounts_db_via_serialization(&accounts, current_slot);

assert_load_account(&accounts, current_slot, pubkey, some_lamport);
assert_load_account(&accounts, current_slot, purged_pubkey1, 0);
assert_load_account(&accounts, current_slot, purged_pubkey2, 0);
assert_load_account(&accounts, current_slot, dummy_pubkey, dummy_lamport);
}

#[test]
#[ignore]
fn test_store_account_stress() {
Expand Down