Modify connection manager state data type #5017
Merged
Conversation
This file contains 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
coot
force-pushed
the
coot/connection-manager-state
branch
2 times, most recently
from
53c6ff0 to
60e1235
Compare
karknu
reviewed
Dec 3, 2024
bolt12
requested changes
Dec 3, 2024
ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/ConnMap.hs
Outdated
Show resolved
Hide resolved
ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/ConnMap.hs
Outdated
Show resolved
Hide resolved
ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/ConnMap.hs
Show resolved
Hide resolved
ouroboros-network-framework/testlib/Ouroboros/Network/ConnectionManager/Test/Experiments.hs
Outdated
Show resolved
Hide resolved
ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/Core.hs
Show resolved
Hide resolved
coot
force-pushed
the
coot/connection-manager-state
branch
2 times, most recently
from
ff9ec67 to
9dbbd0d
Compare
This was referenced Dec 5, 2024
On Linux when a server listens on a given (addr, port), and we connect a socket which is bound to the same address: * the accept call never returns * the connect returns, and the socket acts as a mirror: whatever is sent over it, can be received from it. This patch implements the same behaviour of simulated snockets. It also provides a test which verifies it.
Connections in side connection manager state must be identified by their `ConnectionId`. Since the state map also reserves places for outbound connections for which we only know the remote address the state is slightly more general than `Map (ConnectionId peerAddr) ...`. The data type used by the state is provided in `Ouroboros.Network.ConnectionManager.ConnMap` module, the `Ouroboros.Network.ConnectionManager.State` module provides type alias for the state type and additional APIs needed by the connection manager.
Can we remove the transition tracer, since we use `traceTVar` in `newMutableConnState`?
IOSimPOR discovered that a connection can be removed from the connection manager (by the connection clean-up function), while it is promoted to warm remote.
It wasn't enough to fix IOSimPOR issue fixed in the previous commit.
coot
force-pushed
the
coot/connection-manager-state
branch
from
9dbbd0d to
2cfc7fb
Compare
The fixed schedule doesn't satisfy IOSimPOR invariants anymore.
Note that this is only effective in `IOSim`.
It's useful to provide not only the transitions that didn't match, but also the time and the server name to make it easier to locate the transition in a trace.
coot
force-pushed
the
coot/connection-manager-state
branch
from
2cfc7fb to
e73179a
Compare
bolt12
approved these changes
Dec 11, 2024
7 tasks
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.
Description
We used to identify connections just be remote IP address and its port, but
since we bind to
0.0.0.0, this is not good enough. We might get twoconnections from the same remote host to different interfaces (or the same
interface but a different IP). Such connections wouldn't be distinguished. For
this reason we switch the internal data type to something isomorphic to
The reason we use
Maybeis that we insert outbound connections before we knowtheir local address.
traceTMVarChecklist
Quality
Maintenance
ouroboros-networkproject.