fix(fpss): evict in-flight subscribe correlation on unsubscribe so a superseded reject cannot drop a live sub#883
Merged
Merged
Conversation
…superseded reject cannot drop a live sub A per-contract or full-stream subscribe records a pending correlation keyed by its req_id so a rejecting REQ_RESPONSE can untrack exactly that subscription. The correlation is only an authority to untrack while the tracked entry it created is still live. An unsubscribe removed the tracked entry but left the correlation resident, so a subscribe -> unsubscribe -> re-subscribe of the same identity, all before the first subscribe's REQ_RESPONSE landed, left the first request's correlation pointing at a slot now occupied by a new live entry. A late rejection of that obsolete req_id then untracked the re-subscribed entry by value and dropped it from the reconnect-replay set. Evict the in-flight correlation for an identity at the unsubscribe boundary, alongside removing the tracked entry. This restores the invariant that at most one correlation per (kind, contract) or (kind, sec_type) identity is resident and it always names the current live entry, so apply_req_response can untrack purely by req_id lookup and a rejection of a superseded request is a no-op rather than a value match against a newer entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 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.
What
A per-contract or full-stream subscribe records a pending correlation keyed by its
req_idso a rejectingREQ_RESPONSEcan untrack exactly that subscription. That correlation is only an authority to untrack while the tracked entry it created is still live.An unsubscribe removed the tracked entry from
active_subs/active_full_subsbut left the correlation resident in the pending registry. So asubscribe X->unsubscribe X->re-subscribe Xof the same identity, all before the first subscribe'sREQ_RESPONSElanded, left the first request's correlation pointing at a slot now occupied by a new live entry. A late rejection of that obsoletereq_idthen untracked the re-subscribed entry by value and dropped it from the reconnect-replay set: the on-wire stream stayed live until the next reconnect, where the contract was no longer replayed.Fix
Evict the in-flight correlation for an identity at the unsubscribe boundary, alongside removing the tracked entry, for both the per-contract and full-stream paths. This restores the invariant that at most one correlation per
(kind, contract)or(kind, sec_type)identity is resident and it always names the current live entry.apply_req_responsecan then untrack purely byreq_idlookup, and a rejection of a superseded request is a no-op rather than a value match against a newer entry.This keeps the existing duplicate-subscribe guard (a duplicate shares the one live entry and registers no second correlation), the TTL / cap eviction, and the reconnect clear unchanged.
Tests
Two regression tests added: the per-contract and full-stream
unsubscribe -> re-subscribe -> reject-supersededsequences each assert that exactly one correlation is resident after the re-subscribe and that the live entry stays tracked (and therefore replayed on reconnect) after the obsolete request is rejected.cargo test -p thetadatadx --lib fpss: 270 passed, 0 failed.🤖 Generated with Claude Code