Implement connection actor with prioritised write loop#129
Conversation
Implement ConnectionActor using tokio::select! with biased polling.Actor reads high and low priority queues as well as an optional\nresponse stream, terminating on shutdown or channel closure.\nIncluded tests cover shutdown handling and prioritisation of sources.
Reviewer's GuideThis PR introduces a new ConnectionActor for prioritized outbound frame delivery using a biased tokio::select loop with high/low priority queues and optional response streams, integrates CancellationToken-based shutdown coordination, refines PushQueues lifecycle, exposes the actor in the public API with updated docs, adds tokio-util dependency, and adds comprehensive unit tests for ordering, shutdown, and error cases. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ConnectionActor
participant PushQueues
participant ResponseStream
participant ShutdownToken
loop While active
alt High-priority frame available
ConnectionActor ->> PushQueues: Poll high-priority queue
PushQueues -->> ConnectionActor: Frame
ConnectionActor ->> Client: Output frame
else Low-priority frame available
ConnectionActor ->> PushQueues: Poll low-priority queue
PushQueues -->> ConnectionActor: Frame
ConnectionActor ->> Client: Output frame
else Response stream frame available
ConnectionActor ->> ResponseStream: Poll next frame
ResponseStream -->> ConnectionActor: Frame or Error
ConnectionActor ->> Client: Output frame or return error
else Shutdown signalled
ShutdownToken -->> ConnectionActor: Cancel
ConnectionActor ->> Client: Exit
end
end
Possibly related PRs
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 (
|
![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:
- Consider whether breaking immediately on shutdown might drop queued frames—if you want a graceful shutdown you may need to drain high- and low-priority queues before exiting.
- The
biasedselect gives strict priority to high then low over the response stream, which could starve streaming frames under load—verify that this ordering matches your throughput goals.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider whether breaking immediately on shutdown might drop queued frames—if you want a graceful shutdown you may need to drain high- and low-priority queues before exiting.
- The `biased` select gives strict priority to high then low over the response stream, which could starve streaming frames under load—verify that this ordering matches your throughput goals.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates Failed
(1 file with Bumpy Road Ahead)
(1 file with Complex Method, Complex Conditional)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 1 critical rule | 10.00 → 9.24 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 2 advisory rules | 10.00 → 9.24 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.toml(1 hunks)docs/wireframe-1-0-detailed-development-roadmap.md(2 hunks)src/connection.rs(1 hunks)src/lib.rs(2 hunks)tests/connection_actor.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`Cargo.toml`: Use explicit version ranges in `Cargo.toml` and keep dependencies up-to-date.
Cargo.toml: Use explicit version ranges inCargo.tomland keep dependencies up-to-date.
Cargo.toml
`**/*.rs`: Every Rust module must begin with a module level (`//!`) comment explaining the module's purpose and utility. Document public Rust APIs using Rustdoc comments (`///`) so...
**/*.rs: Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
src/lib.rssrc/connection.rstests/connection_actor.rs
`docs/**/*.md`: Use the markdown files within the `docs/` directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decision...
docs/**/*.md: Use the markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Proactively update the relevant file(s) in thedocs/directory to reflect the latest state when new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve.
Documentation in thedocs/directory must use en-GB-oxendict spelling and grammar, except for the word 'license'.
docs/wireframe-1-0-detailed-development-roadmap.md
`**/*.md`: Validate Markdown files using `markdownlint *.md **/*.md`. Run `mdformat-all` after any documentation changes to format all Markdown files and fix table markup. Validate...
**/*.md: Validate Markdown files usingmarkdownlint *.md **/*.md.
Runmdformat-allafter any documentation changes to format all Markdown files and fix table markup.
Validate Markdown Mermaid diagrams using thenixieCLI by runningnixie *.md **/*.md.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown files must be wrapped at 120 columns.
Tables and headings in Markdown files must not be wrapped.
docs/wireframe-1-0-detailed-development-roadmap.md
`docs/**/wireframe-1-0-detailed-development-roadmap.md`: Track detailed development roadmap items, including unified response types, trait and builder integration, and benchmarks.
docs/**/wireframe-1-0-detailed-development-roadmap.md: Track detailed development roadmap items, including unified response types, trait and builder integration, and benchmarks.
docs/wireframe-1-0-detailed-development-roadmap.md
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (12)
docs/wireframe-1-0-detailed-development-roadmap.md (2)
29-29: Good improvement to document structure.The blank line addition improves visual separation between phases, enhancing readability of the roadmap document.
42-42: Consistent formatting improvement.The additional blank line maintains consistent visual separation between all phases in the document.
src/lib.rs (2)
10-10: Correct module declaration.The connection module is properly declared as public, making it accessible to users of the crate.
21-21: Appropriate re-export for public API.Re-exporting
ConnectionActorat the crate root provides convenient access to this key component without requiring users to navigate the module hierarchy.src/connection.rs (4)
1-7: Excellent module documentation.The module-level documentation clearly explains the purpose and implementation details, including the biased select! behaviour. This follows the coding guidelines for Rust modules.
16-21: Well-structured actor definition.The
ConnectionActorstruct appropriately encapsulates the necessary components for managing outbound frames with clear ownership semantics.
28-39: Proper constructor implementation.The constructor correctly applies the
#[must_use]attribute and follows standard Rust patterns for struct initialisation.
48-96: Robust implementation of the actor's main loop.The
runmethod implementation demonstrates several excellent practices:
- Uses
biasedselect! to ensure proper prioritisation of high-priority frames- Properly tracks closure state of all sources
- Handles errors from the response stream appropriately
- Terminates correctly when shutdown is signalled or all sources are exhausted
The async block for the response stream handling (lines 73-81) is a clever way to conditionally participate in the select! macro.
tests/connection_actor.rs (4)
6-12: Good use of fixtures for test setup.The fixtures provide clean, reusable setup for the tests. The
#[allow(unused_braces)]attribute addresses clippy warnings appropriately.
14-31: Comprehensive priority verification test.This test effectively verifies the biased select! behaviour by ensuring high-priority frames (1) are processed before low-priority frames (2), and both are processed before streamed frames (3). The test setup and assertions are clear and correct.
33-45: Proper shutdown behaviour verification.The test correctly verifies that the actor terminates immediately when the shutdown token is cancelled, without processing any frames. This ensures the shutdown signal takes precedence over all other operations.
47-63: Thorough source exhaustion test.This test verifies that the actor continues processing until all sources are closed, which is critical for ensuring no frames are lost during normal operation. The test correctly drops the handle to close the queues and uses a finite stream.
There was a problem hiding this comment.
Gates Failed
(1 file with Bumpy Road Ahead)
(1 file with Complex Method, Complex Conditional)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 1 critical rule | 10.00 → 9.24 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 2 advisory rules | 10.00 → 9.24 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/connection_actor.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.rs`: Every Rust module must begin with a module level (`//!`) comment expl...
**/*.rs: Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
tests/connection_actor.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-test
- GitHub Check: Analyze (actions)
🔇 Additional comments (5)
tests/connection_actor.rs (5)
23-40: Excellent test coverage for priority ordering.The test correctly verifies that high-priority frames are processed before low-priority frames, followed by response stream frames. The test setup and assertions are clear and comprehensive.
42-54: Good test for shutdown signal precedence.The test properly verifies that when a shutdown token is cancelled before the actor runs, no frames are processed. This ensures the shutdown signal takes precedence over all other operations.
56-72: Comprehensive test for complete source draining.The test effectively verifies that all sources (push queues and response streams) are completely drained before the actor terminates. This ensures no frames are lost during normal operation.
74-100: Robust error propagation testing.The test properly verifies that errors from the response stream are correctly propagated whilst ensuring that frames processed before the error are still delivered. The custom
TestErrortype and error matching are well implemented.
102-128: Effective test for interleaved shutdown behaviour.The test cleverly uses a spawned task to trigger shutdown during stream processing, verifying that the actor respects shutdown signals during operation. The timing-based approach is appropriate for this scenario.
There was a problem hiding this comment.
Gates Failed
(1 file with Bumpy Road Ahead)
(1 file with Complex Method, Complex Conditional)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 1 critical rule | 10.00 → 9.10 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 2 advisory rules | 10.00 → 9.10 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/connection.rs (1)
90-104: Simplify the response stream handling logic.The async block with nested conditions is unnecessarily complex. Consider refactoring for better readability.
- res = async { - if resp_closed { - None - } else if let Some(stream) = &mut self.response { - stream.next().await - } else { - None - } - }, if !shutting_down => { + res = async { + match &mut self.response { + Some(stream) if !resp_closed => stream.next().await, + _ => None, + } + }, if !shutting_down && !resp_closed => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/connection.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.rs`: Every Rust module must begin with a module level (`//!`) comment expl...
**/*.rs: Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
src/connection.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (3)
src/connection.rs (3)
1-7: Module documentation looks good!The module-level documentation clearly explains the actor's purpose and prioritisation strategy.
27-39: Constructor implementation follows best practices.The
#[must_use]attribute is correctly placed after the doc comment, and the method provides a clean interface for creating the actor.
100-102: Consider documenting error handling behaviour.When the response stream yields an error, the method returns immediately without processing any remaining queued frames. This might be intentional, but it's worth documenting this behaviour explicitly.
Is this the intended behaviour? If so, consider adding a note to the method documentation clarifying that response stream errors cause immediate termination without draining queues.
There was a problem hiding this comment.
Gates Failed
(1 file with Complex Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 1 advisory rule | 10.00 → 9.64 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/connection.rs (1)
48-97: Address previous review feedback on shutdown documentation.The complex
runmethod (cyclomatic complexity 13) manages multiple concerns effectively, but there's an outstanding request to improve documentation about shutdown behavior for response streams. The current comment at line 102 could be expanded as previously suggested.Consider the past review suggestion:
// Drop any streaming response so shutdown is prompt. + // Note: This means streamed responses are not guaranteed to complete during shutdown, + // while queued frames will still be drained.Additionally, consider extracting some of the state management logic into helper methods to reduce complexity:
+ fn is_done(&self, high_closed: bool, low_closed: bool, resp_closed: bool, shutting_down: bool) -> bool { + let push_drained = high_closed && low_closed; + push_drained && (resp_closed || shutting_down) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
docs/asynchronous-outbound-messaging-roadmap.md(1 hunks)docs/wireframe-1-0-detailed-development-roadmap.md(3 hunks)src/connection.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
`docs/**/*.md`: Use the markdown files within the `docs/` directory as a knowled...
docs/**/*.md: Use the markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Proactively update the relevant file(s) in thedocs/directory to reflect the latest state when new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve.
Documentation in thedocs/directory must use en-GB-oxendict spelling and grammar, except for the word 'license'.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
docs/asynchronous-outbound-messaging-roadmap.mddocs/wireframe-1-0-detailed-development-roadmap.md
`**/*.md`: Validate Markdown files using `markdownlint *.md **/*.md`. Run `mdfor...
**/*.md: Validate Markdown files usingmarkdownlint *.md **/*.md.
Runmdformat-allafter any documentation changes to format all Markdown files and fix table markup.
Validate Markdown Mermaid diagrams using thenixieCLI by runningnixie *.md **/*.md.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown files must be wrapped at 120 columns.
Tables and headings in Markdown files must not be wrapped.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
docs/asynchronous-outbound-messaging-roadmap.mddocs/wireframe-1-0-detailed-development-roadmap.md
`docs/**/wireframe-1-0-detailed-development-roadmap.md`: Track detailed development roadmap items, including unified response types, trait and builder integration, and benchmarks.
docs/**/wireframe-1-0-detailed-development-roadmap.md: Track detailed development roadmap items, including unified response types, trait and builder integration, and benchmarks.
📄 Source: CodeRabbit Inference Engine (docs/asynchronous-outbound-messaging-roadmap.md)
List of files the instruction was applied to:
docs/wireframe-1-0-detailed-development-roadmap.md
`**/*.rs`: Every Rust module must begin with a module level (`//!`) comment expl...
**/*.rs: Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
src/connection.rs
🔇 Additional comments (8)
src/connection.rs (4)
1-7: Excellent module documentation.The module-level documentation clearly explains the purpose and key implementation details, including the biased
tokio::select!loop and priority handling. This meets the coding guidelines requirement.
16-39: Well-designed struct with proper constructor.The
ConnectionActorstruct design is sound with appropriate generics, public field access, and a properly documented constructor with the#[must_use]attribute.
99-126: Well-structured helper methods.The helper methods provide good separation of concerns with clear, focused responsibilities. The
start_shutdownmethod clearly documents the shutdown sequence.
8-14: Clean imports and module structure.The imports are appropriate, including the new
tokio_util::sync::CancellationTokendependency mentioned in the PR objectives. The module structure is well-organized.docs/asynchronous-outbound-messaging-roadmap.md (1)
13-15: Accurate roadmap update.The task completion marking correctly reflects the implemented
ConnectionActorwith its biasedselect!loop functionality. The description aligns well with the actual implementation.docs/wireframe-1-0-detailed-development-roadmap.md (3)
12-15: Accurate status update reflecting implementation progress.The updated introduction correctly reflects that the foundational components (push queue utilities and connection actor) are now implemented, appropriately shifting Phase 1's focus to integration and API readiness.
29-29: Good visual separation between phases.The added blank line improves document readability by clearly separating the task phases.
42-42: Consistent formatting improvement.The blank line maintains consistent visual separation between phases throughout the document.
There was a problem hiding this comment.
Gates Failed
(1 file with Complex Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 1 advisory rule | 10.00 → 9.69 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/connection.rs (2)
49-55: Early exit behaviour is well-documented.The detailed comments explain the implications of returning immediately when shutdown is already cancelled, addressing the design choice clearly for future maintainers.
101-104: Shutdown behaviour documentation has been improved.The comment clearly explains that streaming responses may be truncated while queued frames are still drained, which addresses the previous concern about documenting this design choice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/connection.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.rs`: Every Rust module must begin with a module level (`//!`) comment expl...
**/*.rs: Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
src/connection.rs
🧬 Code Graph Analysis (1)
src/connection.rs (2)
tests/connection_actor.rs (1)
queues(13-13)src/push.rs (1)
recv(150-156)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-test
- GitHub Check: Analyze (actions)
🔇 Additional comments (4)
src/connection.rs (4)
1-7: Module documentation follows guidelines well.The module-level documentation clearly explains the purpose and utility of the connection actor, including implementation details about the biased select loop and prioritisation behaviour.
16-21: Well-documented public struct with appropriate field visibility.The struct is properly documented and exposes its fields as public, which aligns with the actor pattern where external code needs access to configure the queues and response stream.
27-39: Constructor follows Rust conventions properly.The
#[must_use]attribute is appropriate for a constructor, and the documentation clearly describes the method's purpose.
118-137: Helper methods are well-implemented.The
handle_pushandhandle_responsemethods properly separate concerns and handle the different result types appropriately. The error propagation inhandle_responseis correct.
Extract tokio::select! logic into a helper and consolidate state into a dedicated struct. This lowers cyclomatic complexity and clarifies shutdown behaviour.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/connection.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.rs`: Every Rust module must begin with a module level (`//!`) comment expl...
**/*.rs: Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
src/connection.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-test
- GitHub Check: Analyze (actions)
🔇 Additional comments (1)
src/connection.rs (1)
41-158: Well-structured actor implementation with proper state management.The refactored implementation successfully addresses the complexity concerns raised in previous reviews by:
- Extracting
poll_sourcesas a helper method- Using
ActorStatestruct instead of multiple boolean parameters- Clearly documenting the shutdown behaviour for response streams
- Properly handling prioritization with the biased
select!The code is clean, maintainable, and follows Rust best practices.
|
@coderabbitai review |
|
@sourcery-ai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- The manual tracking of push and response closure flags in
ActorStateis verbose; consider consolidating state management (e.g., counting closed channels) for clearer state transitions. - The
tokio::select!loop inpoll_sourcesis fairly large—extracting individual branches into helper async functions could improve readability and maintainability. - Right now high‐priority frames can starve low‐priority ones under constant load—consider a configurable fairness mechanism or occasional yield to the low‐priority branch.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The manual tracking of push and response closure flags in `ActorState` is verbose; consider consolidating state management (e.g., counting closed channels) for clearer state transitions.
- The `tokio::select!` loop in `poll_sources` is fairly large—extracting individual branches into helper async functions could improve readability and maintainability.
- Right now high‐priority frames can starve low‐priority ones under constant load—consider a configurable fairness mechanism or occasional yield to the low‐priority branch.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai new GitHub issues, please:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@leynos I'll create three GitHub issues for the improvements you've identified. Let me generate them now: 🏁 Script executed: Length of output: 2256 Perfect! I've successfully created three GitHub issues to track the ConnectionActor improvements you identified: Created Issues:
All issues have been assigned to you and include proper context and references back to this PR and discussion. These can now be tracked and addressed in future development iterations. 🐰✨ |
leynos
deleted the
codex/implement-connection-actor-with-biased-select-loop
branch
1 participant
Summary
ConnectionActorfor managing outbound framestokio-utilforCancellationTokenTesting
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_685b4579902c8322aee724c49f0ed82a
Summary by Sourcery
Implement a new
ConnectionActorfor driving outbound frames with prioritized push queues and cancellation support, expose it in the library, add the necessary dependency, update documentation, and enhance tests.New Features:
ConnectionActorto manage prioritized outbound frame delivery with a biased write loopConnectionActorin the public APIEnhancements:
close()method toPushQueuesand restrict queue receiver visibilityBuild:
tokio-utildependency forCancellationTokensupportDocumentation:
Tests:
ConnectionActorcovering push ordering, shutdown, error propagation, interleaved cancellation, and backpressure