From a21be4b7a0e2164c8c2a541a9e0c8dbc6a4d0b55 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Sat, 12 Feb 2022 18:27:50 -0800 Subject: [PATCH 1/2] Minor refactor on Shreds column family descriptor construction. --- ledger/src/blockstore_db.rs | 74 ++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/ledger/src/blockstore_db.rs b/ledger/src/blockstore_db.rs index 3b43c0079b5ea5..6ff4956be87697 100644 --- a/ledger/src/blockstore_db.rs +++ b/ledger/src/blockstore_db.rs @@ -318,40 +318,18 @@ impl Rocks { new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), - match options.shred_storage_type { - ShredStorageType::RocksLevel => { - new_cf_descriptor::(&access_type, &oldest_slot) - } - ShredStorageType::RocksFifo => { - if options.shred_data_cf_size > FIFO_WRITE_BUFFER_SIZE { - new_cf_descriptor_fifo::(&options.shred_data_cf_size) - } else { - warn!( - "shred_data_cf_size is must be greater than {} for RocksFifo.", - FIFO_WRITE_BUFFER_SIZE - ); - warn!("Fall back to ShredStorageType::RocksLevel for cf::ShredData."); - new_cf_descriptor::(&access_type, &oldest_slot) - } - } - }, - match options.shred_storage_type { - ShredStorageType::RocksLevel => { - new_cf_descriptor::(&access_type, &oldest_slot) - } - ShredStorageType::RocksFifo => { - if options.shred_code_cf_size > FIFO_WRITE_BUFFER_SIZE { - new_cf_descriptor_fifo::(&options.shred_code_cf_size) - } else { - warn!( - "shred_code_cf_size is must be greater than {} for RocksFifo.", - FIFO_WRITE_BUFFER_SIZE - ); - warn!("Fall back to ShredStorageType::RocksLevel for cf::ShredCode."); - new_cf_descriptor::(&access_type, &oldest_slot) - } - } - }, + new_cf_descriptor_shreds::( + &options.shred_storage_type, + &access_type, + &oldest_slot, + &options.shred_data_cf_size, + ), + new_cf_descriptor_shreds::( + &options.shred_storage_type, + &access_type, + &oldest_slot, + &options.shred_code_cf_size, + ), new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), @@ -1437,6 +1415,34 @@ fn get_cf_options( options } +/// Constructs and returns a ColumnFamilyDescriptor based on the +/// specified ShredStorageType. +fn new_cf_descriptor_shreds( + storage_type: &ShredStorageType, + access_type: &AccessType, + oldest_slot: &OldestSlot, + max_cf_size: &u64, +) -> ColumnFamilyDescriptor { + match storage_type { + ShredStorageType::RocksLevel => new_cf_descriptor::(access_type, oldest_slot), + ShredStorageType::RocksFifo => { + if *max_cf_size > FIFO_WRITE_BUFFER_SIZE { + new_cf_descriptor_fifo::(max_cf_size) + } else { + warn!( + " is must be greater than {} for RocksFifo.", + FIFO_WRITE_BUFFER_SIZE + ); + warn!( + "Fall back to ShredStorageType::RocksLevel for cf::{}.", + C::NAME + ); + new_cf_descriptor::(access_type, oldest_slot) + } + } + } +} + fn new_cf_descriptor_fifo( max_cf_size: &u64, ) -> ColumnFamilyDescriptor { From a3c995d15202541b7b3aeaf1e229412b21adf73f Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang Date: Sat, 12 Feb 2022 18:27:50 -0800 Subject: [PATCH 2/2] Minor refactor on Shreds column family descriptor construction. --- core/tests/ledger_cleanup.rs | 7 ++- ledger/src/blockstore_db.rs | 96 +++++++++++++++++++++++++----------- 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/core/tests/ledger_cleanup.rs b/core/tests/ledger_cleanup.rs index 904a9fc3777214..8c3d440fbd4360 100644 --- a/core/tests/ledger_cleanup.rs +++ b/core/tests/ledger_cleanup.rs @@ -9,7 +9,7 @@ mod tests { solana_core::ledger_cleanup_service::LedgerCleanupService, solana_ledger::{ blockstore::{make_many_slot_shreds, Blockstore}, - blockstore_db::{BlockstoreOptions, ShredStorageType}, + blockstore_db::{BlockstoreOptions, BlockstoreRocksFifoOptions, ShredStorageType}, get_tmp_ledger_path, }, solana_measure::measure::Measure, @@ -310,7 +310,10 @@ mod tests { if config.fifo_compaction { BlockstoreOptions { shred_storage_type: ShredStorageType::RocksFifo, - shred_data_cf_size: config.shred_data_cf_size, + fifo_options: Some(BlockstoreRocksFifoOptions { + shred_data_cf_size: config.shred_data_cf_size, + ..BlockstoreRocksFifoOptions::default() + }), ..BlockstoreOptions::default() } } else { diff --git a/ledger/src/blockstore_db.rs b/ledger/src/blockstore_db.rs index 6ff4956be87697..b0671c9737b7d7 100644 --- a/ledger/src/blockstore_db.rs +++ b/ledger/src/blockstore_db.rs @@ -309,6 +309,13 @@ impl Rocks { let oldest_slot = OldestSlot::default(); // Get column family descriptors and names + let (cf_descriptor_shred_data, cf_descriptor_shred_code) = + new_cf_descriptor_pair_shreds::( + &options.shred_storage_type, + &access_type, + &oldest_slot, + &options.fifo_options, + ); let cfs = vec![ new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), @@ -318,18 +325,8 @@ impl Rocks { new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), - new_cf_descriptor_shreds::( - &options.shred_storage_type, - &access_type, - &oldest_slot, - &options.shred_data_cf_size, - ), - new_cf_descriptor_shreds::( - &options.shred_storage_type, - &access_type, - &oldest_slot, - &options.shred_code_cf_size, - ), + cf_descriptor_shred_data, + cf_descriptor_shred_code, new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), new_cf_descriptor::(&access_type, &oldest_slot), @@ -993,37 +990,42 @@ pub struct BlockstoreOptions { pub enforce_ulimit_nofile: bool, // Determine how to store both data and coding shreds. Default: RocksLevel. pub shred_storage_type: ShredStorageType, + // Options that are used when ShredStorageType is RocksFIFO. + pub fifo_options: Option, +} + +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, + shred_storage_type: ShredStorageType::RocksLevel, + fifo_options: None, + } + } +} + +pub struct BlockstoreRocksFifoOptions { // The maximum storage size for storing data shreds in column family // [`cf::DataShred`]. Typically, data shreds contribute around 25% of the // ledger store storage size if the RPC service is enabled, or 50% if RPC // service is not enabled. - // - // Currently, this setting is only used when shred_storage_type is set to - // [`ShredStorageType::RocksFifo`]. pub shred_data_cf_size: u64, // The maximum storage size for storing coding shreds in column family // [`cf::CodeShred`]. Typically, coding shreds contribute around 20% of the // ledger store storage size if the RPC service is enabled, or 40% if RPC // service is not enabled. - // - // Currently, this setting is only used when shred_storage_type is set to - // [`ShredStorageType::RocksFifo`]. pub shred_code_cf_size: u64, } -impl Default for BlockstoreOptions { - /// The default options are the values used by [`Blockstore::open`]. +impl Default for BlockstoreRocksFifoOptions { fn default() -> Self { Self { - access_type: AccessType::PrimaryOnly, - recovery_mode: None, - enforce_ulimit_nofile: true, - shred_storage_type: ShredStorageType::RocksLevel, - // Maximum size of cf::DataShred. Used when `shred_storage_type` - // is set to ShredStorageType::RocksFifo. + // Maximum size of cf::ShredData. shred_data_cf_size: DEFAULT_FIFO_COMPACTION_DATA_CF_SIZE, - // Maximum size of cf::CodeShred. Used when `shred_storage_type` - // is set to ShredStorageType::RocksFifo. + // Maximum size of cf::ShredCode. shred_code_cf_size: DEFAULT_FIFO_COMPACTION_CODING_CF_SIZE, } } @@ -1415,6 +1417,42 @@ fn get_cf_options( options } +/// Creates and returns the column family descriptors for both data shreds and +/// coding shreds column families. +/// +/// @return a pair of ColumnFamilyDescriptor where the first / second elements +/// are associated to the first / second template class respectively. +fn new_cf_descriptor_pair_shreds< + D: 'static + Column + ColumnName, // Column Family for Data Shred + C: 'static + Column + ColumnName, // Column Family for Coding Shred +>( + shred_storage_type: &ShredStorageType, + access_type: &AccessType, + oldest_slot: &OldestSlot, + fifo_options: &Option, +) -> (ColumnFamilyDescriptor, ColumnFamilyDescriptor) { + match &fifo_options { + Some(fifo) => ( + new_cf_descriptor_shreds::( + shred_storage_type, + access_type, + oldest_slot, + &fifo.shred_data_cf_size, + ), + new_cf_descriptor_shreds::( + shred_storage_type, + access_type, + oldest_slot, + &fifo.shred_code_cf_size, + ), + ), + None => ( + new_cf_descriptor::(access_type, oldest_slot), + new_cf_descriptor::(access_type, oldest_slot), + ), + } +} + /// Constructs and returns a ColumnFamilyDescriptor based on the /// specified ShredStorageType. fn new_cf_descriptor_shreds( @@ -1430,7 +1468,7 @@ fn new_cf_descriptor_shreds( new_cf_descriptor_fifo::(max_cf_size) } else { warn!( - " is must be greater than {} for RocksFifo.", + "max_cf_size must be greater than {} for RocksFifo.", FIFO_WRITE_BUFFER_SIZE ); warn!(