Skip to content

Commit

Permalink
Remove needless last_root for better reclaims (#8148)
Browse files Browse the repository at this point in the history
* Restore last_root to fix unintended storage delete

* Remove last_root thing altogether

* Remove unneeded test...
  • Loading branch information
ryoqun authored Feb 12, 2020
1 parent 741d148 commit 5872746
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 50 deletions.
23 changes: 8 additions & 15 deletions runtime/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,6 @@ impl AccountsDB {
reclaims.extend(new_reclaims);
}

let last_root = accounts_index.last_root;

drop(accounts_index);
drop(storage);

Expand All @@ -698,16 +696,16 @@ impl AccountsDB {
}
}

self.handle_reclaims(&reclaims, last_root);
self.handle_reclaims(&reclaims);
}

fn handle_reclaims(&self, reclaims: &[(Slot, AccountInfo)], last_root: Slot) {
fn handle_reclaims(&self, reclaims: &[(Slot, AccountInfo)]) {
let mut dead_accounts = Measure::start("reclaims::remove_dead_accounts");
let mut dead_slots = self.remove_dead_accounts(reclaims);
dead_accounts.stop();

let mut cleanup_dead_slots = Measure::start("reclaims::purge_slots");
self.cleanup_dead_slots(&mut dead_slots, last_root);
self.cleanup_dead_slots(&mut dead_slots);
cleanup_dead_slots.stop();

let mut purge_slots = Measure::start("reclaims::purge_slots");
Expand Down Expand Up @@ -1112,7 +1110,7 @@ impl AccountsDB {
slot_id: Slot,
infos: Vec<AccountInfo>,
accounts: &[(&Pubkey, &Account)],
) -> (Vec<(Slot, AccountInfo)>, u64) {
) -> Vec<(Slot, AccountInfo)> {
let mut reclaims: Vec<(Slot, AccountInfo)> = Vec::with_capacity(infos.len() * 2);
let index = self.accounts_index.read().unwrap();
let mut update_index_work = Measure::start("update_index_work");
Expand All @@ -1127,7 +1125,6 @@ impl AccountsDB {
})
.collect();

let last_root = index.last_root;
drop(index);
if !inserts.is_empty() {
let mut index = self.accounts_index.write().unwrap();
Expand All @@ -1136,7 +1133,7 @@ impl AccountsDB {
}
}
update_index_work.stop();
(reclaims, last_root)
reclaims
}

fn remove_dead_accounts(&self, reclaims: &[(Slot, AccountInfo)]) -> HashSet<Slot> {
Expand Down Expand Up @@ -1171,9 +1168,7 @@ impl AccountsDB {
dead_slots
}

fn cleanup_dead_slots(&self, dead_slots: &mut HashSet<Slot>, last_root: u64) {
// a slot is not totally dead until it is older than the root
dead_slots.retain(|slot| *slot < last_root);
fn cleanup_dead_slots(&self, dead_slots: &mut HashSet<Slot>) {
if !dead_slots.is_empty() {
{
let mut index = self.accounts_index.write().unwrap();
Expand Down Expand Up @@ -1234,11 +1229,11 @@ impl AccountsDB {
store_accounts.stop();

let mut update_index = Measure::start("store::update_index");
let (reclaims, last_root) = self.update_index(slot_id, infos, accounts);
let reclaims = self.update_index(slot_id, infos, accounts);
update_index.stop();
trace!("reclaim: {}", reclaims.len());

self.handle_reclaims(&reclaims, last_root);
self.handle_reclaims(&reclaims);
}

pub fn add_root(&self, slot: Slot) {
Expand Down Expand Up @@ -1797,9 +1792,7 @@ pub mod tests {
let (list, idx) = index.get(&pubkey, &ancestors).unwrap();
list[idx].1.store_id
};
//slot 0 is behind root, but it is not root, therefore it is purged
accounts.add_root(1);
assert!(accounts.accounts_index.read().unwrap().is_purged(0));

//slot is still there, since gc is lazy
assert!(accounts.storage.read().unwrap().0[&0].get(&id).is_some());
Expand Down
35 changes: 0 additions & 35 deletions runtime/src/accounts_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ pub struct AccountsIndex<T> {
pub account_maps: HashMap<Pubkey, RwLock<SlotList<T>>>,

pub roots: HashSet<Slot>,

// This value that needs to be stored to recover the index from AppendVec
pub last_root: Slot,
}

impl<T: Clone> AccountsIndex<T> {
Expand Down Expand Up @@ -148,10 +145,6 @@ impl<T: Clone> AccountsIndex<T> {
entry.write().unwrap().push((slot, account_info));
}

pub fn is_purged(&self, slot: Slot) -> bool {
slot < self.last_root
}

pub fn can_purge(max_root: Slot, slot: Slot) -> bool {
slot < max_root
}
Expand All @@ -161,11 +154,6 @@ impl<T: Clone> AccountsIndex<T> {
}

pub fn add_root(&mut self, slot: Slot) {
assert!(
(self.last_root == 0 && slot == 0) || (slot >= self.last_root),
"new roots must be increasing"
);
self.last_root = slot;
self.roots.insert(slot);
}
/// Remove the slot when the storage for the slot is freed
Expand Down Expand Up @@ -270,29 +258,6 @@ mod tests {
assert_eq!(list[idx], (0, true));
}

#[test]
fn test_is_purged() {
let mut index = AccountsIndex::<bool>::default();
assert!(!index.is_purged(0));
index.add_root(1);
assert!(index.is_purged(0));
}

#[test]
fn test_max_last_root() {
let mut index = AccountsIndex::<bool>::default();
index.add_root(1);
assert_eq!(index.last_root, 1);
}

#[test]
#[should_panic]
fn test_max_last_root_old() {
let mut index = AccountsIndex::<bool>::default();
index.add_root(1);
index.add_root(0);
}

#[test]
fn test_cleanup_first() {
let mut index = AccountsIndex::<bool>::default();
Expand Down

0 comments on commit 5872746

Please sign in to comment.