Skip to content

Commit

Permalink
Remove prune_hot_diff field
Browse files Browse the repository at this point in the history
  • Loading branch information
dapplion committed Dec 15, 2024
1 parent 3fd8f4b commit 385f55b
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 56 deletions.
8 changes: 2 additions & 6 deletions beacon_node/beacon_chain/src/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,12 +709,8 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> BackgroundMigrator<E, Ho
]
})
.chain(abandoned_states.into_iter().flat_map(|(slot, state_hash)| {
[StoreOp::DeleteState {
state_root: state_hash.into(),
state_slot: Some(slot),
// Abandoned forks will never be used, so we can safely delete the diffs
prune_hot_diff: true,
}]
// Abandoned forks will never be used, so we can safely delete the diffs
[StoreOp::DeleteState(state_hash.into(), Some(slot))]
}))
.collect();

Expand Down
8 changes: 2 additions & 6 deletions beacon_node/store/src/garbage_collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,8 @@ where
self.iter_temporary_state_roots()
.try_fold(vec![], |mut ops, state_root| {
let state_root = state_root?;
ops.push(StoreOp::DeleteState {
state_root,
state_slot: None,
// This states will never be used, safe to delete the hot diffs
prune_hot_diff: true,
});
// This states will never be used, safe to delete the hot diffs
ops.push(StoreOp::DeleteState(state_root, None));
Result::<_, Error>::Ok(ops)
})?;

Expand Down
58 changes: 19 additions & 39 deletions beacon_node/store/src/hot_cold_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1051,17 +1051,11 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
/// than the split point. You shouldn't delete states from the finalized portion of the chain
/// (which are frozen, and won't be deleted), or valid descendents of the finalized checkpoint
/// (which will be deleted by this function but shouldn't be).
pub fn delete_state(
&self,
state_root: &Hash256,
slot: Slot,
prune_hot_diff: bool,
) -> Result<(), Error> {
self.do_atomically_with_block_and_blobs_cache(vec![StoreOp::DeleteState {
state_root: *state_root,
state_slot: Some(slot),
prune_hot_diff,
}])
pub fn delete_state(&self, state_root: &Hash256, slot: Slot) -> Result<(), Error> {
self.do_atomically_with_block_and_blobs_cache(vec![StoreOp::DeleteState(
*state_root,
Some(slot),
)])
}

pub fn forwards_block_roots_iterator(
Expand Down Expand Up @@ -1202,11 +1196,7 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
}
}

StoreOp::DeleteState {
state_root,
state_slot: slot,
prune_hot_diff,
} => {
StoreOp::DeleteState(state_root, slot) => {
// Delete the hot state summary.
let state_summary_key =
get_key_for_col(DBColumn::BeaconStateSummary.into(), state_root.as_slice());
Expand All @@ -1220,13 +1210,12 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
);
key_value_batch.push(KeyValueStoreOp::DeleteKey(state_temp_key));

// TODO(hdiff): Should delete the diff under this state root if any
if prune_hot_diff {
key_value_batch.push(KeyValueStoreOp::DeleteKey(get_key_for_col(
DBColumn::BeaconStateHotDiff.into(),
state_root.as_slice(),
)));
}
// Always delete diffs with summary. If a diff must be kept beyond finalization,
// do not issue a DeleteState op at that time.
key_value_batch.push(KeyValueStoreOp::DeleteKey(get_key_for_col(
DBColumn::BeaconStateHotDiff.into(),
state_root.as_slice(),
)));

// TODO(hdiff): Review under HDiff
if slot.map_or(true, |slot| slot % E::slots_per_epoch() == 0) {
Expand Down Expand Up @@ -1384,7 +1373,7 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
self.state_cache.lock().delete_block_states(&block_root);
}

StoreOp::DeleteState { state_root, .. } => {
StoreOp::DeleteState(state_root, _) => {
self.state_cache.lock().delete_state(&state_root)
}

Expand Down Expand Up @@ -3080,12 +3069,8 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
"slot" => summary.slot,
"reason" => reason,
);
state_delete_batch.push(StoreOp::DeleteState {
state_root,
state_slot: Some(summary.slot),
// Delete the diff as descendants of this state will never be used.
prune_hot_diff: true,
});
// Delete the diff as descendants of this state will never be used.
state_delete_batch.push(StoreOp::DeleteState(state_root, Some(summary.slot)));
}
}
}
Expand Down Expand Up @@ -3192,16 +3177,11 @@ pub fn migrate_database<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>>(
non_checkpoint_block_roots.remove(&block_root);
}

// Delete the old summary, and the full state if we lie on an epoch boundary.
// Note: we checked that this diff is not necessary for the finalized section
// of hot hdiffs, delete.
// TODO(hdiff): This diff has to be pruned eventually, implement routine.
if !required_finalized_diff_state_slots.contains(&slot) {
hot_db_ops.push(StoreOp::DeleteState {
state_root,
state_slot: Some(slot),
// Note: we checked that this diff is not necessary for the finalized section
// of hot hdiffs, delete.
// TODO(hdiff): This diff has to be pruned eventually, implement routine.
prune_hot_diff: true,
});
hot_db_ops.push(StoreOp::DeleteState(state_root, Some(slot)));
}

// Do not try to store states if a restore point is yet to be stored, or will never be
Expand Down
6 changes: 1 addition & 5 deletions beacon_node/store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,7 @@ pub enum StoreOp<'a, E: EthSpec> {
DeleteBlock(Hash256),
DeleteBlobs(Hash256),
DeleteDataColumns(Hash256, Vec<ColumnIndex>),
DeleteState {
state_root: Hash256,
state_slot: Option<Slot>,
prune_hot_diff: bool,
},
DeleteState(Hash256, Option<Slot>),
DeleteExecutionPayload(Hash256),
DeleteSyncCommitteeBranch(Hash256),
KeyValueOp(KeyValueStoreOp),
Expand Down

0 comments on commit 385f55b

Please sign in to comment.