Skip to content

Commit

Permalink
reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
HaoranYi committed Oct 25, 2023
1 parent b6ebeb7 commit 238dda6
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 104 deletions.
122 changes: 58 additions & 64 deletions accounts-db/src/accounts_db.rs
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion ledger-tool/src/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub fn get_accounts_db_config(
skip_initial_hash_calc: arg_matches.is_present("accounts_db_skip_initial_hash_calculation"),
test_partitioned_epoch_rewards,
test_skip_rewrites_but_include_in_bank_hash: arg_matches
.is_present("accounts_db_test_skip_rewrites_but_include_in_bank_hash"),
.is_present("accounts_db_test_skip_rewrites"),
..AccountsDbConfig::default()
}
}
Expand Down
20 changes: 10 additions & 10 deletions ledger-tool/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1128,10 +1128,10 @@ fn main() {
"Debug option to scan all AppendVecs and verify account index refcounts prior to clean",
)
.hidden(hidden_unless_forced());
let accountsdb_test_skip_rewrites_but_include_in_bank_hash = Arg::with_name("accounts_db_test_skip_rewrites_but_include_in_bank_hash")
.long("accounts-db-test-skip-rewrites-but-include-in-bank-hash")
let accounts_db_test_skip_rewrites_but_include_in_bank_hash = Arg::with_name("accounts_db_test_skip_rewrites")
.long("accounts-db-test-skip-rewrites")
.help(
"Debug option to skip rewrites for rent exempted accounts but still added them in bank delta hash calculation",
"Debug option to skip rewrites for rent-exempt accounts but still add them in bank delta hash calculation",
)
.hidden(hidden_unless_forced());
let accounts_filler_count = Arg::with_name("accounts_filler_count")
Expand Down Expand Up @@ -1562,7 +1562,7 @@ fn main() {
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.arg(&accounts_db_skip_initial_hash_calc_arg)
.arg(&accountsdb_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
)
.subcommand(
SubCommand::with_name("shred-meta")
Expand All @@ -1580,7 +1580,7 @@ fn main() {
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.arg(&accounts_db_skip_initial_hash_calc_arg)
.arg(&accountsdb_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
)
.subcommand(
SubCommand::with_name("bounds")
Expand Down Expand Up @@ -1616,7 +1616,7 @@ fn main() {
.arg(&disable_disk_index)
.arg(&accountsdb_skip_shrink)
.arg(&accountsdb_verify_refcounts)
.arg(&accountsdb_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_filler_count)
.arg(&accounts_filler_size)
.arg(&verify_index_arg)
Expand Down Expand Up @@ -1697,7 +1697,7 @@ fn main() {
.arg(&accounts_index_limit)
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.arg(&accountsdb_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_skip_initial_hash_calc_arg)
.arg(&halt_at_slot_arg)
.arg(&hard_forks_arg)
Expand Down Expand Up @@ -1734,7 +1734,7 @@ fn main() {
.arg(&accounts_index_limit)
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.arg(&accountsdb_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_skip_initial_hash_calc_arg)
.arg(&accountsdb_skip_shrink)
.arg(&ancient_append_vecs)
Expand Down Expand Up @@ -1929,7 +1929,7 @@ fn main() {
.arg(&accounts_index_limit)
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.arg(&accountsdb_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_skip_initial_hash_calc_arg)
.arg(&halt_at_slot_arg)
.arg(&hard_forks_arg)
Expand Down Expand Up @@ -1964,7 +1964,7 @@ fn main() {
.arg(&accounts_index_limit)
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.arg(&accountsdb_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
.arg(&accounts_db_skip_initial_hash_calc_arg)
.arg(&halt_at_slot_arg)
.arg(&hard_forks_arg)
Expand Down
4 changes: 1 addition & 3 deletions runtime/benches/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,7 @@ fn test_accounts_delta_hash(bencher: &mut Bencher) {
let mut pubkeys: Vec<Pubkey> = vec![];
create_test_accounts(&accounts, &mut pubkeys, 100_000, 0);
bencher.iter(|| {
accounts
.accounts_db
.calculate_accounts_delta_hash(0, Vec::default());
accounts.accounts_db.calculate_accounts_delta_hash(0);
});
}

Expand Down
38 changes: 17 additions & 21 deletions runtime/src/serde_snapshot/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,7 @@ mod serde_snapshot_tests {
create_test_accounts(&accounts, &mut pubkeys, 100, slot);
check_accounts_local(&accounts, &pubkeys, 100);
accounts.add_root(slot);
let accounts_delta_hash = accounts
.accounts_db
.calculate_accounts_delta_hash(slot, Vec::default());
let accounts_delta_hash = accounts.accounts_db.calculate_accounts_delta_hash(slot);
let accounts_hash = AccountsHash(Hash::new_unique());
accounts
.accounts_db
Expand Down Expand Up @@ -272,9 +270,7 @@ mod serde_snapshot_tests {
.unwrap(),
);
check_accounts_local(&daccounts, &pubkeys, 100);
let daccounts_delta_hash = daccounts
.accounts_db
.calculate_accounts_delta_hash(slot, Vec::default());
let daccounts_delta_hash = daccounts.accounts_db.calculate_accounts_delta_hash(slot);
assert_eq!(accounts_delta_hash, daccounts_delta_hash);
let daccounts_hash = daccounts.accounts_db.get_accounts_hash(slot).unwrap().0;
assert_eq!(accounts_hash, daccounts_hash);
Expand Down Expand Up @@ -304,7 +300,7 @@ mod serde_snapshot_tests {
db.store_for_tests(new_root, &[(&key2, &account0)]);
db.add_root_and_flush_write_cache(new_root);

db.calculate_accounts_delta_hash(new_root, Vec::default());
db.calculate_accounts_delta_hash(new_root);
db.update_accounts_hash_for_tests(new_root, &linear_ancestors(new_root), false, false);

// Simulate reconstruction from snapshot
Expand Down Expand Up @@ -343,7 +339,7 @@ mod serde_snapshot_tests {
accounts.add_root_and_flush_write_cache(0);
accounts.check_storage(0, 100);
accounts.check_accounts(&pubkeys, 0, 100, 2);
accounts.calculate_accounts_delta_hash(0, Vec::default());
accounts.calculate_accounts_delta_hash(0);

let mut pubkeys1: Vec<Pubkey> = vec![];

Expand All @@ -361,7 +357,7 @@ mod serde_snapshot_tests {
// accounts
accounts.create_account(&mut pubkeys1, latest_slot, 10, 0, 0);

accounts.calculate_accounts_delta_hash(latest_slot, Vec::default());
accounts.calculate_accounts_delta_hash(latest_slot);
accounts.add_root_and_flush_write_cache(latest_slot);
accounts.check_storage(1, 21);

Expand All @@ -381,7 +377,7 @@ mod serde_snapshot_tests {
// 21 + 10 = 31 accounts
accounts.create_account(&mut pubkeys2, latest_slot, 10, 0, 0);

accounts.calculate_accounts_delta_hash(latest_slot, Vec::default());
accounts.calculate_accounts_delta_hash(latest_slot);
accounts.add_root_and_flush_write_cache(latest_slot);
accounts.check_storage(2, 31);

Expand Down Expand Up @@ -479,7 +475,7 @@ mod serde_snapshot_tests {

accounts.print_accounts_stats("accounts_post_purge");

accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.update_accounts_hash_for_tests(
current_slot,
&linear_ancestors(current_slot),
Expand Down Expand Up @@ -536,7 +532,7 @@ mod serde_snapshot_tests {
accounts.add_root_and_flush_write_cache(current_slot);

accounts.print_accounts_stats("pre_f");
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.update_accounts_hash_for_tests(4, &Ancestors::default(), false, false);

let accounts = f(accounts, current_slot);
Expand Down Expand Up @@ -633,7 +629,7 @@ mod serde_snapshot_tests {
accounts.add_root_and_flush_write_cache(current_slot);

accounts.print_count_and_status("before reconstruct");
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.update_accounts_hash_for_tests(
current_slot,
&linear_ancestors(current_slot),
Expand Down Expand Up @@ -676,7 +672,7 @@ mod serde_snapshot_tests {
current_slot += 1;
accounts.store_for_tests(current_slot, &[(&pubkey1, &account)]);
accounts.store_for_tests(current_slot, &[(&pubkey2, &account)]);
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.add_root(current_slot);

// B: Test multiple updates to pubkey1 in a single slot/storage
Expand All @@ -691,15 +687,15 @@ mod serde_snapshot_tests {
// Stores to same pubkey, same slot only count once towards the
// ref count
assert_eq!(2, accounts.ref_count_for_pubkey(&pubkey1));
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);

// C: Yet more update to trigger lazy clean of step A
current_slot += 1;
assert_eq!(2, accounts.ref_count_for_pubkey(&pubkey1));
accounts.store_for_tests(current_slot, &[(&pubkey1, &account3)]);
accounts.add_root_and_flush_write_cache(current_slot);
assert_eq!(3, accounts.ref_count_for_pubkey(&pubkey1));
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.add_root_and_flush_write_cache(current_slot);

// D: Make pubkey1 0-lamport; also triggers clean of step B
Expand Down Expand Up @@ -731,13 +727,13 @@ mod serde_snapshot_tests {
3, /* == 3 - 1 + 1 */
accounts.ref_count_for_pubkey(&pubkey1)
);
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.add_root(current_slot);

// E: Avoid missing bank hash error
current_slot += 1;
accounts.store_for_tests(current_slot, &[(&dummy_pubkey, &dummy_account)]);
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.add_root(current_slot);

accounts.assert_load_account(current_slot, pubkey1, zero_lamport);
Expand Down Expand Up @@ -768,7 +764,7 @@ mod serde_snapshot_tests {
// F: Finally, make Step A cleanable
current_slot += 1;
accounts.store_for_tests(current_slot, &[(&pubkey2, &account)]);
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.add_root(current_slot);

// Do clean
Expand Down Expand Up @@ -809,7 +805,7 @@ mod serde_snapshot_tests {
accounts.store_for_tests(current_slot, &[(pubkey, &account)]);
}
let shrink_slot = current_slot;
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.add_root_and_flush_write_cache(current_slot);

current_slot += 1;
Expand All @@ -819,7 +815,7 @@ mod serde_snapshot_tests {
for pubkey in updated_pubkeys {
accounts.store_for_tests(current_slot, &[(pubkey, &account)]);
}
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.add_root_and_flush_write_cache(current_slot);

accounts.clean_accounts_for_tests();
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/snapshot_minimizer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ mod tests {
minimized_account_set.insert(*pubkey);
}
}
accounts.calculate_accounts_delta_hash(current_slot, Vec::default());
accounts.calculate_accounts_delta_hash(current_slot);
accounts.add_root_and_flush_write_cache(current_slot);
}

Expand Down
6 changes: 3 additions & 3 deletions validator/src/cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1195,9 +1195,9 @@ pub fn app<'a>(version: &'a str, default_args: &'a DefaultArgs) -> App<'a, 'a> {
.hidden(hidden_unless_forced())
)
.arg(
Arg::with_name("accounts_db_test_skip_rewrites_but_include_in_bank_hash")
.long("accounts-db-test-skip-rewrites-but-include-in-bank-hash")
.help("Debug option to skip rewrites for rent exempted accounts but still added them in bank delta hash calculation")
Arg::with_name("accounts_db_test_skip_rewrites")
.long("accounts-db-test-skip-rewrites")
.help("Debug option to skip rewrites for rent-exempt accounts but still add them in bank delta hash calculation")
.hidden(hidden_unless_forced())
)
.arg(
Expand Down
2 changes: 1 addition & 1 deletion validator/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@ pub fn main() {
.unwrap_or_default(),
test_partitioned_epoch_rewards,
test_skip_rewrites_but_include_in_bank_hash: matches
.is_present("accounts_db_test_skip_rewrites_but_include_in_bank_hash"),
.is_present("accounts_db_test_skip_rewrites"),
..AccountsDbConfig::default()
};

Expand Down

0 comments on commit 238dda6

Please sign in to comment.