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

allow test feature to skip rewrites #33851

Merged
merged 13 commits into from
Oct 27, 2023
42 changes: 37 additions & 5 deletions accounts-db/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ pub const ACCOUNTS_DB_CONFIG_FOR_TESTING: AccountsDbConfig = AccountsDbConfig {
exhaustively_verify_refcounts: false,
create_ancient_storage: CreateAncientStorage::Pack,
test_partitioned_epoch_rewards: TestPartitionedEpochRewards::CompareResults,
test_skip_rewrites_but_include_in_bank_hash: false,
};
pub const ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS: AccountsDbConfig = AccountsDbConfig {
index: Some(ACCOUNTS_INDEX_CONFIG_FOR_BENCHMARKS),
Expand All @@ -498,6 +499,7 @@ pub const ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS: AccountsDbConfig = AccountsDbConfig
exhaustively_verify_refcounts: false,
create_ancient_storage: CreateAncientStorage::Pack,
test_partitioned_epoch_rewards: TestPartitionedEpochRewards::None,
test_skip_rewrites_but_include_in_bank_hash: false,
};

pub type BinnedHashData = Vec<Vec<CalculateHashIntermediate>>;
Expand Down Expand Up @@ -557,6 +559,7 @@ pub struct AccountsDbConfig {
/// if None, ancient append vecs are set to ANCIENT_APPEND_VEC_DEFAULT_OFFSET
/// Some(offset) means include slots up to (max_slot - (slots_per_epoch - 'offset'))
pub ancient_append_vec_offset: Option<i64>,
pub test_skip_rewrites_but_include_in_bank_hash: bool,
pub skip_initial_hash_calc: bool,
pub exhaustively_verify_refcounts: bool,
/// how to create ancient storages
Expand Down Expand Up @@ -1440,6 +1443,9 @@ pub struct AccountsDb {
/// from AccountsDbConfig
create_ancient_storage: CreateAncientStorage,

/// true if this client should skip rewrites but still include those rewrites in the bank hash as if rewrites had occurred.
pub test_skip_rewrites_but_include_in_bank_hash: bool,

pub accounts_cache: AccountsCache,

write_cache_limit_bytes: Option<u64>,
Expand Down Expand Up @@ -1573,6 +1579,7 @@ pub struct AccountsStats {
delta_hash_scan_time_total_us: AtomicU64,
delta_hash_accumulate_time_total_us: AtomicU64,
delta_hash_num: AtomicU64,
skipped_rewrites_num: AtomicUsize,

last_store_report: AtomicInterval,
store_hash_accounts: AtomicU64,
Expand Down Expand Up @@ -2547,6 +2554,7 @@ impl AccountsDb {
exhaustively_verify_refcounts: false,
partitioned_epoch_rewards_config: PartitionedEpochRewardsConfig::default(),
epoch_accounts_hash_manager: EpochAccountsHashManager::new_invalid(),
test_skip_rewrites_but_include_in_bank_hash: false,
}
}

Expand Down Expand Up @@ -2622,6 +2630,11 @@ impl AccountsDb {
.map(|config| config.test_partitioned_epoch_rewards)
.unwrap_or_default();

let test_skip_rewrites_but_include_in_bank_hash = accounts_db_config
.as_ref()
.map(|config| config.test_skip_rewrites_but_include_in_bank_hash)
.unwrap_or_default();

let partitioned_epoch_rewards_config: PartitionedEpochRewardsConfig =
PartitionedEpochRewardsConfig::new(test_partitioned_epoch_rewards);

Expand All @@ -2647,6 +2660,7 @@ impl AccountsDb {
.and_then(|x| x.write_cache_limit_bytes),
partitioned_epoch_rewards_config,
exhaustively_verify_refcounts,
test_skip_rewrites_but_include_in_bank_hash,
..Self::default_with_accounts_index(
accounts_index,
base_working_path,
Expand Down Expand Up @@ -6944,6 +6958,11 @@ impl AccountsDb {
.swap(0, Ordering::Relaxed),
i64
),
(
"skipped_rewrites_num",
self.stats.skipped_rewrites_num.swap(0, Ordering::Relaxed),
i64
),
);
}

Expand Down Expand Up @@ -7908,7 +7927,6 @@ impl AccountsDb {
slot: Slot,
) -> (Vec<(Pubkey, AccountHash)>, u64, Measure) {
let mut scan = Measure::start("scan");

let scan_result: ScanStorageResult<(Pubkey, AccountHash), DashMap<Pubkey, AccountHash>> =
self.scan_account_storage(
slot,
Expand All @@ -7928,6 +7946,7 @@ impl AccountsDb {
ScanStorageResult::Cached(cached_result) => cached_result,
ScanStorageResult::Stored(stored_result) => stored_result.into_iter().collect(),
};

(hashes, scan.as_us(), accumulate)
}

Expand Down Expand Up @@ -7968,12 +7987,12 @@ impl AccountsDb {
}
}

/// Calculate accounts delta hash for `slot`
/// Wrapper function to calculate accounts delta hash for `slot` (only used for testing and benchmarking.)
///
/// As part of calculating the accounts delta hash, get a list of accounts modified this slot
/// (aka dirty pubkeys) and add them to `self.uncleaned_pubkeys` for future cleaning.
pub fn calculate_accounts_delta_hash(&self, slot: Slot) -> AccountsDeltaHash {
self.calculate_accounts_delta_hash_internal(slot, None)
self.calculate_accounts_delta_hash_internal(slot, None, HashMap::default())
}

/// Calculate accounts delta hash for `slot`
Expand All @@ -7984,9 +8003,20 @@ impl AccountsDb {
&self,
slot: Slot,
ignore: Option<Pubkey>,
mut skipped_rewrites: HashMap<Pubkey, AccountHash>,
) -> AccountsDeltaHash {
let (mut hashes, scan_us, mut accumulate) = self.get_pubkey_hash_for_slot(slot);
let dirty_keys = hashes.iter().map(|(pubkey, _hash)| *pubkey).collect();

hashes.iter().for_each(|(k, _h)| {
skipped_rewrites.remove(k);
});

let num_skipped_rewrites = skipped_rewrites.len();
hashes.extend(skipped_rewrites);

info!("skipped rewrite hashes {} {}", slot, num_skipped_rewrites);

if let Some(ignore) = ignore {
hashes.retain(|k| k.0 != ignore);
}
Expand Down Expand Up @@ -8015,6 +8045,10 @@ impl AccountsDb {
.delta_hash_accumulate_time_total_us
.fetch_add(accumulate.as_us(), Ordering::Relaxed);
self.stats.delta_hash_num.fetch_add(1, Ordering::Relaxed);
self.stats
.skipped_rewrites_num
.fetch_add(num_skipped_rewrites, Ordering::Relaxed);

accounts_delta_hash
}

Expand Down Expand Up @@ -15020,7 +15054,6 @@ pub mod tests {
db.store_uncached(1, &[(&account_key1, &account2)]);
db.calculate_accounts_delta_hash(0);
db.calculate_accounts_delta_hash(1);

db.print_accounts_stats("pre-clean1");

// clean accounts - no accounts should be cleaned, since no rooted slots
Expand All @@ -15042,7 +15075,6 @@ pub mod tests {
db.store_uncached(2, &[(&account_key2, &account3)]);
db.store_uncached(2, &[(&account_key1, &account3)]);
db.calculate_accounts_delta_hash(2);

db.clean_accounts_for_tests();
db.print_accounts_stats("post-clean2");

Expand Down
2 changes: 1 addition & 1 deletion accounts-db/src/accounts_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1243,7 +1243,7 @@ pub enum ZeroLamportAccounts {

/// Hash of an account
#[repr(transparent)]
#[derive(Debug, Copy, Clone, Eq, PartialEq, Pod, Zeroable)]
#[derive(Debug, Copy, Clone, Eq, PartialEq, Pod, Zeroable, AbiExample)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this is caused by the addition of the rewrites on Bank?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah.

pub struct AccountHash(pub Hash);

// Ensure the newtype wrapper never changes size from the underlying Hash
Expand Down
2 changes: 2 additions & 0 deletions ledger-tool/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ pub fn get_accounts_db_config(
exhaustively_verify_refcounts: arg_matches.is_present("accounts_db_verify_refcounts"),
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"),
..AccountsDbConfig::default()
}
}
Expand Down
13 changes: 13 additions & 0 deletions ledger-tool/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,12 @@ fn main() {
"Debug option to scan all AppendVecs and verify account index refcounts prior to clean",
)
.hidden(hidden_unless_forced());
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-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")
.long("accounts-filler-count")
.value_name("COUNT")
Expand Down Expand Up @@ -1556,6 +1562,7 @@ fn main() {
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.arg(&accounts_db_skip_initial_hash_calc_arg)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
)
.subcommand(
SubCommand::with_name("shred-meta")
Expand All @@ -1573,6 +1580,7 @@ fn main() {
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.arg(&accounts_db_skip_initial_hash_calc_arg)
.arg(&accounts_db_test_skip_rewrites_but_include_in_bank_hash)
)
.subcommand(
SubCommand::with_name("bounds")
Expand Down Expand Up @@ -1608,6 +1616,7 @@ fn main() {
.arg(&disable_disk_index)
.arg(&accountsdb_skip_shrink)
.arg(&accountsdb_verify_refcounts)
.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 @@ -1688,6 +1697,7 @@ fn main() {
.arg(&accounts_index_limit)
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.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 @@ -1724,6 +1734,7 @@ fn main() {
.arg(&accounts_index_limit)
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.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 @@ -1918,6 +1929,7 @@ fn main() {
.arg(&accounts_index_limit)
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.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 @@ -1952,6 +1964,7 @@ fn main() {
.arg(&accounts_index_limit)
.arg(&disable_disk_index)
.arg(&accountsdb_verify_refcounts)
.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
Loading