retry: consolidate RTA and MPSD backoff - #44
Open
HashimTheArab wants to merge 6 commits into
Open
HashimTheArab wants to merge 6 commits into
HashimTheArab wants to merge 6 commits into
Conversation
A dropped RTA WebSocket got four redials about twenty seconds apart and then the Conn closed itself with "max reconnect attempt reached" as the cause. A service outage lasts longer than that, so every long-lived Conn ended the outage dead: each later Subscribe returned the cached cause and nothing short of building a new Conn recovered. The same budget applied to a socket that kept dropping mid-handshake. Reconnect now retries with capped exponential backoff (1s doubling to 60s, 50% jitter) until the dial lands or the Conn is closed, and an interrupted resubscribe round backs off and goes again instead of counting toward a limit. Subscriptions stay owned by the reconnect the whole time; if the Conn closes underneath it they get the close cause. Tests drive an outage through a server that refuses upgrades and a handshake the server keeps cutting, and cover Close during an outage. (cherry picked from commit 7ab4616)
…d Close If Close ran while a resubscribe handshake was in flight, its deactivation loop saw an empty map: the reconnect had taken the subscriptions, and the successful handshake then tracked an active subscription back onto the closed Conn with no HandleError. The reconnect now finishes that deactivation when it sees the Conn closed. Also: only the first failed redial logs at Error, later ones at Warn, so a long outage is not an Error stream; and tests shorten the backoff through a package variable set before Dial instead of writing to a live dialer. (cherry picked from commit 6ebe74f)
A dial that completed just as Close swept c.conn could be stored after the sweep, leaving an open socket behind a closed Conn. The reconnect now publishes under connMu with a ctx check, so either Close sees the socket or the reconnect sees the cancelled ctx and closes it itself.
Contributor
Author
|
ill update this pr after #42 is merged |
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.
RTA redial, interrupted resubscribe, and MPSD join each maintained their own retry loop and waits. This consolidates those timed retries on
github.com/cenkalti/backoff/v7, with each operation retaining its own retry rules.Two small cancellation differences accompany the refactor: when Join reaches the retry operation with an already-canceled context, it returns the context error directly before constructing an HTTP request, rather than going through http.Client and its error wrapper. RTA similarly skips already-canceled dial attempts, and the retry notifier does not log a failed attempt when cancellation has already ended recovery. Retry eligibility, attempt limits, and delay ranges are otherwise preserved relative to #42.
The public Subscribe loop still waits on the reconnect gate. Social's Retry-After metadata, authentication invalidation, and OAuth device polling retain their existing behavior. This does not add automatic retries to session writes or other HTTP operations; Microsoft documents that session writes can have side effects when repeated after an uncertain failure: calling guidance.
Depends on #42 and should be merged after it. The current diff includes that prerequisite; the follow-up itself is 0ec19f6.
Validation:
go test -race ./rta ./mpsd,go test ./...,go vet ./..., andstaticcheck ./...pass. Added coverage for long reconnect waits, cancellation, independent schedules, ETag fallback, request-body replay, response cleanup, and errors that must not trigger another write. An independent code review andcodex review --base upstream/mainusing Astra with low reasoning found no actionable issues.Implemented and tested with Codex assistance.