Draft-20 backlog: relay shutdown and lifecycle - #115
Merged
Merged
Conversation
…on them Start documents cancelling its ctx as terminating live sessions, but it only ended each session's handler: the session was unregistered, so Stop no longer closed it, and left open. A relay-scoped reader of an upstream SUBSCRIBE on such a session then never returned, and Stop waited on it without bound. A handler with an inbound subgroup stream open did not even end, so its session stayed open and registered. handleConn now closes the session with NO_ERROR (§3.5: no GOAWAY was sent, so none ran out) as soon as Start's ctx ends, via context.AfterFunc, and again once serveSession returns if the ctx has ended: stop can otherwise win against an AfterFunc that has not started yet. Run is unaffected: it hands Start a ctx that is never cancelled. Pooled upstream sessions run under the pool's context and are closed by Stop as before. TestRelay_StartCtxClosesSessions, idle and with a publisher's subgroup stream open, was verified red before the change. Closing only after serveSession returns fails the open-stream case every time. A bare `defer stop()` failed the idle case intermittently under -race with -count=20. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A session that registers after Stop took its snapshot drains on its own: GOAWAY, grace period, force-close. That wait ignored Stop's ctx, so a cancelled Stop could still wait up to GoawayTimeout for it, although Stop's bulk drain cuts short on the same ctx. beginShutdown now records Stop's ctx in place of the shuttingDown flag, and drainStraggler also ends on it, closing with NO_ERROR: the grace period did not run out, so GOAWAY_TIMEOUT does not apply (§3.5), as in the bulk drain. TestRelay_addSessionDrainsStraggler gains a case with a one-hour grace period and Stop's ctx ending at 100ms; it fails with the ctx case removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…aining (§10.4)
A SUBSCRIBE whose only candidate was a relay reached through the upstream
pool that had sent GOAWAY got DOES_NOT_EXIST: the pool skips such a relay
before any request (§10.4), so no answer of its own existed. A draining
local publisher yields GOING_AWAY ("The endpoint has received a GOAWAY",
§10.6.2).
resolveUpstreams now reports whether it skipped a draining relay, and
subscribeUpstream counts that as a GOING_AWAY answer, ranked with the other
"no publisher yet" errors; under RENDEZVOUS_TIMEOUT the hold continues, as
before.
Interpretation, marked in the code: GOING_AWAY is taken to outrank §10.2.6's
DOES_NOT_EXIST for "no publisher is available", since the publisher is known
and GOING_AWAY says to retry. The last "no publisher yet" answer still sets
the code, so the order of candidates matters there; that, and an upstream
relay's own GOING_AWAY answer being passed on as INTERNAL_ERROR, are
recorded in STATUS.md.
TestCrossRelay_DrainingUpstreamRelayAnswersGoingAway (alone, and with a
local publisher refusing too) uses a Discovery store that keeps a draining
relay listed, as an eventually-consistent backend may; it fails with the
handler change removed. TestResolveUpstreamsSkipsGoingAwayRelay asserts the
new result.
Co-Authored-By: Claude Opus 5.5 (1M context) <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
1 participant
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.
Batch 1 of the remaining draft-20 backlog: relay shutdown and lifecycle. One commit per item, each reviewed by moqt-reviewer, each with a regression test verified red first (by running it before the fix, or with a mutant where the fix changes a signature).
Commits
427de7fClose the sessionsStart's ctx ends, soStopcannot hang on them.Startdocuments cancelling its ctx as terminating live sessions. In practice it only ended each handler; the session was dropped from the setStopcloses and left open. A relay-scoped reader of an upstream SUBSCRIBE on it then never returned, soStopwaited without bound.handleConnnow closes the session with NO_ERROR (§3.5) as soon as the ctx ends (viacontext.AfterFunc), and again afterserveSessionreturns if the ctx has ended, becausestopcan otherwise win the race.Runis unaffected.39e4f13Bound a straggler session's drain byStop's ctx.Stop's snapshot now stops waiting whenStop's ctx ends, as the bulk drain already did, and closes with NO_ERROR (§3.5).beginShutdown(ctx)records the ctx in place of theshuttingDownflag.942585aRefuse with GOING_AWAY when the only upstream relay is draining (§10.4, §10.6.2).resolveUpstreamsnow also reports a relay it skipped for GOAWAY.subscribeUpstreamcounts that as a GOING_AWAY answer, ranked with the other "no publisher yet" errors.Decisions and interpretations
Start's ctx cancel closes the sessions, rather than leaving them forStopor merely boundingStop's wait. This was a maintainer decision; the code now matches the existing doc.Known gaps (added to STATUS.md)
Verification
go test ./...andgolangci-lint runpass.go test -race ./pkg/relay/...passes.TestRelay_StartCtxClosesSessionswas stressed with 5×20 runs under-race.🤖 Generated with Claude Code