connmgr: Implement exponential backoff with jitter.#3700
Open
davecgh wants to merge 27 commits into
Open
Conversation
davecgh
commented
May 26, 2026
jholdstock
approved these changes
Jun 8, 2026
matthawkins90
approved these changes
Jul 4, 2026
Currently the whitelisting logic happens in the server which makes it inaccessible to the connection manager. In order to pave the way for supporting various connection-related logic that currently happens in the server, but ideally should be happening in the connection manager, this adds basic support for whitelisting CIDR prefixes to the connection manager. The connection manager config struct now accepts a slice of prefixes and a new method named IsWhitelisted is added. Note that this only adds support . It does not update anything to use the new functionality yet.
This adds tests to ensure the new whitelist detection method works as expected.
This modifies the server to pass in the parsed whitelist entries to the connection manager config and the relevant code to make use of the new method it exposes. Finally, it removes the no longer used local isWhitelisted method.
This adds a new TryAcquire method to the context-aware semaphore. As the name implies, the method supports conditionally acquiring the semaphore only when resources are immediately available. In other words, it will not block when there are no resources immediately available.
This adds tests for the new TryAcquire method on the context-aware semaphore to ensure the semantics work as expected.
The current overall total connection limits are enforced by the server rather than the connection manager. This is not ideal for many reasons, but one of the most important consequences is that it makes DoS attacks easier. Another example of some less than ideal behavior that it allows is that some rare combinations of events can lead to temporary extra connection churn. It is much more robust and natural to perform the limiting in the connection manager itself via semaphores. That approach not only significantly hardens the server against DoS attacks and solves various edge cases present in the current code, it also paves the way for even more advanced features such as traffic shaping in the future. To that end, this adds semaphore-based limiting for the total overall number of normal connections to the connection manager and removes the relevant current limiting for it from the server. Normal connections are the automatic outbound, manual outbound, and inbound connections. Persistent connections, on the other hand, are not subject to the limit since they have their own limiting. This is consistent with them not being subject to the automatic target outbound limit either.
This adds tests to ensure that the new max normal connection limiting properly enforces the limit including automatic outbound, manual outbound, and inbound connections. It also ensures that it not applied to persistent connections.
This modifies the persistent connection and target outbound handler shutdown logic to wait for any goroutines they have launched to finish before returning. This ensures there are no dangling goroutines from them once Run returns.
This updates the primary parsing method in the connection manager tests to return a concrete addrmgr address instead of a stdlib net.TCPAddr. The goal is to eventually use the concrete address type almost everywhere to avoid a lot of the less than ideal address reparsing and repeated host/port splitting and joining.
This modifies the address handling to parse the stdlib net.Addr to a concrete addrmgr address earlier in the connect and persistent paths rather than doing it in the dial method and switch the callback to get new addresses to return the concrete type. The server is updated to simply the return the chosen address which no longer needs to be converted. Note that this is theoretically a semantics change because the code in server previously potentially resolved the address via DNS and that no longer is the case. In practice, nothing is really changing in terms of resolution though because the address manager only ever works with resolved addresses since it needs to gossip addresses via the wire protocol which only deals with resolved addresses.
Similar to the recent total normal connection limiting, the current per-host connection limits are enforced by the server. For similar reasons, it is much more robust and natural to perform the limiting early in the connection manager. With that in mind, this implements the per-host connection limiting in the connection manager and removes the relevant current limiting for it from the server. The limiting is applied to inbound, outbound, and persistent connections. The new limiting is handled early in both the inbound and outbound paths now which allows it to take advantage of fast connection shedding for inbound connections and to preemptively prevent all outbound attempts that would exceed the limit regardless of source. It also provides the flexibility to apply independent special permissions in the future. This also slightly changes the semantics to exempt whitelisted addresses for both inbound and outbound connections as opposed to only inbound connections.
This adds tests to ensure that the new max connections per host limiting properly enforces the limit including automatic outbound, manual outbound, inbound, and persistent connections. It also tests whitelisted addresses are exempt.
This moves the code related to mock addresses, connections, and listeners to a separate file. This helps keep it from cluttering up the main test code and also makes it easier to reuse in other packages.
Most of the tests in this package were written well before some of the more modern test conveniences like t.Cleanup were added. As a result, almost all of the tests repeat the code related to waiting for and asserting clean shutdown. This updates the tests so that the main method used to run the connection manager in all of the tests now registers a cleanup func via t.Cleanup to wait for clean shutdown and assert the internal state is clean as expected. This approach is much less error prone and convenient.
This makes the name of the connection manager instances in the test code match the naming used in the implmentation code.
Most of the tests in this package were written when the connection manager was based the older async model and before t.Context was available. As a result, the main method used to run the connection manager in all of the tests takes a context that is always set to a new background context. This updates the method to remove the parameter and instead use the test context via t.Context.
Now that the Connect method is synchronous and returns an error, this modifies the test for detecting dial timeouts to use that error for more more accurate detection that the failure is actually the result of dial timeout as expected.
This converts the max retry duration test over to use the synctest which makes it more robust and no longer reliant on real time.
There are various aspects of the connection manager that would benefit from making use of randomness. For example, adding random jitter to connection backoffs. However, by default, randomness often makes reproducing test failures extremely difficult since the next run will not not necessarily be following the same code branches due to different random values. In order to provide deterministic reproducibility, this adds a csprng interface that makes use of the dcrd crypto/rand module by default. All code that sources randomness moving forward is expected to use the interface. It also adds test infrastructure to automatically generate a new seed on each test iteration that is logged in the event of failure along with the ability to specify the seed via the -seed parameter. When running tests, the seed is then used to create a deterministic math/v2/chacha8 instance to implement the csprng interface.
The current code uses a simple deterministic linear backoff with a maximum capacity. While it works well enough, it has a big downside in that all of the attempts will tend to coalesce over time. This is especially true during a network outage since all connections will drop at the same time. Also, exponential backoffs are preferred over linear to reduce the retry frequency more quickly and give the remote more time to come back online. This addresses both of those points by changing the linear backoff for persistent peers to use an exponential backoff with jitter instead.
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
3 participants
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.
This requires #3699.
The current code uses a simple deterministic linear backoff with a maximum capacity. While it works well enough, it has a big downside in that all of the attempts will tend to coalesce over time. This is especially true during a network outage since all connections will drop at the same time.
Also, exponential backoffs are preferred over linear to reduce the retry frequency more quickly and give the remote more time to come back online.
This addresses both of those points by changing the linear backoff for persistent peers to use an exponential backoff with jitter instead.
Due to the introduction of nondeterministic behavior, and considering there are various other aspects of the connection manager that would benefit from making use of randomness, this also includes a separate commit to introduce full infrastructure and support for deterministic reproducibility when running the tests.
See the individual commit descriptions for more details.