feat(output): change resubscription method#1011
Conversation
📝 WalkthroughWalkthroughThe gRPC output client now requests a fresh broadcast receiver per connection through an mpsc/oneshot handshake. The output module adds an ChangesSubscriber-based event receiver plumbing
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1011 +/- ##
==========================================
- Coverage 33.85% 32.64% -1.21%
==========================================
Files 21 21
Lines 2812 2916 +104
Branches 2812 2916 +104
==========================================
Hits 952 952
- Misses 1857 1961 +104
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fact/src/output/mod.rs`:
- Around line 60-65: The `tokio::select!` branch in `output::mod::start` that
reads from `subs_rx.recv()` should treat a closed subscription channel as
terminal instead of continuing. Update the `req = subs_rx.recv()` handling so
`None` breaks the loop or returns an error/finished state, and keep the existing
`req.send(rx)` path for valid requests. This prevents the `subs_rx` branch from
becoming immediately ready forever and lets the output task shut down cleanly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 3b392f6c-31a6-4820-9a4e-d01a8281fa7f
📒 Files selected for processing (3)
fact/src/output/grpc.rsfact/src/output/mod.rsfact/src/output/stdout.rs
The new way for output components to resubscribe the broadcast channel distributing events is to send a oneshot sender back to the main task which will resubscribe from the broadcast sender and forward the receiver back. While this approach might seem overly complicated, it ensures there will be no lingering receivers laying around, which in turn allows for Arc::unwrap_or_clone to properly unwrap all messages when only one output is in use.
c51ec30 to
c98bda1
Compare
Description
The new way for output components to resubscribe the broadcast channel distributing events is to send a oneshot sender back to the main task which will resubscribe from the broadcast sender and forward the receiver back.
While this approach might seem overly complicated, it ensures there will be no lingering receivers laying around, which in turn allows for Arc::unwrap_or_clone to properly unwrap all messages when only one output is in use.
Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
Used the changes from #1010 to run fact under DHAT with and without these changes. Without these changes the call to Arc::unwrap_or_clone in fact/src/output/grpc.rs is showing up as making a clone, with the changes this doesn't happen anymore.