diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index fd0743ba5..a58874fa0 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -48,7 +48,7 @@ impl AccountsHashVerifier { known_validators: Option>, halt_on_known_validators_accounts_hash_mismatch: bool, fault_injection_rate_slots: u64, - snapshot_config: Option, + snapshot_config: SnapshotConfig, ) -> Self { // If there are no accounts packages to process, limit how often we re-check const LOOP_LIMITER: Duration = Duration::from_millis(SLOT_MS); @@ -83,7 +83,7 @@ impl AccountsHashVerifier { &mut hashes, &exit, fault_injection_rate_slots, - snapshot_config.as_ref(), + &snapshot_config, )); datapoint_info!( @@ -184,7 +184,7 @@ impl AccountsHashVerifier { hashes: &mut Vec<(Slot, Hash)>, exit: &Arc, fault_injection_rate_slots: u64, - snapshot_config: Option<&SnapshotConfig>, + snapshot_config: &SnapshotConfig, ) { let accounts_hash = Self::calculate_and_verify_accounts_hash(&accounts_package); @@ -377,13 +377,11 @@ impl AccountsHashVerifier { fn submit_for_packaging( accounts_package: AccountsPackage, pending_snapshot_package: Option<&PendingSnapshotPackage>, - snapshot_config: Option<&SnapshotConfig>, + snapshot_config: &SnapshotConfig, accounts_hash: AccountsHash, ) { if pending_snapshot_package.is_none() - || !snapshot_config - .map(|snapshot_config| snapshot_config.should_generate_snapshots()) - .unwrap_or(false) + || !snapshot_config.should_generate_snapshots() || !matches!( accounts_package.package_type, AccountsPackageType::Snapshot(_) @@ -555,7 +553,7 @@ mod tests { &mut hashes, &exit, 0, - Some(&snapshot_config), + &snapshot_config, ); // sleep for 1ms to create a newer timestamp for gossip entry diff --git a/core/src/validator.rs b/core/src/validator.rs index 6a025846a..21763a32e 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -130,7 +130,7 @@ pub struct ValidatorConfig { pub geyser_plugin_config_files: Option>, pub rpc_addrs: Option<(SocketAddr, SocketAddr)>, // (JsonRpc, JsonRpcPubSub) pub pubsub_config: PubSubConfig, - pub snapshot_config: Option, + pub snapshot_config: SnapshotConfig, pub max_ledger_shreds: Option, pub broadcast_stage_type: BroadcastStageType, pub turbine_disabled: Arc, @@ -194,7 +194,7 @@ impl Default for ValidatorConfig { geyser_plugin_config_files: None, rpc_addrs: None, pubsub_config: PubSubConfig::default(), - snapshot_config: Some(SnapshotConfig::new_load_only()), + snapshot_config: SnapshotConfig::new_load_only(), broadcast_stage_type: BroadcastStageType::Standard, turbine_disabled: Arc::::default(), enforce_ulimit_nofile: true, @@ -577,17 +577,13 @@ impl Validator { cluster_info.restore_contact_info(ledger_path, config.contact_save_interval); let cluster_info = Arc::new(cluster_info); - // A snapshot config is required. Remove the Option<> wrapper in the future. - assert!(config.snapshot_config.is_some()); - let snapshot_config = config.snapshot_config.clone().unwrap(); - assert!(is_snapshot_config_valid( - &snapshot_config, + &config.snapshot_config, config.accounts_hash_interval_slots, )); let (pending_snapshot_package, snapshot_packager_service) = - if snapshot_config.should_generate_snapshots() { + if config.snapshot_config.should_generate_snapshots() { // filler accounts make snapshots invalid for use // so, do not publish that we have snapshots let enable_gossip_push = config @@ -601,7 +597,7 @@ impl Validator { starting_snapshot_hashes, &exit, &cluster_info, - snapshot_config.clone(), + config.snapshot_config.clone(), enable_gossip_push, ); ( @@ -629,7 +625,7 @@ impl Validator { let accounts_background_request_sender = AbsRequestSender::new(snapshot_request_sender.clone()); let snapshot_request_handler = SnapshotRequestHandler { - snapshot_config, + snapshot_config: config.snapshot_config.clone(), snapshot_request_sender, snapshot_request_receiver, accounts_package_sender, @@ -795,7 +791,7 @@ impl Validator { let json_rpc_service = JsonRpcService::new( rpc_addr, config.rpc_config.clone(), - config.snapshot_config.clone(), + Some(config.snapshot_config.clone()), bank_forks.clone(), block_commitment_cache.clone(), blockstore.clone(), @@ -1446,7 +1442,7 @@ fn load_blockstore( &blockstore, config.account_paths.clone(), config.account_shrink_paths.clone(), - config.snapshot_config.as_ref(), + Some(&config.snapshot_config), &process_options, transaction_history_services .cache_block_meta_sender @@ -1481,7 +1477,7 @@ fn load_blockstore( leader_schedule_cache.set_fixed_leader_schedule(config.fixed_leader_schedule.clone()); { let mut bank_forks = bank_forks.write().unwrap(); - bank_forks.set_snapshot_config(config.snapshot_config.clone()); + bank_forks.set_snapshot_config(Some(config.snapshot_config.clone())); bank_forks.set_accounts_hash_interval_slots(config.accounts_hash_interval_slots); if let Some(ref shrink_paths) = config.account_shrink_paths { bank_forks @@ -1658,11 +1654,6 @@ fn maybe_warp_slot( accounts_background_request_sender: &AbsRequestSender, ) -> Result<(), String> { if let Some(warp_slot) = config.warp_slot { - let snapshot_config = match config.snapshot_config.as_ref() { - Some(config) => config, - None => return Err("warp slot requires a snapshot config".to_owned()), - }; - process_blockstore.process()?; let mut bank_forks = bank_forks.write().unwrap(); @@ -1695,11 +1686,15 @@ fn maybe_warp_slot( ledger_path, &bank_forks.root_bank(), None, - &snapshot_config.full_snapshot_archives_dir, - &snapshot_config.incremental_snapshot_archives_dir, - snapshot_config.archive_format, - snapshot_config.maximum_full_snapshot_archives_to_retain, - snapshot_config.maximum_incremental_snapshot_archives_to_retain, + &config.snapshot_config.full_snapshot_archives_dir, + &config.snapshot_config.incremental_snapshot_archives_dir, + config.snapshot_config.archive_format, + config + .snapshot_config + .maximum_full_snapshot_archives_to_retain, + config + .snapshot_config + .maximum_incremental_snapshot_archives_to_retain, ) { Ok(archive_info) => archive_info, Err(e) => return Err(format!("Unable to create snapshot: {e}")), diff --git a/core/tests/epoch_accounts_hash.rs b/core/tests/epoch_accounts_hash.rs index 797f6fa07..6a2a2e63a 100755 --- a/core/tests/epoch_accounts_hash.rs +++ b/core/tests/epoch_accounts_hash.rs @@ -199,7 +199,7 @@ impl BackgroundServices { None, false, 0, - Some(snapshot_config.clone()), + snapshot_config.clone(), ); let (snapshot_request_sender, snapshot_request_receiver) = crossbeam_channel::unbounded(); diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 6151748d6..b0f05c002 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -231,7 +231,7 @@ fn run_bank_forks_snapshot_n( None, false, 0, - Some(snapshot_test_config.snapshot_config.clone()), + snapshot_test_config.snapshot_config.clone(), ); let (snapshot_request_sender, snapshot_request_receiver) = unbounded(); @@ -745,7 +745,7 @@ fn test_bank_forks_incremental_snapshot( None, false, 0, - Some(snapshot_test_config.snapshot_config.clone()), + snapshot_test_config.snapshot_config.clone(), ); let (snapshot_request_sender, snapshot_request_receiver) = unbounded(); @@ -1036,7 +1036,7 @@ fn test_snapshots_with_background_services( None, false, 0, - Some(snapshot_test_config.snapshot_config.clone()), + snapshot_test_config.snapshot_config.clone(), ); let accounts_background_service = AccountsBackgroundService::new( diff --git a/local-cluster/src/local_cluster.rs b/local-cluster/src/local_cluster.rs index 5f8e3403a..d59d93ada 100644 --- a/local-cluster/src/local_cluster.rs +++ b/local-cluster/src/local_cluster.rs @@ -148,14 +148,14 @@ impl LocalCluster { ) { config.account_paths = vec![ledger_path.join("accounts")]; config.tower_storage = Arc::new(FileTowerStorage::new(ledger_path.to_path_buf())); - if let Some(snapshot_config) = &mut config.snapshot_config { - let dummy: PathBuf = DUMMY_SNAPSHOT_CONFIG_PATH_MARKER.into(); - if snapshot_config.full_snapshot_archives_dir == dummy { - snapshot_config.full_snapshot_archives_dir = ledger_path.to_path_buf(); - } - if snapshot_config.bank_snapshots_dir == dummy { - snapshot_config.bank_snapshots_dir = ledger_path.join("snapshot"); - } + + let snapshot_config = &mut config.snapshot_config; + let dummy: PathBuf = DUMMY_SNAPSHOT_CONFIG_PATH_MARKER.into(); + if snapshot_config.full_snapshot_archives_dir == dummy { + snapshot_config.full_snapshot_archives_dir = ledger_path.to_path_buf(); + } + if snapshot_config.bank_snapshots_dir == dummy { + snapshot_config.bank_snapshots_dir = ledger_path.join("snapshot"); } } diff --git a/local-cluster/tests/common.rs b/local-cluster/tests/common.rs index 73f5e5fb7..b96ccc475 100644 --- a/local-cluster/tests/common.rs +++ b/local-cluster/tests/common.rs @@ -494,7 +494,7 @@ impl SnapshotValidatorConfig { // Create the validator config let validator_config = ValidatorConfig { - snapshot_config: Some(snapshot_config), + snapshot_config, account_paths: account_storage_paths, accounts_hash_interval_slots, ..ValidatorConfig::default_for_test() diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index fbd445339..4f3135134 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -488,14 +488,10 @@ fn test_snapshot_download() { let full_snapshot_archives_dir = &leader_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .full_snapshot_archives_dir; let incremental_snapshot_archives_dir = &leader_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .incremental_snapshot_archives_dir; trace!("Waiting for snapshot"); @@ -518,14 +514,10 @@ fn test_snapshot_download() { validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_full_snapshot_archives_to_retain, validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_incremental_snapshot_archives_to_retain, false, &mut None, @@ -580,14 +572,10 @@ fn test_incremental_snapshot_download() { let full_snapshot_archives_dir = &leader_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .full_snapshot_archives_dir; let incremental_snapshot_archives_dir = &leader_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .incremental_snapshot_archives_dir; debug!("snapshot config:\n\tfull snapshot interval: {}\n\tincremental snapshot interval: {}\n\taccounts hash interval: {}", @@ -651,14 +639,10 @@ fn test_incremental_snapshot_download() { validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_full_snapshot_archives_to_retain, validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_incremental_snapshot_archives_to_retain, false, &mut None, @@ -677,14 +661,10 @@ fn test_incremental_snapshot_download() { validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_full_snapshot_archives_to_retain, validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_incremental_snapshot_archives_to_retain, false, &mut None, @@ -824,14 +804,10 @@ fn test_incremental_snapshot_download_with_crossing_full_snapshot_interval_at_st validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_full_snapshot_archives_to_retain, validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_incremental_snapshot_archives_to_retain, false, &mut None, @@ -865,14 +841,10 @@ fn test_incremental_snapshot_download_with_crossing_full_snapshot_interval_at_st validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_full_snapshot_archives_to_retain, validator_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .maximum_incremental_snapshot_archives_to_retain, false, &mut None, @@ -1259,8 +1231,6 @@ fn test_snapshot_restart_tower() { let full_snapshot_archives_dir = &leader_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .full_snapshot_archives_dir; let full_snapshot_archive_info = cluster.wait_for_next_full_snapshot( @@ -1313,8 +1283,6 @@ fn test_snapshots_blockstore_floor() { let full_snapshot_archives_dir = &leader_snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .full_snapshot_archives_dir; let mut config = ClusterConfig { @@ -1415,8 +1383,6 @@ fn test_snapshots_restart_validity() { let full_snapshot_archives_dir = &snapshot_test_config .validator_config .snapshot_config - .as_ref() - .unwrap() .full_snapshot_archives_dir; // Set up the cluster with 1 snapshotting validator @@ -2228,7 +2194,7 @@ fn test_hard_fork_with_gap_in_roots() { let validator_b_pubkey = validators[1]; let validator_config = ValidatorConfig { - snapshot_config: Some(LocalCluster::create_dummy_load_only_snapshot_config()), + snapshot_config: LocalCluster::create_dummy_load_only_snapshot_config(), accounts_db_caching_enabled: true, ..ValidatorConfig::default() }; diff --git a/test-validator/src/lib.rs b/test-validator/src/lib.rs index 53dacedc2..a21415776 100644 --- a/test-validator/src/lib.rs +++ b/test-validator/src/lib.rs @@ -807,14 +807,14 @@ impl TestValidator { accounts_hash_interval_slots: 100, account_paths: vec![ledger_path.join("accounts")], poh_verify: false, // Skip PoH verification of ledger on startup for speed - snapshot_config: Some(SnapshotConfig { + snapshot_config: SnapshotConfig { full_snapshot_archive_interval_slots: 100, incremental_snapshot_archive_interval_slots: Slot::MAX, bank_snapshots_dir: ledger_path.join("snapshot"), full_snapshot_archives_dir: ledger_path.to_path_buf(), incremental_snapshot_archives_dir: ledger_path.to_path_buf(), ..SnapshotConfig::default() - }), + }, enforce_ulimit_nofile: false, warp_slot: config.warp_slot, validator_exit: config.validator_exit.clone(), diff --git a/validator/src/bootstrap.rs b/validator/src/bootstrap.rs index 852c62928..ee3c5d9ed 100644 --- a/validator/src/bootstrap.rs +++ b/validator/src/bootstrap.rs @@ -15,10 +15,7 @@ use { solana_runtime::{ snapshot_archive_info::SnapshotArchiveInfoGetter, snapshot_package::SnapshotType, - snapshot_utils::{ - self, DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, - DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, - }, + snapshot_utils::{self}, }, solana_sdk::{ clock::Slot, @@ -1243,18 +1240,13 @@ fn download_snapshot( desired_snapshot_hash: (Slot, Hash), snapshot_type: SnapshotType, ) -> Result<(), String> { - let (maximum_full_snapshot_archives_to_retain, maximum_incremental_snapshot_archives_to_retain) = - if let Some(snapshot_config) = validator_config.snapshot_config.as_ref() { - ( - snapshot_config.maximum_full_snapshot_archives_to_retain, - snapshot_config.maximum_incremental_snapshot_archives_to_retain, - ) - } else { - ( - DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, - DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, - ) - }; + let maximum_full_snapshot_archives_to_retain = validator_config + .snapshot_config + .maximum_full_snapshot_archives_to_retain; + let maximum_incremental_snapshot_archives_to_retain = validator_config + .snapshot_config + .maximum_incremental_snapshot_archives_to_retain; + *start_progress.write().unwrap() = ValidatorStartProgress::DownloadingSnapshot { slot: desired_snapshot_hash.0, rpc_addr: rpc_contact_info.rpc, diff --git a/validator/src/main.rs b/validator/src/main.rs index 3971f80c6..cb7e6eb69 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -1285,7 +1285,7 @@ pub fn main() { (Slot::MAX, Slot::MAX) }; - validator_config.snapshot_config = Some(SnapshotConfig { + validator_config.snapshot_config = SnapshotConfig { usage: if full_snapshot_archive_interval_slots == Slot::MAX { SnapshotUsage::LoadOnly } else { @@ -1302,14 +1302,12 @@ pub fn main() { maximum_incremental_snapshot_archives_to_retain, accounts_hash_debug_verify: validator_config.accounts_db_test_hash_calculation, packager_thread_niceness_adj: snapshot_packager_niceness_adj, - }); + }; validator_config.accounts_hash_interval_slots = value_t_or_exit!(matches, "accounts-hash-interval-slots", u64); if !is_snapshot_config_valid( - // SAFETY: Calling `.unwrap()` is safe here because `validator_config.snapshot_config` must - // be `Some`. The Option<> wrapper will be removed later to solidify this requirement. - validator_config.snapshot_config.as_ref().unwrap(), + &validator_config.snapshot_config, validator_config.accounts_hash_interval_slots, ) { eprintln!("Invalid snapshot configuration provided: snapshot intervals are incompatible. \