Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ShrinkCandidates only holds slot #33173

Merged
merged 1 commit into from
Sep 7, 2023
Merged

ShrinkCandidates only holds slot #33173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 59 additions & 86 deletions accounts-db/src/accounts_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ type AccountSlots = HashMap<Pubkey, HashSet<Slot>>;
type SlotOffsets = HashMap<Slot, HashSet<usize>>;
type ReclaimResult = (AccountSlots, SlotOffsets);
type PubkeysRemovedFromAccountsIndex = HashSet<Pubkey>;
type ShrinkCandidates = HashMap<Slot, Arc<AccountStorageEntry>>;
type ShrinkCandidates = HashSet<Slot>;

trait Versioned {
fn version(&self) -> u64;
Expand Down Expand Up @@ -2481,7 +2481,7 @@ impl AccountsDb {
recycle_stores: RwLock::new(RecycleStores::default()),
uncleaned_pubkeys: DashMap::new(),
next_id: AtomicAppendVecId::new(0),
shrink_candidate_slots: Mutex::new(HashMap::new()),
shrink_candidate_slots: Mutex::new(ShrinkCandidates::new()),
write_cache_limit_bytes: None,
write_version: AtomicU64::new(0),
paths: vec![],
Expand Down Expand Up @@ -4268,6 +4268,7 @@ impl AccountsDb {
/// achieved, it will stop and return the filtered-down candidates and the candidates which
/// are skipped in this round and might be eligible for the future shrink.
fn select_candidates_by_total_usage(
&self,
shrink_slots: &ShrinkCandidates,
shrink_ratio: f64,
oldest_non_ancient_slot: Option<Slot>,
Expand All @@ -4283,14 +4284,17 @@ impl AccountsDb {
let mut candidates_count: usize = 0;
let mut total_bytes: u64 = 0;
let mut total_candidate_stores: usize = 0;
for (slot, store) in shrink_slots {
for slot in shrink_slots {
if oldest_non_ancient_slot
.map(|oldest_non_ancient_slot| slot < &oldest_non_ancient_slot)
.unwrap_or_default()
{
// this slot will be 'shrunk' by ancient code
continue;
}
let Some(store) = self.storage.get_slot_storage_entry(*slot) else {
continue;
};
Comment on lines +4295 to +4297
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

candidates_count += 1;
total_alive_bytes += Self::page_align(store.alive_bytes() as u64);
total_bytes += store.capacity();
Expand Down Expand Up @@ -4326,7 +4330,7 @@ impl AccountsDb {
usage.slot, total_alive_bytes, total_bytes, alive_ratio, shrink_ratio
);
if usage.alive_ratio < shrink_ratio {
shrink_slots_next_batch.insert(usage.slot, store.clone());
shrink_slots_next_batch.insert(usage.slot);
} else {
break;
}
Expand All @@ -4335,7 +4339,7 @@ impl AccountsDb {
let after_shrink_size = Self::page_align(store.alive_bytes() as u64);
let bytes_saved = current_store_size.saturating_sub(after_shrink_size);
total_bytes -= bytes_saved;
shrink_slots.insert(usage.slot, store.clone());
shrink_slots.insert(usage.slot);
}
}
measure.stop();
Expand Down Expand Up @@ -4743,8 +4747,8 @@ impl AccountsDb {

let (shrink_slots, shrink_slots_next_batch) = {
if let AccountShrinkThreshold::TotalSpace { shrink_ratio } = self.shrink_ratio {
let (shrink_slots, shrink_slots_next_batch) =
Self::select_candidates_by_total_usage(
let (shrink_slots, shrink_slots_next_batch) = self
.select_candidates_by_total_usage(
&shrink_candidates_slots,
shrink_ratio,
self.ancient_append_vec_offset
Expand All @@ -4771,14 +4775,14 @@ impl AccountsDb {
let num_candidates = shrink_slots.len();
let shrink_candidates_count = shrink_slots.len();
self.thread_pool_clean.install(|| {
shrink_slots
.into_par_iter()
.for_each(|(slot, slot_shrink_candidate)| {
let mut measure = Measure::start("shrink_candidate_slots-ms");
shrink_slots.into_par_iter().for_each(|slot| {
let mut measure = Measure::start("shrink_candidate_slots-ms");
if let Some(slot_shrink_candidate) = self.storage.get_slot_storage_entry(slot) {
brooksprumo marked this conversation as resolved.
Show resolved Hide resolved
self.do_shrink_slot_store(slot, &slot_shrink_candidate);
measure.stop();
inc_new_counter_info!("shrink_candidate_slots-ms", measure.as_ms() as usize);
});
}
measure.stop();
inc_new_counter_info!("shrink_candidate_slots-ms", measure.as_ms() as usize);
});
});
measure_shrink_all_candidates.stop();
inc_new_counter_info!(
Expand All @@ -4790,8 +4794,8 @@ impl AccountsDb {
if let Some(shrink_slots_next_batch) = shrink_slots_next_batch {
let mut shrink_slots = self.shrink_candidate_slots.lock().unwrap();
pended_counts += shrink_slots_next_batch.len();
for (slot, store) in shrink_slots_next_batch {
shrink_slots.insert(slot, store);
for slot in shrink_slots_next_batch {
shrink_slots.insert(slot);
}
}
inc_new_counter_info!("shrink_pended_stores-count", pended_counts);
Expand Down Expand Up @@ -8199,7 +8203,7 @@ impl AccountsDb {
// because slots should only have one storage entry, namely the one that was
// created by `flush_slot_cache()`.
{
new_shrink_candidates.insert(*slot, store);
new_shrink_candidates.insert(*slot);
}
}
}
Expand All @@ -8211,18 +8215,8 @@ impl AccountsDb {

let mut measure = Measure::start("shrink");
let mut shrink_candidate_slots = self.shrink_candidate_slots.lock().unwrap();
for (slot, store) in new_shrink_candidates {
debug!(
"adding: {} {} to shrink candidates: count: {}/{} bytes: {}/{}",
store.append_vec_id(),
slot,
store.approx_stored_count(),
store.count(),
store.alive_bytes(),
store.capacity()
);

shrink_candidate_slots.insert(slot, store);
for slot in new_shrink_candidates {
shrink_candidate_slots.insert(slot);
measure.stop();
self.clean_accounts_stats
.remove_dead_accounts_shrink_us
Expand Down Expand Up @@ -13250,13 +13244,11 @@ pub mod tests {
fn test_select_candidates_by_total_usage_no_candidates() {
// no input candidates -- none should be selected
solana_logger::setup();
let candidates = ShrinkCandidates::new();
let candidates: ShrinkCandidates = ShrinkCandidates::new();
let db = AccountsDb::new_single_for_tests();

let (selected_candidates, next_candidates) = AccountsDb::select_candidates_by_total_usage(
&candidates,
DEFAULT_ACCOUNTS_SHRINK_RATIO,
None,
);
let (selected_candidates, next_candidates) =
db.select_candidates_by_total_usage(&candidates, DEFAULT_ACCOUNTS_SHRINK_RATIO, None);

assert_eq!(0, selected_candidates.len());
assert_eq!(0, next_candidates.len());
Expand All @@ -13267,6 +13259,7 @@ pub mod tests {
// three candidates, one selected for shrink, one is put back to the candidate list and one is ignored
solana_logger::setup();
let mut candidates = ShrinkCandidates::new();
let db = AccountsDb::new_single_for_tests();

let common_store_path = Path::new("");
let slot_id_1 = 12;
Expand All @@ -13281,7 +13274,7 @@ pub mod tests {
));
store1.alive_bytes.store(0, Ordering::Release);

candidates.insert(slot_id_1, store1.clone());
candidates.insert(slot_id_1);

let slot_id_2 = 13;

Expand All @@ -13298,7 +13291,7 @@ pub mod tests {
store2
.alive_bytes
.store(store2_alive_bytes, Ordering::Release);
candidates.insert(slot_id_2, store2.clone());
candidates.insert(slot_id_2);

let slot_id_3 = 14;
let store3_id = 55;
Expand All @@ -13309,37 +13302,36 @@ pub mod tests {
store_file_size,
));

db.storage.insert(slot_id_1, Arc::clone(&store1));
db.storage.insert(slot_id_2, Arc::clone(&store2));
db.storage.insert(slot_id_3, Arc::clone(&entry3));

// The store3's alive ratio is 1.0 as its page-aligned alive size is 2 pages
let store3_alive_bytes = (PAGE_SIZE + 1) as usize;
entry3
.alive_bytes
.store(store3_alive_bytes, Ordering::Release);

candidates.insert(slot_id_3, entry3);
candidates.insert(slot_id_3);

// Set the target alive ratio to 0.6 so that we can just get rid of store1, the remaining two stores
// alive ratio can be > the target ratio: the actual ratio is 0.75 because of 3 alive pages / 4 total pages.
// The target ratio is also set to larger than store2's alive ratio: 0.5 so that it would be added
// to the candidates list for next round.
let target_alive_ratio = 0.6;
let (selected_candidates, next_candidates) =
AccountsDb::select_candidates_by_total_usage(&candidates, target_alive_ratio, None);
db.select_candidates_by_total_usage(&candidates, target_alive_ratio, None);
assert_eq!(1, selected_candidates.len());
assert_eq!(
selected_candidates[&slot_id_1].append_vec_id(),
store1.append_vec_id()
);
assert!(selected_candidates.contains(&slot_id_1));
assert_eq!(1, next_candidates.len());
assert_eq!(
next_candidates[&slot_id_2].append_vec_id(),
store2.append_vec_id()
);
assert!(next_candidates.contains(&slot_id_2));
}

#[test]
fn test_select_candidates_by_total_usage_2_way_split_condition() {
// three candidates, 2 are selected for shrink, one is ignored
solana_logger::setup();
let db = AccountsDb::new_single_for_tests();
let mut candidates = ShrinkCandidates::new();

let common_store_path = Path::new("");
Expand All @@ -13354,8 +13346,8 @@ pub mod tests {
store_file_size,
));
store1.alive_bytes.store(0, Ordering::Release);

candidates.insert(slot_id_1, store1.clone());
db.storage.insert(slot_id_1, Arc::clone(&store1));
candidates.insert(slot_id_1);

let slot_id_2 = 13;
let store2_id = 44;
Expand All @@ -13365,13 +13357,14 @@ pub mod tests {
store2_id,
store_file_size,
));
db.storage.insert(slot_id_2, Arc::clone(&store2));

// The store2's alive_ratio is 0.5: as its page aligned alive size is 1 page.
let store2_alive_bytes = (PAGE_SIZE - 1) as usize;
store2
.alive_bytes
.store(store2_alive_bytes, Ordering::Release);
candidates.insert(slot_id_2, store2.clone());
candidates.insert(slot_id_2);

let slot_id_3 = 14;
let store3_id = 55;
Expand All @@ -13388,28 +13381,23 @@ pub mod tests {
.alive_bytes
.store(store3_alive_bytes, Ordering::Release);

candidates.insert(slot_id_3, entry3);
candidates.insert(slot_id_3);

// Set the target ratio to default (0.8), both store1 and store2 must be selected and store3 is ignored.
let target_alive_ratio = DEFAULT_ACCOUNTS_SHRINK_RATIO;
let (selected_candidates, next_candidates) =
AccountsDb::select_candidates_by_total_usage(&candidates, target_alive_ratio, None);
db.select_candidates_by_total_usage(&candidates, target_alive_ratio, None);
assert_eq!(2, selected_candidates.len());
assert_eq!(
selected_candidates[&slot_id_1].append_vec_id(),
store1.append_vec_id()
);
assert_eq!(
selected_candidates[&slot_id_2].append_vec_id(),
store2.append_vec_id()
);
assert!(selected_candidates.contains(&slot_id_1));
assert!(selected_candidates.contains(&slot_id_2));
assert_eq!(0, next_candidates.len());
}

#[test]
fn test_select_candidates_by_total_usage_all_clean() {
// 2 candidates, they must be selected to achieve the target alive ratio
solana_logger::setup();
let db = AccountsDb::new_single_for_tests();
let mut candidates = ShrinkCandidates::new();

let slot1 = 12;
Expand All @@ -13430,7 +13418,8 @@ pub mod tests {
.alive_bytes
.store(store1_alive_bytes, Ordering::Release);

candidates.insert(slot1, store1.clone());
candidates.insert(slot1);
db.storage.insert(slot1, Arc::clone(&store1));

let store2_id = 44;
let slot2 = 44;
Expand All @@ -13447,17 +13436,17 @@ pub mod tests {
.alive_bytes
.store(store2_alive_bytes, Ordering::Release);

candidates.insert(slot2, store2.clone());
candidates.insert(slot2);
db.storage.insert(slot2, Arc::clone(&store2));

for newest_ancient_slot in [None, Some(slot1), Some(slot2)] {
// Set the target ratio to default (0.8), both stores from the two different slots must be selected.
let target_alive_ratio = DEFAULT_ACCOUNTS_SHRINK_RATIO;
let (selected_candidates, next_candidates) =
AccountsDb::select_candidates_by_total_usage(
&candidates,
target_alive_ratio,
newest_ancient_slot.map(|newest_ancient_slot| newest_ancient_slot + 1),
);
let (selected_candidates, next_candidates) = db.select_candidates_by_total_usage(
&candidates,
target_alive_ratio,
newest_ancient_slot.map(|newest_ancient_slot| newest_ancient_slot + 1),
);
assert_eq!(
if newest_ancient_slot == Some(slot1) {
1
Expand All @@ -13473,19 +13462,8 @@ pub mod tests {
selected_candidates.contains(&slot1)
);

if newest_ancient_slot.is_none() {
assert_eq!(
selected_candidates[&slot1].append_vec_id(),
store1.append_vec_id()
);
}
if newest_ancient_slot != Some(slot2) {
assert!(selected_candidates.contains(&slot2));

assert_eq!(
selected_candidates[&slot2].append_vec_id(),
store2.append_vec_id()
);
}
assert_eq!(0, next_candidates.len());
}
Expand Down Expand Up @@ -14989,10 +14967,9 @@ pub mod tests {
db.clean_accounts(Some(1), false, None, &EpochSchedule::default());

// Shrink Slot 0
let slot0_store = db.get_and_assert_single_storage(0);
{
let mut shrink_candidate_slots = db.shrink_candidate_slots.lock().unwrap();
shrink_candidate_slots.insert(0, slot0_store);
shrink_candidate_slots.insert(0);
}
db.shrink_candidate_slots(&epoch_schedule);

Expand Down Expand Up @@ -15323,11 +15300,7 @@ pub mod tests {
return;
}
// Simulate adding shrink candidates from clean_accounts()
let store = db.get_and_assert_single_storage(slot);
db.shrink_candidate_slots
.lock()
.unwrap()
.insert(slot, store.clone());
db.shrink_candidate_slots.lock().unwrap().insert(slot);
db.shrink_candidate_slots(&epoch_schedule);
})
.unwrap()
Expand Down
11 changes: 2 additions & 9 deletions accounts-db/src/ancient_append_vecs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1347,10 +1347,7 @@ pub mod tests {
let storage = db.storage.get_slot_storage_entry(slot);
assert!(storage.is_some());
if in_shrink_candidate_slots {
db.shrink_candidate_slots
.lock()
.unwrap()
.insert(slot, storage.unwrap());
db.shrink_candidate_slots.lock().unwrap().insert(slot);
}
});

Expand Down Expand Up @@ -1379,11 +1376,7 @@ pub mod tests {
);

slots.clone().for_each(|slot| {
assert!(!db
.shrink_candidate_slots
.lock()
.unwrap()
.contains_key(&slot));
assert!(!db.shrink_candidate_slots.lock().unwrap().contains(&slot));
});

let roots_after = db
Expand Down