From 8b32e80ee2170f56b1e494ba6289b85105abae29 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Tue, 22 Feb 2022 09:40:25 -0600 Subject: [PATCH] DiskIdx: rename 'remove' to 'evict' (#23188) --- runtime/src/in_mem_accounts_index.rs | 114 ++++++++++++++------------- 1 file changed, 59 insertions(+), 55 deletions(-) diff --git a/runtime/src/in_mem_accounts_index.rs b/runtime/src/in_mem_accounts_index.rs index 7d3b638206..1245143fc7 100644 --- a/runtime/src/in_mem_accounts_index.rs +++ b/runtime/src/in_mem_accounts_index.rs @@ -44,10 +44,10 @@ pub struct InMemAccountsIndex { // pubkey ranges that this bin must hold in the cache while the range is present in this vec pub(crate) cache_ranges_held: CacheRangesHeld, - // incremented each time stop_flush is changed - stop_flush_changes: AtomicU64, + // incremented each time stop_evictions is changed + stop_evictions_changes: AtomicU64, // true while ranges are being manipulated. Used to keep an async flush from removing things while a range is being held. - stop_flush: AtomicU64, + stop_evictions: AtomicU64, // set to true when any entry in this bin is marked dirty bin_dirty: AtomicBool, // set to true while this bin is being actively flushed @@ -79,8 +79,8 @@ impl InMemAccountsIndex { .map(|disk| disk.get_bucket_from_index(bin)) .map(Arc::clone), cache_ranges_held: CacheRangesHeld::default(), - stop_flush_changes: AtomicU64::default(), - stop_flush: AtomicU64::default(), + stop_evictions_changes: AtomicU64::default(), + stop_evictions: AtomicU64::default(), bin_dirty: AtomicBool::default(), flushing_active: AtomicBool::default(), // initialize this to max, to make it clear we have not flushed at age 0, the starting age @@ -142,10 +142,10 @@ impl InMemAccountsIndex { pub fn keys(&self) -> Vec { Self::update_stat(&self.stats().keys, 1); // easiest implementation is to load evrything from disk into cache and return the keys - self.start_stop_flush(true); + self.start_stop_evictions(true); self.put_range_in_cache(&None::<&RangeInclusive>); let keys = self.map().read().unwrap().keys().cloned().collect(); - self.start_stop_flush(false); + self.start_stop_evictions(false); keys } @@ -720,17 +720,17 @@ impl InMemAccountsIndex { already_held } - /// called with 'stop'=true to stop bg flusher from removing any entries from in-mem idx - /// called with 'stop'=false to allow bg flusher to remove eligible (not in held ranges) entries from in-mem idx - fn start_stop_flush(&self, stop: bool) { + /// called with 'stop'=true to stop bg flusher from evicting any entries from in-mem idx + /// called with 'stop'=false to allow bg flusher to evict eligible (not in held ranges) entries from in-mem idx + fn start_stop_evictions(&self, stop: bool) { if stop { - self.stop_flush.fetch_add(1, Ordering::Release); - } else if 1 == self.stop_flush.fetch_sub(1, Ordering::Release) { - // stop_flush went to 0, so this bucket could now be ready to be aged + self.stop_evictions.fetch_add(1, Ordering::Release); + } else if 1 == self.stop_evictions.fetch_sub(1, Ordering::Release) { + // stop_evictions went to 0, so this bucket could now be ready to be aged self.storage.wait_dirty_or_aged.notify_one(); } // note that this value has changed - self.stop_flush_changes.fetch_add(1, Ordering::Release); + self.stop_evictions_changes.fetch_add(1, Ordering::Release); } /// if 'start_holding'=true, then: @@ -744,7 +744,7 @@ impl InMemAccountsIndex { where R: RangeBounds + Debug, { - self.start_stop_flush(true); + self.start_stop_evictions(true); if !start_holding || !self.add_hold_range_in_memory_if_already_held(range) { if start_holding { @@ -755,14 +755,14 @@ impl InMemAccountsIndex { self.just_set_hold_range_in_memory(range, start_holding); } - self.start_stop_flush(false); + self.start_stop_evictions(false); } fn put_range_in_cache(&self, range: &Option<&R>) where R: RangeBounds, { - assert!(self.get_stop_flush()); // caller should be controlling the lifetime of how long this needs to be present + assert!(self.get_stop_evictions()); // caller should be controlling the lifetime of how long this needs to be present let m = Measure::start("range"); let mut added_to_mem = 0; @@ -791,12 +791,14 @@ impl InMemAccountsIndex { Self::update_time_stat(&self.stats().get_range_us, m); } - fn get_stop_flush(&self) -> bool { - self.stop_flush.load(Ordering::Acquire) > 0 + /// returns true if there are active requests to stop evictions + fn get_stop_evictions(&self) -> bool { + self.stop_evictions.load(Ordering::Acquire) > 0 } - fn get_stop_flush_changes(&self) -> u64 { - self.stop_flush_changes.load(Ordering::Acquire) + /// return count of calls to 'start_stop_evictions', indicating changes could have been made to eviction strategy + fn get_stop_evictions_changes(&self) -> u64 { + self.stop_evictions_changes.load(Ordering::Acquire) } pub(crate) fn flush(&self) { @@ -817,6 +819,8 @@ impl InMemAccountsIndex { self.storage.wait_dirty_or_aged.notify_one(); } + /// returns true if a dice roll indicates this call should result in a random eviction. + /// This causes non-determinism in cache contents per validator. fn random_chance_of_eviction() -> bool { // random eviction const N: usize = 1000; @@ -831,8 +835,8 @@ impl InMemAccountsIndex { + std::mem::size_of::>() } - /// return true if 'entry' should be removed from the in-mem index - fn should_remove_from_mem( + /// return true if 'entry' should be evicted from the in-mem index + fn should_evict_from_mem( &self, current_age: Age, entry: &AccountMapEntry, @@ -856,11 +860,11 @@ impl InMemAccountsIndex { false // keep 0 and > 1 slot lists in mem. They will be cleaned or shrunk soon. } else { // keep items with slot lists that contained cached items - let remove = !slot_list.iter().any(|(_, info)| info.is_cached()); - if !remove && update_stats { + let evict = !slot_list.iter().any(|(_, info)| info.is_cached()); + if !evict && update_stats { Self::update_stat(&self.stats().held_in_mem_slot_list_cached, 1); } - remove + evict } } } else { @@ -887,8 +891,8 @@ impl InMemAccountsIndex { // may have to loop if disk has to grow and we have to restart loop { - let mut removes; - let mut removes_random = Vec::default(); + let mut evictions; + let mut evictions_random = Vec::default(); let disk = self.bucket.as_ref().unwrap(); let mut flush_entries_updated_on_disk = 0; @@ -897,13 +901,13 @@ impl InMemAccountsIndex { // holds read lock { let map = self.map().read().unwrap(); - removes = Vec::with_capacity(map.len()); + evictions = Vec::with_capacity(map.len()); let m = Measure::start("flush_scan_and_update"); // we don't care about lock time in this metric - bg threads can wait for (k, v) in map.iter() { - if self.should_remove_from_mem(current_age, v, startup, true, exceeds_budget) { - removes.push(*k); + if self.should_evict_from_mem(current_age, v, startup, true, exceeds_budget) { + evictions.push(*k); } else if Self::random_chance_of_eviction() { - removes_random.push(*k); + evictions_random.push(*k); } else { // not planning to remove this item from memory now, so don't write it to disk yet continue; @@ -936,17 +940,17 @@ impl InMemAccountsIndex { flush_entries_updated_on_disk, ); - let m = Measure::start("flush_remove_or_grow"); + let m = Measure::start("flush_evict_or_grow"); match disk_resize { Ok(_) => { - if !self.flush_remove_from_cache( - removes, + if !self.evict_from_cache( + evictions, current_age, startup, false, exceeds_budget, - ) || !self.flush_remove_from_cache( - removes_random, + ) || !self.evict_from_cache( + evictions_random, current_age, startup, true, @@ -973,23 +977,23 @@ impl InMemAccountsIndex { } } - // remove keys in 'removes' from in-mem cache due to age + // remove keys in 'evictions' from in-mem cache, likely due to age // return true if the removal was completed - fn flush_remove_from_cache( + fn evict_from_cache( &self, - removes: Vec, + evictions: Vec, current_age: Age, startup: bool, randomly_evicted: bool, exceeds_budget: bool, ) -> bool { let mut completed_scan = true; - if removes.is_empty() { + if evictions.is_empty() { return completed_scan; // completed, don't need to get lock or do other work } - let stop_flush_changes_at_start = self.get_stop_flush_changes(); - if self.get_stop_flush() { + let stop_evictions_changes_at_start = self.get_stop_evictions_changes(); + if self.get_stop_evictions() { return false; // did NOT complete, ranges were changed, so have to restart } let ranges = self.cache_ranges_held.read().unwrap().clone(); @@ -997,7 +1001,7 @@ impl InMemAccountsIndex { let mut removed = 0; // consider chunking these so we don't hold the write lock too long let mut map = self.map().write().unwrap(); - for k in removes { + for k in evictions { if let Entry::Occupied(occupied) = map.entry(k) { let v = occupied.get(); if Arc::strong_count(v) > 1 { @@ -1008,7 +1012,7 @@ impl InMemAccountsIndex { if v.dirty() || (!randomly_evicted - && !self.should_remove_from_mem( + && !self.should_evict_from_mem( current_age, v, startup, @@ -1028,7 +1032,7 @@ impl InMemAccountsIndex { continue; } - if stop_flush_changes_at_start != self.get_stop_flush_changes() { + if stop_evictions_changes_at_start != self.get_stop_evictions_changes() { return false; // did NOT complete, ranges were changed, so have to restart } @@ -1098,7 +1102,7 @@ mod tests { } #[test] - fn test_should_remove_from_mem() { + fn test_should_evict_from_mem() { solana_logger::setup(); let bucket = new_for_test::(); let mut startup = false; @@ -1112,7 +1116,7 @@ mod tests { )); // exceeded budget - assert!(bucket.should_remove_from_mem( + assert!(bucket.should_evict_from_mem( current_age, &Arc::new(AccountMapEntryInner::new( vec![], @@ -1124,7 +1128,7 @@ mod tests { true, )); // empty slot list - assert!(!bucket.should_remove_from_mem( + assert!(!bucket.should_evict_from_mem( current_age, &Arc::new(AccountMapEntryInner::new( vec![], @@ -1136,7 +1140,7 @@ mod tests { false, )); // 1 element slot list - assert!(bucket.should_remove_from_mem( + assert!(bucket.should_evict_from_mem( current_age, &one_element_slot_list_entry, startup, @@ -1144,7 +1148,7 @@ mod tests { false, )); // 2 element slot list - assert!(!bucket.should_remove_from_mem( + assert!(!bucket.should_evict_from_mem( current_age, &Arc::new(AccountMapEntryInner::new( vec![(0, 0), (1, 1)], @@ -1159,7 +1163,7 @@ mod tests { { let bucket = new_for_test::(); // 1 element slot list with a CACHED item - f64 acts like cached - assert!(!bucket.should_remove_from_mem( + assert!(!bucket.should_evict_from_mem( current_age, &Arc::new(AccountMapEntryInner::new( vec![(0, 0.0)], @@ -1173,7 +1177,7 @@ mod tests { } // 1 element slot list, age is now - assert!(bucket.should_remove_from_mem( + assert!(bucket.should_evict_from_mem( current_age, &one_element_slot_list_entry, startup, @@ -1183,7 +1187,7 @@ mod tests { // 1 element slot list, but not current age current_age = 1; - assert!(!bucket.should_remove_from_mem( + assert!(!bucket.should_evict_from_mem( current_age, &one_element_slot_list_entry, startup, @@ -1193,7 +1197,7 @@ mod tests { // 1 element slot list, but at startup and age not current startup = true; - assert!(bucket.should_remove_from_mem( + assert!(bucket.should_evict_from_mem( current_age, &one_element_slot_list_entry, startup,