fix: quiesce peer discovery before shutdown - #9809
Closed
GrapeBaBa wants to merge 2 commits into
Closed
Conversation
Contributor
Author
|
Closing in favor of libp2p/js-libp2p#3597. The controlled minimal candidate used unmodified Lodestar at de5f89e with only the js-libp2p socket reset guard and completed 50 clean mainnet shutdowns with 0 hangs, so the Lodestar-side changes are not required. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Lodestar can hang during shutdown when new peer discovery or dial work overlaps libp2p connection teardown. This change quiesces peer management before sending goodbye messages and closing libp2p.
Issue reproduction: https://gist.github.com/nflaig/5f41cfc50f38baf5046a034162943dc3
Description
PeerManager.quiesce()that clears peer-management intervals and stops discovery.quiesce()before taking the goodbye peer snapshot and disconnecting peers.PeerManager.close()responsible for callingquiesce()so direct close callers retain the same cleanup.The direct socket reset/shutdown race is addressed by libp2p/js-libp2p#3596. Both draft PR heads correspond exactly to the image currently under test.
Validation
d8ab5dbb3eb8623ed0d12319b77d75b707cce490c20c70314f5328ef240f6b061242a313eb38d860lodestar:shutdown-fix-v6-reset-close-racesha256:2c2bcfbe1a9adf72cb9f73dfc605fb3a149e283d8aeae6b0325bbefd5e1b13f9docker stop -t 60followed bydocker start, with about six minutes of synced soak before shutdown.pnpm lintpnpm check-typespnpm test:unit(339 files passed, 1 skipped; 3487 tests passed, 9 skipped, 1 todo)Local checks ran under Node.js 25.2.1 and emitted the repository engine warning requesting Node.js
^24.13.0.AI Assistance Disclosure