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

remove should_retain from mark_dirty_dead_stores #29358

Merged
merged 1 commit into from
Jan 3, 2023
Merged

remove should_retain from mark_dirty_dead_stores #29358

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
97 changes: 34 additions & 63 deletions runtime/src/accounts_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3835,7 +3835,6 @@ impl AccountsDb {
// Purge old, overwritten storage entries
let (remaining_stores, dead_storages) = self.mark_dirty_dead_stores(
slot,
|store| !shrink_collect.store_ids.contains(&store.append_vec_id()),
// If all accounts are zero lamports, then we want to mark the entire OLD append vec as dirty.
// otherwise, we'll call 'add_uncleaned_pubkeys_after_shrink' just on the unref'd keys below.
shrink_collect.all_are_zero_lamports,
Expand Down Expand Up @@ -3991,15 +3990,13 @@ impl AccountsDb {
}

/// get stores for 'slot'
/// retain only the stores where 'should_retain(store)' == true
/// for stores not retained, insert in 'dead_storages' and optionally 'dirty_stores'
/// Drop 'shrink_in_progress', which will cause the old store to be removed from the storage map.
/// For 'shrink_in_progress'.'old_storage' which is not retained, insert in 'dead_storages' and optionally 'dirty_stores'
/// This is the end of the life cycle of `shrink_in_progress`.
/// Dropping 'shrink_in_progress' here will cause the old store to be removed from the storage map.
/// returns: (# of remaining stores for this slot, dead storages)
pub(crate) fn mark_dirty_dead_stores(
&self,
slot: Slot,
should_retain: impl Fn(&AccountStorageEntry) -> bool,
add_dirty_stores: bool,
shrink_in_progress: Option<ShrinkInProgress>,
) -> (usize, SnapshotStorage) {
Expand All @@ -4022,17 +4019,12 @@ impl AccountsDb {
drop(shrink_in_progress);
slot_stores.read().unwrap().len()
} else {
// no shrink_in_progress, so retain append vecs depending on 'should_retain'
// no shrink in progress, so all append vecs in this slot are dead
let mut list = slot_stores.write().unwrap();
list.retain(|_key, store| {
if !should_retain(store) {
not_retaining_store(store);
false
} else {
true
}
list.drain().for_each(|(_key, store)| {
not_retaining_store(&store);
});
list.len()
0
}
} else {
0
Expand Down Expand Up @@ -16495,7 +16487,7 @@ pub mod tests {
let slot = 0;
for add_dirty_stores in [false, true] {
let (remaining_stores, dead_storages) =
db.mark_dirty_dead_stores(slot, |_| true, add_dirty_stores, None);
db.mark_dirty_dead_stores(slot, add_dirty_stores, None);
assert_eq!(remaining_stores, 0);
assert!(dead_storages.is_empty());
assert!(db.dirty_stores.is_empty());
Expand All @@ -16515,7 +16507,7 @@ pub mod tests {
let existing_store = db.create_and_insert_store(slot, size, "test");
let old_id = existing_store.append_vec_id();
let (remaining_stores, dead_storages) =
db.mark_dirty_dead_stores(slot, |_| false, add_dirty_stores, None);
db.mark_dirty_dead_stores(slot, add_dirty_stores, None);
assert_eq!(0, remaining_stores);
assert_eq!(dead_storages.len(), 1);
assert_eq!(dead_storages.first().unwrap().append_vec_id(), old_id);
Expand All @@ -16532,55 +16524,34 @@ pub mod tests {

#[test]
fn test_mark_dirty_dead_stores() {
let db = AccountsDb::new_single_for_tests();
let slot = 0;
let called = AtomicUsize::default();
let add_dirty_stores = false;
let (remaining_stores, _) = db.mark_dirty_dead_stores(
slot,
|_store| {
called.fetch_add(1, Ordering::Relaxed);
false
},
add_dirty_stores,
None,
);
assert_eq!(0, called.load(Ordering::Relaxed));
assert_eq!(0, remaining_stores);

let size = 1;
let inserted_store = db.create_and_insert_store(slot, size, "test");
let (remaining_stores, _) = db.mark_dirty_dead_stores(
slot,
|store| {
assert_eq!(store.append_vec_id(), inserted_store.append_vec_id());
called.fetch_add(1, Ordering::Relaxed);
true // retain
},
add_dirty_stores,
None,
);
assert_eq!(1, called.load(Ordering::Relaxed));
assert_eq!(1, remaining_stores);

let called = AtomicUsize::default();
let (remaining_stores, _) = db.mark_dirty_dead_stores(
slot,
|store| {
assert_eq!(store.append_vec_id(), inserted_store.append_vec_id());
called.fetch_add(1, Ordering::Relaxed);
false // don't retain
},
add_dirty_stores,
None,
);
assert_eq!(1, called.load(Ordering::Relaxed));
assert!(db
.get_storages_for_slot(slot)
.unwrap_or_default()
.is_empty());
assert_eq!(0, remaining_stores);
assert!(db.dirty_stores.is_empty());
// use shrink_in_progress to cause us to drop the initial store
for add_dirty_stores in [false, true] {
let db = AccountsDb::new_single_for_tests();
let size = 1;
let old_store = db.create_and_insert_store(slot, size, "test");
let old_id = old_store.append_vec_id();
let shrink_in_progress = db.get_store_for_shrink(slot, 100);
let (remaining_stores, dead_storages) =
db.mark_dirty_dead_stores(slot, add_dirty_stores, Some(shrink_in_progress));
assert_eq!(1, remaining_stores);
assert_eq!(dead_storages.len(), 1);
assert_eq!(dead_storages.first().unwrap().append_vec_id(), old_id);
if add_dirty_stores {
assert_eq!(1, db.dirty_stores.len());
let dirty_store = db.dirty_stores.get(&(slot, old_id)).unwrap();
assert_eq!(dirty_store.append_vec_id(), old_id);
} else {
assert!(db.dirty_stores.is_empty());
}
assert_eq!(
1,
db.get_storages_for_slot(slot)
.map(|storages| storages.len())
.unwrap_or_default()
);
}
}

#[test]
Expand Down
3 changes: 1 addition & 2 deletions runtime/src/snapshot_minimizer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,7 @@ impl<'a> SnapshotMinimizer<'a> {

let (_, mut dead_storages_this_time) = self.accounts_db().mark_dirty_dead_stores(
slot,
|_store| true, /* ignored if shrink_in_progress is passed, otherwise retain all */
true, // add_dirty_stores
true, // add_dirty_stores
shrink_in_progress,
);
dead_storages
Expand Down