Time-based gradual gossip #4176
Conversation
|
This heavily conflicts with #4125 |
|
I'd like to get it in soon, but since a bug in the logic has the potential to halt finality, it's probably best to test a little first on @andresilva 's chaos setup, and roll out gradually. would nonetheless be nice to get it in soon after we bump |
|
For what it's worth I'm fine with that PR being merged. I don't know enough about how this part of the gossiping system works to review this, however. |
andresilva
left a comment
andresilva
left a comment
There was a problem hiding this comment.
Not sure if the round_start should be kept per round rather than globally. The downside with the current approachis that we might delay gossiping older messages a little bit (as we progress to a new round and restart the timer). Although that may be a good thing in order to reduce gossip further. I think the current implementation should be fine and shouldn't lead to any stalls as we always broadcast to everyone eventually.
| const PROPAGATION_ALL: u32 = 4; //in rounds; | ||
| const PROPAGATION_ALL_AUTHORITIES: u32 = 2; //in rounds; | ||
| const PROPAGATION_SOME_NON_AUTHORITIES: u32 = 3; //in rounds; | ||
| const ROUND_DURATION: u32 = 4; // measured in gossip durations |
There was a problem hiding this comment.
Shouldn't this be 2 (prevote and precommit)? Or are the extra 2 gossip durations to account for network/gossip delays?
| } | ||
|
|
||
| fn reshuffle(&mut self) { | ||
| let mut lucky_peers : Vec<_> = self.inner |
There was a problem hiding this comment.
| let mut lucky_peers : Vec<_> = self.inner | |
| let mut lucky_peers: Vec<_> = self.inner |
This reworks #4050 to use time-based gossip progression, rather than relying on the number of attempts, which is quite random.