CS-1992: Skip claim race fix - #123
Open
alexlivekit wants to merge 1 commit into
Open
Conversation
erikhortsch
added a commit
that referenced
this pull request
Aug 21, 2026
CS-1992: a handler that errors on receipt publishes its response right behind the announcement, on a different channel, and nothing orders their delivery. When the response won the race, selection stashed it as a fallback and the announcement then sent the caller off to wait on a channel already drained, turning an instant error into a request timeout. The stash exists for broadcast, where an early error is one server rejecting a request it could not read and another may yet bid. On a queue rpc the responder is the only server that received the request, so nothing else can ever arrive: return the response the moment it appears. This also stops a queue caller from waiting out the selection timeout to surface a malformed-request rejection, the case with no announcement at all. Reported in #123, which surfaced the stashed error once the announcement arrived; resolving it at the response site keeps selection from consuming the answer in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
erikhortsch
added a commit
that referenced
this pull request
Aug 24, 2026
* server: let a server skip the claim for every queue rpc it handles WithClientSkipClaim only reaches clients built through ClientParams.Options(). Constructors that hand-pick options off the params struct drop it silently, and some of those live in other repos, so the caller-side switch cannot cover the fleet. The skipped round trip is server->client->server before the handler runs, so the server already holds the decision; the request bit is only authorization. Let the server elect it too, for every queue rpc it handles. Queue is still re-checked, and the outcome is still observed as ClaimSkipped. Takes func() bool rather than bool to keep the same runtime revocability as the client option. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * client: advertise the skip instead of electing it Two switches for one decision left the caller able to force an announcement at a server that never opted in, and put the compatibility risk on the wrong end: a server electing to skip would announce to callers predating the field. Make the request bit an advertisement -- always set on a queue rpc, since a caller built from this version can always accept an announcement -- and let the server alone decide whether to make one. Both ends must now agree, so an older caller is never sent an announcement it cannot read, and WithClientSkipClaim has nothing left to configure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * client: a queue response is the answer, even when it is an error CS-1992: a handler that errors on receipt publishes its response right behind the announcement, on a different channel, and nothing orders their delivery. When the response won the race, selection stashed it as a fallback and the announcement then sent the caller off to wait on a channel already drained, turning an instant error into a request timeout. The stash exists for broadcast, where an early error is one server rejecting a request it could not read and another may yet bid. On a queue rpc the responder is the only server that received the request, so nothing else can ever arrive: return the response the moment it appears. This also stops a queue caller from waiting out the selection timeout to surface a malformed-request rejection, the case with no announcement at all. Reported in #123, which surfaced the stashed error once the announcement arrived; resolving it at the response site keeps selection from consuming the answer in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * proto: move the advertisement off the election's field number Field 9 carried the caller's election of the skip, which pre-election servers honor unconditionally. A caller that advertises on the same number would switch those servers on for every queue rpc, with no opt-in and no kill switch, since they have no server-side election to revoke. At 10, the advertisement is invisible to every server that predates it: they see field 9 absent and negotiate. The reverse skew already degraded safely -- an old caller electing on 9 is simply negotiated with. Field 9 is reserved so it cannot come back meaning something else. Also catches pkg/client tests up to selectServer's queue parameter, which the previous commit missed, and covers the queue/broadcast split directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: cut every comment down to the fact it carries reserved 9 stays: protoc rejects any later field reusing the number, so it is enforcement, not documentation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <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.
No description provided.