Repository navigation
Stop the aggregator stress test dropping events on a slow runner - #88
Conversation
The stress test floods 10K events into an unlimited inbox, but the dispatch queue keeps its default capacity of 16. When the dispatcher thread is not scheduled for a moment, the collector fills the queue and drops whole batches by design, so the latch never reaches 10K and the test fails after 60s. Reproducible every time on a single-thread dispatcher. Size the queue to the event count, an upper bound on queued items. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe 10,000-event stress test now sets ChangesStress-test queue capacity
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other Merge Risk: ⚪ Minimal · up to This test-only change prevents the stress test from failing due to an undersized dispatch queue. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. A rabbit checks the queue with care Comment |
|
🚀 Available in v5.1.1 |
3 participants
Goal
Closes AND-1605.
StreamEventAggregatorImplTest > stress - 10K events with realistic type distributionfails intermittently in CI with10K events not delivered in time. It has failed ondevelop(run), on a dependabot PR (run) and on #87 (run), and passed on rerun each time.The cause is dropped events, not a slow runner. A slow runner only exposes the drop. The 60s timeout was never close to being needed:
DEFAULT_DISPATCH_QUEUE_CAPACITY = 16. At a threshold of 50, the run produces about 200 batches.trySendfails. As the class KDoc says, the batch is then dropped with a warning. The test has no logger, so the drop is silent.Implementation
The stress test now sets
dispatchQueueCapacity = totalEvents. Every queued item holds at least one event, so the queue can never fill up. I did not usetotalEvents / threshold: batches can be smaller than the threshold when the 500ms window closes before 50 events arrive, so it is not a guaranteed bound.The test still covers all three of its guarantees: every event delivered, batches no larger than the threshold, and fewer handler calls than events. The drop path is still covered by the existing full-dispatch-queue test.
Test-only, no production code touched. The drop under load is intended behaviour per the KDoc, but the test calls "all events delivered" a guarantee, and in production that only holds while the dispatcher keeps up.
Testing
Dispatchers.Default.limitedParallelism(1)the collector never yields to the dispatcher, and the test fails every time../gradlew :stream-android-core:testDebugUnitTest --tests '*StreamEventAggregatorImplTest*'is green, and spotless is clean.🤖 Generated with Claude Code
Summary by CodeRabbit