diff --git a/core/src/validator.rs b/core/src/validator.rs index 05583a741..78a27517c 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -194,7 +194,7 @@ impl Default for ValidatorConfig { geyser_plugin_config_files: None, rpc_addrs: None, pubsub_config: PubSubConfig::default(), - snapshot_config: None, + snapshot_config: Some(SnapshotConfig::new_load_only()), broadcast_stage_type: BroadcastStageType::Standard, turbine_disabled: Arc::::default(), enforce_ulimit_nofile: true, @@ -575,29 +575,17 @@ impl Validator { cluster_info.restore_contact_info(ledger_path, config.contact_save_interval); let cluster_info = Arc::new(cluster_info); - let ( - accounts_background_service, - accounts_hash_verifier, - snapshot_packager_service, - accounts_background_request_sender, - ) = { - let pending_accounts_package = PendingAccountsPackage::default(); - let ( - accounts_background_request_sender, - snapshot_request_handler, - pending_snapshot_package, - snapshot_packager_service, - ) = if let Some(snapshot_config) = config.snapshot_config.clone() { - if !is_snapshot_config_valid( - snapshot_config.full_snapshot_archive_interval_slots, - snapshot_config.incremental_snapshot_archive_interval_slots, - config.accounts_hash_interval_slots, - ) { - error!("Snapshot config is invalid"); - } + // 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(); - let pending_snapshot_package = PendingSnapshotPackage::default(); + assert!(is_snapshot_config_valid( + &snapshot_config, + config.accounts_hash_interval_slots, + )); + let (pending_snapshot_package, snapshot_packager_service) = + if 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 @@ -605,7 +593,7 @@ impl Validator { .as_ref() .map(|config| config.filler_accounts_config.count == 0) .unwrap_or(true); - + let pending_snapshot_package = PendingSnapshotPackage::default(); let snapshot_packager_service = SnapshotPackagerService::new( pending_snapshot_package.clone(), starting_snapshot_hashes, @@ -614,56 +602,48 @@ impl Validator { snapshot_config.clone(), enable_gossip_push, ); - - let (snapshot_request_sender, snapshot_request_receiver) = unbounded(); ( - AbsRequestSender::new(snapshot_request_sender), - Some(SnapshotRequestHandler { - snapshot_config, - snapshot_request_receiver, - pending_accounts_package: pending_accounts_package.clone(), - }), Some(pending_snapshot_package), Some(snapshot_packager_service), ) } else { - (AbsRequestSender::default(), None, None, None) + (None, None) }; - let accounts_hash_verifier = AccountsHashVerifier::new( - Arc::clone(&pending_accounts_package), - pending_snapshot_package, - &exit, - &cluster_info, - config.known_validators.clone(), - config.halt_on_known_validators_accounts_hash_mismatch, - config.accounts_hash_fault_injection_slots, - config.snapshot_config.clone(), - ); + let pending_accounts_package = PendingAccountsPackage::default(); + let accounts_hash_verifier = AccountsHashVerifier::new( + Arc::clone(&pending_accounts_package), + pending_snapshot_package, + &exit, + &cluster_info, + config.known_validators.clone(), + config.halt_on_known_validators_accounts_hash_mismatch, + config.accounts_hash_fault_injection_slots, + config.snapshot_config.clone(), + ); - let pruned_banks_request_handler = PrunedBanksRequestHandler { - pruned_banks_receiver, - }; - let last_full_snapshot_slot = starting_snapshot_hashes.map(|x| x.full.hash.0); - let accounts_background_service = AccountsBackgroundService::new( - bank_forks.clone(), - &exit, - AbsRequestHandlers { - snapshot_request_handler, - pruned_banks_request_handler, - }, - config.accounts_db_caching_enabled, - config.accounts_db_test_hash_calculation, - last_full_snapshot_slot, - ); - - ( - accounts_background_service, - accounts_hash_verifier, - snapshot_packager_service, - accounts_background_request_sender, - ) + let (snapshot_request_sender, snapshot_request_receiver) = unbounded(); + let accounts_background_request_sender = AbsRequestSender::new(snapshot_request_sender); + let snapshot_request_handler = SnapshotRequestHandler { + snapshot_config, + snapshot_request_receiver, + pending_accounts_package, }; + let pruned_banks_request_handler = PrunedBanksRequestHandler { + pruned_banks_receiver, + }; + let last_full_snapshot_slot = starting_snapshot_hashes.map(|x| x.full.hash.0); + let accounts_background_service = AccountsBackgroundService::new( + bank_forks.clone(), + &exit, + AbsRequestHandlers { + snapshot_request_handler, + pruned_banks_request_handler, + }, + config.accounts_db_caching_enabled, + config.accounts_db_test_hash_calculation, + last_full_snapshot_slot, + ); let leader_schedule_cache = Arc::new(leader_schedule_cache); let mut process_blockstore = ProcessBlockStore::new( @@ -2158,15 +2138,18 @@ fn cleanup_accounts_paths(config: &ValidatorConfig) { } pub fn is_snapshot_config_valid( - full_snapshot_interval_slots: Slot, - incremental_snapshot_interval_slots: Slot, + snapshot_config: &SnapshotConfig, accounts_hash_interval_slots: Slot, ) -> bool { - // if full snapshot interval is MAX, that means snapshots are turned off, so yes, valid - if full_snapshot_interval_slots == Slot::MAX { + // if the snapshot config is configured to *not* take snapshots, then it is valid + if !snapshot_config.should_generate_snapshots() { return true; } + let full_snapshot_interval_slots = snapshot_config.full_snapshot_archive_interval_slots; + let incremental_snapshot_interval_slots = + snapshot_config.incremental_snapshot_archive_interval_slots; + let is_incremental_config_valid = if incremental_snapshot_interval_slots == Slot::MAX { true } else { @@ -2413,40 +2396,97 @@ mod tests { #[test] fn test_interval_check() { - assert!(is_snapshot_config_valid(300, 200, 100)); + fn new_snapshot_config( + full_snapshot_archive_interval_slots: Slot, + incremental_snapshot_archive_interval_slots: Slot, + ) -> SnapshotConfig { + SnapshotConfig { + full_snapshot_archive_interval_slots, + incremental_snapshot_archive_interval_slots, + ..SnapshotConfig::default() + } + } + + assert!(is_snapshot_config_valid( + &new_snapshot_config(300, 200), + 100 + )); let default_accounts_hash_interval = snapshot_utils::DEFAULT_INCREMENTAL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS; assert!(is_snapshot_config_valid( - snapshot_utils::DEFAULT_FULL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS, - snapshot_utils::DEFAULT_INCREMENTAL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS, + &new_snapshot_config( + snapshot_utils::DEFAULT_FULL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS, + snapshot_utils::DEFAULT_INCREMENTAL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS + ), default_accounts_hash_interval, )); - assert!(is_snapshot_config_valid( - Slot::MAX, - snapshot_utils::DEFAULT_INCREMENTAL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS, + &new_snapshot_config( + snapshot_utils::DEFAULT_FULL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS, + Slot::MAX + ), default_accounts_hash_interval )); assert!(is_snapshot_config_valid( - snapshot_utils::DEFAULT_FULL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS, - Slot::MAX, + &new_snapshot_config( + snapshot_utils::DEFAULT_INCREMENTAL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS, + Slot::MAX + ), default_accounts_hash_interval )); assert!(is_snapshot_config_valid( - snapshot_utils::DEFAULT_INCREMENTAL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS, - Slot::MAX, - default_accounts_hash_interval + &new_snapshot_config(Slot::MAX, Slot::MAX), + Slot::MAX )); - assert!(!is_snapshot_config_valid(0, 100, 100)); - assert!(!is_snapshot_config_valid(100, 0, 100)); - assert!(!is_snapshot_config_valid(42, 100, 100)); - assert!(!is_snapshot_config_valid(100, 42, 100)); - assert!(!is_snapshot_config_valid(100, 100, 100)); - assert!(!is_snapshot_config_valid(100, 200, 100)); - assert!(!is_snapshot_config_valid(444, 200, 100)); - assert!(!is_snapshot_config_valid(400, 222, 100)); + assert!(!is_snapshot_config_valid(&new_snapshot_config(0, 100), 100)); + assert!(!is_snapshot_config_valid(&new_snapshot_config(100, 0), 100)); + assert!(!is_snapshot_config_valid( + &new_snapshot_config(42, 100), + 100 + )); + assert!(!is_snapshot_config_valid( + &new_snapshot_config(100, 42), + 100 + )); + assert!(!is_snapshot_config_valid( + &new_snapshot_config(100, 100), + 100 + )); + assert!(!is_snapshot_config_valid( + &new_snapshot_config(100, 200), + 100 + )); + assert!(!is_snapshot_config_valid( + &new_snapshot_config(444, 200), + 100 + )); + assert!(!is_snapshot_config_valid( + &new_snapshot_config(400, 222), + 100 + )); + + assert!(is_snapshot_config_valid( + &SnapshotConfig::new_load_only(), + 100 + )); + assert!(is_snapshot_config_valid( + &SnapshotConfig { + full_snapshot_archive_interval_slots: 41, + incremental_snapshot_archive_interval_slots: 37, + ..SnapshotConfig::new_load_only() + }, + 100 + )); + assert!(is_snapshot_config_valid( + &SnapshotConfig { + full_snapshot_archive_interval_slots: Slot::MAX, + incremental_snapshot_archive_interval_slots: Slot::MAX, + ..SnapshotConfig::new_load_only() + }, + 100 + )); } #[test] diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 69c0cb4ed..bd219f600 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -925,11 +925,11 @@ fn test_snapshots_with_background_services( } let abs_request_sender = AbsRequestSender::new(snapshot_request_sender); - let snapshot_request_handler = Some(SnapshotRequestHandler { + let snapshot_request_handler = SnapshotRequestHandler { snapshot_config: snapshot_test_config.snapshot_config.clone(), snapshot_request_receiver, pending_accounts_package: Arc::clone(&pending_accounts_package), - }); + }; let pruned_banks_request_handler = PrunedBanksRequestHandler { pruned_banks_receiver, }; diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index ecf86c65f..5331b100b 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -38,7 +38,7 @@ use { solana_runtime::{ accounts_background_service::{ AbsRequestHandlers, AbsRequestSender, AccountsBackgroundService, - PrunedBanksRequestHandler, + PrunedBanksRequestHandler, SnapshotRequestHandler, }, accounts_db::{AccountsDbConfig, FillerAccountsConfig}, accounts_index::{AccountsIndexConfig, IndexLimitMb, ScanConfig}, @@ -53,6 +53,7 @@ use { snapshot_config::SnapshotConfig, snapshot_hash::StartingSnapshotHashes, snapshot_minimizer::SnapshotMinimizer, + snapshot_package::PendingAccountsPackage, snapshot_utils::{ self, ArchiveFormat, SnapshotVersion, DEFAULT_ARCHIVE_COMPRESSION, DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, @@ -1095,13 +1096,20 @@ fn load_bank_forks( accounts_update_notifier, ); + let (snapshot_request_sender, snapshot_request_receiver) = crossbeam_channel::unbounded(); + let accounts_background_request_sender = AbsRequestSender::new(snapshot_request_sender); + let snapshot_request_handler = SnapshotRequestHandler { + snapshot_config: SnapshotConfig::new_load_only(), + snapshot_request_receiver, + pending_accounts_package: PendingAccountsPackage::default(), + }; let pruned_banks_receiver = AccountsBackgroundService::setup_bank_drop_callback(bank_forks.clone()); let pruned_banks_request_handler = PrunedBanksRequestHandler { pruned_banks_receiver, }; let abs_request_handler = AbsRequestHandlers { - snapshot_request_handler: None, + snapshot_request_handler, pruned_banks_request_handler, }; let exit = Arc::new(AtomicBool::new(false)); @@ -1121,7 +1129,7 @@ fn load_bank_forks( &process_options, None, None, - &AbsRequestSender::default(), + &accounts_background_request_sender, ) .map(|_| (bank_forks, starting_snapshot_hashes)); diff --git a/local-cluster/src/local_cluster.rs b/local-cluster/src/local_cluster.rs index 1007dcc72..4cd28a6a2 100644 --- a/local-cluster/src/local_cluster.rs +++ b/local-cluster/src/local_cluster.rs @@ -24,7 +24,7 @@ use { solana_sdk::{ account::{Account, AccountSharedData}, client::SyncClient, - clock::{Slot, DEFAULT_DEV_SLOTS_PER_EPOCH, DEFAULT_TICKS_PER_SLOT}, + clock::{DEFAULT_DEV_SLOTS_PER_EPOCH, DEFAULT_TICKS_PER_SLOT}, commitment_config::CommitmentConfig, epoch_schedule::EpochSchedule, genesis_config::{ClusterType, GenesisConfig}, @@ -741,8 +741,6 @@ impl LocalCluster { // node lifecycle. // There must be some place holder for now... SnapshotConfig { - full_snapshot_archive_interval_slots: Slot::MAX, - incremental_snapshot_archive_interval_slots: Slot::MAX, full_snapshot_archives_dir: DUMMY_SNAPSHOT_CONFIG_PATH_MARKER.into(), bank_snapshots_dir: DUMMY_SNAPSHOT_CONFIG_PATH_MARKER.into(), ..SnapshotConfig::new_load_only() diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 102ec8405..59f381ce1 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -394,7 +394,7 @@ impl PrunedBanksRequestHandler { } pub struct AbsRequestHandlers { - pub snapshot_request_handler: Option, + pub snapshot_request_handler: SnapshotRequestHandler, pub pruned_banks_request_handler: PrunedBanksRequestHandler, } @@ -407,16 +407,12 @@ impl AbsRequestHandlers { non_snapshot_time_us: u128, last_full_snapshot_slot: &mut Option, ) -> Option> { - self.snapshot_request_handler - .as_ref() - .and_then(|snapshot_request_handler| { - snapshot_request_handler.handle_snapshot_requests( - accounts_db_caching_enabled, - test_hash_calculation, - non_snapshot_time_us, - last_full_snapshot_slot, - ) - }) + self.snapshot_request_handler.handle_snapshot_requests( + accounts_db_caching_enabled, + test_hash_calculation, + non_snapshot_time_us, + last_full_snapshot_slot, + ) } } diff --git a/validator/src/main.rs b/validator/src/main.rs index 1470a82ee..1bc2d1291 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -2957,8 +2957,9 @@ pub fn main() { validator_config.accounts_hash_interval_slots = value_t_or_exit!(matches, "accounts-hash-interval-slots", u64); if !is_snapshot_config_valid( - full_snapshot_archive_interval_slots, - incremental_snapshot_archive_interval_slots, + // 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.accounts_hash_interval_slots, ) { eprintln!("Invalid snapshot configuration provided: snapshot intervals are incompatible. \