Skip to content

Commit

Permalink
[Narwhal] disable min_delay_timer after skipping rounds in proposer (#…
Browse files Browse the repository at this point in the history
…12580)

## Description 

Currently `min_delay_timer` is reset in proposer for the next round,
after skipping rounds because of receiving higher round parents. The
network mostly increments rounds at an interval of `min_header_delay`.
So the following can happen,
- Primary A receives parents from round 15, skips proposing round 14.
- Primary A waits for min_header_delay=0.5s to propose at round 15.
- Primary A receives parents from round 16, skips proposing round 15.

This change avoids the additional wait in `min_delay_timer` when the
proposer skips rounds, to help the lagging primary become get in sync
with the network.

This change is intentionally simplified. A bigger refactor along with
some fixes to proposer will be done in a future PR.


## Test Plan 

This has shown to make header proposal rate more consistent with less
dips across validators in private testnet.

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
mwtian committed Jun 21, 2023
1 parent d816269 commit 9d0fba4
Showing 1 changed file with 25 additions and 7 deletions.
32 changes: 25 additions & 7 deletions narwhal/primary/src/proposer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,9 @@ impl Proposer {
}
}

// Reset advance flag.
advance = false;

// Reschedule the timer.
let timer_start = Instant::now();
max_delay_timer
Expand All @@ -478,6 +481,9 @@ impl Proposer {
min_delay_timer
.as_mut()
.reset(timer_start + self.min_delay());

// Recheck condition and reset time out flags.
continue;
}

tokio::select! {
Expand Down Expand Up @@ -581,16 +587,28 @@ impl Proposer {
let _ = self.tx_narwhal_round_updates.send(self.round);
self.last_parents = parents;

// we re-calculate the timeout to give the opportunity to the node
// to propose earlier if it's a leader for the round
// Reschedule the timer.
// Reset advance flag.
advance = false;

// Extend max_delay_timer to properly wait for leader from the
// previous round.
//
// But min_delay_timer should not be extended: the network moves at
// the interval of min_header_delay. Delaying header creation for
// another min_header_delay after receiving parents from a higher
// round and cancelling proposing, makes it very likely that higher
// round parents will be received and header creation will be cancelled
// again. So min_delay_timer is disabled to get the proposer in sync
// with the quorum.
// If the node becomes leader, disabling min_delay_timer to propose as
// soon as possible is the right thing to do as well.
let timer_start = Instant::now();
max_delay_timer
.as_mut()
.reset(timer_start + self.max_delay());
min_delay_timer
.as_mut()
.reset(timer_start + self.min_delay());
.reset(timer_start);
},
Ordering::Less => {
// Ignore parents from older rounds.
Expand Down Expand Up @@ -624,9 +642,9 @@ impl Proposer {
};

self.metrics
.proposer_ready_to_advance
.with_label_values(&[&advance.to_string(), round_type])
.inc();
.proposer_ready_to_advance
.with_label_values(&[&advance.to_string(), round_type])
.inc();
}

// Receive digests from our workers.
Expand Down

0 comments on commit 9d0fba4

Please sign in to comment.