Skip to content

types: make timely predicate adaptive after 10 rounds#7739

Merged
mergify[bot] merged 7 commits intomasterfrom
wb/adaptive-synchrony
Feb 2, 2022
Merged

types: make timely predicate adaptive after 10 rounds#7739
mergify[bot] merged 7 commits intomasterfrom
wb/adaptive-synchrony

Conversation

@williambanfield
Copy link
Contributor

This change adds logic to double the message delay bound after every 10 rounds. Alternatives to this somewhat magic number were discussed. Specifically, whether or not to make '10' modifiable as a parameter was discussed. Since this behavior only exists to ensure liveness in the case that these values were poorly chosen to begin with, a method to configure this value was not created. Chains that notice many 'untimely' rounds per the relevant metric are expected to take action to increase the configured message delay to more accurately match the conditions of the network.

closes: tendermint/spec#371

@williambanfield williambanfield added the S:automerge Automatically merge PR when requirements pass label Feb 2, 2022
@mergify mergify bot merged commit de04f57 into master Feb 2, 2022
@mergify mergify bot deleted the wb/adaptive-synchrony branch February 2, 2022 15:25
// proceed in the case that the chosen value was too small for the given network conditions.
// For more information and discussion on this mechanism, see the relevant github issue:
// https://github.com/tendermint/spec/issues/371
maxShift := bits.LeadingZeros64(uint64(sp.MessageDelay)) - 1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a closed PR, but I feel that this treatment should be restricted to when the shift is applied, namely round > 10, Don't you think that enclosing this code inside an if would be a better solution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S:automerge Automatically merge PR when requirements pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PBTS: should synchrony parameters be adaptive?

4 participants