test(network): give the reconnect test a budget above the backoff (backport of #4569) - #4570
Open
stevenvegt wants to merge 1 commit into
Open
stevenvegt wants to merge 1 commit into
stevenvegt wants to merge 1 commit into
Conversation
TestNetworkIntegration_OutboundConnection11Reconnects waits defaultTimeout, 5 seconds, for node1 to reconnect to node2 after node2 was restarted. That reconnect races the backoff: connect() resets it to a random value between 1 and 5 seconds after a connection that was up is closed cleanly, and the connect loop only picks up expired backoffs once per second. The worst case is therefore already over the budget before dialling starts, and the test fails intermittently under load. The reconnect wait now has its own budget of 15 seconds. The two other waits in the test are an initial connect and a disconnect, neither of which races a backoff, so they keep defaultTimeout. Test-only. The tight budget dates back to #1874, which introduced the random reset. Assisted-by: AI (cherry picked from commit 71c1629)
stevenvegt
requested review from
gerardsn,
reinkrul and
woutslakhorst
as code owners
September 18, 2026 15:51
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 #4569.
Problem
TestNetworkIntegration_OutboundConnection11Reconnectsfails intermittently under load with "time-out while waiting for node 1 to reconnect to node 2".The test restarts node2 and gives node1
defaultTimeout, 5 seconds, to reconnect by itself. That reconnect races the backoff: when a connection that was up is closed cleanly,connect()resets the backoff toRandomBackoff(time.Second, 5*time.Second), andconnectLooponly picks up expired backoffs on a one second ticker. The worst case is therefore already over the budget before dialling and the TLS handshake start.This is not new and is not caused by any recent change. The random reset came in with #1874 (2023-02-24) and
defaultTimeoutpredates it.Change
The reconnect wait gets its own budget of 15 seconds, with the arithmetic written down next to the constant. The two other waits in the test are an initial connect and a disconnect, neither of which races a backoff, so they keep
defaultTimeout.Test-only, no production code touched. Cherry-picked without conflicts.
Verification
This is a timing budget, so there is no deterministic RED: the test passes before and after when the machine is quiet.
-count=5passes on this branch. What the change removes is a worst case that was outside the budget by construction, visible in the two constants rather than in a test run.