From e8b7f96a89bc60c3ddddb7bf73f47b292df54c84 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang <93241502+yhchiang-sol@users.noreply.github.com> Date: Mon, 3 Jan 2022 18:30:45 -1000 Subject: [PATCH] Add struct BlockstoreOptions (#22121) --- core/src/tvu.rs | 3 +- core/src/validator.rs | 9 +++-- ledger-tool/src/main.rs | 22 +++++++++-- ledger/src/blockstore.rs | 59 +++++++++++----------------- ledger/src/blockstore_db.rs | 33 ++++++++++------ local-cluster/tests/local_cluster.rs | 25 ++++++++---- replica-node/src/replica_node.rs | 9 +++-- 7 files changed, 93 insertions(+), 67 deletions(-) diff --git a/core/src/tvu.rs b/core/src/tvu.rs index 6b516f2fb..a8325973a 100644 --- a/core/src/tvu.rs +++ b/core/src/tvu.rs @@ -398,6 +398,7 @@ pub mod tests { solana_gossip::cluster_info::{ClusterInfo, Node}, solana_ledger::{ blockstore::BlockstoreSignals, + blockstore_db::BlockstoreOptions, create_new_tmp_ledger, genesis_utils::{create_genesis_config, GenesisConfigInfo}, }, @@ -438,7 +439,7 @@ pub mod tests { blockstore, ledger_signal_receiver, .. - } = Blockstore::open_with_signal(&blockstore_path, None, true) + } = Blockstore::open_with_signal(&blockstore_path, BlockstoreOptions::default()) .expect("Expected to successfully open ledger"); let blockstore = Arc::new(blockstore); let bank = bank_forks.working_bank(); diff --git a/core/src/validator.rs b/core/src/validator.rs index 5f2af1d7d..50b747a25 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -36,7 +36,7 @@ use { solana_ledger::{ bank_forks_utils, blockstore::{Blockstore, BlockstoreSignals, CompletedSlotsReceiver, PurgeType}, - blockstore_db::BlockstoreRecoveryMode, + blockstore_db::{BlockstoreOptions, BlockstoreRecoveryMode}, blockstore_processor::{self, TransactionStatusSender}, leader_schedule::FixedSchedule, leader_schedule_cache::LeaderScheduleCache, @@ -1265,8 +1265,11 @@ fn new_banks_from_ledger( .. } = Blockstore::open_with_signal( ledger_path, - config.wal_recovery_mode.clone(), - enforce_ulimit_nofile, + BlockstoreOptions { + recovery_mode: config.wal_recovery_mode.clone(), + enforce_ulimit_nofile, + ..BlockstoreOptions::default() + }, ) .expect("Failed to open ledger database"); blockstore.set_no_compaction(config.no_rocksdb_compaction); diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index e9f01b4f8..5d49ed784 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -22,7 +22,9 @@ use { ancestor_iterator::AncestorIterator, bank_forks_utils, blockstore::{create_new_ledger, Blockstore, PurgeType}, - blockstore_db::{self, AccessType, BlockstoreRecoveryMode, Column, Database}, + blockstore_db::{ + self, AccessType, BlockstoreOptions, BlockstoreRecoveryMode, Column, Database, + }, blockstore_processor::ProcessOptions, shred::Shred, }, @@ -676,7 +678,14 @@ fn open_blockstore( access_type: AccessType, wal_recovery_mode: Option, ) -> Blockstore { - match Blockstore::open_with_access_type(ledger_path, access_type, wal_recovery_mode, true) { + match Blockstore::open_with_access_type( + ledger_path, + BlockstoreOptions { + access_type, + recovery_mode: wal_recovery_mode, + enforce_ulimit_nofile: true, + }, + ) { Ok(blockstore) => blockstore, Err(err) => { eprintln!("Failed to open ledger at {:?}: {:?}", ledger_path, err); @@ -686,7 +695,14 @@ fn open_blockstore( } fn open_database(ledger_path: &Path, access_type: AccessType) -> Database { - match Database::open(&ledger_path.join("rocksdb"), access_type, None) { + match Database::open( + &ledger_path.join("rocksdb"), + BlockstoreOptions { + access_type, + recovery_mode: None, + ..BlockstoreOptions::default() + }, + ) { Ok(database) => database, Err(err) => { eprintln!("Unable to read the Ledger rocksdb: {:?}", err); diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 58212761d..4246d7a9c 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -6,7 +6,7 @@ use { crate::{ ancestor_iterator::AncestorIterator, blockstore_db::{ - columns as cf, AccessType, BlockstoreRecoveryMode, Column, Database, IteratorDirection, + columns as cf, AccessType, BlockstoreOptions, Column, Database, IteratorDirection, IteratorMode, LedgerColumn, Result, WriteBatch, }, blockstore_meta::*, @@ -341,38 +341,26 @@ impl Blockstore { /// Opens a Ledger in directory, provides "infinite" window of shreds pub fn open(ledger_path: &Path) -> Result { - Self::do_open(ledger_path, AccessType::PrimaryOnly, None, true) + Self::do_open(ledger_path, BlockstoreOptions::default()) } pub fn open_with_access_type( ledger_path: &Path, - access_type: AccessType, - recovery_mode: Option, - enforce_ulimit_nofile: bool, + options: BlockstoreOptions, ) -> Result { - Self::do_open( - ledger_path, - access_type, - recovery_mode, - enforce_ulimit_nofile, - ) + Self::do_open(ledger_path, options) } - fn do_open( - ledger_path: &Path, - access_type: AccessType, - recovery_mode: Option, - enforce_ulimit_nofile: bool, - ) -> Result { + fn do_open(ledger_path: &Path, options: BlockstoreOptions) -> Result { fs::create_dir_all(&ledger_path)?; let blockstore_path = ledger_path.join(BLOCKSTORE_DIRECTORY); - adjust_ulimit_nofile(enforce_ulimit_nofile)?; + adjust_ulimit_nofile(options.enforce_ulimit_nofile)?; // Open the database let mut measure = Measure::start("open"); info!("Opening database at {:?}", blockstore_path); - let db = Database::open(&blockstore_path, access_type, recovery_mode)?; + let db = Database::open(&blockstore_path, options)?; // Create the metadata column family let meta_cf = db.column(); @@ -467,15 +455,9 @@ impl Blockstore { pub fn open_with_signal( ledger_path: &Path, - recovery_mode: Option, - enforce_ulimit_nofile: bool, + options: BlockstoreOptions, ) -> Result { - let mut blockstore = Self::open_with_access_type( - ledger_path, - AccessType::PrimaryOnly, - recovery_mode, - enforce_ulimit_nofile, - )?; + let mut blockstore = Self::open_with_access_type(ledger_path, options)?; let (ledger_signal_sender, ledger_signal_receiver) = sync_channel(1); let (completed_slots_sender, completed_slots_receiver) = sync_channel(MAX_COMPLETED_SLOTS_IN_CHANNEL); @@ -3773,7 +3755,14 @@ pub fn create_new_ledger( genesis_config.write(ledger_path)?; // Fill slot 0 with ticks that link back to the genesis_config to bootstrap the ledger. - let blockstore = Blockstore::open_with_access_type(ledger_path, access_type, None, false)?; + let blockstore = Blockstore::open_with_access_type( + ledger_path, + BlockstoreOptions { + access_type, + recovery_mode: None, + enforce_ulimit_nofile: false, + }, + )?; let ticks_per_slot = genesis_config.ticks_per_slot; let hashes_per_tick = genesis_config.poh_config.hashes_per_tick.unwrap_or(0); let entries = create_ticks(ticks_per_slot, hashes_per_tick, genesis_config.hash()); @@ -4716,7 +4705,7 @@ pub mod tests { fn test_data_set_completed_on_insert() { let ledger_path = get_tmp_ledger_path_auto_delete!(); let BlockstoreSignals { blockstore, .. } = - Blockstore::open_with_signal(ledger_path.path(), None, true).unwrap(); + Blockstore::open_with_signal(ledger_path.path(), BlockstoreOptions::default()).unwrap(); // Create enough entries to fill 2 shreds, only the later one is data complete let slot = 0; @@ -4757,8 +4746,7 @@ pub mod tests { blockstore, ledger_signal_receiver: recvr, .. - } = Blockstore::open_with_signal(ledger_path.path(), None, true).unwrap(); - //let blockstore = Arc::new(blockstore); + } = Blockstore::open_with_signal(ledger_path.path(), BlockstoreOptions::default()).unwrap(); let entries_per_slot = 50; // Create entries for slot 0 @@ -4841,8 +4829,7 @@ pub mod tests { blockstore, completed_slots_receiver: recvr, .. - } = Blockstore::open_with_signal(ledger_path.path(), None, true).unwrap(); - // let blockstore = Arc::new(blockstore); + } = Blockstore::open_with_signal(ledger_path.path(), BlockstoreOptions::default()).unwrap(); let entries_per_slot = 10; @@ -4867,8 +4854,7 @@ pub mod tests { blockstore, completed_slots_receiver: recvr, .. - } = Blockstore::open_with_signal(ledger_path.path(), None, true).unwrap(); - // let blockstore = Arc::new(blockstore); + } = Blockstore::open_with_signal(ledger_path.path(), BlockstoreOptions::default()).unwrap(); let entries_per_slot = 10; let slots = vec![2, 5, 10]; @@ -4913,8 +4899,7 @@ pub mod tests { blockstore, completed_slots_receiver: recvr, .. - } = Blockstore::open_with_signal(ledger_path.path(), None, true).unwrap(); - // let blockstore = Arc::new(blockstore); + } = Blockstore::open_with_signal(ledger_path.path(), BlockstoreOptions::default()).unwrap(); let entries_per_slot = 10; let mut slots = vec![2, 5, 10]; diff --git a/ledger/src/blockstore_db.rs b/ledger/src/blockstore_db.rs index c7632931f..f1ca33414 100644 --- a/ledger/src/blockstore_db.rs +++ b/ledger/src/blockstore_db.rs @@ -270,12 +270,10 @@ impl OldestSlot { struct Rocks(rocksdb::DB, ActualAccessType, OldestSlot); impl Rocks { - fn open( - path: &Path, - access_type: AccessType, - recovery_mode: Option, - ) -> Result { + fn open(path: &Path, options: BlockstoreOptions) -> Result { use columns::*; + let access_type = options.access_type; + let recovery_mode = options.recovery_mode; fs::create_dir_all(&path)?; @@ -1016,13 +1014,26 @@ pub struct WriteBatch<'a> { map: HashMap<&'static str, &'a ColumnFamily>, } +pub struct BlockstoreOptions { + pub access_type: AccessType, + pub recovery_mode: Option, + pub enforce_ulimit_nofile: bool, +} + +impl Default for BlockstoreOptions { + /// The default options are the values used by [`Blockstore::open`]. + fn default() -> Self { + Self { + access_type: AccessType::PrimaryOnly, + recovery_mode: None, + enforce_ulimit_nofile: true, + } + } +} + impl Database { - pub fn open( - path: &Path, - access_type: AccessType, - recovery_mode: Option, - ) -> Result { - let backend = Arc::new(Rocks::open(path, access_type, recovery_mode)?); + pub fn open(path: &Path, options: BlockstoreOptions) -> Result { + let backend = Arc::new(Rocks::open(path, options)?); Ok(Database { backend, diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index 0ad14e823..d901174d9 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -31,7 +31,7 @@ use { solana_ledger::{ ancestor_iterator::AncestorIterator, blockstore::{Blockstore, PurgeType}, - blockstore_db::AccessType, + blockstore_db::{AccessType, BlockstoreOptions}, leader_schedule::{FixedSchedule, LeaderSchedule}, }, solana_local_cluster::{ @@ -3043,10 +3043,17 @@ fn test_validator_saves_tower() { } fn open_blockstore(ledger_path: &Path) -> Blockstore { - Blockstore::open_with_access_type(ledger_path, AccessType::TryPrimaryThenSecondary, None, true) - .unwrap_or_else(|e| { - panic!("Failed to open ledger at {:?}, err: {}", ledger_path, e); - }) + Blockstore::open_with_access_type( + ledger_path, + BlockstoreOptions { + access_type: AccessType::TryPrimaryThenSecondary, + recovery_mode: None, + enforce_ulimit_nofile: true, + }, + ) + .unwrap_or_else(|e| { + panic!("Failed to open ledger at {:?}, err: {}", ledger_path, e); + }) } fn purge_slots(blockstore: &Blockstore, start_slot: Slot, slot_count: Slot) { @@ -3402,9 +3409,11 @@ fn do_test_optimistic_confirmation_violation_with_or_without_tower(with_tower: b a_votes.push(last_vote); let blockstore = Blockstore::open_with_access_type( &val_a_ledger_path, - AccessType::TryPrimaryThenSecondary, - None, - true, + BlockstoreOptions { + access_type: AccessType::TryPrimaryThenSecondary, + recovery_mode: None, + enforce_ulimit_nofile: true, + }, ) .unwrap(); let mut ancestors = AncestorIterator::new(last_vote, &blockstore); diff --git a/replica-node/src/replica_node.rs b/replica-node/src/replica_node.rs index 021783a2f..221661d04 100644 --- a/replica-node/src/replica_node.rs +++ b/replica-node/src/replica_node.rs @@ -6,7 +6,7 @@ use { solana_genesis_utils::download_then_check_genesis_hash, solana_gossip::{cluster_info::ClusterInfo, contact_info::ContactInfo}, solana_ledger::{ - blockstore::Blockstore, blockstore_db::AccessType, blockstore_processor, + blockstore::Blockstore, blockstore_db::BlockstoreOptions, blockstore_processor, leader_schedule_cache::LeaderScheduleCache, }, solana_replica_lib::accountsdb_repl_client::AccountsDbReplClientServiceConfig, @@ -177,9 +177,10 @@ fn start_client_rpc_services( let blockstore = Arc::new( Blockstore::open_with_access_type( &replica_config.ledger_path, - AccessType::PrimaryOnly, - None, - false, + BlockstoreOptions { + enforce_ulimit_nofile: false, + ..BlockstoreOptions::default() + }, ) .unwrap(), );