Repository navigation
refactor(peer): run Manager connections through the protocol core - #144
Merged
Merged
Conversation
peer.Manager made its own pre-CER, packet, control, watchdog and retirement decisions for every connection it owned. Each dialed and accepted connection generation now runs through one private protocol core, the one StateMachine and Client already use. Manager keeps what lies above a single connection: peer authorization and selection, election (RFC 6733 §5.6.4), dialing, routing, the shared identifier space, queues, pending requests and failover (RFC 6733 §5.5.4), reconnection, notifications, renewal and retirement. Native connections are still supervised through DispatchDone, and the public API does not change. Behaviour changes, each with a test that fails on main: - Inbound activity that did not refresh the watchdog interval on main now does, without counting toward health: a malformed or repeated CER, other recoverable decode errors, an invalid header, invalid grammar, a request rejected by optional validation, a wrong bound host, an unsolicited CEA or DPA, and decoded traffic while reopening. - During local Closing, the first DPR received records the peer's cause; a later DPR no longer replaces reconnect suppression. - For every adapter, a request rejected during Closing is now dropped before its error answer is built. When building that answer failed, StateMachine and Client used to close the connection, report ProtocolReplyFailed and end Disconnect with ErrDisconnectClosed; Closing now continues to its DPA or deadline. Where Manager still differs from StateMachine, a named core option selects each diagnostic or disposition policy. A differential test of StateMachine and Client against main matches 3,758 of 3,760 observations; the two differences are the Closing change above. Core effects shrink from 1,208 to 304 bytes and are appended into storage the caller lends; a successful queued answer completes without waking the Manager's actor; each core keeps one spare operation record. BenchmarkInboundMessages at one processor, bytes and allocations per exchange, main → this change (dialed exchanges match accepted ones): | Exchange | B/op | allocs/op | | ------------------------------------------ | ------------ | --------- | | peer Accepted/ApplicationRequestRejection | 7627 → 6394 | 141 → 126 | | peer Accepted/CorrelatedApplicationAnswer | 5441 → 5169 | 121 → 118 | | peer Accepted/UnsolicitedApplicationAnswer | 3312 → 3104 | 59 → 57 | | peer Accepted/WatchdogExchange | 3928 → 2264 | 75 → 57 | | sm Accepted/ApplicationRequest | 11041 → 4128 | 60 → 59 | | sm Accepted/ApplicationAnswer | 9760 → 2848 | 23 → 22 | | sm Accepted/WatchdogExchange | 14409 → 5176 | 84 → 81 | Closes #112.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
peer.Manager made its own pre-CER, packet, control, watchdog and
retirement decisions for every connection it owned. Each dialed and
accepted connection generation now runs through one private protocol
core, the one StateMachine and Client already use. Manager keeps what
lies above a single connection: peer authorization and selection,
election (RFC 6733 §5.6.4), dialing, routing, the shared identifier
space, queues, pending requests and failover (RFC 6733 §5.5.4),
reconnection, notifications, renewal and retirement. Native connections
are still supervised through DispatchDone, and the public API does not
change.
Behaviour changes, each with a test that fails on main:
does, without counting toward health: a malformed or repeated CER,
other recoverable decode errors, an invalid header, invalid grammar, a
request rejected by optional validation, a wrong bound host, an
unsolicited CEA or DPA, and decoded traffic while reopening.
a later DPR no longer replaces reconnect suppression.
before its error answer is built. When building that answer failed,
StateMachine and Client used to close the connection, report
ProtocolReplyFailed and end Disconnect with ErrDisconnectClosed;
Closing now continues to its DPA or deadline.
Where Manager still differs from StateMachine, a named core option
selects each diagnostic or disposition policy. A differential test of
StateMachine and Client against main matches 3,758 of 3,760
observations; the two differences are the Closing change above.
Core effects shrink from 1,208 to 304 bytes and are appended into
storage the caller lends; a successful queued answer completes without
waking the Manager's actor; each core keeps one spare operation record.
BenchmarkInboundMessages at one processor, bytes and allocations per
exchange, main → this change (dialed exchanges match accepted ones):
Closes #112.
Summary by cubic
Runs all peer.Manager connections through the shared protocol core that StateMachine and Client already use, removing duplicate pre-CER, packet, control, watchdog, and retirement logic. The public API is unchanged; Manager still handles peer selection, election, dialing, routing, queuing, failover, and renewal.
Behavior changes
Performance
Closes #112.
Written for commit d21c2be. Summary will update on new commits.