From 3dd348802fe6eaf39fcd33eacd759d8873bc31a6 Mon Sep 17 00:00:00 2001 From: steviez Date: Fri, 19 Jan 2024 09:25:46 -0600 Subject: [PATCH] Bubble up genesis load errors instead of exiting (#34851) The function open_genesis_config() performs several operations that could fail. If any of these fail, the process exits immediately. Instead of exiting immediately, bubble up the error and let the caller decide the appropriate action. solana-validator and solana-ledger-tool will functionally be unchanged, but this consolidates startup failures for both of these processes. --- accounts-db/src/hardened_unpack.rs | 39 ++++++++++++++++------------ core/src/validator.rs | 8 +++--- ledger-tool/src/ledger_utils.rs | 6 ++++- local-cluster/tests/local_cluster.rs | 2 +- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/accounts-db/src/hardened_unpack.rs b/accounts-db/src/hardened_unpack.rs index 4d99f93ae9..39eca4f9cd 100644 --- a/accounts-db/src/hardened_unpack.rs +++ b/accounts-db/src/hardened_unpack.rs @@ -446,28 +446,35 @@ fn is_valid_snapshot_archive_entry(parts: &[&str], kind: tar::EntryType) -> bool } } +#[derive(Error, Debug)] +pub enum OpenGenesisConfigError { + #[error("unpack error: {0}")] + Unpack(#[from] UnpackError), + #[error("Genesis load error: {0}")] + Load(#[from] std::io::Error), +} + pub fn open_genesis_config( ledger_path: &Path, max_genesis_archive_unpacked_size: u64, -) -> GenesisConfig { - GenesisConfig::load(ledger_path).unwrap_or_else(|load_err| { - let genesis_package = ledger_path.join(DEFAULT_GENESIS_ARCHIVE); - unpack_genesis_archive( - &genesis_package, - ledger_path, - max_genesis_archive_unpacked_size, - ) - .unwrap_or_else(|unpack_err| { +) -> std::result::Result { + match GenesisConfig::load(ledger_path) { + Ok(genesis_config) => Ok(genesis_config), + Err(load_err) => { warn!( - "Failed to open ledger genesis_config at {:?}: {}, {}", - ledger_path, load_err, unpack_err, + "Failed to load genesis_config at {ledger_path:?}: {load_err}. \ + Will attempt to unpack genesis archive and then retry loading." ); - std::process::exit(1); - }); - // loading must succeed at this moment - GenesisConfig::load(ledger_path).unwrap() - }) + let genesis_package = ledger_path.join(DEFAULT_GENESIS_ARCHIVE); + unpack_genesis_archive( + &genesis_package, + ledger_path, + max_genesis_archive_unpacked_size, + )?; + GenesisConfig::load(ledger_path).map_err(OpenGenesisConfigError::Load) + } + } } pub fn unpack_genesis_archive( diff --git a/core/src/validator.rs b/core/src/validator.rs index c2dd6c50f1..a8751e6d28 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -565,9 +565,10 @@ impl Validator { "ledger directory does not exist or is not accessible: {ledger_path:?}" )); } - let genesis_config = - open_genesis_config(ledger_path, config.max_genesis_archive_unpacked_size); + open_genesis_config(ledger_path, config.max_genesis_archive_unpacked_size) + .map_err(|err| format!("Failed to open genesis config: {err}"))?; + metrics_config_sanity_check(genesis_config.cluster_type)?; if let Some(expected_shred_version) = config.expected_shred_version { @@ -1764,7 +1765,8 @@ fn load_blockstore( > { info!("loading ledger from {:?}...", ledger_path); *start_progress.write().unwrap() = ValidatorStartProgress::LoadingLedger; - let genesis_config = open_genesis_config(ledger_path, config.max_genesis_archive_unpacked_size); + let genesis_config = open_genesis_config(ledger_path, config.max_genesis_archive_unpacked_size) + .map_err(|err| format!("Failed to open genesis config: {err}"))?; // This needs to be limited otherwise the state in the VoteAccount data // grows too large diff --git a/ledger-tool/src/ledger_utils.rs b/ledger-tool/src/ledger_utils.rs index 5c2713ef18..1476c2df06 100644 --- a/ledger-tool/src/ledger_utils.rs +++ b/ledger-tool/src/ledger_utils.rs @@ -553,7 +553,11 @@ fn open_blockstore_with_temporary_primary_access( pub fn open_genesis_config_by(ledger_path: &Path, matches: &ArgMatches<'_>) -> GenesisConfig { let max_genesis_archive_unpacked_size = value_t_or_exit!(matches, "max_genesis_archive_unpacked_size", u64); - open_genesis_config(ledger_path, max_genesis_archive_unpacked_size) + + open_genesis_config(ledger_path, max_genesis_archive_unpacked_size).unwrap_or_else(|err| { + eprintln!("Exiting. Failed to open genesis config: {err}"); + exit(1); + }) } pub fn get_program_ids(tx: &VersionedTransaction) -> impl Iterator + '_ { diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index 4ed4ee46f7..74039f1a64 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -2196,7 +2196,7 @@ fn create_snapshot_to_hard_fork( ..ProcessOptions::default() }; let ledger_path = blockstore.ledger_path(); - let genesis_config = open_genesis_config(ledger_path, u64::max_value()); + let genesis_config = open_genesis_config(ledger_path, u64::max_value()).unwrap(); let snapshot_config = create_simple_snapshot_config(ledger_path); let (bank_forks, ..) = bank_forks_utils::load( &genesis_config,