Make RocksFifo storage size API expose an Option<u64> (solana-labs#28192

)

A fifo rocksdb instance must be opened with max size parameter on the
fifo columns. To support this, we previously plumbed a constant up to
callers that provided a default if unbounded growth desired.

This change attempts to be more rusty by exposing an option for this
value, and converting the option to a constant at the lowest level
possible.

ledger-tool/src/main.rs

@@ -33,7 +33,6 @@ use {
         blockstore_options::{
             AccessType, BlockstoreOptions, BlockstoreRecoveryMode, LedgerColumnOptions,
             ShredStorageType, BLOCKSTORE_DIRECTORY_ROCKS_FIFO,
-            MAX_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES,
         },
         blockstore_processor::{self, BlockstoreProcessorError, ProcessOptions},
         shred::Shred,
@@ -875,9 +874,9 @@ fn open_blockstore_with_temporary_primary_access(
 fn get_shred_storage_type(ledger_path: &Path, warn_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
-    // temporary use of MAX_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES could
-    // affect the persisted rocksdb options file.
-    match ShredStorageType::from_ledger_path(ledger_path, MAX_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES) {
+    // 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 => {
             warn!("{}", warn_message);

ledger-tool/tests/basic.rs

@@ -87,7 +87,6 @@ fn insert_test_shreds(ledger_path: &Path, ending_slot: u64) {
 }
 
 fn ledger_tool_copy_test(src_shred_compaction: &str, dst_shred_compaction: &str) {
-    const TEST_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES: u64 = std::u64::MAX;
     let genesis_config = create_genesis_config(100).genesis_config;
 
     let (ledger_path, _blockhash) = match src_shred_compaction {
@@ -101,10 +100,9 @@ fn ledger_tool_copy_test(src_shred_compaction: &str, dst_shred_compaction: &str)
 
     let target_ledger_path = get_tmp_ledger_path_auto_delete!();
     if dst_shred_compaction == "fifo" {
-        let rocksdb_fifo_path = target_ledger_path.path().join(
-            ShredStorageType::rocks_fifo(TEST_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES)
-                .blockstore_directory(),
-        );
+        let rocksdb_fifo_path = target_ledger_path
+            .path()
+            .join(ShredStorageType::rocks_fifo(None).blockstore_directory());
         fs::create_dir_all(rocksdb_fifo_path).unwrap();
     }
     let target_ledger_path = target_ledger_path.path().to_str().unwrap();

ledger/src/blockstore_options.rs

@@ -142,11 +142,10 @@ pub const BLOCKSTORE_DIRECTORY_ROCKS_LEVEL: &str = "rocksdb";
 pub 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
-    /// and shred_code_cf_size.
-    pub fn rocks_fifo(shred_storage_size: u64) -> ShredStorageType {
-        ShredStorageType::RocksFifo(BlockstoreRocksFifoOptions::new(shred_storage_size))
+    /// Returns a ShredStorageType::RocksFifo, see BlockstoreRocksFifoOptions
+    /// for more details on how `max_shred_storage_size` is interpreted.
+    pub fn rocks_fifo(max_shred_storage_size: Option<u64>) -> ShredStorageType {
+        ShredStorageType::RocksFifo(BlockstoreRocksFifoOptions::new(max_shred_storage_size))
     }
 
     /// The directory under `ledger_path` to the underlying blockstore.
@@ -163,7 +162,7 @@ impl ShredStorageType {
     /// None will be returned if the ShredStorageType cannot be inferred.
     pub fn from_ledger_path(
         ledger_path: &Path,
-        fifo_shred_storage_size: u64,
+        max_fifo_shred_storage_size: Option<u64>,
     ) -> Option<ShredStorageType> {
         let mut result: Option<ShredStorageType> = None;
 
@@ -180,7 +179,7 @@ impl ShredStorageType {
         {
             if result.is_none() {
                 result = Some(ShredStorageType::RocksFifo(
-                    BlockstoreRocksFifoOptions::new(fifo_shred_storage_size),
+                    BlockstoreRocksFifoOptions::new(max_fifo_shred_storage_size),
                 ));
             } else {
                 result = None;
@@ -215,10 +214,22 @@ pub struct BlockstoreRocksFifoOptions {
 pub const MAX_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES: u64 = std::u64::MAX;
 
 impl BlockstoreRocksFifoOptions {
-    fn new(shred_storage_size: u64) -> Self {
-        Self {
-            shred_data_cf_size: shred_storage_size / 2,
-            shred_code_cf_size: shred_storage_size / 2,
+    /// Returns a BlockstoreRocksFifoOptions where the specified
+    /// `max_shred_storage_size` is equally split between shred_data_cf_size
+    /// and shred_code_cf_size. A `None` value for `max_shred_storage_size`
+    /// will (functionally) allow unbounded growth in these two columns. Once
+    /// a column's total size exceeds the configured value, the oldest file(s)
+    /// will be purged to get back within the limit.
+    fn new(max_shred_storage_size: Option<u64>) -> Self {
+        match max_shred_storage_size {
+            Some(size) => Self {
+                shred_data_cf_size: size / 2,
+                shred_code_cf_size: size / 2,
+            },
+            None => Self {
+                shred_data_cf_size: MAX_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES,
+                shred_code_cf_size: MAX_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES,
+            },
         }
     }
 

validator/src/main.rs

@@ -28,7 +28,6 @@ use {
     solana_gossip::{cluster_info::Node, contact_info::ContactInfo},
     solana_ledger::blockstore_options::{
         BlockstoreCompressionType, BlockstoreRecoveryMode, LedgerColumnOptions, ShredStorageType,
-        MAX_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES,
     },
     solana_net_utils::VALIDATOR_PORT_RANGE,
     solana_perf::recycler::enable_recycler_warming,
@@ -392,19 +391,18 @@ fn hash_validator(hash: String) -> Result<(), String> {
         .map_err(|e| format!("{:?}", e))
 }
 
-/// Returns the default shred storage size (include both data and coding
+/// Returns the default fifo shred storage size (include both data and coding
 /// shreds) based on the validator config.
-fn default_fifo_shred_storage_size(vc: &ValidatorConfig) -> u64 {
+fn default_fifo_shred_storage_size(vc: &ValidatorConfig) -> Option<u64> {
     // The max shred size is around 1228 bytes.
     // Here we reserve a little bit more than that to give extra storage for FIFO
     // to prevent it from purging data that have not yet being marked as obsoleted
     // by LedgerCleanupService.
     const RESERVED_BYTES_PER_SHRED: u64 = 1500;
-    match vc.max_ledger_shreds {
+    vc.max_ledger_shreds.map(|max_ledger_shreds| {
         // x2 as we have data shred and coding shred.
-        Some(max_ledger_shreds) => max_ledger_shreds * RESERVED_BYTES_PER_SHRED * 2,
-        None => MAX_ROCKS_FIFO_SHRED_STORAGE_SIZE_BYTES,
-    }
+        max_ledger_shreds * RESERVED_BYTES_PER_SHRED * 2
+    })
 }
 
 // This function is duplicated in ledger-tool/src/main.rs...
@@ -3055,11 +3053,11 @@ pub fn main() {
                     None => ShredStorageType::rocks_fifo(default_fifo_shred_storage_size(
                         &validator_config,
                     )),
-                    Some(_) => ShredStorageType::rocks_fifo(value_t_or_exit!(
+                    Some(_) => ShredStorageType::rocks_fifo(Some(value_t_or_exit!(
                         matches,
                         "rocksdb_fifo_shred_storage_size",
                         u64
-                    )),
+                    ))),
                 },
                 _ => panic!(
                     "Unrecognized rocksdb-shred-compaction: {}",