failover: retry the same host when no fallback region exists, and cover the Cloud API hosts - #1002
Conversation
06d9061 to
4d7185e
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 2 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| // or Cloud API domain (or force is set). | ||
| func (c failoverConfig) attempts(hostname string) int { | ||
| if c.enabled && (c.force || isCloud(hostname)) { | ||
| if c.enabled && (c.force || isCloud(hostname) || isCloudAPI(hostname)) { |
There was a problem hiding this comment.
🟡 Cloud API failures trigger futile discovery
A retryable Cloud API failure runs unsupported region discovery before failover retries or returns. A nil result triggers discovery again after another transport error, adding up to four seconds.
Learn more
Cloud project hosts use /settings/regions to choose another region. Cloud API hosts have one origin and return 404 from that endpoint. Enabling their failover enters the same discovery branch after every retryable response. A failed fetch leaves regions nil, so the next transport failure fetches again. Each fetch uses the independent two-second region discovery timeout, outside the attempt timeout.
Example: A dead Cloud API origin consumes a 10-second attempt, a 2-second discovery timeout, another 10-second attempt, and another 2-second discovery timeout before its final attempt. A fast 503 still waits for the unsupported discovery request before being returned.
Recommended fix: Skip regionCache.get for isCloudAPI(req.URL.Hostname()) and proceed directly to the same-host transport-error branch. Preserve immediate return for Cloud API 5xx responses.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Is there a way to avoid this call to regions?
| func isCloudAPI(hostname string) bool { | ||
| return strings.HasPrefix(hostname, "cloud-api.") && strings.HasSuffix(hostname, ".livekit.io") | ||
| } |
There was a problem hiding this comment.
🟡 Mixed-case Cloud API hosts lose retries
A mixed-case Cloud API hostname makes isCloudAPI return false. Valid URLs such as https://CLOUD-API.LIVEKIT.IO therefore retain one attempt.
| func isCloudAPI(hostname string) bool { | |
| return strings.HasPrefix(hostname, "cloud-api.") && strings.HasSuffix(hostname, ".livekit.io") | |
| } | |
| func isCloudAPI(hostname string) bool { | |
| hostname = strings.ToLower(hostname) | |
| return strings.HasPrefix(hostname, "cloud-api.") && strings.HasSuffix(hostname, ".livekit.io") | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| nextScheme, nextHost, ok := nextRegion(regions, tried) | ||
| if !ok { | ||
| return terminate(resp, err, cancel) // no untried region left | ||
| // With no fallback region, a 5xx is final; only a transport error is |
There was a problem hiding this comment.
Why aren't 5xx retried?
There was a problem hiding this comment.
Two reasons. First, scope: the incident this fixes was a request that got no response at all, and a transport error is the only signal that the origin never saw it.
A 5xx means the origin, or traefik in front of it, processed it and answered, so a same-host retry against the same origin is much less likely to change the outcome.
Second, safety: most twirp calls here are not idempotent, and re-sending one that the server already executed and then 500'd on is a double-execution risk.
The X-Livekit-Request-Id header is meant for server-side dedup, but cloud-api doesn't dedup on it today.
This also preserves the existing contract that TestAPI_RegionDiscoveryUnreachable pins: a 5xx with no fallback is surfaced.
If you'd rather retry the proxy-class codes, 502/503/504 from traefik do mean the request most likely never reached the app, so retrying those same-host while surfacing 500 would be a reasonable middle ground. Happy to do that in this PR if you prefer it.
There was a problem hiding this comment.
I'd argue that the non idempotence issue already exists in case of transport error: the request may make its way to the server, but the response may never make it back. Unless Cloudflare synthesizes an HTTP error at the proxy level in case of timeout (before the SDK http timeout fires)?
| // or Cloud API domain (or force is set). | ||
| func (c failoverConfig) attempts(hostname string) int { | ||
| if c.enabled && (c.force || isCloud(hostname)) { | ||
| if c.enabled && (c.force || isCloud(hostname) || isCloudAPI(hostname)) { |
There was a problem hiding this comment.
Is there a way to avoid this call to regions?
4d7185e to
d64aa1c
Compare
Yes, and fixed in the latest push. |
d64aa1c to
a05ab5a
Compare
A retryable failure with no untried region used to be surfaced after a single attempt. Retry it against the same host instead, bounded by the existing attempt count and backoff. This matches the cross-region path, which already retries both transport errors and 5xx responses.
cloud-api.livekit.io has a single origin and no region list, so a lost request was surfaced after one attempt. Treat cloud-api.<env>.livekit.io like a cloud project host so the transport's same-host retry applies.
a05ab5a to
1621d8b
Compare
u9g
changed the title
2 participants
Why
A CLI call to
cloud-api.livekit.iohung for 10 seconds and timed out on 2026-09-11 at 14:44Z. The request reached Cloudflare's edge and was acknowledged there, but no LiveKit system ever saw it: traefik's per-backend counters, the OCI load balancer's metrics, and cloud-api's request spans were all clean for that minute. Every other call in the surrounding minutes succeeded. The failure was a single lost request between Cloudflare and the origin.The SDK's failover transport could not help, for two reasons:
isCloudonly matches*.livekit.cloud, so acloud-api.livekit.iorequest always got exactly one attempt./settings/regionsreturns 404.What
Two commits, each test-first:
TestAPI_RegionDiscoveryUnreachablestill holds: a region that fails deterministically is surfaced once attempts are exhausted.isCloudAPIcheck matchescloud-api.<env>.livekit.iocase-insensitively and joinsisCloudin the attempts policy. Cloud API hosts skip/settings/regionsdiscovery entirely, since they have a single origin and the endpoint 404s; otherwise every retry would pay the independent 2-second discovery timeout.isClouditself is unchanged since it also drives region-URL parsing for project hosts.The existing
minFailoverTimeoutgate still applies, so requests with a budget under 5 seconds get a single attempt as before.Behavior change
For cloud-api calls with a budget of 5 seconds or more, a lost request now costs up to three attempts with backoff. A truly dead origin takes up to three times the per-attempt budget to surface instead of one. A single lost request becomes a sub-second blip.
Cross-SDK parity
Same change in the other server SDKs: livekit/python-sdks#811 and livekit/node-sdks#723.
Testing
go test ./ -run 'Failover|PrepareContext|DialContext'passes.TestFailoverRetriesSameHostOnTransportError,TestFailoverRetriesSameHostOn5xx,TestFailoverCloudAPISkipsRegionDiscovery, plus cloud-api rows inTestFailoverAttempts. Each was confirmed failing before its implementation commit.integration_test.gocases need a running LiveKit server and fail identically onmain.