From 493108f02688a03ddba570600fafc082bde7ba74 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang <93241502+yhchiang-sol@users.noreply.github.com> Date: Wed, 13 Jul 2022 00:05:54 +0800 Subject: [PATCH] Move Blockstore::blockstore_directory() to ShredStorageType (#26179) #### Summary of Changes Blockstore::blockstore_directory() function takes a ShredStorageType and returns a path. As it's a ShredStorageType related function, moving it under ShredStorageType as a member function is a better fit. In addition, as we start doing automatic shred compaction type selection under ledger-tool, it becomes more convenient to reuse the function and const under blockstore_options namespace instead of blockstore. --- ledger/src/blockstore.rs | 60 ++++++++++++-------------------- ledger/src/blockstore_options.rs | 23 ++++++++++++ 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 01e3bfb2a..19985d1ee 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -11,7 +11,8 @@ use { }, blockstore_meta::*, blockstore_options::{ - AccessType, BlockstoreOptions, LedgerColumnOptions, ShredStorageType, + AccessType, BlockstoreOptions, BlockstoreRocksFifoOptions, LedgerColumnOptions, + ShredStorageType, }, leader_schedule_cache::LeaderScheduleCache, next_slots_iterator::NextSlotsIterator, @@ -84,9 +85,6 @@ pub use { rocksdb::properties as RocksProperties, }; -pub const BLOCKSTORE_DIRECTORY_ROCKS_LEVEL: &str = "rocksdb"; -pub const BLOCKSTORE_DIRECTORY_ROCKS_FIFO: &str = "rocksdb_fifo"; - // get_max_thread_count to match number of threads in the old code. // see: https://github.com/solana-labs/solana/pull/24853 lazy_static! { @@ -226,14 +224,6 @@ impl Blockstore { &self.ledger_path } - /// The directory under `ledger_path` to the underlying blockstore. - pub fn blockstore_directory(shred_storage_type: &ShredStorageType) -> &str { - match shred_storage_type { - ShredStorageType::RocksLevel => BLOCKSTORE_DIRECTORY_ROCKS_LEVEL, - ShredStorageType::RocksFifo(_) => BLOCKSTORE_DIRECTORY_ROCKS_FIFO, - } - } - /// Opens a Ledger in directory, provides "infinite" window of shreds pub fn open(ledger_path: &Path) -> Result { Self::do_open(ledger_path, BlockstoreOptions::default()) @@ -245,9 +235,12 @@ impl Blockstore { fn do_open(ledger_path: &Path, options: BlockstoreOptions) -> Result { fs::create_dir_all(&ledger_path)?; - let blockstore_path = ledger_path.join(Self::blockstore_directory( - &options.column_options.shred_storage_type, - )); + let blockstore_path = ledger_path.join( + options + .column_options + .shred_storage_type + .blockstore_directory(), + ); adjust_ulimit_nofile(options.enforce_ulimit_nofile)?; @@ -434,9 +427,15 @@ impl Blockstore { pub fn destroy(ledger_path: &Path) -> Result<()> { // Database::destroy() fails if the root directory doesn't exist fs::create_dir_all(ledger_path)?; - Database::destroy(&Path::new(ledger_path).join(BLOCKSTORE_DIRECTORY_ROCKS_LEVEL)).and( - Database::destroy(&Path::new(ledger_path).join(BLOCKSTORE_DIRECTORY_ROCKS_FIFO)), + Database::destroy( + &Path::new(ledger_path).join(ShredStorageType::RocksLevel.blockstore_directory()), ) + .and(Database::destroy( + &Path::new(ledger_path).join( + ShredStorageType::RocksFifo(BlockstoreRocksFifoOptions::default()) + .blockstore_directory(), + ), + )) } /// Returns the SlotMeta of the specified slot. @@ -3825,7 +3824,7 @@ 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_dir = Blockstore::blockstore_directory(&column_options.shred_storage_type); + let blockstore_dir = column_options.shred_storage_type.blockstore_directory(); let blockstore = Blockstore::open_with_options( ledger_path, BlockstoreOptions { @@ -4385,9 +4384,7 @@ pub mod tests { assert_eq!(ticks, entries); assert!(Path::new(ledger_path.path()) - .join(Blockstore::blockstore_directory( - &ShredStorageType::RocksLevel, - )) + .join(ShredStorageType::RocksLevel.blockstore_directory()) .exists()); } @@ -4416,26 +4413,13 @@ pub mod tests { assert_eq!(ticks, entries); assert!(Path::new(ledger_path.path()) - .join(Blockstore::blockstore_directory( - &ShredStorageType::RocksFifo(BlockstoreRocksFifoOptions::default()) - )) + .join( + ShredStorageType::RocksFifo(BlockstoreRocksFifoOptions::default()) + .blockstore_directory() + ) .exists()); } - #[test] - fn test_rocksdb_directory() { - assert_eq!( - Blockstore::blockstore_directory(&ShredStorageType::RocksLevel), - BLOCKSTORE_DIRECTORY_ROCKS_LEVEL - ); - assert_eq!( - Blockstore::blockstore_directory(&ShredStorageType::RocksFifo( - BlockstoreRocksFifoOptions::default() - )), - BLOCKSTORE_DIRECTORY_ROCKS_FIFO - ); - } - #[test] fn test_insert_get_bytes() { // Create enough entries to ensure there are at least two shreds created diff --git a/ledger/src/blockstore_options.rs b/ledger/src/blockstore_options.rs index 19522b46e..2609cec05 100644 --- a/ledger/src/blockstore_options.rs +++ b/ledger/src/blockstore_options.rs @@ -135,6 +135,9 @@ impl Default for ShredStorageType { } } +const BLOCKSTORE_DIRECTORY_ROCKS_LEVEL: &str = "rocksdb"; +const BLOCKSTORE_DIRECTORY_ROCKS_FIFO: &str = "rocksdb_fifo"; + impl ShredStorageType { /// Returns ShredStorageType::RocksFifo where the specified /// `shred_storage_size` is equally allocated to shred_data_cf_size @@ -142,6 +145,14 @@ impl ShredStorageType { pub fn rocks_fifo(shred_storage_size: u64) -> ShredStorageType { ShredStorageType::RocksFifo(BlockstoreRocksFifoOptions::new(shred_storage_size)) } + + /// The directory under `ledger_path` to the underlying blockstore. + pub fn blockstore_directory(&self) -> &str { + match self { + ShredStorageType::RocksLevel => BLOCKSTORE_DIRECTORY_ROCKS_LEVEL, + ShredStorageType::RocksFifo(_) => BLOCKSTORE_DIRECTORY_ROCKS_FIFO, + } + } } #[derive(Debug, Clone)] @@ -210,3 +221,15 @@ impl BlockstoreCompressionType { } } } + +#[test] +fn test_rocksdb_directory() { + assert_eq!( + ShredStorageType::RocksLevel.blockstore_directory(), + BLOCKSTORE_DIRECTORY_ROCKS_LEVEL + ); + assert_eq!( + ShredStorageType::RocksFifo(BlockstoreRocksFifoOptions::default()).blockstore_directory(), + BLOCKSTORE_DIRECTORY_ROCKS_FIFO + ); +}