fix(events): surface the SUBSCRIBE error and refuse callers instead of admitting a dead stream (BUG-2764) - #1215
Merged
Merged
Conversation
…f admitting a dead stream (BUG-2764)
…resubscribe (BUG-2764)
… install-nothing reason (BUG-2764)
…efuse after Close (BUG-2764 codex round 1)
…ting a live entry (BUG-2764 codex round 2)
…ong it takes (BUG-2764 codex round 3)
…or shutdown, replacement refusals do not count as cycles (BUG-2764 codex round 4)
… docs scope the refusal to the activity stream (BUG-2764 codex round 5, BUG-2800)
… closing (BUG-2764 codex round 6)
…cribers remain (BUG-2764 codex round 7)
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
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.
Fixes BUG-2764. Files BUG-2799 and BUG-2800 for the two adjacent gaps review found that are different mechanisms.
The defect
establishSubscriptionopened the workspace subscription withclient.Subscribe(ctx, channel). In go-redis v9.22.0 that call discards the SUBSCRIBE write's error (redis.go:_ = pubsub.Subscribe(ctx, channels...)), so a SUBSCRIBE the client could not deliver — refused dial, write failure, a caller's context ending mid-dial — came back as a PubSub indistinguishable from a healthy one. It was installed, its acknowledgement never arrived, and its callers were admitted as an unconfirmed subscription promised a reconcile that could never come: a stream subscribed to nothing, for the life of the process on heartbeat phase 1, and re-cycled every idle interval on phase 2.The fix
client.Subscribe(ctx)thenpubsub.Subscribe(ctx, channel)), at all three sites:internal/events(the filed one) and bothinternal/watcheventssites, where the failure used to surface only as the 5s confirmation wait timing out with an error that named the wait.establishSubscription's deciding section, retiring the record exactly as the other two do.SubscribeFailed, a new outcome: a caller whose loop ends with no live subscription and nobody establishing — after the loop's one built-in retry — is refused, not handed a channel wired to nothing. The post-loop check reads the pending record before the live entry (section 1's order, for the same reason), waits on an in-flight record and re-evaluates, and refuses on a closed bus.503+Retry-After: 5+subscription_failed; admission slot released.cycleIfIdle's retry gate (b.pubsub == nil) can open on phase 2;resubscribereturns the write error immediately.idle_timeoutmetric Help, and threedocs/deployment.mdclauses now name the third reason; the BUG-2738 narrative is rewritten as fixed and scoped to the activity stream.Evidence (all on the head SHA unless stated)
go build ./...,go vet ./...,make lint0 issues;go test ./...28 ok / 0 FAIL / 4 no-test (233aef2; the final commit changes one log string);go test -raceok oninternal/events,internal/watchevents,internal/metrics.Dialerwhosenet.Connfails the SUBSCRIBE write (a proxy close races the kernel buffer). Events: all-fail → refused with nothing left behind; first-fail → the retry serves establisher and a mid-attempt joiner, each once; all-fail with a joiner → both refused; after Close → refused; failed replacement → next pass re-establishes and the original channel receives. Watchevents: slot left nil,resubscribereturns the injected error itself. Server: 503 on both subscribe branches, through the router.doneand re-decides (the loop's documented defence in depth), so there is no observable strand; the same-section retire is kept as the file's stated invariant.Recorded, not fixed
-NOPERMetc.) arrives as the first reply, which go-redis's channel loop swallows; different mechanism, lands on the ack-consumption design BUG-2739/2747 built.pubsub == nil, retryable on phase 2).Closedoes not wait for maintenance-pass workers — pre-existing for cycle workers; the uncovered workers inherit exactly that posture (record retired, PubSub closed, eventual).