feat: implement rendezvous timeout feature in MoqxRelay - #549
Open
akash-a-n wants to merge 3 commits into
Open
Conversation
akash-a-n
force-pushed
the
d18/rendezvous-timeout
branch
from
August 6, 2026 13:28
d1d0f2d to
2718863
Compare
akash-a-n
force-pushed
the
d18/rendezvous-timeout
branch
from
August 17, 2026 13:23
2718863 to
f48ea74
Compare
akash-a-n
force-pushed
the
d18/rendezvous-timeout
branch
5 times, most recently
from
August 27, 2026 18:31
a383ff1 to
ffede66
Compare
afrind
reviewed
Aug 27, 2026
afrind
left a comment
afrind
left a comment
Contributor
There was a problem hiding this comment.
(C) Correctness
- LF claim race — a woken SUBSCRIBE fails despite a publisher being available, because a neighbouring subscriber's
failed claim is still open when it joins. Surfaces as INTERNAL_ERROR "local forwarder setup failed". Not
reproducible in the fixture. - Ready-but-draining forwarder short-circuits the park → makeAddSubscriberError instead of waiting. Matches the
non-rendezvous path, so possibly intended.
(T) Test coverage
- The co_stopped_may_throw cancellation branch.
- Trie pruning after a timeout — unobservable today; extracting the trie into its own class (as NamespaceTree is)
makes it testable. - The 30s clamp — not observable without waiting it out.
Also, I think we should encapsulate the rendezvous trie into it's own class/file.
@afrind made 1 comment.
Reviewable status: 0 of 6 files reviewed, all discussions resolved.
- Introduce PendingRendezvous structure to manage parked subscriptions. - Add support for RENDEZVOUS_TIMEOUT in subscription and publish flows. - Implement wake and erase functions for pending rendezvous. - Add tests for rendezvous timeout scenarios in MoqxRelayRendezvousTests.
findPublisherSession() read publisherSession_ off the last node the walk reached. addPublish() and subscriber registration also create nodes carrying no publisher, so a SUBSCRIBE descending through one failed even when a broader namespace above it had a publisher, and a publisher at the root was never found at all. It now keeps the deepest publisher it passes, seeded from the root. Nothing else needed a prefix match, so MatchType is gone. Ports moxygen a682b57a, which never reached moqx because NamespaceTree is a separate implementation of the same tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USngBUv8nzrbMHQCokzJBF
makeUpstreamSubReq stripped track request param key 0x04 unconditionally. That key is RENDEZVOUS_TIMEOUT only at draft 18+; below it the same key is MAX_CACHE_DURATION and is legal to forward, so a pre-v18 client's value was dropped on the way upstream. The two halves now sit where their information lives. The downstream half is consumed by the screen that already reads it -- takeRendezvousTimeout(), renamed from requestedRendezvousTimeout since it no longer just reads -- so a v18 client's per-hop value dies at ingress. makeUpstreamSubReq strips only when the upstream is v18+ and would misread the key. Net effect matches moxygen's downstream-or-upstream-is-v18 gate: forward only when both sides are pre-v18. Also: park timers fire on the relay's own EventBase instead of folly's global timekeeper thread, and the clamp log drops from WARN to DBG1 -- the value is client-supplied and nothing acts on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USngBUv8nzrbMHQCokzJBF
akash-a-n
force-pushed
the
d18/rendezvous-timeout
branch
from
September 7, 2026 13:39
3080345 to
454b75d
Compare
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.
This change is