chore: wait in the precommit step without re-scheduling #2619
Conversation
|
won't merge until I run the script long enough and we're confident |
consensus/state.go
Outdated
| if !cs.rs.StartedPrecommitSleep.Load() { | ||
| cs.rs.StartedPrecommitSleep.Store(true) |
evan-forbes
left a comment
evan-forbes
left a comment
There was a problem hiding this comment.
I wouldn't call this a tolerance, but a wait (for the pr title) 🤷
the tolerance I would associate with #2621
cmwaters
left a comment
cmwaters
left a comment
There was a problem hiding this comment.
I'm a little nervous about the logic but I can't think of any scenario that would be problematic
| cs.scheduleTimeout(waitTime, height, round, cstypes.RoundStepPrevoteWait) | ||
| if !cs.rs.StartedPrecommitSleep.Load() { | ||
| cs.rs.StartedPrecommitSleep.Store(true) | ||
| waitTime := cs.precommitDelay() |
There was a problem hiding this comment.
Can we avoid the timer and unlocking relocking, if the waitTime is 0
We could even add the check prior like:
waitTime := ps.precommitDelay()
if !cs.rs.StartedPrecommitSleep.Load() && waitTime > 0 {
...
There was a problem hiding this comment.
adding the check above would complicate the if statement given the else branch that will return
rach-id
changed the title
…ecommit-race # Conflicts: # consensus/state.go
| decideProposal func(height int64, round int32) | ||
| doPrevote func(height int64, round int32) | ||
| setProposal func(proposal *types.Proposal) error | ||
| StartedPrecommitSleep atomic.Bool |
There was a problem hiding this comment.
moved this to the state instead of the round state because we copy the latter in multiple places and it's not worth handling that now.
Closes celestiaorg/celestia-app#6124