blockworks-foundation
/
solana
mirror of https://github.com/blockworks-foundation/solana.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Bubble up errors in bank_fork_utils instead of exiting process (#34277) 

There are operations in bank_fork_utils that may fail; we explicitly
call std::process::exit() on several of these. Granted we may end up
exiting the process higher up the callstack, bubbling the errors up
allow a caller that could handle the error to do so.
This commit is contained in:
steviez 2023-11-30 16:35:59 -06:00 committed by GitHub
parent c5368a3d35
commit 479b7ee9f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 71 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
core/src/validator.rs
View File

@ -1801,7 +1801,8 @@ fn load_blockstore(
.map(|service| service.sender()),
accounts_update_notifier,
exit,
);
)
.map_err(|err| err.to_string())?;
// Before replay starts, set the callbacks in each of the banks in BankForks so that
// all dropped banks come through the `pruned_banks_receiver` channel. This way all bank

12
ledger-tool/src/ledger_utils.rs
View File

@ -16,9 +16,7 @@ use {
AccessType, BlockstoreOptions, BlockstoreRecoveryMode, LedgerColumnOptions,
ShredStorageType,
},
blockstore_processor::{
self, BlockstoreProcessorError, ProcessOptions, TransactionStatusSender,
},
blockstore_processor::{self, ProcessOptions, TransactionStatusSender},
},
solana_measure::measure,
solana_rpc::transaction_status_service::TransactionStatusService,
@ -71,7 +69,7 @@ pub fn load_and_process_ledger(
process_options: ProcessOptions,
snapshot_archive_path: Option<PathBuf>,
incremental_snapshot_archive_path: Option<PathBuf>,
) -> Result<(Arc<RwLock<BankForks>>, Option<StartingSnapshotHashes>), BlockstoreProcessorError> {
) -> Result<(Arc<RwLock<BankForks>>, Option<StartingSnapshotHashes>), String> {
let bank_snapshots_dir = if blockstore.is_primary_access() {
blockstore.ledger_path().join("snapshot")
} else {
@ -245,7 +243,8 @@ pub fn load_and_process_ledger(
None, // Maybe support this later, though
accounts_update_notifier,
exit.clone(),
);
)
.map_err(|err| err.to_string())?;
let block_verification_method = value_t!(
arg_matches,
"block_verification_method",
@ -345,7 +344,8 @@ pub fn load_and_process_ledger(
None, // Maybe support this later, though
&accounts_background_request_sender,
)
.map(|_| (bank_forks, starting_snapshot_hashes));
.map(|_| (bank_forks, starting_snapshot_hashes))
.map_err(|err| err.to_string());
exit.store(true, Ordering::Relaxed);
accounts_background_service.join().unwrap();

106
ledger/src/bank_forks_utils.rs
View File

@ -25,18 +25,50 @@ use {
solana_sdk::genesis_config::GenesisConfig,
std::{
path::PathBuf,
process, result,
result,
sync::{atomic::AtomicBool, Arc, RwLock},
},
thiserror::Error,
};
#[derive(Error, Debug)]
pub enum BankForksUtilsError {
#[error("accounts path(s) not present when booting from snapshot")]
AccountPathsNotPresent,
#[error(
"failed to load bank: {source}, full snapshot archive: {full_snapshot_archive}, \
incremental snapshot archive: {incremental_snapshot_archive}"
)]
BankFromSnapshotsArchive {
source: snapshot_utils::SnapshotError,
full_snapshot_archive: String,
incremental_snapshot_archive: String,
},
#[error(
"there is no local state to startup from. \
Ensure --{flag} is NOT set to \"{value}\" and restart"
)]
NoBankSnapshotDirectory { flag: String, value: String },
#[error("failed to load bank: {source}, snapshot: {path}")]
BankFromSnapshotsDirectory {
source: snapshot_utils::SnapshotError,
path: PathBuf,
},
#[error("failed to process blockstore from root: {0}")]
ProcessBlockstoreFromRoot(#[source] BlockstoreProcessorError),
}
pub type LoadResult = result::Result<
(
Arc<RwLock<BankForks>>,
LeaderScheduleCache,
Option<StartingSnapshotHashes>,
),
BlockstoreProcessorError,
BankForksUtilsError,
>;
/// Load the banks via genesis or a snapshot then processes all full blocks in blockstore
@ -68,8 +100,7 @@ pub fn load(
entry_notification_sender,
accounts_update_notifier,
exit,
);
)?;
blockstore_processor::process_blockstore_from_root(
blockstore,
&bank_forks,
@ -80,7 +111,9 @@ pub fn load(
entry_notification_sender,
&AbsRequestSender::default(),
)
.map(|_| (bank_forks, leader_schedule_cache, starting_snapshot_hashes))
.map_err(BankForksUtilsError::ProcessBlockstoreFromRoot)?;
Ok((bank_forks, leader_schedule_cache, starting_snapshot_hashes))
}
#[allow(clippy::too_many_arguments)]
@ -95,11 +128,7 @@ pub fn load_bank_forks(
entry_notification_sender: Option<&EntryNotifierSender>,
accounts_update_notifier: Option<AccountsUpdateNotifier>,
exit: Arc<AtomicBool>,
) -> (
Arc<RwLock<BankForks>>,
LeaderScheduleCache,
Option<StartingSnapshotHashes>,
) {
) -> LoadResult {
fn get_snapshots_to_load(
snapshot_config: Option<&SnapshotConfig>,
) -> Option<(
@ -157,7 +186,7 @@ pub fn load_bank_forks(
process_options,
accounts_update_notifier,
exit,
);
)?;
(bank_forks, Some(starting_snapshot_hashes))
} else {
info!("Processing ledger from genesis");
@ -193,7 +222,7 @@ pub fn load_bank_forks(
.for_each(|hard_fork_slot| root_bank.register_hard_fork(*hard_fork_slot));
}
(bank_forks, leader_schedule_cache, starting_snapshot_hashes)
Ok((bank_forks, leader_schedule_cache, starting_snapshot_hashes))
}
#[allow(clippy::too_many_arguments)]
@ -207,11 +236,10 @@ fn bank_forks_from_snapshot(
process_options: &ProcessOptions,
accounts_update_notifier: Option<AccountsUpdateNotifier>,
exit: Arc<AtomicBool>,
) -> (Arc<RwLock<BankForks>>, StartingSnapshotHashes) {
) -> Result<(Arc<RwLock<BankForks>>, StartingSnapshotHashes), BankForksUtilsError> {
// Fail hard here if snapshot fails to load, don't silently continue
if account_paths.is_empty() {
error!("Account paths not present when booting from snapshot");
process::exit(1);
return Err(BankForksUtilsError::AccountPathsNotPresent);
}
let latest_snapshot_archive_slot = std::cmp::max(
@ -261,29 +289,21 @@ fn bank_forks_from_snapshot(
accounts_update_notifier,
exit,
)
.unwrap_or_else(|err| {
error!(
"Failed to load bank: {err} \
\nfull snapshot archive: {} \
\nincremental snapshot archive: {}",
full_snapshot_archive_info.path().display(),
incremental_snapshot_archive_info
.as_ref()
.map(|archive| archive.path().display().to_string())
.unwrap_or("none".to_string()),
);
process::exit(1);
});
.map_err(|err| BankForksUtilsError::BankFromSnapshotsArchive {
source: err,
full_snapshot_archive: full_snapshot_archive_info.path().display().to_string(),
incremental_snapshot_archive: incremental_snapshot_archive_info
.as_ref()
.map(|archive| archive.path().display().to_string())
.unwrap_or("none".to_string()),
})?;
bank
} else {
let Some(bank_snapshot) = latest_bank_snapshot else {
error!(
"There is no local state to startup from. Ensure --{} is *not* set to \"{}\" and restart.",
use_snapshot_archives_at_startup::cli::LONG_ARG,
UseSnapshotArchivesAtStartup::Never.to_string(),
);
process::exit(1);
};
let bank_snapshot =
latest_bank_snapshot.ok_or_else(|| BankForksUtilsError::NoBankSnapshotDirectory {
flag: use_snapshot_archives_at_startup::cli::LONG_ARG.to_string(),
value: UseSnapshotArchivesAtStartup::Never.to_string(),
})?;
// If a newer snapshot archive was downloaded, it is possible that its slot is
// higher than the local bank we will load. Did the user intend for this?
@ -318,14 +338,10 @@ fn bank_forks_from_snapshot(
accounts_update_notifier,
exit,
)
.unwrap_or_else(|err| {
error!(
"Failed to load bank: {err} \
\nsnapshot: {}",
bank_snapshot.snapshot_path().display(),
);
process::exit(1);
});
.map_err(|err| BankForksUtilsError::BankFromSnapshotsDirectory {
source: err,
path: bank_snapshot.snapshot_path(),
})?;
bank
};
@ -349,5 +365,5 @@ fn bank_forks_from_snapshot(
incremental: incremental_snapshot_hash,
};
(BankForks::new_rw_arc(bank), starting_snapshot_hashes)
Ok((BankForks::new_rw_arc(bank), starting_snapshot_hashes))
}

3
ledger/src/blockstore_processor.rs
View File

@ -751,7 +751,8 @@ pub fn test_process_blockstore(
None,
None,
exit,
);
)
.unwrap();
process_blockstore_from_root(
blockstore,