Skip to content

Commit

Permalink
[Consensus] DagState to evict blocks based on GC round (#19465)
Browse files Browse the repository at this point in the history
## Description 

Currently we evict the cached ref entries in DagState whenever we
`flush`. At this point we evict the entries for each authority by
dropping all the blocks which are `<= evict_round`, where `evict_round =
authority_latest_commit_round - CACHED_ROUNDS` . The `CACHED_ROUNDS`
here allow us to keep around for a little longer committed blocks. Of
course all the blocks that are `> evict_round` are kept.

This can work fine so far where we don't use GC , as we expect
eventually to include blocks from other peers as weak links - no matter
how far back they are - and that will move the
`authority_latest_commit_round` and trigger the eviction of their blocks
from our memory. Now with GC we don't have those guarantees. It is
possible to get to a scenario where even a group of slow nodes that are
constantly behind `gc_round`, they keep proposing but their blocks never
get committed. Although their blocks should not end up in others DAGs ,
they will remain in their own and fill up their memory. Overall, the
current approach will provide weaker guarantees.

This PR is changing the eviction strategy so it's driven by the
`gc_round`. Doing though the eviction purely on the `gc_round` will
change a lot the semantics of the `DagState` as one of the intentions
was to keep recent cached data from each authority. That would also be
particularly visible for authorities for which we do not have frequent
block proposals, as we could end up always evicting all their blocks if
they are behind the `gc_round`. Then this would not allow us to do
certain operations we used to do before with cached data(ex get latest
cached block per authority).

For that reason this PR is changing a bit the semantics of the
`CACHED_ROUNDS` and from now on it will be the minimum/desired amount of
rounds we want to keep in cache for each of authority. The eviction
algorithm will still attempt to clean up records that are `<= gc_round`,
but it will also make sure that `CACHED_ROUNDS` worth of data are kept
around.
Especially for more edge case situation where a node has not produced
blocks `> gc_round`, we guarantee to keep `CACHED_ROUNDS` even when all
of them are `<= gc_round`, but we'll eventually evict anything before -
practically like a moving window.

## Test plan 

CI/PT

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
  • Loading branch information
akichidis authored Oct 15, 2024
1 parent 72e5405 commit 34416bf
Show file tree
Hide file tree
Showing 6 changed files with 643 additions and 333 deletions.
26 changes: 20 additions & 6 deletions consensus/core/src/base_committer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,17 +227,31 @@ impl BaseCommitter {
leader_block: &VerifiedBlock,
all_votes: &mut HashMap<BlockRef, bool>,
) -> bool {
let (gc_enabled, gc_round) = {
let dag_state = self.dag_state.read();
(dag_state.gc_enabled(), dag_state.gc_round())
};

let mut votes_stake_aggregator = StakeAggregator::<QuorumThreshold>::new();
for reference in potential_certificate.ancestors() {
let is_vote = if let Some(is_vote) = all_votes.get(reference) {
*is_vote
} else {
let potential_vote = self
.dag_state
.read()
.get_block(reference)
.unwrap_or_else(|| panic!("Block not found in storage: {:?}", reference));
let is_vote = self.is_vote(&potential_vote, leader_block);
let potential_vote = self.dag_state.read().get_block(reference);

let is_vote = if gc_enabled {
if let Some(potential_vote) = potential_vote {
self.is_vote(&potential_vote, leader_block)
} else {
assert!(reference.round <= gc_round, "Block not found in storage: {:?} , and is not below gc_round: {gc_round}", reference);
false
}
} else {
let potential_vote = potential_vote
.unwrap_or_else(|| panic!("Block not found in storage: {:?}", reference));
self.is_vote(&potential_vote, leader_block)
};

all_votes.insert(*reference, is_vote);
is_vote
};
Expand Down
Loading

0 comments on commit 34416bf

Please sign in to comment.