Skip stale JetStream pull statuses in paced consumer - #25
Open
i-surkov wants to merge 3 commits into
Open
Conversation
…tale pull statuses The engine arms its local window deadline when a pull is published, while the server starts the same expiresIn clock only on receipt, so on an idle consumer the client always timed out first and the server's routine 408 for the just-expired pull arrived at the head of the next window. There it looked like a terminus arriving a full window early - a dead consumer answering instantly - and tripped the 500ms EarlyEmptyGuard: a non-draining guard sleep (delaying anything published meanwhile by up to 500ms) and a redundant second pull, every window, on a perfectly healthy consumer. Statuses are now matched to the pull they answer: jnats publishes every pull request with a distinct reply subject, and the transport takes that subject from the internal jnats call that computes it (pullReturningSubject in the io.nats.client.impl package object, following the existing NatsKeyValue precedent there), skipping any status that does not answer the current pull - the same subject matching jnats's own sync fetch path uses. A stale status can no longer end (or fail) the live pull's window, whatever the round trip or scheduling delays. Genuine early termini (e.g. a deleted consumer answering the fresh pull instantly) still carry the current pull's subject and still hit the guard. No API or configuration change; stop/reconnect timing is unchanged; the callback engine is unaffected.
stasimus
approved these changes
Aug 6, 2026
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughPaced pull responses now correlate with their originating reply subjects. Stale timeout statuses are skipped. The engine polls buffered directives before deadline-bound waits. Tests and design documentation cover the new behavior. ChangesPaced consume engine
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant PacedPullEngine
participant BufferedPullTransport
participant JetStreamSubscription
participant MessageHandler
PacedPullEngine->>BufferedPullTransport: request pull
BufferedPullTransport->>JetStreamSubscription: pullReturningSubject(options)
JetStreamSubscription-->>BufferedPullTransport: reply subject
BufferedPullTransport-->>PacedPullEngine: classified directive
PacedPullEngine->>MessageHandler: handle buffered Deliver directive
MessageHandler-->>PacedPullEngine: directive outcome
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The engine arms its local window deadline at pull publish; the server starts the same
expiresInclock at receipt. On an idle consumer the client therefore always times out first,and the server's routine
408for the just-expired pull lands at the head of the next window —indistinguishable from a dead consumer answering instantly, so it trips the 500 ms
EarlyEmptyGuard: a non-draining sleep (delaying anything published meanwhile by up to 500 ms)plus a redundant second pull, every idle window.
Fix
Match statuses to the pull they answer. jnats publishes every pull request with a distinct reply
subject and the server addresses that pull's statuses to it — jnats's own sync
fetchusesexactly this equality check; the dispatcher path just never had it. The transport now takes the
subject from the internal
_pullcall that computes it (pullReturningSubjectin theio.nats.client.implpackage object, alongside the existingNatsKeyValueaccessor) and skipsany status that does not answer the current pull. Genuine early termini carry the current pull's
subject and still hit the guard.
Same diagnosis as #22, different mechanism: removes the stale-status class instead of
compensating the 408 case with a timing slack — no RTT assumption, no change to stop/reconnect
timing. No API or configuration change; the callback engine is unaffected.
Summary by CodeRabbit
Bug Fixes
Documentation
Tests