connmgr: Convert to synchronous model.#3646
Open
davecgh wants to merge 9 commits intodecred:masterfrom
Open
Conversation
This was referenced Mar 15, 2026
davecgh
force-pushed
the
connmgr_concurrent
branch
from
4d1e97e to
1a4d8a0
Compare
davecgh
force-pushed
the
connmgr_concurrent
branch
2 times, most recently
from
6ba66b0 to
2581a47
Compare
jholdstock
reviewed
Apr 18, 2026
jholdstock
reviewed
Apr 18, 2026
jholdstock
reviewed
Apr 18, 2026
Member
jholdstock
left a comment
jholdstock
left a comment
There was a problem hiding this comment.
Very cool that all of this could change without having to update the tests at all!
This is the first in a series of commits that will ultimately convert all of the code related to connection manager handling events to make use of separate mutexes and atomics to protect concurrent access. The motivation for the change in architecture follows. Currently, nearly all operations performed by the connection manager are implemented via a single event handler goroutine and a single message channel to protect concurrent access. Generally speaking, mutexes and atomics are best suited to caches and state tracking, while channels are better suited to distributing units of work and communicating async results. While the existing implementation has worked well for many years, it has several downsides such as: - Adding any new functionality requires additional plumbing to provide access to any related information - The state is mostly inaccessible since it is limited to a single goroutine - The use of channels significantly hinders dynamically adjusting to changing conditions based on the state due to the previous There are several other improvements that would be ideal to make, but in the effort of making it easier to follow the changes and assert correctness, this series of commits focuses only on converting it to a synchronous model. With all of the in mind, the commit starts the conversion by introducing a separate mutex to protect the connection requests and moving the map that tracks the connection requests out of the event handler to the connection manager itself. This will allow future commits to methodically refactor the various operations without introducing races.
This moves the pending connection requests map out of the event handler goroutine to the connection manager itself and makes it concurrent safe by protecting it with the new connection mutex. This is part of the overall effort to convert the code related to handling the various connection manager events to synchronous code.
This refactors the logic related to registering a pending connection out of the event handler since it is now independently concurrent safe. This is a part of the overall effort to convert the code related to handling the various connection manager events to synchronous code.
This refactors the logic related to canceling a pending connection out of the event handler since it is now independently concurrent safe. This is a part of the overall effort to convert the code related to handling the various connection manager events to synchronous code.
This refactors the logic related to iterating active connections out of the event handler since it is now independently concurrent safe. This is a part of the overall effort to convert the code related to handling the various connection manager events to synchronous code.
This refactors the logic related to handled established connections out of the event handler since it is now independently concurrent safe. This is a part of the overall effort to convert the code related to handling the various connection manager events to synchronous code.
This refactors the logic related to handled failed connections out of the event handler since it is now independently concurrent safe. This is a part of the overall effort to convert the code related to handling the various connection manager events to synchronous code.
This refactors the logic related to disconnection out of the event handler since it is now independently concurrent safe. This is a part of the overall effort to convert the code related to handling the various connection manager events to synchronous code.
This removes the requests message channel and connection handler since they are no longer used. This is a part of the overall effort to convert the code related to handling the various connection manager events to synchronous code.
davecgh
force-pushed
the
connmgr_concurrent
branch
from
2581a47 to
81e6cd7
Compare
Member
Author
For sure. The tests are fairly well written in there in terms of testing overall expected behavior, handling timeouts, etc. Not to worry though, some code that is still a WIP which builds on this PR will have several new tests 😉. |
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
2 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 converts all of the code related to connection manager handling events to make use of separate mutexes and atomics to protect concurrent access.
The motivation for the change in architecture follows.
Currently, nearly all operations performed by the connection manager are implemented via a single event handler goroutine and a single message channel to protect concurrent access.
Generally speaking, mutexes and atomics are best suited to caches and state tracking, while channels are better suited to distributing units of work and communicating async results.
While the existing implementation has worked well for many years, it has several downsides such as:
There are several other improvements that would be ideal to make, but in the effort of making it easier to follow the changes and assert correctness, this series of commits focuses only on converting it to a synchronous model.
This consists of a series of commits to help ease the review process. Each commit is intended to be a self-contained and logically easy to follow change such that the code continues to compile and works properly at each step.
See the description of each commit for further details.