-
Notifications
You must be signed in to change notification settings - Fork 851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Round change upon f+1 RC messages (experimental option) #7838
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Bhanu Pulluri <[email protected]>
Signed-off-by: Bhanu Pulluri <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with some small comments
consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftHelpers.java
Outdated
Show resolved
Hide resolved
...sts/tests/src/test/java/org/hyperledger/besu/tests/acceptance/bftsoak/BftMiningSoakTest.java
Outdated
Show resolved
Hide resolved
I forgot about this, just having a look now. Hopefully won't take me log to get through it. |
Co-authored-by: Matt Whitehead <[email protected]> Signed-off-by: Bhanu Pulluri <[email protected]>
083956e
to
468e194
Compare
Signed-off-by: Bhanu Pulluri <[email protected]>
Signed-off-by: Bhanu Pulluri <[email protected]>
Signed-off-by: Bhanu Pulluri <[email protected]>
consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BftHelpers.java
Outdated
Show resolved
Hide resolved
doRoundChange(qbftRound.getRoundIdentifier().getRoundNumber() + 1); | ||
} | ||
|
||
private synchronized void doRoundChange(final int newRoundNumber) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this need to be synchronized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is called by two threads one handling round timer expiry and the other handling RC payload (upon f+1 RC quorum) . So made it synchronized as there's no scope of IO blocking or no scope for deadlock without calls to other synchronized methods. Can finegrain it if needed
...ain/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerFactory.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Bhanu Pulluri <[email protected]>
Signed-off-by: Bhanu Pulluri <[email protected]>
Signed-off-by: Bhanu Pulluri <[email protected]>
PR description
Plan going ahead
Next release
--Xqbft-enable-early-round-change = false
--Xqbft-enable-early-round-change = true
Besu 25.6.0
Remove --Xqbft-enable-early-round-change (NOT make it --qbft-enable-early-round-change)
Fixed Issue(s)
#1822