From 660d62f07b4d8bc9f1929b4720ccf0fccb356158 Mon Sep 17 00:00:00 2001 From: Steven Czabaniuk Date: Tue, 26 Dec 2023 14:09:28 -0500 Subject: [PATCH 1/2] Move get_shred_storage_type() closer to caller --- ledger-tool/src/ledger_utils.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/ledger-tool/src/ledger_utils.rs b/ledger-tool/src/ledger_utils.rs index e72804c201c136..5288d9464b4b66 100644 --- a/ledger-tool/src/ledger_utils.rs +++ b/ledger-tool/src/ledger_utils.rs @@ -94,20 +94,6 @@ pub(crate) enum LoadAndProcessLedgerError { ProcessBlockstoreFromRoot(#[source] BlockstoreProcessorError), } -pub fn get_shred_storage_type(ledger_path: &Path, message: &str) -> ShredStorageType { - // TODO: the following shred_storage_type inference must be updated once - // the rocksdb options can be constructed via load_options_file() as the - // value picked by passing None for `max_shred_storage_size` could affect - // the persisted rocksdb options file. - match ShredStorageType::from_ledger_path(ledger_path, None) { - Some(s) => s, - None => { - info!("{}", message); - ShredStorageType::RocksLevel - } - } -} - pub fn load_and_process_ledger_or_exit( arg_matches: &ArgMatches, genesis_config: &GenesisConfig, @@ -508,6 +494,20 @@ pub fn open_blockstore( } } +pub fn get_shred_storage_type(ledger_path: &Path, message: &str) -> ShredStorageType { + // TODO: the following shred_storage_type inference must be updated once + // the rocksdb options can be constructed via load_options_file() as the + // value picked by passing None for `max_shred_storage_size` could affect + // the persisted rocksdb options file. + match ShredStorageType::from_ledger_path(ledger_path, None) { + Some(s) => s, + None => { + info!("{}", message); + ShredStorageType::RocksLevel + } + } +} + /// Open blockstore with temporary primary access to allow necessary, /// persistent changes to be made to the blockstore (such as creation of new /// column family(s)). Then, continue opening with `original_access_type` From 1add55cad1afdfc2a529fd18486b5a6083bacc1f Mon Sep 17 00:00:00 2001 From: Steven Czabaniuk Date: Tue, 26 Dec 2023 16:16:30 -0500 Subject: [PATCH 2/2] Move blockstore option parsing to open function Avoids logic that is duplicated across multiples files --- ledger-tool/src/bigtable.rs | 6 +- ledger-tool/src/ledger_utils.rs | 13 +- ledger-tool/src/main.rs | 224 +++++--------------------------- ledger-tool/src/program.rs | 18 +-- 4 files changed, 45 insertions(+), 216 deletions(-) diff --git a/ledger-tool/src/bigtable.rs b/ledger-tool/src/bigtable.rs index 987a03f2ef99cc..cf153aae6ce2cb 100644 --- a/ledger-tool/src/bigtable.rs +++ b/ledger-tool/src/bigtable.rs @@ -1075,9 +1075,7 @@ pub fn bigtable_process_command(ledger_path: &Path, matches: &ArgMatches<'_>) { let runtime = tokio::runtime::Runtime::new().unwrap(); let verbose = matches.is_present("verbose"); - let force_update_to_open = matches.is_present("force_update_to_open"); let output_format = OutputFormat::from_matches(matches, "output_format", verbose); - let enforce_ulimit_nofile = !matches.is_present("ignore_ulimit_nofile_error"); let (subcommand, sub_matches) = matches.subcommand(); let instance_name = get_global_subcommand_arg( @@ -1100,10 +1098,8 @@ pub fn bigtable_process_command(ledger_path: &Path, matches: &ArgMatches<'_>) { let force_reupload = arg_matches.is_present("force_reupload"); let blockstore = crate::open_blockstore( &canonicalize_ledger_path(ledger_path), + arg_matches, AccessType::Secondary, - None, - force_update_to_open, - enforce_ulimit_nofile, ); let config = solana_storage_bigtable::LedgerStorageConfig { read_only: false, diff --git a/ledger-tool/src/ledger_utils.rs b/ledger-tool/src/ledger_utils.rs index 5288d9464b4b66..e50f2477326c1f 100644 --- a/ledger-tool/src/ledger_utils.rs +++ b/ledger-tool/src/ledger_utils.rs @@ -362,10 +362,8 @@ pub fn load_and_process_ledger( let tss_blockstore = if enable_rpc_transaction_history { Arc::new(open_blockstore( blockstore.ledger_path(), + arg_matches, AccessType::PrimaryForMaintenance, - None, - false, - false, )) } else { blockstore.clone() @@ -416,11 +414,14 @@ pub fn load_and_process_ledger( pub fn open_blockstore( ledger_path: &Path, + matches: &ArgMatches, access_type: AccessType, - wal_recovery_mode: Option, - force_update_to_open: bool, - enforce_ulimit_nofile: bool, ) -> Blockstore { + let wal_recovery_mode = matches + .value_of("wal_recovery_mode") + .map(BlockstoreRecoveryMode::from); + let force_update_to_open = matches.is_present("force_update_to_open"); + let enforce_ulimit_nofile = !matches.is_present("ignore_ulimit_nofile_error"); let shred_storage_type = get_shred_storage_type( ledger_path, &format!( diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 742d85c44f0e8a..a23cc1fe227bd1 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -40,10 +40,7 @@ use { ancestor_iterator::AncestorIterator, blockstore::{create_new_ledger, Blockstore, PurgeType}, blockstore_db::{self, columns as cf, Column, ColumnName, Database}, - blockstore_options::{ - AccessType, BlockstoreRecoveryMode, LedgerColumnOptions, - BLOCKSTORE_DIRECTORY_ROCKS_FIFO, - }, + blockstore_options::{AccessType, LedgerColumnOptions, BLOCKSTORE_DIRECTORY_ROCKS_FIFO}, blockstore_processor::ProcessOptions, shred::Shred, use_snapshot_archives_at_startup::{self, UseSnapshotArchivesAtStartup}, @@ -2267,11 +2264,6 @@ fn main() { .ok() .map(PathBuf::from); - let wal_recovery_mode = matches - .value_of("wal_recovery_mode") - .map(BlockstoreRecoveryMode::from); - let force_update_to_open = matches.is_present("force_update_to_open"); - let enforce_ulimit_nofile = !matches.is_present("ignore_ulimit_nofile_error"); let verbose_level = matches.occurrences_of("verbose"); if let ("bigtable", Some(arg_matches)) = matches.subcommand() { @@ -2289,13 +2281,7 @@ fn main() { let allow_dead_slots = arg_matches.is_present("allow_dead_slots"); let only_rooted = arg_matches.is_present("only_rooted"); output_ledger( - open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ), + open_blockstore(&ledger_path, arg_matches, AccessType::Secondary), starting_slot, ending_slot, allow_dead_slots, @@ -2310,13 +2296,7 @@ fn main() { let ending_slot = value_t_or_exit!(arg_matches, "ending_slot", Slot); let target_db = PathBuf::from(value_t_or_exit!(arg_matches, "target_db", String)); - let source = open_blockstore( - &ledger_path, - AccessType::Secondary, - None, - force_update_to_open, - enforce_ulimit_nofile, - ); + let source = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); // Check if shred storage type can be inferred; if not, a new // ledger is being created. open_blockstore() will attempt to @@ -2332,13 +2312,7 @@ fn main() { &target_db ), ); - let target = open_blockstore( - &target_db, - AccessType::Primary, - None, - force_update_to_open, - enforce_ulimit_nofile, - ); + let target = open_blockstore(&target_db, arg_matches, AccessType::Primary); for (slot, _meta) in source.slot_meta_iterator(starting_slot).unwrap() { if slot > ending_slot { break; @@ -2414,13 +2388,8 @@ fn main() { ..ProcessOptions::default() }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); - let blockstore = open_blockstore( - &ledger_path, - get_access_type(&process_options), - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = + open_blockstore(&ledger_path, arg_matches, get_access_type(&process_options)); let (bank_forks, _) = load_and_process_ledger_or_exit( arg_matches, &genesis_config, @@ -2453,13 +2422,7 @@ fn main() { } let starting_slot = value_t_or_exit!(arg_matches, "starting_slot", Slot); let ending_slot = value_t!(arg_matches, "ending_slot", Slot).unwrap_or(Slot::MAX); - let ledger = open_blockstore( - &ledger_path, - AccessType::Secondary, - None, - force_update_to_open, - enforce_ulimit_nofile, - ); + let ledger = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); for (slot, _meta) in ledger .slot_meta_iterator(starting_slot) .unwrap() @@ -2494,13 +2457,8 @@ fn main() { ..ProcessOptions::default() }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); - let blockstore = open_blockstore( - &ledger_path, - get_access_type(&process_options), - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = + open_blockstore(&ledger_path, arg_matches, get_access_type(&process_options)); let (bank_forks, _) = load_and_process_ledger_or_exit( arg_matches, &genesis_config, @@ -2514,13 +2472,7 @@ fn main() { ("slot", Some(arg_matches)) => { let slots = values_t_or_exit!(arg_matches, "slots", Slot); let allow_dead_slots = arg_matches.is_present("allow_dead_slots"); - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); for slot in slots { println!("Slot {slot}"); if let Err(err) = output_slot( @@ -2539,13 +2491,7 @@ fn main() { let starting_slot = value_t_or_exit!(arg_matches, "starting_slot", Slot); let allow_dead_slots = arg_matches.is_present("allow_dead_slots"); output_ledger( - open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ), + open_blockstore(&ledger_path, arg_matches, AccessType::Secondary), starting_slot, Slot::MAX, allow_dead_slots, @@ -2556,26 +2502,14 @@ fn main() { ); } ("dead-slots", Some(arg_matches)) => { - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); let starting_slot = value_t_or_exit!(arg_matches, "starting_slot", Slot); for slot in blockstore.dead_slots_iterator(starting_slot).unwrap() { println!("{slot}"); } } ("duplicate-slots", Some(arg_matches)) => { - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); let starting_slot = value_t_or_exit!(arg_matches, "starting_slot", Slot); for slot in blockstore.duplicate_slots_iterator(starting_slot).unwrap() { println!("{slot}"); @@ -2583,13 +2517,7 @@ fn main() { } ("set-dead-slot", Some(arg_matches)) => { let slots = values_t_or_exit!(arg_matches, "slots", Slot); - let blockstore = open_blockstore( - &ledger_path, - AccessType::Primary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Primary); for slot in slots { match blockstore.set_dead_slot(slot) { Ok(_) => println!("Slot {slot} dead"), @@ -2599,13 +2527,7 @@ fn main() { } ("remove-dead-slot", Some(arg_matches)) => { let slots = values_t_or_exit!(arg_matches, "slots", Slot); - let blockstore = open_blockstore( - &ledger_path, - AccessType::Primary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Primary); for slot in slots { match blockstore.remove_dead_slot(slot) { Ok(_) => println!("Slot {slot} not longer marked dead"), @@ -2618,13 +2540,7 @@ fn main() { ("parse_full_frozen", Some(arg_matches)) => { let starting_slot = value_t_or_exit!(arg_matches, "starting_slot", Slot); let ending_slot = value_t_or_exit!(arg_matches, "ending_slot", Slot); - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); let mut ancestors = BTreeSet::new(); assert!( blockstore.meta(ending_slot).unwrap().is_some(), @@ -2738,13 +2654,8 @@ fn main() { let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); info!("genesis hash: {}", genesis_config.hash()); - let blockstore = open_blockstore( - &ledger_path, - get_access_type(&process_options), - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = + open_blockstore(&ledger_path, arg_matches, get_access_type(&process_options)); let (bank_forks, _) = load_and_process_ledger_or_exit( arg_matches, &genesis_config, @@ -2794,13 +2705,8 @@ fn main() { }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); - let blockstore = open_blockstore( - &ledger_path, - get_access_type(&process_options), - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = + open_blockstore(&ledger_path, arg_matches, get_access_type(&process_options)); let (bank_forks, _) = load_and_process_ledger_or_exit( arg_matches, &genesis_config, @@ -2920,10 +2826,8 @@ fn main() { }; let blockstore = Arc::new(open_blockstore( &ledger_path, + arg_matches, get_access_type(&process_options), - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, )); let snapshot_slot = if Some("ROOT") == arg_matches.value_of("snapshot_slot") { @@ -3340,13 +3244,8 @@ fn main() { }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); let include_sysvars = arg_matches.is_present("include_sysvars"); - let blockstore = open_blockstore( - &ledger_path, - get_access_type(&process_options), - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = + open_blockstore(&ledger_path, arg_matches, get_access_type(&process_options)); let (bank_forks, _) = load_and_process_ledger_or_exit( arg_matches, &genesis_config, @@ -3430,13 +3329,8 @@ fn main() { ..ProcessOptions::default() }; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); - let blockstore = open_blockstore( - &ledger_path, - get_access_type(&process_options), - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = + open_blockstore(&ledger_path, arg_matches, get_access_type(&process_options)); let (bank_forks, _) = load_and_process_ledger_or_exit( arg_matches, &genesis_config, @@ -3926,13 +3820,8 @@ fn main() { let dead_slots_only = arg_matches.is_present("dead_slots_only"); let batch_size = value_t_or_exit!(arg_matches, "batch_size", usize); - let blockstore = open_blockstore( - &ledger_path, - AccessType::PrimaryForMaintenance, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = + open_blockstore(&ledger_path, arg_matches, AccessType::PrimaryForMaintenance); let end_slot = match end_slot { Some(end_slot) => end_slot, @@ -4001,13 +3890,7 @@ fn main() { } } ("list-roots", Some(arg_matches)) => { - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); let max_height = value_t!(arg_matches, "max_height", usize).unwrap_or(usize::MAX); let start_root = value_t!(arg_matches, "start_root", Slot).unwrap_or(0); @@ -4044,13 +3927,7 @@ fn main() { }); } ("latest-optimistic-slots", Some(arg_matches)) => { - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); let num_slots = value_t_or_exit!(arg_matches, "num_slots", usize); let exclude_vote_only_slots = arg_matches.is_present("exclude_vote_only_slots"); let slots = @@ -4080,13 +3957,7 @@ fn main() { } } ("repair-roots", Some(arg_matches)) => { - let blockstore = open_blockstore( - &ledger_path, - AccessType::Primary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Primary); let start_root = value_t!(arg_matches, "start_root", Slot) .unwrap_or_else(|_| blockstore.max_root()); @@ -4112,13 +3983,7 @@ fn main() { println!("Successfully repaired {num_repaired_roots} roots"); } ("bounds", Some(arg_matches)) => { - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); match blockstore.slot_meta_iterator(0) { Ok(metas) => { @@ -4183,26 +4048,13 @@ fn main() { } }; } - ("analyze-storage", _) => { + ("analyze-storage", Some(arg_matches)) => { analyze_storage( - &open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ) - .db(), + &open_blockstore(&ledger_path, arg_matches, AccessType::Secondary).db(), ); } ("compute-slot-cost", Some(arg_matches)) => { - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - force_update_to_open, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); let mut slots: Vec = vec![]; if !arg_matches.is_present("slots") { @@ -4220,13 +4072,7 @@ fn main() { } } ("print-file-metadata", Some(arg_matches)) => { - let blockstore = open_blockstore( - &ledger_path, - AccessType::Secondary, - wal_recovery_mode, - false, - enforce_ulimit_nofile, - ); + let blockstore = open_blockstore(&ledger_path, arg_matches, AccessType::Secondary); let sst_file_name = arg_matches.value_of("file_name"); if let Err(err) = print_blockstore_file_metadata(&blockstore, &sst_file_name) { eprintln!("{err}"); diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index de8e5f93772b5d..97d676aff7831f 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -10,10 +10,7 @@ use { }, solana_clap_utils::input_parsers::pubkeys_of, solana_cli_output::{OutputFormat, QuietDisplay, VerboseDisplay}, - solana_ledger::{ - blockstore_options::{AccessType, BlockstoreRecoveryMode}, - blockstore_processor::ProcessOptions, - }, + solana_ledger::{blockstore_options::AccessType, blockstore_processor::ProcessOptions}, solana_program_runtime::{ invoke_context::InvokeContext, loaded_programs::{LoadProgramMetrics, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET}, @@ -77,8 +74,6 @@ fn load_accounts(path: &Path) -> Result { fn load_blockstore(ledger_path: &Path, arg_matches: &ArgMatches<'_>) -> Arc { let debug_keys = pubkeys_of(arg_matches, "debug_key") .map(|pubkeys| Arc::new(pubkeys.into_iter().collect::>())); - let force_update_to_open = arg_matches.is_present("force_update_to_open"); - let enforce_ulimit_nofile = !arg_matches.is_present("ignore_ulimit_nofile_error"); let process_options = ProcessOptions { new_hard_forks: hardforks_of(arg_matches, "hard_forks"), run_verification: false, @@ -108,18 +103,9 @@ fn load_blockstore(ledger_path: &Path, arg_matches: &ArgMatches<'_>) -> Arc