diff --git a/accounts-db/src/accounts_index.rs b/accounts-db/src/accounts_index.rs index 3b4ddbe3927e5d..6c0a1ec62c8f27 100644 --- a/accounts-db/src/accounts_index.rs +++ b/accounts-db/src/accounts_index.rs @@ -50,7 +50,7 @@ pub const ACCOUNTS_INDEX_CONFIG_FOR_TESTING: AccountsIndexConfig = AccountsIndex bins: Some(BINS_FOR_TESTING), flush_threads: Some(FLUSH_THREADS_TESTING), drives: None, - index_limit_mb: IndexLimitMb::Unspecified, + index_limit_mb: IndexLimitMb::Unlimited, ages_to_stay_in_cache: None, scan_results_limit_bytes: None, started_from_validator: false, @@ -59,7 +59,7 @@ pub const ACCOUNTS_INDEX_CONFIG_FOR_BENCHMARKS: AccountsIndexConfig = AccountsIn bins: Some(BINS_FOR_BENCHMARKS), flush_threads: Some(FLUSH_THREADS_TESTING), drives: None, - index_limit_mb: IndexLimitMb::Unspecified, + index_limit_mb: IndexLimitMb::Unlimited, ages_to_stay_in_cache: None, scan_results_limit_bytes: None, started_from_validator: false, @@ -189,22 +189,16 @@ pub struct AccountSecondaryIndexesIncludeExclude { } /// specification of how much memory in-mem portion of account index can use -#[derive(Debug, Clone)] +#[derive(Debug, Copy, Clone, Default)] pub enum IndexLimitMb { - /// nothing explicit specified, so default - Unspecified, - /// limit was specified, use disk index for rest - Limit(usize), + /// use disk index while allowing to use as much memory as available for + /// in-memory index. + #[default] + Unlimited, /// in-mem-only was specified, no disk index InMemOnly, } -impl Default for IndexLimitMb { - fn default() -> Self { - Self::Unspecified - } -} - #[derive(Debug, Default, Clone)] pub struct AccountsIndexConfig { pub bins: Option, @@ -2487,7 +2481,7 @@ pub mod tests { let mut config = ACCOUNTS_INDEX_CONFIG_FOR_TESTING; config.index_limit_mb = if use_disk { - IndexLimitMb::Limit(10_000) + IndexLimitMb::Unlimited } else { IndexLimitMb::InMemOnly // in-mem only }; diff --git a/accounts-db/src/accounts_index/in_mem_accounts_index.rs b/accounts-db/src/accounts_index/in_mem_accounts_index.rs index 051769ad9e55ad..15610713a69652 100644 --- a/accounts-db/src/accounts_index/in_mem_accounts_index.rs +++ b/accounts-db/src/accounts_index/in_mem_accounts_index.rs @@ -980,13 +980,6 @@ impl + Into> InMemAccountsIndex usize { - std::mem::size_of::() - + std::mem::size_of::() - + std::mem::size_of::>() - } - fn should_evict_based_on_age( current_age: Age, entry: &AccountMapEntry, @@ -1003,16 +996,12 @@ impl + Into> InMemAccountsIndex, startup: bool, update_stats: bool, - exceeds_budget: bool, ages_flushing_now: Age, ) -> (bool, Option>>) { // this could be tunable dynamically based on memory pressure // we could look at more ages or we could throw out more items we are choosing to keep in the cache if Self::should_evict_based_on_age(current_age, entry, startup, ages_flushing_now) { - if exceeds_budget { - // if we are already holding too many items in-mem, then we need to be more aggressive at kicking things out - (true, None) - } else if entry.ref_count() != 1 { + if entry.ref_count() != 1 { Self::update_stat(&self.stats().held_in_mem.ref_count, 1); (false, None) } else { @@ -1199,7 +1188,6 @@ impl + Into> InMemAccountsIndex + Into> InMemAccountsIndex + Into> InMemAccountsIndex bool { - let in_mem_count = self.stats().count_in_mem.load(Ordering::Relaxed); - let limit = self.storage.mem_budget_mb; - let estimate_mem = in_mem_count * Self::approx_size_of_one_entry(); - let exceeds_budget = limit - .map(|limit| estimate_mem >= limit * 1024 * 1024) - .unwrap_or_default(); - self.stats() - .estimate_mem - .store(estimate_mem as u64, Ordering::Relaxed); - exceeds_budget - } - /// for each key in 'keys', look up in map, set age to the future fn move_ages_to_future(&self, next_age: Age, current_age: Age, keys: &[Pubkey]) { let map = self.map_internal.read().unwrap(); @@ -1564,7 +1536,7 @@ impl Drop for EvictionsGuard<'_> { mod tests { use { super::*, - crate::accounts_index::{AccountsIndexConfig, IndexLimitMb, BINS_FOR_TESTING}, + crate::accounts_index::{AccountsIndexConfig, BINS_FOR_TESTING}, assert_matches::assert_matches, itertools::Itertools, }; @@ -1582,10 +1554,7 @@ mod tests { fn new_disk_buckets_for_test() -> InMemAccountsIndex { let holder = Arc::new(BucketMapHolder::new( BINS_FOR_TESTING, - &Some(AccountsIndexConfig { - index_limit_mb: IndexLimitMb::Limit(1), - ..AccountsIndexConfig::default() - }), + &Some(AccountsIndexConfig::default()), 1, )); let bin = 0; @@ -1615,7 +1584,6 @@ mod tests { &one_element_slot_list_entry, startup, false, - false, 1, ) .0, @@ -1695,23 +1663,6 @@ mod tests { AccountMapEntryMeta::default(), )); - // exceeded budget - assert!( - bucket - .should_evict_from_mem( - current_age, - &Arc::new(AccountMapEntryInner::new( - vec![], - ref_count, - AccountMapEntryMeta::default() - )), - startup, - false, - true, - 0, - ) - .0 - ); // empty slot list assert!( !bucket @@ -1724,7 +1675,6 @@ mod tests { )), startup, false, - false, 0, ) .0 @@ -1737,7 +1687,6 @@ mod tests { &one_element_slot_list_entry, startup, false, - false, 0, ) .0 @@ -1754,7 +1703,6 @@ mod tests { )), startup, false, - false, 0, ) .0 @@ -1774,7 +1722,6 @@ mod tests { )), startup, false, - false, 0, ) .0 @@ -1789,7 +1736,6 @@ mod tests { &one_element_slot_list_entry, startup, false, - false, 0, ) .0 @@ -1804,7 +1750,6 @@ mod tests { &one_element_slot_list_entry, startup, false, - false, 0, ) .0 @@ -1819,7 +1764,6 @@ mod tests { &one_element_slot_list_entry, startup, false, - false, 0, ) .0 diff --git a/accounts-db/src/bucket_map_holder.rs b/accounts-db/src/bucket_map_holder.rs index fb90b68d124702..8a6fb5028c1060 100644 --- a/accounts-db/src/bucket_map_holder.rs +++ b/accounts-db/src/bucket_map_holder.rs @@ -29,9 +29,6 @@ const _: () = assert!(std::mem::size_of::() == std::mem::size_of:: = Some(10_000); - pub struct BucketMapHolder + Into> { pub disk: Option>, @@ -59,10 +56,6 @@ pub struct BucketMapHolder + Into> pub threads: usize, - // how much mb are we allowed to keep in the in-mem index? - // Rest goes to disk. - pub mem_budget_mb: Option, - /// how many ages should elapse from the last time an item is used where the item will remain in the cache pub ages_to_stay_in_cache: Age, @@ -217,43 +210,15 @@ impl + Into> BucketMapHolder config.drives.clone() }); - let mem_budget_mb = match config + let disk = match config .as_ref() - .map(|config| &config.index_limit_mb) - .unwrap_or(&IndexLimitMb::Unspecified) + .map(|config| config.index_limit_mb) + .unwrap_or_default() { - // creator said to use disk idx with a specific limit - IndexLimitMb::Limit(mb) => Some(*mb), - // creator said InMemOnly, so no disk index IndexLimitMb::InMemOnly => None, - // whatever started us didn't specify whether to use the acct idx - IndexLimitMb::Unspecified => { - // check env var if we were not started from a validator - let mut use_default = true; - if !config - .as_ref() - .map(|config| config.started_from_validator) - .unwrap_or_default() - { - if let Ok(_limit) = std::env::var("SOLANA_TEST_ACCOUNTS_INDEX_MEMORY_LIMIT_MB") - { - // Note this env var means the opposite of the default. The default now is disk index is on. - // So, if this env var is set, DO NOT allocate with disk buckets if mem budget was not set, we were NOT started from validator, and env var was set - // we do not want the env var to have an effect when running the validator (only tests, benches, etc.) - use_default = false; - } - } - if use_default { - // if validator does not specify disk index limit or specify in mem only, then this is the default - DEFAULT_DISK_INDEX - } else { - None - } - } + IndexLimitMb::Unlimited => Some(BucketMap::new(bucket_config)), }; - // only allocate if mem_budget_mb is Some - let disk = mem_budget_mb.map(|_| BucketMap::new(bucket_config)); Self { disk, ages_to_stay_in_cache, @@ -270,7 +235,7 @@ impl + Into> BucketMapHolder age_timer: AtomicInterval::default(), bins, startup: AtomicBool::default(), - mem_budget_mb, + threads, _phantom: PhantomData, startup_stats: Arc::default(), @@ -512,10 +477,7 @@ pub mod tests { #[test] fn test_disk_index_enabled() { let bins = 1; - let config = AccountsIndexConfig { - index_limit_mb: IndexLimitMb::Limit(0), - ..AccountsIndexConfig::default() - }; + let config = AccountsIndexConfig::default(); let test = BucketMapHolder::::new(bins, &Some(config), 1); assert!(test.is_disk_index_enabled()); } diff --git a/ledger-tool/src/args.rs b/ledger-tool/src/args.rs index 131bc91e9a7914..510cbab05dba84 100644 --- a/ledger-tool/src/args.rs +++ b/ledger-tool/src/args.rs @@ -59,22 +59,12 @@ pub fn accounts_db_args<'a, 'b>() -> Box<[Arg<'a, 'b>]> { .validator(is_pow2) .takes_value(true) .help("Number of bins to divide the accounts index into"), - Arg::with_name("accounts_index_memory_limit_mb") - .long("accounts-index-memory-limit-mb") - .value_name("MEGABYTES") - .validator(is_parsable::) - .takes_value(true) - .help( - "How much memory the accounts index can consume. If this is exceeded, some \ - account index entries will be stored on disk.", - ), Arg::with_name("disable_accounts_disk_index") .long("disable-accounts-disk-index") .help( "Disable the disk-based accounts index. It is enabled by default. The entire \ accounts index will be kept in memory.", - ) - .conflicts_with("accounts_index_memory_limit_mb"), + ), Arg::with_name("accounts_db_skip_shrink") .long("accounts-db-skip-shrink") .help( @@ -241,14 +231,11 @@ pub fn get_accounts_db_config( let ledger_tool_ledger_path = ledger_path.join(LEDGER_TOOL_DIRECTORY); let accounts_index_bins = value_t!(arg_matches, "accounts_index_bins", usize).ok(); - let accounts_index_index_limit_mb = - if let Ok(limit) = value_t!(arg_matches, "accounts_index_memory_limit_mb", usize) { - IndexLimitMb::Limit(limit) - } else if arg_matches.is_present("disable_accounts_disk_index") { - IndexLimitMb::InMemOnly - } else { - IndexLimitMb::Unspecified - }; + let accounts_index_index_limit_mb = if arg_matches.is_present("disable_accounts_disk_index") { + IndexLimitMb::InMemOnly + } else { + IndexLimitMb::Unlimited + }; let accounts_index_drives = values_t!(arg_matches, "accounts_index_path", String) .ok() .map(|drives| drives.into_iter().map(PathBuf::from).collect()) diff --git a/validator/src/cli.rs b/validator/src/cli.rs index 60cdc3044748f6..440cf155d8d892 100644 --- a/validator/src/cli.rs +++ b/validator/src/cli.rs @@ -1400,17 +1400,6 @@ pub fn app<'a>(version: &'a str, default_args: &'a DefaultArgs) -> App<'a, 'a> { this is exceeded, the scan aborts.", ), ) - .arg( - Arg::with_name("accounts_index_memory_limit_mb") - .long("accounts-index-memory-limit-mb") - .value_name("MEGABYTES") - .validator(is_parsable::) - .takes_value(true) - .help( - "How much memory the accounts index can consume. If this is exceeded, some \ - account index entries will be stored on disk.", - ), - ) .arg( Arg::with_name("accounts_index_bins") .long("accounts-index-bins") @@ -2009,6 +1998,18 @@ fn deprecated_arguments() -> Vec { Ok(()) } })); + // deprecated in v2.1 by PR #2721 + add_arg!(Arg::with_name("accounts_index_memory_limit_mb") + .long("accounts-index-memory-limit-mb") + .value_name("MEGABYTES") + .validator(is_parsable::) + .takes_value(true) + .help( + "How much memory the accounts index can consume. If this is exceeded, some \ + account index entries will be stored on disk.", + ), + usage_warning: "index memory limit has been deprecated. The limit arg has no effect now.", + ); add_arg!(Arg::with_name("accountsdb_repl_bind_address") .long("accountsdb-repl-bind-address") .value_name("HOST") diff --git a/validator/src/main.rs b/validator/src/main.rs index b2fb6a7ac275b9..3738eabced96de 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -1187,14 +1187,11 @@ pub fn main() { TestPartitionedEpochRewards::None }; - accounts_index_config.index_limit_mb = - if let Ok(limit) = value_t!(matches, "accounts_index_memory_limit_mb", usize) { - IndexLimitMb::Limit(limit) - } else if matches.is_present("disable_accounts_disk_index") { - IndexLimitMb::InMemOnly - } else { - IndexLimitMb::Unspecified - }; + accounts_index_config.index_limit_mb = if matches.is_present("disable_accounts_disk_index") { + IndexLimitMb::InMemOnly + } else { + IndexLimitMb::Unlimited + }; { let mut accounts_index_paths: Vec = if matches.is_present("accounts_index_path") {