Skip to content

Commit

Permalink
ledger-tool: Move blockstore arg parsing to open_blockstore() (solana…
Browse files Browse the repository at this point in the history
…-labs#34596)

The open_blockstore() helper currently takes multiple configurable
options. While the arguments are parsed at a high enough scope in
main.rs to avoid repeated calls, this parsing is duplicated in the
separate subcommand files (program.rs and bigtable.rs).

The repeated parsing is redundant, and also prone to having to missing
an arg (as was the case with bigtable not having wal_recovery_mode).

So, this PR consolidates the parsing to a single function and uses that
function across the previous callers.
  • Loading branch information
steviez authored Jan 3, 2024
1 parent 5a3a10e commit 0b49d82
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 230 deletions.
6 changes: 1 addition & 5 deletions ledger-tool/src/bigtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down
41 changes: 21 additions & 20 deletions ledger-tool/src/ledger_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -376,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()
Expand Down Expand Up @@ -430,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<BlockstoreRecoveryMode>,
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!(
Expand Down Expand Up @@ -508,6 +495,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`
Expand Down
Loading

0 comments on commit 0b49d82

Please sign in to comment.