fix(network): close peer connections that go idle (backport of #4562) - #4568
Open
stevenvegt wants to merge 1 commit into
Open
stevenvegt wants to merge 1 commit into
stevenvegt wants to merge 1 commit into
Conversation
* fix(network): close peer connections that go idle Peers send gossip and diagnostics messages every few seconds, so a stream without any incoming message is a dead one: a half-open TCP connection, or a reverse proxy that kept the stream open after the other side went away. Such a connection lingered until the proxy or node was restarted, and the peer holding the stale stream rejected every new connection with "already connected". Connections on which nothing was received for network.idletimeout (default 2m) are now closed, after which the regular reconnect logic takes over. Time spent handling a message does not count as idle, so a large transaction list during sync does not trigger a disconnect. Set network.idletimeout to 0 to disable the check. This works through reverse proxies, unlike gRPC keepalive pings, which proxies answer themselves. Assisted-by: AI * refactor(network): start connection goroutines through one helper A connection keeps two pieces of bookkeeping for its goroutines: an atomic counter that the tests assert on, and a WaitGroup that waitForReceivers blocks on. Neither can replace the other. A WaitGroup can be waited on but its value cannot be read; an atomic counter can be read but cannot be waited on. Waiting on all goroutines instead of the receive loops would be wrong as well, because startSending can block in SendMsg for as long as the gRPC transport takes to give up on a dead connection. Both were maintained by hand at each of the three places that start a goroutine, so adding a receive loop meant remembering two separate registrations. They now go through startGoroutine and startReceiveLoop, which do the bookkeeping, and the counter and the WaitGroup are no longer touched anywhere else. The tests read the counter through goroutineCount. No behaviour change. Assisted-by: AI (cherry picked from commit 13187b8)
reinkrul
approved these changes
Sep 19, 2026
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.
Backport of #4562. Follows the backport of #4467 (#4564 on this branch), which fixed the other half of the same incident.
Problem
A peer kept a stream open for our node after our node had already dropped it, so it believed it was still connected and rejected every new connection with
already connected. In our case the stale stream was held by an nginx in front of the peer; a half-open TCP connection does the same. Nothing cleaned that up short of restarting the proxy.Change
New
network.idletimeout(default2m,0disables). Peers send gossip and diagnostics on every stream every few seconds, so a stream with no incoming message for two minutes is dead. The node now closes it and the regular reconnect logic takes over. Time spent handling a message (a large transaction list during sync, for example) does not count as idle. This works through reverse proxies, unlike gRPC keepalive pings, which proxies answer themselves.The second part of the original PR is a refactor with no behaviour change: the three places that start a goroutine for a connection now go through
startGoroutineorstartReceiveLoop, so the goroutine counter and the receive-loop WaitGroup are maintained in one place instead of by hand at each site.Conflicts
docs/pages/release_notes.rst: this branch keeps its own Unreleased section. Resolved by adding the idle-timeout entry in this branch's style, below the three entries from fix(network): back off when a peer rejects the connection (backport of #4467) #4564.docs/pages/deployment/server_options_didnuts.rstdoes not exist on this branch. Dropped it;server_options.rstandcli-reference.rstwere regenerated instead and carry the new option.The code applied without conflicts, including the
/v5module paths, because the changed hunks do not touch import lines.Verification
go test -race ./network/...passes, apart fromTestNetwork_checkHealth, which fails locally for environment reasons on this branch without the change as well. Generated docs are up to date.Note on
TestNetworkIntegration_OutboundConnection11Reconnects: it failed twice during the full-package runs while preparing this backport. It is not caused by this change (6 clean full-package runs with it, 4 without it, 10 clean runs of the test in isolation). The test waitsdefaultTimeout(5s) for a reconnect while a clean disconnect sets a random 1 to 5 second backoff, so the budget is too tight under load. Pre-existing, same family as #4321.Operational notes
For deployments behind nginx,
grpc_read_timeoutandgrpc_send_timeoutat their 60s default (not raised to hours) plusso_keepaliveon the listen directive let the proxy drop dead streams on its own as well.