Skip to content

Commit

Permalink
Adds --no-skip-initial-accounts-db-clean *hidden* CLI flag (#33664)
Browse files Browse the repository at this point in the history
  • Loading branch information
brooksprumo authored Oct 12, 2023
1 parent ac788ab commit 452fd5d
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 5 deletions.
3 changes: 3 additions & 0 deletions core/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ pub struct ValidatorConfig {
pub warp_slot: Option<Slot>,
pub accounts_db_test_hash_calculation: bool,
pub accounts_db_skip_shrink: bool,
pub accounts_db_force_initial_clean: bool,
pub tpu_coalesce: Duration,
pub staked_nodes_overrides: Arc<RwLock<HashMap<Pubkey, u64>>>,
pub validator_exit: Arc<RwLock<Exit>>,
Expand Down Expand Up @@ -313,6 +314,7 @@ impl Default for ValidatorConfig {
warp_slot: None,
accounts_db_test_hash_calculation: false,
accounts_db_skip_shrink: false,
accounts_db_force_initial_clean: false,
tpu_coalesce: DEFAULT_TPU_COALESCE,
staked_nodes_overrides: Arc::new(RwLock::new(HashMap::new())),
validator_exit: Arc::new(RwLock::new(Exit::default())),
Expand Down Expand Up @@ -1759,6 +1761,7 @@ fn load_blockstore(
shrink_ratio: config.accounts_shrink_ratio,
accounts_db_test_hash_calculation: config.accounts_db_test_hash_calculation,
accounts_db_skip_shrink: config.accounts_db_skip_shrink,
accounts_db_force_initial_clean: config.accounts_db_force_initial_clean,
runtime_config: config.runtime_config.clone(),
use_snapshot_archives_at_startup: config.use_snapshot_archives_at_startup,
..blockstore_processor::ProcessOptions::default()
Expand Down
1 change: 1 addition & 0 deletions core/tests/epoch_accounts_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ fn test_snapshots_have_expected_epoch_accounts_hash() {
AccountShrinkThreshold::default(),
true,
true,
false,
true,
None,
None,
Expand Down
3 changes: 3 additions & 0 deletions core/tests/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ fn restore_from_snapshot(
check_hash_calculation,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down Expand Up @@ -893,6 +894,7 @@ fn restore_from_snapshots_and_check_banks_are_equal(
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down Expand Up @@ -1113,6 +1115,7 @@ fn test_snapshots_with_background_services(
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
exit.clone(),
Expand Down
1 change: 1 addition & 0 deletions ledger/src/bank_forks_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ fn bank_forks_from_snapshot(
process_options.shrink_ratio,
process_options.accounts_db_test_hash_calculation,
process_options.accounts_db_skip_shrink,
process_options.accounts_db_force_initial_clean,
process_options.verify_index,
process_options.accounts_db_config.clone(),
accounts_update_notifier,
Expand Down
1 change: 1 addition & 0 deletions ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@ pub struct ProcessOptions {
pub allow_dead_slots: bool,
pub accounts_db_test_hash_calculation: bool,
pub accounts_db_skip_shrink: bool,
pub accounts_db_force_initial_clean: bool,
pub accounts_db_config: Option<AccountsDbConfig>,
pub verify_index: bool,
pub shrink_ratio: AccountShrinkThreshold,
Expand Down
1 change: 1 addition & 0 deletions local-cluster/src/validator_configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub fn safe_clone_config(config: &ValidatorConfig) -> ValidatorConfig {
warp_slot: config.warp_slot,
accounts_db_test_hash_calculation: config.accounts_db_test_hash_calculation,
accounts_db_skip_shrink: config.accounts_db_skip_shrink,
accounts_db_force_initial_clean: config.accounts_db_force_initial_clean,
tpu_coalesce: config.tpu_coalesce,
staked_nodes_overrides: config.staked_nodes_overrides.clone(),
validator_exit: Arc::new(RwLock::new(Exit::default())),
Expand Down
7 changes: 4 additions & 3 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7609,12 +7609,13 @@ impl Bank {
pub fn verify_snapshot_bank(
&self,
test_hash_calculation: bool,
accounts_db_skip_shrink: bool,
skip_shrink: bool,
force_clean: bool,
last_full_snapshot_slot: Slot,
base: Option<(Slot, /*capitalization*/ u64)>,
) -> bool {
let (_, clean_time_us) = measure_us!({
let should_clean = !accounts_db_skip_shrink && self.slot() > 0;
let should_clean = force_clean || (!skip_shrink && self.slot() > 0);
if should_clean {
info!("Cleaning...");
// We cannot clean past the last full snapshot's slot because we are about to
Expand All @@ -7634,7 +7635,7 @@ impl Bank {
});

let (_, shrink_time_us) = measure_us!({
let should_shrink = !accounts_db_skip_shrink && self.slot() > 0;
let should_shrink = !skip_shrink && self.slot() > 0;
if should_shrink {
info!("Shrinking...");
self.rc.accounts.accounts_db.shrink_all_slots(
Expand Down
1 change: 1 addition & 0 deletions runtime/src/bank/serde_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ mod tests {
false,
false,
false,
false,
Some(solana_accounts_db::accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down
4 changes: 2 additions & 2 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3608,11 +3608,11 @@ fn test_verify_snapshot_bank() {
bank.freeze();
add_root_and_flush_write_cache(&bank);
bank.update_accounts_hash_for_tests();
assert!(bank.verify_snapshot_bank(true, false, bank.slot(), None));
assert!(bank.verify_snapshot_bank(true, false, false, bank.slot(), None));

// tamper the bank after freeze!
bank.increment_signature_count(1);
assert!(!bank.verify_snapshot_bank(true, false, bank.slot(), None));
assert!(!bank.verify_snapshot_bank(true, false, false, bank.slot(), None));
}

// Test that two bank forks with the same accounts should not hash to the same value.
Expand Down
11 changes: 11 additions & 0 deletions runtime/src/snapshot_bank_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ pub fn bank_from_snapshot_archives(
shrink_ratio: AccountShrinkThreshold,
test_hash_calculation: bool,
accounts_db_skip_shrink: bool,
accounts_db_force_initial_clean: bool,
verify_index: bool,
accounts_db_config: Option<AccountsDbConfig>,
accounts_update_notifier: Option<AccountsUpdateNotifier>,
Expand Down Expand Up @@ -365,6 +366,7 @@ pub fn bank_from_snapshot_archives(
if !bank.verify_snapshot_bank(
test_hash_calculation,
accounts_db_skip_shrink || !full_snapshot_archive_info.is_remote(),
accounts_db_force_initial_clean,
full_snapshot_archive_info.slot(),
base,
) && limit_load_slot_count_from_snapshot.is_none()
Expand Down Expand Up @@ -427,6 +429,7 @@ pub fn bank_from_latest_snapshot_archives(
shrink_ratio: AccountShrinkThreshold,
test_hash_calculation: bool,
accounts_db_skip_shrink: bool,
accounts_db_force_initial_clean: bool,
verify_index: bool,
accounts_db_config: Option<AccountsDbConfig>,
accounts_update_notifier: Option<AccountsUpdateNotifier>,
Expand Down Expand Up @@ -459,6 +462,7 @@ pub fn bank_from_latest_snapshot_archives(
shrink_ratio,
test_hash_calculation,
accounts_db_skip_shrink,
accounts_db_force_initial_clean,
verify_index,
accounts_db_config,
accounts_update_notifier,
Expand Down Expand Up @@ -1326,6 +1330,7 @@ mod tests {
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down Expand Up @@ -1437,6 +1442,7 @@ mod tests {
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down Expand Up @@ -1568,6 +1574,7 @@ mod tests {
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down Expand Up @@ -1689,6 +1696,7 @@ mod tests {
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down Expand Up @@ -1826,6 +1834,7 @@ mod tests {
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down Expand Up @@ -1891,6 +1900,7 @@ mod tests {
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down Expand Up @@ -2255,6 +2265,7 @@ mod tests {
false,
false,
false,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
Expand Down
7 changes: 7 additions & 0 deletions validator/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,13 @@ pub fn app<'a>(version: &'a str, default_args: &'a DefaultArgs) -> App<'a, 'a> {
.help("Debug option to scan all append vecs and verify account index refcounts prior to clean")
.hidden(hidden_unless_forced())
)
.arg(
Arg::with_name("no_skip_initial_accounts_db_clean")
.long("no-skip-initial-accounts-db-clean")
.help("Do not skip the initial cleaning of accounts when verifying snapshot bank")
.hidden(hidden_unless_forced())
.conflicts_with("accounts_db_skip_shrink")
)
.arg(
Arg::with_name("accounts_db_create_ancient_storage_packed")
.long("accounts-db-create-ancient-storage-packed")
Expand Down
1 change: 1 addition & 0 deletions validator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1390,6 +1390,7 @@ pub fn main() {
accounts_db_test_hash_calculation: matches.is_present("accounts_db_test_hash_calculation"),
accounts_db_config,
accounts_db_skip_shrink: true,
accounts_db_force_initial_clean: matches.is_present("no_skip_initial_accounts_db_clean"),
tpu_coalesce,
no_wait_for_vote_to_start_leader: matches.is_present("no_wait_for_vote_to_start_leader"),
accounts_shrink_ratio,
Expand Down

0 comments on commit 452fd5d

Please sign in to comment.