Add fairness counter for low-priority queue#145
Conversation
Reviewer's GuideAdds a configurable fairness counter to ConnectionActor so that after a burst of high-priority frames or an optional time slice, a low-priority frame is yielded; includes configuration struct, state tracking, integration in the select loop, helper methods, a dedicated test, and docs update. Sequence diagram for fairness counter yielding to low-priority queuesequenceDiagram
participant ConnectionActor
participant HighPriorityQueue
participant LowPriorityQueue
participant Outbound
loop Process high-priority frames
ConnectionActor->>HighPriorityQueue: recv_push()
alt Frame received
ConnectionActor->>Outbound: handle_push()
ConnectionActor->>ConnectionActor: after_high()
alt Fairness threshold or time slice hit
ConnectionActor->>LowPriorityQueue: try_recv()
alt Frame available
ConnectionActor->>Outbound: before_send()
ConnectionActor->>Outbound: push(frame)
ConnectionActor->>ConnectionActor: after_low()
else Queue empty or disconnected
end
else Continue high-priority
end
else No frame
ConnectionActor->>ConnectionActor: reset_high_counter()
end
end
Class diagram for FairnessConfig and ConnectionActor changesclassDiagram
class FairnessConfig {
+usize max_high_before_low
+Option<Duration> time_slice
+default()
}
class ConnectionActor {
-PushQueues<F> queues
-Option<FrameStream<F, E>> response
-CancellationToken shutdown
-ProtocolHooks<F> hooks
-FairnessConfig fairness
-usize high_counter
-Option<Instant> high_start
+set_fairness(fairness: FairnessConfig)
+after_high(out: &mut Vec<F>, low_closed: &mut bool)
+after_low()
+reset_high_counter()
}
ConnectionActor --> FairnessConfig : uses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughA fairness mechanism was added to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ConnectionActor
participant HighQueue
participant LowQueue
loop While processing frames
ConnectionActor->>HighQueue: Poll high-priority frame
alt High-priority frame received
ConnectionActor->>ConnectionActor: after_high()
alt Max high before low or time slice reached
ConnectionActor->>LowQueue: Try receive low-priority frame
alt Low-priority frame available
ConnectionActor->>ConnectionActor: after_low()
end
end
else No high-priority frame
ConnectionActor->>ConnectionActor: reset_high_counter()
end
end
Possibly related issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (6)`docs/**/*.md`: Use the markdown files within the `docs/` directory as a knowled...
📄 Source: CodeRabbit Inference Engine (AGENTS.md) List of files the instruction was applied to:
`**/*.md`: Validate Markdown files using `markdownlint *.md **/*.md`. Run `mdfor...
📄 Source: CodeRabbit Inference Engine (AGENTS.md) List of files the instruction was applied to:
`docs/*.md`: Each file in the docs/ directory is a documentation markdown file s...
📄 Source: CodeRabbit Inference Engine (docs/contents.md) List of files the instruction was applied to:
`docs/**/*.md`: Use British English spelling based on the Oxford English Diction...
📄 Source: CodeRabbit Inference Engine (docs/documentation-style-guide.md) List of files the instruction was applied to:
`docs/**/asynchronous-outbound-messaging-design.md`: Document and reference desi...
📄 Source: CodeRabbit Inference Engine (docs/asynchronous-outbound-messaging-roadmap.md) List of files the instruction was applied to:
`**/*.rs`: Every Rust module must begin with a module level (`//!`) comment expl...
📄 Source: CodeRabbit Inference Engine (AGENTS.md) List of files the instruction was applied to:
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (10)
✨ 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}
There was a problem hiding this comment.
Gates Failed
(1 file with Bumpy Road Ahead)
(1 file with Complex Method)
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.54 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 1 advisory rule | 10.00 → 9.54 | 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.
![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 and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/connection.rs:197` </location>
<code_context>
+ self.high_start = Some(Instant::now());
+ }
+
+ let threshold_hit = self.fairness.max_high_before_low > 0
+ && self.high_counter >= self.fairness.max_high_before_low;
+ let time_hit =
</code_context>
<issue_to_address>
Zero value for max_high_before_low disables threshold logic.
If this is intended, please document that setting max_high_before_low to zero disables the threshold logic and relies solely on the time slice for fairness.
</issue_to_address>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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
docs/asynchronous-outbound-messaging-roadmap.md(1 hunks)src/connection.rs(6 hunks)tests/connection_actor.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`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.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.md
`docs/*.md`: Each file in the docs/ directory is a documentation markdown file s...
docs/*.md: Each file in the docs/ directory is a documentation markdown file summarizing a specific aspect of the project's design, architecture, testing, operations, or reference material.
📄 Source: CodeRabbit Inference Engine (docs/contents.md)
List of files the instruction was applied to:
docs/asynchronous-outbound-messaging-roadmap.md
`docs/**/*.md`: Use British English spelling based on the Oxford English Diction...
docs/**/*.md: Use British English spelling based on the Oxford English Dictionary, except retain US spelling in API names (e.g., 'color').
Use the Oxford comma in lists.
Write headings in sentence case and use Markdown heading levels in order without skipping.
Follow markdownlint recommendations for Markdown formatting.
Always use fenced code blocks with a language identifier; use 'plaintext' for non-code text.
Use '-' as the first level bullet and renumber lists when items change.
Prefer inline links using 'text' or angle brackets around the URL.
Expand any uncommon acronym on first use, e.g., Continuous Integration (CI).
Wrap paragraphs at 80 columns, code at 120 columns, and do not wrap tables.
Use footnotes referenced with '[^label]'.
When embedding figures, use '' and provide concise alt text describing the content.
Add a short description before each Mermaid diagram for screen readers.
📄 Source: CodeRabbit Inference Engine (docs/documentation-style-guide.md)
List of files the instruction was applied to:
docs/asynchronous-outbound-messaging-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:
tests/connection_actor.rssrc/connection.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (5)
src/connection.rs (3)
47-82: Well-structured fairness tracking implementation.The addition of fairness tracking fields and their proper initialisation in both constructors is clean and follows the existing patterns in the codebase.
140-163: Correct integration of fairness logic into the polling loop.The fairness mechanism is properly integrated, with high-priority frames incrementing the counter and both low-priority and response frames resetting it. This ensures the fairness counter accurately tracks consecutive high-priority frame processing.
191-218: Robust fairness enforcement implementation.The
after_highmethod correctly implements both count-based and time-based fairness thresholds. The use oftry_recvfor non-blocking low-priority queue checking is appropriate, and all error cases are properly handled.tests/connection_actor.rs (1)
47-71: Well-designed test for fairness mechanism.The test correctly validates that the fairness counter yields to the low-priority queue after processing the configured number of high-priority frames. The expected output
[1, 2, 99, 3, 4, 5]properly demonstrates the fairness behavior.docs/asynchronous-outbound-messaging-roadmap.md (1)
16-17: Documentation correctly updated to reflect feature completion.The fairness counter checkbox has been appropriately marked as complete, aligning with the implementation delivered in this PR.
There was a problem hiding this comment.
Gates Failed
(1 file with Bumpy Road Ahead)
(1 file with Complex Method)
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.54 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| connection.rs | 1 advisory rule | 10.00 → 9.54 | 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: 3
♻️ Duplicate comments (2)
src/connection.rs (2)
244-249: Address code duplication between after_low and reset_high_counter.Both methods have identical implementations, violating the DRY principle.
fn after_low(&mut self) { - self.high_counter = 0; - self.high_start = None; + self.reset_high_counter(); } fn reset_high_counter(&mut self) { self.high_counter = 0; self.high_start = None; }
221-222: Document the zero value behaviour for max_high_before_low.Setting
max_high_before_lowto zero disables the threshold logic and relies solely ontime_slicefor fairness. This should be clearly documented in the code.let threshold_hit = self.fairness.max_high_before_low > 0 && self.high_counter >= self.fairness.max_high_before_low; +// Note: Setting max_high_before_low to zero disables threshold logic
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
docs/asynchronous-outbound-messaging-design.md(2 hunks)docs/contents.md(1 hunks)src/connection.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
`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/contents.mddocs/asynchronous-outbound-messaging-design.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/contents.mddocs/asynchronous-outbound-messaging-design.md
`docs/*.md`: Each file in the docs/ directory is a documentation markdown file s...
docs/*.md: Each file in the docs/ directory is a documentation markdown file summarizing a specific aspect of the project's design, architecture, testing, operations, or reference material.
📄 Source: CodeRabbit Inference Engine (docs/contents.md)
List of files the instruction was applied to:
docs/contents.mddocs/asynchronous-outbound-messaging-design.md
`docs/**/*.md`: Use British English spelling based on the Oxford English Diction...
docs/**/*.md: Use British English spelling based on the Oxford English Dictionary, except retain US spelling in API names (e.g., 'color').
Use the Oxford comma in lists.
Write headings in sentence case and use Markdown heading levels in order without skipping.
Follow markdownlint recommendations for Markdown formatting.
Always use fenced code blocks with a language identifier; use 'plaintext' for non-code text.
Use '-' as the first level bullet and renumber lists when items change.
Prefer inline links using 'text' or angle brackets around the URL.
Expand any uncommon acronym on first use, e.g., Continuous Integration (CI).
Wrap paragraphs at 80 columns, code at 120 columns, and do not wrap tables.
Use footnotes referenced with '[^label]'.
When embedding figures, use '
Add a short description before each Mermaid diagram for screen readers.
📄 Source: CodeRabbit Inference Engine (docs/documentation-style-guide.md)
List of files the instruction was applied to:
docs/contents.mddocs/asynchronous-outbound-messaging-design.md
`docs/**/asynchronous-outbound-messaging-design.md`: Document and reference desi...
docs/**/asynchronous-outbound-messaging-design.md: Document and reference design details for features such as actor state management, optional dead letter queue, error handling and resilience, fairness for low-priority frames, configuration via the WireframeProtocol trait, the PushHandle, prioritised message queues, the SessionRegistry, and use cases.
📄 Source: CodeRabbit Inference Engine (docs/asynchronous-outbound-messaging-roadmap.md)
List of files the instruction was applied to:
docs/asynchronous-outbound-messaging-design.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
🪛 LanguageTool
docs/contents.md
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ...embling messages. - Streaming response design\ Design for handling multi-packet and streaming...
(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~33-~33: Possible typo: you repeated a word.
Context: ... High-level development roadmap. - 1.0 philosophy\ Philosophy and feature set for Wireframe 1.0. ## ...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (6)
src/connection.rs (1)
21-32: Document the misplaced comment issue has been resolved.The documentation placement issue from the previous review has been properly addressed. The
FairnessConfignow has appropriate documentation describing its purpose and fields.docs/asynchronous-outbound-messaging-design.md (3)
150-152: Excellent clarification of zero value behaviour.The updated documentation clearly explains that setting the counter to zero disables threshold logic and relies solely on
time_slicefor fairness. This addresses the previous concerns about documenting this behaviour.
185-208: Well-structured sequence diagram enhances understanding.The new Mermaid sequence diagram effectively illustrates the runtime behaviour of the fairness mechanism, showing the interaction between the ConnectionActor and the queues. The diagram complements the existing flowchart nicely.
185-185: Minor grammar improvement needed.The text should use "behaviour" (British English) consistently.
-The following sequence diagram illustrates the runtime behaviour: +The following sequence diagram illustrates the runtime behaviour:Wait, this is already correct. The documentation is properly using British English spelling.
docs/contents.md (2)
3-4: Approve text consolidation for better readability.The consolidation of the introductory text into a single line improves readability without changing the meaning.
12-12: False positive from static analysis tool.The LanguageTool warning about "word repetition" appears to be incorrectly flagging the filename
multi-packet-and-streaming-responses-design.md. This is a false positive as the repetition is in a legitimate filename, not duplicated words in prose.
1 participant
Summary
Testing
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_685f1ebd24108322b4214a341609e5d3
Summary by Sourcery
Introduce a configurable fairness counter in ConnectionActor to balance processing between high- and low-priority queues, add corresponding tests, and update documentation.
New Features:
Documentation:
Tests: