Skip to content

Commit

Permalink
Remove unused Option<> around ValidatorConfig's SnapshotConfig (#29090)
Browse files Browse the repository at this point in the history
Remove Option<> around ValidatorConfig's SnapshotConfig

The SnapshotConfig is required and is currently hard-coded to be a
Some().
  • Loading branch information
steviez committed Dec 6, 2022
1 parent 8ce6081 commit aeb6b53
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 102 deletions.
14 changes: 6 additions & 8 deletions core/src/accounts_hash_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl AccountsHashVerifier {
known_validators: Option<HashSet<Pubkey>>,
halt_on_known_validators_accounts_hash_mismatch: bool,
fault_injection_rate_slots: u64,
snapshot_config: Option<SnapshotConfig>,
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);
Expand Down Expand Up @@ -83,7 +83,7 @@ impl AccountsHashVerifier {
&mut hashes,
&exit,
fault_injection_rate_slots,
snapshot_config.as_ref(),
&snapshot_config,
));

datapoint_info!(
Expand Down Expand Up @@ -184,7 +184,7 @@ impl AccountsHashVerifier {
hashes: &mut Vec<(Slot, Hash)>,
exit: &Arc<AtomicBool>,
fault_injection_rate_slots: u64,
snapshot_config: Option<&SnapshotConfig>,
snapshot_config: &SnapshotConfig,
) {
let accounts_hash = Self::calculate_and_verify_accounts_hash(&accounts_package);

Expand Down Expand Up @@ -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(_)
Expand Down Expand Up @@ -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
Expand Down
41 changes: 18 additions & 23 deletions core/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub struct ValidatorConfig {
pub geyser_plugin_config_files: Option<Vec<PathBuf>>,
pub rpc_addrs: Option<(SocketAddr, SocketAddr)>, // (JsonRpc, JsonRpcPubSub)
pub pubsub_config: PubSubConfig,
pub snapshot_config: Option<SnapshotConfig>,
pub snapshot_config: SnapshotConfig,
pub max_ledger_shreds: Option<u64>,
pub broadcast_stage_type: BroadcastStageType,
pub turbine_disabled: Arc<AtomicBool>,
Expand Down Expand Up @@ -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::<AtomicBool>::default(),
enforce_ulimit_nofile: true,
Expand Down Expand Up @@ -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
Expand All @@ -601,7 +597,7 @@ impl Validator {
starting_snapshot_hashes,
&exit,
&cluster_info,
snapshot_config.clone(),
config.snapshot_config.clone(),
enable_gossip_push,
);
(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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}")),
Expand Down
2 changes: 1 addition & 1 deletion core/tests/epoch_accounts_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions core/tests/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ fn run_bank_forks_snapshot_n<F>(
None,
false,
0,
Some(snapshot_test_config.snapshot_config.clone()),
snapshot_test_config.snapshot_config.clone(),
);

let (snapshot_request_sender, snapshot_request_receiver) = unbounded();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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(
Expand Down
16 changes: 8 additions & 8 deletions local-cluster/src/local_cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}

Expand Down
2 changes: 1 addition & 1 deletion local-cluster/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
36 changes: 1 addition & 35 deletions local-cluster/tests/local_cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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,
Expand Down Expand Up @@ -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: {}",
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
};
Expand Down
4 changes: 2 additions & 2 deletions test-validator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit aeb6b53

Please sign in to comment.