Repository navigation
fix(mcpx): make events replay lossless past the subscriber buffer (port of dezren39/nix#193) - #351
Conversation
Port of the dezren39/nix fix for dezren39/darwin-nix#193 (replay half): SubscribeFrom replayed retained history into the 256-event subscriber buffer with non-blocking sends while holding the bus lock, silently dropping everything past the buffer with no gap reported. Collect the replay under the lock, size the channel for the whole replay plus the usual live buffer, and set gap if a replay send ever still cannot land. Identical regression test (300-event replay, all in order, zero dropped).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 29 minutes. View limit details
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. Comment |
Problem → solution
Mirror port of the fix for dezren39/darwin-nix#193 (origin issue for this cross-repo mcpx set; no per-repo issue filed here, per the one-issue-per-set rule).
Bus.SubscribeFromreplayed retained history (1024 by default) into the 256-event subscriber buffer with non-blocking sends while holding the bus lock, so a replay over 256 events silently dropped the rest with nogapreported. Reproduced identically in this copy: 300 published events → 256 received,Dropped=44,gap=false.Fix: collect the replay under the lock, size the channel for the whole replay plus the usual live buffer, and set
gapif a replay send ever still cannot land. Regression testTestAReplayLargerThanTheSubscriberBufferMissesNothing(300-event replay, all in order, zero dropped) fails pre-fix and passes post-fix.Scope: the replay half of the origin issue; live-burst drop visibility (approach C) still depends on the #6 gap contract and remains open on the origin issue.
Verified:
internal/events,internal/daemon,internal/poolsuites green locally; gofmt/vet clean; files byte-identical to the dezren39/nix companion PR.Need help on this PR? Tag
@codesmith-botwith what you need.