fix: treat peer-forwarded PUBLISH_NAMESPACE as discovery-only - #220
Merged
englishm merged 2 commits intoAug 28, 2026
Merged
Conversation
A downstream SUBSCRIBE for an uncached track queues a TrackRequest and
waits on a readiness gate until the session that advertised the namespace
answers the upstream SUBSCRIBE. That session frequently dies without
answering: it can return early, be torn down, or have its task dropped
while parked mid-handshake. The sender then disappeared with the gate
still Pending, and the only trace was a closed channel, which the waiter
reported as a bare internal error naming neither the cause nor the
session responsible.
Resolve the gate from Drop instead. The publisher really is gone in all
of those cases, so failing is the correct outcome rather than just a
better log line, and the reason now travels in the error itself: the
waiter already logs it with the namespace and track attached, and the
downstream REQUEST_ERROR carries an honest reason phrase. The wire error
code is unchanged.
Two details are load-bearing:
- Drop only writes when the gate is still Pending. The sender outlives
established() by the whole lifetime of the subscription, so an
unconditional write would turn every completed track into a reported
failure for later subscribers.
- Drop constructs the error directly rather than via internal_ctx,
which mints a UUID; Uuid::new_v4 panics instead of degrading when the
OS entropy source is unavailable. Drop runs during unwinding, where a
panic aborts the process, and the failure would be self-amplifying:
a dead RNG makes every internal_ctx call panic, and unwinding drops
senders whose Drop would hit the same dead RNG.
established() and failed() now record through send_replace, because send
refuses to update once the last receiver is gone and would leave a
resolved subscription indistinguishable from an abandoned one.
A namespace forwarded by a peer relay was handled as if a local client had published it: registered as a local route, claimed in the coordinator, and given a request queue. All three are wrong for a namespace this relay is only relaying, and they combine into two failures that hide each other. Claiming ownership races the origin for one key and loses. The losing path returns before REQUEST_OK and unwinds the local registration it had already published, which downstream reads as the namespace being withdrawn milliseconds after it appeared while the publisher is still live. The local route is worse, because it points somewhere that cannot serve. A downstream subscribe matches it and is sent back up the forwarding session, where the peer published an advertise-only track container whose request channel is closed at creation. That publication takes priority over the peer's unknown-subscribe fall-through, so the subscribe is answered DoesNotExist even though the peer holds a working route. And a dialed session has no Producer draining subscribed() at all, so it could not be served even if the container were sound. The ownership rejection was masking that: tearing down the local route forced a retry through the coordinator, which worked. So removing only the ownership claim turns a masked defect into a hard failure — verified in the cross-relay harness, where the transit test passes before that change and fails with code=16 after it. Register the namespace for discovery instead, the way the SUBSCRIBE_NAMESPACE pull path already does: advertised to list_namespaces_matching, absent from route_namespace. A downstream subscribe then misses locally and resolves through the coordinator to the owner, which serves it over an accepted session that does have a Producer. With no local route there is no queue to strand either. Also records the resolved interface for every accepted session. Whether a peer is classified internal depends on the platform populating local_ip, and nothing observed it: if it is absent, every peer classifies as a public client and this path is silently never taken, degrading to exactly the previous behaviour with no error anywhere. Confirmed Internal in the harness; unverified in production, which this log makes answerable. The advertise-only containers are left in place but no longer reachable from a subscribe. Both call sites now say so, and a unit test pins that such a container accepts a subscribe it can never serve, so a future change routing one there fails loudly rather than returning DoesNotExist.
englishm-cloudflare
force-pushed
the
fix/peer-namespace-discovery-only
branch
from
August 28, 2026 02:39
ab057d1 to
e5dc1a3
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.
In multi-relay deployments that use a Coordinator implementation for coordination, when relay A fans a
PUBLISH_NAMESPACEout to relay B, B now registers the namespace for discovery only — visible tolist_namespaces_matching, absent fromroute_namespace, with no coordinator ownership claim and no local route. This matches the existing behavior in theSUBSCRIBE_NAMESPACEpull path.Previously, peer-forwarded announces went through the same registration path as client announces, which incorrectly claimed coordinator ownership of a namespace already owned by the originating relay and created a local routing entry that could not serve subscriptions. A downstream
SUBSCRIBEnow resolves through the coordinator to the originating relay as expected.A unit test pins the discovery-only contract on the unservable path so that future routing changes produce an explicit failure rather than a silent
DoesNotExist.Also in this PR:
impl Drop for UpstreamReadyTx— when an upstream session ends before acknowledging a forwardedSUBSCRIBE, the readiness gate now resolves with a diagnostic error instead of closing silently. The check is gated on the pre-write state so it does not fire on gates that have already resolved.session acceptedlog — records the session's internal/public classification at the point it is determined. That classification controls whether a peer'sPUBLISH_NAMESPACEis treated as proxied or as a local client announce.