Conversation
Reviewer's GuideThis PR refactors the outbound messaging design by converting per-request workers into stateful connection actors with a biased Sequence diagram for server-initiated MySQL packet pushsequenceDiagram
participant AppTask as Application Task
participant SessionRegistry
participant ConnectionActor
participant Socket
AppTask->>SessionRegistry: get PushHandle for session
AppTask->>ConnectionActor: push(OK packet or LOCAL INFILE)
ConnectionActor->>ConnectionActor: queue frame in outbox
ConnectionActor->>Socket: write frame (when idle or after command completes)
Sequence diagram for WebSocket heart-beat pushsequenceDiagram
participant Timer as Heart-beat Timer
participant ConnectionActor
participant Socket
Timer->>ConnectionActor: push_high_priority(Ping frame)
ConnectionActor->>ConnectionActor: enqueue Ping in high-priority queue
ConnectionActor->>Socket: write Ping frame (even during response stream)
Sequence diagram for MQTT broker fan-out using PushHandlesequenceDiagram
participant Broker as MQTT Broker
participant SessionRegistry
participant ConnectionActor as Client ConnectionActor
participant Socket
Broker->>SessionRegistry: get PushHandle for subscriber
Broker->>ConnectionActor: try_push(PUBLISH frame)
alt Queue not full
ConnectionActor->>Socket: write PUBLISH frame
else Queue full
ConnectionActor->>ConnectionActor: drop frame
end
Class diagram for stateful connection actor and push queue designclassDiagram
class ConnectionActor {
- context: ConnectionContext
- high_priority_queue: mpsc::Receiver<Frame>
- low_priority_queue: mpsc::Receiver<Frame>
- response_stream: Stream<Response>
- shutdown_signal: CancellationToken
+ run()
+ handle_push()
+ handle_response()
}
class PushHandle {
+ push(frame: Frame)
+ try_push(frame: Frame)
+ push_with_policy(frame: Frame, policy: PushPolicy)
}
class SessionRegistry {
- sessions: DashMap<SessionId, Weak<ConnectionActor>>
+ register(session_id, actor)
+ get_handle(session_id): Option<PushHandle>
}
ConnectionActor o-- PushHandle : exposes
SessionRegistry o-- PushHandle : provides
PushHandle ..> ConnectionActor : queues frames
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe documentation was updated to clarify the transition to an actor-per-connection model for asynchronous outbound messaging, detailing its architecture, use cases, and current implementation status. The roadmap was also revised for clarity, with improved formatting and an introductory note on the current state of implementation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ConnectionActor
participant PushQueue
participant Socket
Client->>ConnectionActor: Initiate connection/request
ConnectionActor->>PushQueue: Enqueue outbound message
Note over ConnectionActor: Manages state and push queue
ConnectionActor->>Socket: Write outbound frame (from queue)
Socket-->>Client: Delivers outbound message
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- The roadmap tables have leftover empty separator rows and inconsistent column spacing; please clean up redundant divider lines to ensure proper markdown table rendering.
- After adding the new “Use Cases” section, the header numbering and table of contents are out of sync—please adjust levels and ToC entries for consistency.
- There are multiple notes about the missing connection actor implementation; consider consolidating them into a single status note to avoid repetition.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The roadmap tables have leftover empty separator rows and inconsistent column spacing; please clean up redundant divider lines to ensure proper markdown table rendering.
- After adding the new “Use Cases” section, the header numbering and table of contents are out of sync—please adjust levels and ToC entries for consistency.
- There are multiple notes about the missing connection actor implementation; consider consolidating them into a single status note to avoid repetition.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/asynchronous-outbound-messaging-design.md(3 hunks)docs/wireframe-1-0-detailed-development-roadmap.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/asynchronous-outbound-messaging-design.md
[style] ~19-~19: Would you like to use the Oxford spelling “synchronization”? The spelling ‘synchronisation’ is also correct.
Context: ...istent state awkward and required extra synchronisation when multiple tasks needed to write to ...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~62-~62: Would you like to use the Oxford spelling “centralizing”? The spelling ‘centralising’ is also correct.
Context: ...tead of writing directly to the socket, centralising all output in one place. ### 3.1 Prior...
(OXFORD_SPELLING_Z_NOT_S)
[misspelling] ~381-~381: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ponse model. Each connection task owns an mpsc outbox channel and exposes a `Sess...
(EN_A_VS_AN)
docs/wireframe-1-0-detailed-development-roadmap.md
[typographical] ~26-~26: To join two clauses or introduce examples, consider using an em dash.
Context: ... | Small | - | | 1.2 | Priority Push Chan...
(DASH_RULE)
[typographical] ~27-~27: To join two clauses or introduce examples, consider using an em dash.
Context: ... | Medium | - | | 1.3 | Connection Actor W...
(DASH_RULE)
[style] ~28-~28: Consider using the typographical ellipsis character here instead.
Context: ...stateful connection actors. Implement a select!(biased; ...) loop that polls for shutdown signals,...
(ELLIPSIS)
[uncategorized] ~28-~28: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...p that polls for shutdown signals, high/low priority pushes and the handler response stream ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[typographical] ~29-~29: To join two clauses or introduce examples, consider using an em dash.
Context: ... | Medium | - | | 1.5 | Basic FragmentAdap...
(DASH_RULE)
[uncategorized] ~30-~30: Possible missing comma found.
Context: ...for a single, non-multiplexed stream of fragments and the outbound logic for splitting a ...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (4)
docs/wireframe-1-0-detailed-development-roadmap.md (1)
12-16: Introductory note greatly improves reader orientation
Clear statement of current implementation status and immediate next steps reads well and sets expectations.docs/asynchronous-outbound-messaging-design.md (3)
18-24: Good justification for actor model
Clear explanation of why per-connection actors simplify state management and concurrency.
30-35: Helpful implementation guide
Explicitly pointing readers atsrc/push.rsand the planned biasedselect!loop gives concrete next actions — nice touch.
58-64: British spelling consistent with project – no change needed
“Centralising” matches en-GB style already used elsewhere in the docs.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary
Testing
mdformat-allmarkdownlint *.md **/*.mdnixie *.md docs/*.mdmake fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_685a02bc6bec83228d5f4a557f324c95
Summary by Sourcery
Update documentation to describe stateful connection actor design and server-initiated packet support with multi-protocol examples, and refine the development roadmap accordingly.
New Features:
Enhancements:
Summary by CodeRabbit