besu icon indicating copy to clipboard operation
besu copied to clipboard

P2P: Don't add peer to maintained connections if local node is not ready

Open vdamle opened this issue 2 years ago • 3 comments

PR description

  • Fixes an issue due to which multiple peer connections are setup over time to the local node if it is present in the static nodes list. The code was not handling the case where local node is not yet ready (encountered at startup). https://github.com/hyperledger/besu/pull/1703 added logic to skip adding local node to the set of maintained peer connections. However, the check fails to take into account the case where the local node is not marked ready (occurs at startup since sanitizePeers is called before the call is made to setup the networkRunner at which time the local node is setup and marked ready.

  • Also adds a one time call to initiate connection to nodes in static nodes list after local node is ready. In the absence of this call, the connection attempt is made after the configured interval (default 60 seconds) to check maintained connections. This is to make sure the node doesn't have to wait for almost a minute before it can start interacting with peers (and sync blocks etc.).

Fixed Issue(s)

Fixes https://github.com/hyperledger/besu/issues/1702

Documentation

  • [ ] ~I thought about documentation and added the doc-change-required label to this PR if updates are required.~

No doc updates necessary.

Changelog

vdamle avatar Oct 03 '22 15:10 vdamle

Need some hand holding to fix the acceptance test https://github.com/hyperledger/besu/blob/main/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/bootstrap/StaticNodesAcceptanceTest.java#L38 . I see there is a separate ProcessBesuNodeRunner() setup but not sure how/where the static nodes are being processed in the acceptance test. Thanks in advance!

vdamle avatar Oct 03 '22 20:10 vdamle

Need some hand holding to fix the acceptance test https://github.com/hyperledger/besu/blob/main/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/bootstrap/StaticNodesAcceptanceTest.java#L38 . I see there is a separate ProcessBesuNodeRunner() setup but not sure how/where the static nodes are being processed in the acceptance test. Thanks in advance!

I'm looking at this @vdamle

macfarla avatar Oct 06 '22 06:10 macfarla

@vdamle the change that breaks the AT isn't required - so then you just need to verify that the additional call to checkMaintainedPeers is actually helpful (run up the code on a besu node and see?) and we can review

macfarla avatar Oct 10 '22 07:10 macfarla

closing this one - I couldn't push to your branch so I made a new PR fixing the changelog so we could get it in the RC #4543

macfarla avatar Oct 19 '22 13:10 macfarla