From 6ab92226586fb2dca4d79c11d65f9b318217569b Mon Sep 17 00:00:00 2001 From: Kristofer Peterson Date: Wed, 13 May 2020 13:34:39 +0100 Subject: [PATCH] Remove CLI options and runtime support for selection output snapshot version. Address some clippy complaints. --- core/src/accounts_hash_verifier.rs | 2 -- core/src/rpc_service.rs | 2 -- core/src/snapshot_packager_service.rs | 3 +-- core/tests/bank_forks.rs | 20 +------------------- ledger-tool/src/main.rs | 27 ++------------------------- ledger/src/bank_forks.rs | 13 +------------ ledger/src/snapshot_package.rs | 4 ---- ledger/src/snapshot_utils.rs | 17 +++++++++-------- local-cluster/tests/local_cluster.rs | 1 - runtime/src/serde_utils.rs | 10 +++++----- validator/src/main.rs | 22 +--------------------- 11 files changed, 20 insertions(+), 101 deletions(-) diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index 6f01177aa0816c..4959b172b23017 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -177,7 +177,6 @@ mod tests { use crate::cluster_info::make_accounts_hashes_message; use crate::contact_info::ContactInfo; use solana_ledger::bank_forks::CompressionType; - use solana_ledger::snapshot_utils::SnapshotVersion; use solana_sdk::{ hash::hash, signature::{Keypair, Signer}, @@ -240,7 +239,6 @@ mod tests { tar_output_file: PathBuf::from("."), storages: vec![], compression: CompressionType::Bzip2, - snapshot_version: SnapshotVersion::default(), }; AccountsHashVerifier::process_accounts_package( diff --git a/core/src/rpc_service.rs b/core/src/rpc_service.rs index abc5971215c3a6..b361847be8fb97 100644 --- a/core/src/rpc_service.rs +++ b/core/src/rpc_service.rs @@ -356,7 +356,6 @@ mod tests { bank_forks::CompressionType, genesis_utils::{create_genesis_config, GenesisConfigInfo}, get_tmp_ledger_path, - snapshot_utils::SnapshotVersion, }; use solana_runtime::bank::Bank; use solana_sdk::signature::Signer; @@ -428,7 +427,6 @@ mod tests { snapshot_package_output_path: PathBuf::from("/"), snapshot_path: PathBuf::from("/"), compression: CompressionType::Bzip2, - snapshot_version: SnapshotVersion::default(), }), cluster_info, None, diff --git a/core/src/snapshot_packager_service.rs b/core/src/snapshot_packager_service.rs index 4bd6595c390122..626fdb735e3b2e 100644 --- a/core/src/snapshot_packager_service.rs +++ b/core/src/snapshot_packager_service.rs @@ -81,7 +81,7 @@ mod tests { use solana_ledger::bank_forks::CompressionType; use solana_ledger::{ snapshot_package::AccountsPackage, - snapshot_utils::{self, SnapshotVersion, SNAPSHOT_STATUS_CACHE_FILE_NAME}, + snapshot_utils::{self, SNAPSHOT_STATUS_CACHE_FILE_NAME}, }; use solana_runtime::{accounts_db::AccountStorageEntry, bank::BankSlotDelta}; use solana_sdk::hash::Hash; @@ -173,7 +173,6 @@ mod tests { output_tar_path.clone(), Hash::default(), CompressionType::Bzip2, - SnapshotVersion::default(), ); // Make tarball from packageable snapshot diff --git a/core/tests/bank_forks.rs b/core/tests/bank_forks.rs index 6f7548bc902a17..7eef7234ef22fc 100644 --- a/core/tests/bank_forks.rs +++ b/core/tests/bank_forks.rs @@ -57,7 +57,6 @@ mod tests { snapshot_package_output_path: PathBuf::from(snapshot_output_path.path()), snapshot_path: PathBuf::from(snapshot_dir.path()), compression: CompressionType::Bzip2, - snapshot_version: snapshot_utils::SnapshotVersion::default(), }; bank_forks.set_snapshot_config(Some(snapshot_config.clone())); SnapshotTestConfig { @@ -131,11 +130,6 @@ mod tests { let accounts_dir = &snapshot_test_config.accounts_dir; let snapshot_config = &snapshot_test_config.snapshot_config; let mint_keypair = &snapshot_test_config.genesis_config_info.mint_keypair; - let snapshot_version = bank_forks - .snapshot_config - .as_ref() - .unwrap() - .snapshot_version; let (s, _r) = channel(); let sender = Some(s); @@ -165,7 +159,6 @@ mod tests { &snapshot_config.snapshot_package_output_path, storages, CompressionType::Bzip2, - snapshot_version, ) .unwrap(); @@ -226,22 +219,11 @@ mod tests { let snapshot_config = &snapshot_test_config.snapshot_config; let mint_keypair = &snapshot_test_config.genesis_config_info.mint_keypair; let genesis_config = &snapshot_test_config.genesis_config_info.genesis_config; - let snapshot_version = bank_forks - .snapshot_config - .as_ref() - .unwrap() - .snapshot_version; // Take snapshot of zeroth bank let bank0 = bank_forks.get(0).unwrap(); let storages: Vec<_> = bank0.get_snapshot_storages(); - snapshot_utils::add_snapshot( - &snapshot_config.snapshot_path, - bank0, - &storages, - snapshot_version, - ) - .unwrap(); + snapshot_utils::add_snapshot(&snapshot_config.snapshot_path, bank0, &storages).unwrap(); // Set up snapshotting channels let (sender, receiver) = channel(); diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 8dfc7da206ca00..025ad0c0afae55 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -561,7 +561,6 @@ fn load_bank_forks( snapshot_package_output_path: ledger_path.clone(), snapshot_path: ledger_path.clone().join("snapshot"), compression: CompressionType::Bzip2, - snapshot_version: snapshot_utils::SnapshotVersion::default(), }) }; let account_paths = if let Some(account_paths) = arg_matches.value_of("account_paths") { @@ -629,18 +628,6 @@ fn main() { .takes_value(true) .default_value(&default_genesis_archive_unpacked_size) .help("maximum total uncompressed size of unpacked genesis archive"); - let snapshot_version_arg = Arg::with_name("snapshot_version") - .long("snapshot-version") - .value_name("VERSION") - .validator(|arg| { - arg.parse::() - .map(|_| ()) - .map_err(|e| e.to_string()) - }) - .multiple(false) - .required(false) - .takes_value(true) - .help("Output snapshot in this version"); let matches = App::new(crate_name!()) .about(crate_description!()) @@ -762,7 +749,6 @@ fn main() { .arg(&account_paths_arg) .arg(&hard_forks_arg) .arg(&max_genesis_archive_unpacked_size_arg) - .arg(&snapshot_version_arg) .arg( Arg::with_name("snapshot_slot") .index(1) @@ -1028,15 +1014,7 @@ fn main() { ("create-snapshot", Some(arg_matches)) => { let snapshot_slot = value_t_or_exit!(arg_matches, "snapshot_slot", Slot); let output_directory = value_t_or_exit!(arg_matches, "output_directory", String); - let snapshot_version = match arg_matches.value_of("snapshot_version") { - Some(s) => s - .parse::() - .unwrap_or_else(|e| { - eprintln!("Error: {}", e); - exit(1) - }), - None => snapshot_utils::SnapshotVersion::default(), - }; + let process_options = ProcessOptions { dev_halt_at_slot: Some(snapshot_slot), new_hard_forks: hardforks_of(arg_matches, "hard_forks"), @@ -1060,7 +1038,7 @@ fn main() { }); let storages: Vec<_> = bank.get_snapshot_storages(); - snapshot_utils::add_snapshot(&temp_dir, &bank, &storages, snapshot_version) + snapshot_utils::add_snapshot(&temp_dir, &bank, &storages) .and_then(|slot_snapshot_paths| { snapshot_utils::package_snapshot( &bank, @@ -1070,7 +1048,6 @@ fn main() { output_directory, storages, CompressionType::Bzip2, - snapshot_version, ) }) .and_then(|package| { diff --git a/ledger/src/bank_forks.rs b/ledger/src/bank_forks.rs index b5c2acfa1c4bdf..b3bf1474f5890c 100644 --- a/ledger/src/bank_forks.rs +++ b/ledger/src/bank_forks.rs @@ -16,8 +16,6 @@ use std::{ }; use thiserror::Error; -pub use crate::snapshot_utils::SnapshotVersion; - #[derive(Clone, Debug, Eq, PartialEq)] pub enum CompressionType { Bzip2, @@ -38,9 +36,6 @@ pub struct SnapshotConfig { pub snapshot_path: PathBuf, pub compression: CompressionType, - - // Snapshot version to generate - pub snapshot_version: SnapshotVersion, } #[derive(Error, Debug)] @@ -324,12 +319,7 @@ impl BankForks { let storages: Vec<_> = bank.get_snapshot_storages(); let mut add_snapshot_time = Measure::start("add-snapshot-ms"); - snapshot_utils::add_snapshot( - &config.snapshot_path, - &bank, - &storages, - config.snapshot_version, - )?; + snapshot_utils::add_snapshot(&config.snapshot_path, &bank, &storages)?; add_snapshot_time.stop(); inc_new_counter_info!("add-snapshot-ms", add_snapshot_time.as_ms() as usize); @@ -348,7 +338,6 @@ impl BankForks { &config.snapshot_package_output_path, storages, config.compression.clone(), - config.snapshot_version, )?; accounts_package_sender.send(package)?; diff --git a/ledger/src/snapshot_package.rs b/ledger/src/snapshot_package.rs index 5ee587cfaa1790..a426ff2e90f54a 100644 --- a/ledger/src/snapshot_package.rs +++ b/ledger/src/snapshot_package.rs @@ -1,5 +1,4 @@ use crate::bank_forks::CompressionType; -use crate::snapshot_utils::SnapshotVersion; use solana_runtime::{accounts_db::SnapshotStorages, bank::BankSlotDelta}; use solana_sdk::clock::Slot; use solana_sdk::hash::Hash; @@ -23,7 +22,6 @@ pub struct AccountsPackage { pub tar_output_file: PathBuf, pub hash: Hash, pub compression: CompressionType, - pub snapshot_version: SnapshotVersion, } impl AccountsPackage { @@ -36,7 +34,6 @@ impl AccountsPackage { tar_output_file: PathBuf, hash: Hash, compression: CompressionType, - snapshot_version: SnapshotVersion, ) -> Self { Self { root, @@ -47,7 +44,6 @@ impl AccountsPackage { tar_output_file, hash, compression, - snapshot_version, } } } diff --git a/ledger/src/snapshot_utils.rs b/ledger/src/snapshot_utils.rs index 4380cac74bed15..6b5891f1a4e128 100644 --- a/ledger/src/snapshot_utils.rs +++ b/ledger/src/snapshot_utils.rs @@ -36,6 +36,7 @@ pub const TAR_VERSION_FILE: &str = "version"; const MAX_SNAPSHOT_DATA_FILE_SIZE: u64 = 32 * 1024 * 1024 * 1024; // 32 GiB const VERSION_STRING_V1_1_0: &str = "1.1.0"; const VERSION_STRING_V1_1_1: &str = "1.1.1"; +const DEFAULT_SNAPSHOT_VERSION: SnapshotVersion = SnapshotVersion::V1_1_1; #[derive(Copy, Clone, Eq, PartialEq, Debug)] pub enum SnapshotVersion { @@ -45,7 +46,7 @@ pub enum SnapshotVersion { impl Default for SnapshotVersion { fn default() -> Self { - Self::V1_1_0 + DEFAULT_SNAPSHOT_VERSION } } @@ -138,7 +139,6 @@ pub fn package_snapshot, Q: AsRef>( snapshot_package_output_path: P, snapshot_storages: SnapshotStorages, compression: CompressionType, - snapshot_version: SnapshotVersion, ) -> Result { // Hard link all the snapshots we need for this package let snapshot_hard_links_dir = tempfile::tempdir_in(snapshot_path)?; @@ -169,7 +169,6 @@ pub fn package_snapshot, Q: AsRef>( snapshot_package_output_file, bank.get_accounts_hash(), compression, - snapshot_version, ); Ok(package) @@ -240,7 +239,7 @@ pub fn archive_snapshot_package(snapshot_package: &AccountsPackage) -> Result<() // Write version file { let mut f = std::fs::File::create(staging_version_file)?; - f.write_all(snapshot_package.snapshot_version.as_str().as_bytes())?; + f.write_all(DEFAULT_SNAPSHOT_VERSION.as_str().as_bytes())?; } let (compression_option, file_ext) = get_compression_ext(&snapshot_package.compression); @@ -447,7 +446,7 @@ fn create_versioned_bank_snapshot_serializer<'a>( Ok(()) }, )), - //If additional snapshot version were defined but not implemented... + //If additional snapshot versions were defined but not implemented... //_ => Err(get_io_error(&format!("unsupported snapshot version: {}", snapshot_version.to_string()))), } } @@ -508,7 +507,6 @@ pub fn add_snapshot>( snapshot_path: P, bank: &Bank, snapshot_storages: &[SnapshotStorage], - snapshot_version: SnapshotVersion, ) -> Result { let slot = bank.slot(); // snapshot_path/slot @@ -523,8 +521,11 @@ pub fn add_snapshot>( ); let mut bank_serialize = Measure::start("bank-serialize-ms"); - let bank_snapshot_serializer = - create_versioned_bank_snapshot_serializer(snapshot_version, bank, snapshot_storages)?; + let bank_snapshot_serializer = create_versioned_bank_snapshot_serializer( + DEFAULT_SNAPSHOT_VERSION, + bank, + snapshot_storages, + )?; let consumed_size = serialize_snapshot_data_file(&snapshot_bank_file_path, bank_snapshot_serializer)?; bank_serialize.stop(); diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index 2246723b38ff5d..34f4fda9c56d6a 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -1229,7 +1229,6 @@ fn setup_snapshot_validator_config( snapshot_package_output_path: PathBuf::from(snapshot_output_path.path()), snapshot_path: PathBuf::from(snapshot_dir.path()), compression: CompressionType::Bzip2, - snapshot_version: snapshot_utils::SnapshotVersion::default(), }; // Create the account paths diff --git a/runtime/src/serde_utils.rs b/runtime/src/serde_utils.rs index e38f70a00ce3d5..8c2aeeac081f52 100644 --- a/runtime/src/serde_utils.rs +++ b/runtime/src/serde_utils.rs @@ -5,6 +5,7 @@ use { AccountStorage, AccountStorageEntry, AccountStorageStatus, AccountsDB, AppendVecId, BankHashInfo, SlotStores, }, + accounts_index::Ancestors, append_vec::AppendVec, bank::BankRc, }, @@ -68,7 +69,7 @@ fn accountsdb_to_io_error(error: T) -> IoError { pub fn bankrc_from_stream( account_paths: &[PathBuf], slot: Slot, - ancestors: &HashMap, + ancestors: &Ancestors, frozen_account_pubkeys: &[Pubkey], stream: &mut BufReader, stream_append_vecs_path: P, @@ -101,7 +102,7 @@ where #[cfg(test)] pub(crate) fn accounts_from_stream( account_paths: &[PathBuf], - ancestors: &HashMap, + ancestors: &Ancestors, frozen_account_pubkeys: &[Pubkey], stream: &mut BufReader, stream_append_vecs_path: P, @@ -157,7 +158,7 @@ where pub fn context_bankrc_from_stream<'a, C, R, P>( account_paths: &[PathBuf], slot: Slot, - ancestors: &HashMap, + ancestors: &Ancestors, frozen_account_pubkeys: &[Pubkey], mut stream: &mut BufReader, stream_append_vecs_path: P, @@ -237,7 +238,7 @@ where pub(crate) fn context_accounts_from_stream<'a, C, R, P>( account_paths: &[PathBuf], - ancestors: &HashMap, + ancestors: &Ancestors, frozen_account_pubkeys: &[Pubkey], stream: &mut BufReader, stream_append_vecs_path: P, @@ -583,7 +584,6 @@ impl From<&AccountStorageEntry> for SerializableAccountStorageEntryV1_1_1 { Self { id: rhs.id, accounts_current_len: rhs.accounts.len(), - ..Self::default() } } } diff --git a/validator/src/main.rs b/validator/src/main.rs index eb9644cff75c04..0ed34a9c16cf7d 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -22,7 +22,7 @@ use solana_core::{ }; use solana_download_utils::{download_genesis_if_missing, download_snapshot}; use solana_ledger::{ - bank_forks::{CompressionType, SnapshotConfig, SnapshotVersion}, + bank_forks::{CompressionType, SnapshotConfig}, hardened_unpack::{unpack_genesis_archive, MAX_GENESIS_ARCHIVE_UNPACKED_SIZE}, }; use solana_perf::recycler::enable_recycler_warming; @@ -682,18 +682,6 @@ pub fn main() { .default_value("100") .help("Number of slots between generating accounts hash."), ) - .arg( - Arg::with_name("snapshot_version") - .long("snapshot-version") - .value_name("SNAPSHOT_VERSION") - .validator(|arg| arg.parse::() - .map(|_| ()) - .map_err(|e| e.to_string())) - .takes_value(true) - .multiple(false) - .required(false) - .help("Number of slots between generating snapshots, 0 to disable snapshots"), - ) .arg( Arg::with_name("limit_ledger_size") .long("limit-ledger-size") @@ -966,13 +954,6 @@ pub fn main() { _ => panic!("Compression type not recognized: {}", compression_str), } } - let snapshot_version = match matches.value_of("snapshot_version") { - Some(s) => s.parse::().unwrap_or_else(|e| { - eprintln!("Error: {}", e); - exit(1) - }), - None => SnapshotVersion::default(), - }; validator_config.snapshot_config = Some(SnapshotConfig { snapshot_interval_slots: if snapshot_interval_slots > 0 { snapshot_interval_slots @@ -982,7 +963,6 @@ pub fn main() { snapshot_path, snapshot_package_output_path: ledger_path.clone(), compression: snapshot_compression, - snapshot_version, }); validator_config.accounts_hash_interval_slots =