Removes CreateAncientStorage enum (#6126)

accounts-db/src/accounts_db.rs

@@ -23,6 +23,11 @@ mod scan_account_storage;
 pub mod stats;
 pub mod tests;
 
+#[cfg(test)]
+use crate::{
+    ancient_append_vecs::{AccountsToStore, StorageSelector},
+    u64_align,
+};
 #[cfg(feature = "dev-context-only-utils")]
 use qualifier_attr::qualifiers;
 use {
@@ -39,7 +44,7 @@ use {
         },
         accounts_file::{
             AccountsFile, AccountsFileError, AccountsFileProvider, MatchAccountOwnerError,
-            StorageAccess, ALIGN_BOUNDARY_OFFSET,
+            StorageAccess,
         },
         accounts_hash::{
             AccountHash, AccountLtHash, AccountsDeltaHash, AccountsHash, AccountsHashKind,
@@ -59,9 +64,7 @@ use {
         accounts_update_notifier_interface::AccountsUpdateNotifier,
         active_stats::{ActiveStatItem, ActiveStats},
         ancestors::Ancestors,
-        ancient_append_vecs::{
-            get_ancient_append_vec_capacity, is_ancient, AccountsToStore, StorageSelector,
-        },
+        ancient_append_vecs::get_ancient_append_vec_capacity,
         append_vec::{aligned_stored_size, StoredAccountMeta, STORE_META_OVERHEAD},
         cache_hash_data::{CacheHashData, DeletionPolicy as CacheHashDeletionPolicy},
         contains::Contains,
@@ -73,7 +76,7 @@ use {
         read_only_accounts_cache::ReadOnlyAccountsCache,
         sorted_storages::SortedStorages,
         storable_accounts::{StorableAccounts, StorableAccountsBySlot},
-        u64_align, utils,
+        utils,
         verify_accounts_hash_in_background::VerifyAccountsHashInBackground,
     },
     crossbeam_channel::{unbounded, Receiver, Sender, TryRecvError},
@@ -147,15 +150,6 @@ const SHRINK_COLLECT_CHUNK_SIZE: usize = 50;
 /// candidates for shrinking.
 const SHRINK_INSERT_ANCIENT_THRESHOLD: usize = 10;
 
-#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
-pub enum CreateAncientStorage {
-    /// ancient storages are created by appending
-    Append,
-    /// ancient storages are created by 1-shot write to pack multiple accounts together more efficiently with new formats
-    #[default]
-    Pack,
-}
-
 #[derive(Debug)]
 enum StoreTo<'a> {
     /// write to cache
@@ -337,11 +331,16 @@ pub enum StoreReclaims {
 ///
 /// If a caller uses it before initializing it, it will be a runtime unwrap() error, similar to an assert.
 /// That condition is an illegal use pattern and is justifiably an assertable condition.
+//
+// NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+#[cfg(test)]
 #[derive(Default)]
 struct CurrentAncientAccountsFile {
     slot_and_accounts_file: Option<(Slot, Arc<AccountStorageEntry>)>,
 }
 
+// NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+#[cfg(test)]
 impl CurrentAncientAccountsFile {
     fn new(slot: Slot, append_vec: Arc<AccountStorageEntry>) -> CurrentAncientAccountsFile {
         Self {
@@ -427,17 +426,20 @@ enum LoadZeroLamports {
     SomeWithZeroLamportAccountForTests,
 }
 
+#[cfg(test)]
 #[derive(Debug)]
 struct AncientSlotPubkeysInner {
     pubkeys: HashSet<Pubkey>,
     slot: Slot,
 }
 
+#[cfg(test)]
 #[derive(Debug, Default)]
 struct AncientSlotPubkeys {
     inner: Option<AncientSlotPubkeysInner>,
 }
 
+#[cfg(test)]
 impl AncientSlotPubkeys {
     /// All accounts in 'slot' will be moved to 'current_ancient'
     /// If 'slot' is different than the 'current_ancient'.slot, then an account in 'slot' may ALREADY be in the current ancient append vec.
@@ -509,7 +511,6 @@ pub const ACCOUNTS_DB_CONFIG_FOR_TESTING: AccountsDbConfig = AccountsDbConfig {
     max_ancient_storages: None,
     skip_initial_hash_calc: false,
     exhaustively_verify_refcounts: false,
-    create_ancient_storage: CreateAncientStorage::Pack,
     partitioned_epoch_rewards_config: DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG,
     test_skip_rewrites_but_include_in_bank_hash: false,
     storage_access: StorageAccess::File,
@@ -537,7 +538,6 @@ pub const ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS: AccountsDbConfig = AccountsDbConfig
     max_ancient_storages: None,
     skip_initial_hash_calc: false,
     exhaustively_verify_refcounts: false,
-    create_ancient_storage: CreateAncientStorage::Pack,
     partitioned_epoch_rewards_config: DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG,
     test_skip_rewrites_but_include_in_bank_hash: false,
     storage_access: StorageAccess::File,
@@ -669,8 +669,6 @@ pub struct AccountsDbConfig {
     pub test_skip_rewrites_but_include_in_bank_hash: bool,
     pub skip_initial_hash_calc: bool,
     pub exhaustively_verify_refcounts: bool,
-    /// how to create ancient storages
-    pub create_ancient_storage: CreateAncientStorage,
     pub partitioned_epoch_rewards_config: PartitionedEpochRewardsConfig,
     pub storage_access: StorageAccess,
     pub scan_filter_for_shrinking: ScanFilter,
@@ -1489,9 +1487,6 @@ pub struct AccountsDb {
 
     pub storage: AccountStorage,
 
-    /// from AccountsDbConfig
-    create_ancient_storage: CreateAncientStorage,
-
     /// true if this client should skip rewrites but still include those rewrites in the bank hash as if rewrites had occurred.
     pub test_skip_rewrites_but_include_in_bank_hash: bool,
 
@@ -2033,7 +2028,6 @@ impl AccountsDb {
             account_indexes: accounts_db_config.account_indexes.unwrap_or_default(),
             shrink_ratio: accounts_db_config.shrink_ratio,
             accounts_update_notifier,
-            create_ancient_storage: accounts_db_config.create_ancient_storage,
             read_only_accounts_cache: ReadOnlyAccountsCache::new(
                 read_cache_size.0,
                 read_cache_size.1,
@@ -4228,17 +4222,16 @@ impl AccountsDb {
         let oldest_non_ancient_slot = self.get_oldest_non_ancient_slot(epoch_schedule);
         let can_randomly_shrink = true;
         let sorted_slots = self.get_sorted_potential_ancient_slots(oldest_non_ancient_slot);
-        if self.create_ancient_storage == CreateAncientStorage::Append {
-            self.combine_ancient_slots(sorted_slots, can_randomly_shrink);
-        } else {
-            self.combine_ancient_slots_packed(sorted_slots, can_randomly_shrink);
-        }
+        self.combine_ancient_slots_packed(sorted_slots, can_randomly_shrink);
     }
 
     /// 'accounts' that exist in the current slot we are combining into a different ancient slot
     /// 'existing_ancient_pubkeys': pubkeys that exist currently in the ancient append vec slot
     /// returns the pubkeys that are in 'accounts' that are already in 'existing_ancient_pubkeys'
     /// Also updated 'existing_ancient_pubkeys' to include all pubkeys in 'accounts' since they will soon be written into the ancient slot.
+    //
+    // NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+    #[cfg(test)]
     fn get_keys_to_unref_ancient<'a>(
         accounts: &'a [&AccountFromStorage],
         existing_ancient_pubkeys: &mut HashSet<Pubkey>,
@@ -4260,6 +4253,9 @@ impl AccountsDb {
     /// 'accounts' are about to be appended to an ancient append vec. That ancient append vec may already have some accounts.
     /// Unref each account in 'accounts' that already exists in 'existing_ancient_pubkeys'.
     /// As a side effect, on exit, 'existing_ancient_pubkeys' will now contain all pubkeys in 'accounts'.
+    //
+    // NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+    #[cfg(test)]
     fn unref_accounts_already_in_storage(
         &self,
         accounts: &[&AccountFromStorage],
@@ -4277,6 +4273,7 @@ impl AccountsDb {
     /// get the storage from 'slot' to squash
     /// or None if this slot should be skipped
     /// side effect could be updating 'current_ancient'
+    #[cfg(test)]
     fn get_storage_to_move_to_ancient_accounts_file(
         &self,
         slot: Slot,
@@ -4302,6 +4299,9 @@ impl AccountsDb {
     /// can_randomly_shrink: true if ancient append vecs that otherwise don't qualify to be shrunk can be randomly shrunk
     ///  this is convenient for a running system
     ///  this is not useful for testing
+    //
+    // NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+    #[cfg(test)]
     fn should_move_to_ancient_accounts_file(
         &self,
         storage: &Arc<AccountStorageEntry>,
@@ -4364,6 +4364,9 @@ impl AccountsDb {
 
     /// Combine all account data from storages in 'sorted_slots' into ancient append vecs.
     /// This keeps us from accumulating append vecs for each slot older than an epoch.
+    //
+    // NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+    #[cfg(test)]
     fn combine_ancient_slots(&self, sorted_slots: Vec<Slot>, can_randomly_shrink: bool) {
         if sorted_slots.is_empty() {
             return;
@@ -4423,6 +4426,7 @@ impl AccountsDb {
     }
 
     /// put entire alive contents of 'old_storage' into the current ancient append vec or a newly created ancient append vec
+    #[cfg(test)]
     fn combine_one_store_into_ancient(
         &self,
         slot: Slot,
@@ -6876,27 +6880,12 @@ impl AccountsDb {
     /// If ancient append vecs are not enabled, return 0.
     fn get_oldest_non_ancient_slot_for_hash_calc_scan(
         &self,
-        max_slot_inclusive: Slot,
-        config: &CalcAccountsHashConfig<'_>,
+        _max_slot_inclusive: Slot,
+        _config: &CalcAccountsHashConfig<'_>,
     ) -> Option<Slot> {
-        if self.create_ancient_storage == CreateAncientStorage::Pack {
-            // oldest_non_ancient_slot is only applicable when ancient storages are created with `Append`. When ancient storages are created with `Pack`, ancient storages
-            // can be created in between non-ancient storages. Return None, because oldest_non_ancient_slot is not applicable here.
-            None
-        } else if self.ancient_append_vec_offset.is_some() {
-            // For performance, this is required when ancient appendvecs are enabled
-            Some(
-                self.get_oldest_non_ancient_slot_from_slot(
-                    config.epoch_schedule,
-                    max_slot_inclusive,
-                ),
-            )
-        } else {
-            // This causes the entire range to be chunked together, treating older append vecs just like new ones.
-            // This performs well if there are many old append vecs that haven't been cleaned yet.
-            // 0 will have the effect of causing ALL older append vecs to be chunked together, just like every other append vec.
-            Some(0)
-        }
+        // oldest_non_ancient_slot is only applicable when ancient storages are created with `Append`. When ancient storages are created with `Pack`, ancient storages
+        // can be created in between non-ancient storages. Return None, because oldest_non_ancient_slot is not applicable here.
+        None
     }
 
     /// hash info about 'storage' into 'hasher'
@@ -7737,14 +7726,7 @@ impl AccountsDb {
     pub(crate) fn is_candidate_for_shrink(&self, store: &AccountStorageEntry) -> bool {
         // appended ancient append vecs should not be shrunk by the normal shrink codepath.
         // It is not possible to identify ancient append vecs when we pack, so no check for ancient when we are not appending.
-        let total_bytes = if self.create_ancient_storage == CreateAncientStorage::Append
-            && is_ancient(&store.accounts)
-            && store.accounts.can_append()
-        {
-            store.written_bytes()
-        } else {
-            store.capacity()
-        };
+        let total_bytes = store.capacity();
 
         let alive_bytes = store.alive_bytes_exclude_zero_lamport_single_ref_accounts() as u64;
         match self.shrink_ratio {

accounts-db/src/accounts_db/tests.rs

@@ -6,7 +6,7 @@ use {
         accounts_file::AccountsFileProvider,
         accounts_hash::MERKLE_FANOUT,
         accounts_index::{tests::*, AccountIndex, AccountSecondaryIndexesIncludeExclude},
-        ancient_append_vecs,
+        ancient_append_vecs::{self, is_ancient},
         append_vec::{
             aligned_stored_size, test_utils::TempFile, AccountMeta, AppendVec, StoredAccountMeta,
             StoredMeta,
@@ -6057,21 +6057,9 @@ define_accounts_db_test!(test_many_unrefs, |db| {
     assert_eq!(db.accounts_index.ref_count_from_storage(&pk1), 0);
 });
 
-#[test_case(CreateAncientStorage::Append; "append")]
-#[test_case(CreateAncientStorage::Pack; "pack")]
-fn test_get_oldest_non_ancient_slot_for_hash_calc_scan(
-    create_ancient_storage: CreateAncientStorage,
-) {
-    let expected = |v| {
-        if create_ancient_storage == CreateAncientStorage::Append {
-            Some(v)
-        } else {
-            None
-        }
-    };
-
+#[test]
+fn test_get_oldest_non_ancient_slot_for_hash_calc_scan() {
     let mut db = AccountsDb::new_single_for_tests();
-    db.create_ancient_storage = create_ancient_storage;
 
     let config = CalcAccountsHashConfig::default();
     let slot = config.epoch_schedule.slots_per_epoch;
@@ -6080,23 +6068,23 @@ fn test_get_oldest_non_ancient_slot_for_hash_calc_scan(
     let offset = 10;
     assert_eq!(
         db.get_oldest_non_ancient_slot_for_hash_calc_scan(slots_per_epoch + offset, &config),
-        expected(db.ancient_append_vec_offset.unwrap() as u64 + offset + 1)
+        None,
     );
     // ancient append vecs enabled (but at 0 offset), so can be non-zero
     db.ancient_append_vec_offset = Some(0);
     // 0..=(slots_per_epoch - 1) are all non-ancient
     assert_eq!(
         db.get_oldest_non_ancient_slot_for_hash_calc_scan(slots_per_epoch - 1, &config),
-        expected(0)
+        None,
     );
     // 1..=slots_per_epoch are all non-ancient, so 1 is oldest non ancient
     assert_eq!(
         db.get_oldest_non_ancient_slot_for_hash_calc_scan(slots_per_epoch, &config),
-        expected(1)
+        None,
     );
     assert_eq!(
         db.get_oldest_non_ancient_slot_for_hash_calc_scan(slots_per_epoch + offset, &config),
-        expected(offset + 1)
+        None,
     );
 }
 

accounts-db/src/accounts_file.rs

@@ -28,7 +28,8 @@ pub const ALIGN_BOUNDARY_OFFSET: usize = mem::size_of::<u64>();
 #[macro_export]
 macro_rules! u64_align {
     ($addr: expr) => {
-        ($addr + (ALIGN_BOUNDARY_OFFSET - 1)) & !(ALIGN_BOUNDARY_OFFSET - 1)
+        ($addr + ($crate::accounts_file::ALIGN_BOUNDARY_OFFSET - 1))
+            & !($crate::accounts_file::ALIGN_BOUNDARY_OFFSET - 1)
     };
 }
 
@@ -88,6 +89,9 @@ impl AccountsFile {
     }
 
     /// true if this storage can possibly be appended to (independent of capacity check)
+    //
+    // NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+    #[cfg(test)]
     pub(crate) fn can_append(&self) -> bool {
         match self {
             Self::AppendVec(av) => av.can_append(),

accounts-db/src/ancient_append_vecs.rs

@@ -4,6 +4,8 @@
 //! 2. multiple 'slots' squashed into a single older (ie. ancient) slot for convenience and performance
 //!
 //! Otherwise, an ancient append vec is the same as any other append vec
+#[cfg(test)]
+use crate::accounts_file::AccountsFile;
 use {
     crate::{
         account_storage::ShrinkInProgress,
@@ -12,7 +14,6 @@ use {
             AccountFromStorage, AccountStorageEntry, AccountsDb, AliveAccounts,
             GetUniqueAccountsResult, ShrinkCollect, ShrinkCollectAliveSeparatedByRefs,
         },
-        accounts_file::AccountsFile,
         active_stats::ActiveStatItem,
         storable_accounts::{StorableAccounts, StorableAccountsBySlot},
     },
@@ -1073,6 +1074,9 @@ impl<'a> PackedAncientStorage<'a> {
 
 /// a set of accounts need to be stored.
 /// If there are too many to fit in 'Primary', the rest are put in 'Overflow'
+//
+// NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+#[cfg(test)]
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum StorageSelector {
     Primary,
@@ -1084,6 +1088,9 @@ pub enum StorageSelector {
 /// The 'store' functions need data stored in a slice of specific type.
 /// We need 1-2 of these slices constructed based on available bytes and individual account sizes.
 /// The slice arithmetic across both hashes and account data gets messy. So, this struct abstracts that.
+//
+// NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+#[cfg(test)]
 pub struct AccountsToStore<'a> {
     accounts: &'a [&'a AccountFromStorage],
     /// if 'accounts' contains more items than can be contained in the primary storage, then we have to split these accounts.
@@ -1096,6 +1103,8 @@ pub struct AccountsToStore<'a> {
     bytes_overflow: usize,
 }
 
+// NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+#[cfg(test)]
 impl<'a> AccountsToStore<'a> {
     /// break 'stored_accounts' into primary and overflow
     /// available_bytes: how many bytes remain in the primary storage. Excess accounts will be directed to an overflow storage
@@ -1187,6 +1196,9 @@ pub const fn get_ancient_append_vec_capacity() -> u64 {
 }
 
 /// is this a max-size append vec designed to be used as an ancient append vec?
+//
+// NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+#[cfg(test)]
 pub fn is_ancient(storage: &AccountsFile) -> bool {
     storage.capacity() >= get_ancient_append_vec_capacity()
 }

accounts-db/src/append_vec.rs

@@ -21,7 +21,7 @@ use {
         account_storage::stored_account_info::{StoredAccountInfo, StoredAccountInfoWithoutData},
         accounts_file::{
             AccountsFileError, InternalsForArchive, MatchAccountOwnerError, Result, StorageAccess,
-            StoredAccountsInfo, ALIGN_BOUNDARY_OFFSET,
+            StoredAccountsInfo,
         },
         accounts_hash::AccountHash,
         buffered_reader::{BufferedReader, BufferedReaderStatus, Stack},
@@ -1312,6 +1312,8 @@ impl AppendVec {
         })
     }
 
+    // NOTE: Only used by ancient append vecs "append" method, which is test-only now.
+    #[cfg(test)]
     pub(crate) fn can_append(&self) -> bool {
         match &self.backing {
             AppendVecFileBacking::File(_file) => false,

accounts-db/src/tiered_storage/mmap_utils.rs

@@ -1,9 +1,4 @@
-use {
-    crate::{accounts_file::ALIGN_BOUNDARY_OFFSET, u64_align},
-    log::*,
-    memmap2::Mmap,
-    std::io::Result as IoResult,
-};
+use {crate::u64_align, log::*, memmap2::Mmap, std::io::Result as IoResult};
 
 /// Borrows a value of type `T` from `mmap`
 ///

ledger-tool/src/args.rs

@@ -2,7 +2,7 @@ use {
     crate::LEDGER_TOOL_DIRECTORY,
     clap::{value_t, value_t_or_exit, values_t, values_t_or_exit, Arg, ArgMatches},
     solana_accounts_db::{
-        accounts_db::{AccountsDb, AccountsDbConfig, CreateAncientStorage},
+        accounts_db::{AccountsDb, AccountsDbConfig},
         accounts_file::StorageAccess,
         accounts_index::{AccountsIndexConfig, IndexLimitMb, ScanFilter},
         utils::create_and_canonicalize_directories,
@@ -358,7 +358,6 @@ pub fn get_accounts_db_config(
         exhaustively_verify_refcounts: arg_matches.is_present("accounts_db_verify_refcounts"),
         skip_initial_hash_calc: arg_matches.is_present("accounts_db_skip_initial_hash_calculation"),
         test_skip_rewrites_but_include_in_bank_hash: false,
-        create_ancient_storage: CreateAncientStorage::Pack,
         storage_access,
         scan_filter_for_shrinking,
         enable_experimental_accumulator_hash: !arg_matches

validator/src/commands/run/execute.rs

@@ -10,7 +10,7 @@ use {
     log::*,
     rand::{seq::SliceRandom, thread_rng},
     solana_accounts_db::{
-        accounts_db::{AccountShrinkThreshold, AccountsDb, AccountsDbConfig, CreateAncientStorage},
+        accounts_db::{AccountShrinkThreshold, AccountsDb, AccountsDbConfig},
         accounts_file::StorageAccess,
         accounts_index::{
             AccountIndex, AccountSecondaryIndexes, AccountSecondaryIndexesIncludeExclude,
@@ -521,7 +521,6 @@ pub fn execute(
         )
         .ok(),
         exhaustively_verify_refcounts: matches.is_present("accounts_db_verify_refcounts"),
-        create_ancient_storage: CreateAncientStorage::Pack,
         test_skip_rewrites_but_include_in_bank_hash: false,
         storage_access,
         scan_filter_for_shrinking,