Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 3ae213f The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
69eff5c to
d19c32b
Compare
A retryable failure with no untried origin left used to be surfaced after a single attempt. Retry it against the same origin instead, bounded by the existing attempt count and backoff. This matches the cross-region path, which already retries both transport errors and 5xx responses.
u9g
changed the title
d19c32b to
3f51ac7
Compare
| } | ||
| next = pickNext(regions, attempted); | ||
| // With no fallback origin, a retryable failure is retried against the same host. | ||
| next = pickNext(regions, attempted) ?? current; |
There was a problem hiding this comment.
when there is no fallback origin, what will happen to a dead origin ? with this change, it will fail after 30s rather than 10, can we do something better here ?
As minimal, probably we should update the comment
There was a problem hiding this comment.
And my concern is, saying the service is already busy and return a 5xx (indicating busy traffic or whatever), with this change, we will retry 3 times, that might congest the traffic more.
There was a problem hiding this comment.
A dead origin, ie. connection refused, DNS failure, TLS reject will fail in milliseconds.
Three attempts cost only the backoff, so ~600ms total, not 30s. The 30s worst case (10 + 0.2 + 10 + 0.4 + 10 = 30.6s at the 10s default) only happens when the origin hangs until the per-attempt timeout on every attempt. That's exactly the failure mode this PR exists to fix: on 2026-09-11 the request was acknowledged at Cloudflare's edge and never reached any LiveKit system, and the client burned its full timeout with no response. The only way to recover that is to send it again, and the only signal we have is the timeout itself.
The alternative is a single shared deadline across attempts instead of a per-attempt timeout. I don't think it belongs here: requestTimeout is documented and used as per-request today, so changing it to a total budget is a semantic change to a user-facing option, and it would have to land in server-sdk-go (already merged) and python-sdks at the same time. Happy to open that separately if you want it.
There was a problem hiding this comment.
The amplification factor isn't new: FAILOVER_MAX_ATTEMPTS is 3, and cross-region failover has always retried 5xx at that same cap for .livekit.cloud hosts. This PR extends the existing 3x ceiling to single-origin hosts rather than raising it.
There was a problem hiding this comment.
But the real congestion risk you're pointing at is one neither SDK guards against today: the backoff has no jitter. It's failoverBackoffMs * 2 ** attempt here and backoffBase << attempt in Go, both fully deterministic, so under a shared 5xx event every client retries in lockstep at exactly 200ms and 600ms.
Should I add jitter to the backoff?
There was a problem hiding this comment.
Yes to jitter if that is not too complicated to do.
There was a problem hiding this comment.
btw, I wonder if we should keep 5xx retry but exclude 503 and honor Retry After, if that makes more sense.
xianshijing-lk
left a comment
xianshijing-lk
left a comment
There was a problem hiding this comment.
I approve the PR to save another iteration of review, assuming you will address the comment.
Every client that failed together retried at exactly base * 2^attempt, so a shared 5xx event produced synchronized retry waves. Draw each delay uniformly from [0, base * 2^attempt) instead.
A 503 means the server is shedding load, so blind retries add to the overload. Retry it only when the server says when via Retry-After. Any retried response that carries Retry-After waits that long instead of the jittered backoff.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if (/^\d+$/.test(value)) { | ||
| return Number(value) * 1000; |
There was a problem hiding this comment.
🟡 Long Retry-After delays trigger immediate retries
For a 503 with Retry-After: 2147484, parseRetryAfter returns a delay beyond Node's timer limit. sleep then schedules an approximately one-millisecond timer, so the request retries immediately instead of waiting.
Learn more
A Retry-After delay is measured in seconds, but JavaScript timers cannot represent every resulting millisecond duration. Node clamps a setTimeout delay exceeding 2,147,483,647 milliseconds to roughly one millisecond. Both numeric values and HTTP-dates can exceed that range, and sleep passes the delay directly to the timer.
Example: A server responds with 503 and Retry-After: 2147484. The parsed delay is 2,147,484,000 ms. The SDK retries almost immediately instead of waiting the requested delay.
Recommended fix: Bound or split long waits in sleep, and ensure parseRetryAfter rejects non-finite values. If enforcing an upper bound, define the documented retry policy explicitly rather than accidentally retrying immediately.
Was this helpful? React with 👍 or 👎 to provide feedback.
| `livekit API request to ${new URL(current).host} failed (${reason}), retrying with fallback url ${next}`, | ||
| ); | ||
| await sleep(this.failoverBackoffMs * 2 ** attempt); | ||
| await sleep(retryAfter ?? backoffMs(attempt, this.failoverBackoffMs)); |
There was a problem hiding this comment.
🟡 Region discovery extends Retry-After delays
For a project host's 503, retryAfter starts counting before region discovery. regionOrigins can take two seconds, then the full delay runs, postponing the retry beyond the requested time.
Learn more
A project host discovers fallback regions after a retryable response. The Retry-After value is converted to milliseconds before regionOrigins runs; that lookup can await fetchRegions for up to two seconds. Sleeping for the original full delay afterward counts that lookup time twice against the response's indicated retry time.
Example: A 503 includes Retry-After: 3, and region discovery takes two seconds. The next attempt begins about five seconds after the response rather than after three seconds.
Recommended fix: Record a retry deadline when the response arrives, then sleep only for the remaining time after region discovery. Keep jittered backoff for responses without Retry-After.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Why
A request to
cloud-api.livekit.iowas lost between Cloudflare and the origin on 2026-09-11: the edge acknowledged it, but no LiveKit system ever saw it, and the client waited out its full timeout. Every surrounding call succeeded, so a single retry would have recovered it.The SDK's failover loop could not help, for two reasons:
*.livekit.cloud, so acloud-api.livekit.iorequest always got exactly one attempt./settings/regionsreturns 404.What
Four commits, each with tests:
Retry the same host when a retryable failure has no fallback origin. When
pickNextfinds no untried origin, a retryable failure (a thrown fetch error or an HTTP 5xx) now re-sends the request to the same host instead of surfacing it. Transport errors and 5xx are treated alike: a transport error can also mean the server executed the request and only the response was lost, so distinguishing them buys no idempotency safety, and cross-region failover already retries 5xx. Every attempt carries the sameX-Livekit-Request-Id, so the server can dedup a replay. Max attempts and backoff are unchanged; a 4xx is still terminal.Enable retries for the LiveKit Cloud API hosts. A new
isCloudApicheck matchescloud-api.livekit.ioandcloud-api.<env>.livekit.io(case-insensitively) and joinsisCloudin the attempts policy. A Cloud API host has a single origin, so the retry loop never fetches/settings/regionsfor it and goes straight to the same-host retry; otherwise every failed attempt would also pay the discovery timeout. The existing.livekit.cloudcheck is unchanged.Add full jitter to the backoff. Each retry waited exactly
200ms * 2^attempt, so clients that failed during a shared 5xx event retried in lockstep. A newbackoffMs(attempt, baseMs)draws each delay uniformly from[0, base * 2^attempt).Retry a 503 only with
Retry-After, and honorRetry-After. A 503 means the server is shedding load, so a 503 withoutRetry-Afteris now terminal. A 503 withRetry-Afteris retried. Any retried response that carriesRetry-Afterwaits that long instead of the jittered backoff.parseRetryAfteraccepts delay-seconds and an HTTP-date (RFC 9110).Includes a patch changeset for
livekit-server-sdk.Behavior change
For cloud-api calls with a request timeout at or above
MIN_FAILOVER_TIMEOUT_SECONDS(5s, including the 10s default), a lost request or a 5xx now costs up toFAILOVER_MAX_ATTEMPTS(3) attempts with exponential backoff instead of failing after one. A truly dead or persistently erroring origin takes up to three times the per-attempt budget to surface. Requests with a shorter timeout keep getting a single attempt. For.livekit.cloudproject hosts, the same-host retry only engages once every discovered region has been tried, so their behavior is unchanged in practice.Retry delays are now random in
[0, 200ms)and[0, 400ms). A 503 withoutRetry-Afterfails after one attempt, including during cross-region failover on.livekit.cloudhosts. A 5xx withRetry-Afterwaits the indicated delay, which is controlled by the server and has no cap.Commits 1–2 mirror livekit/server-sdk-go#1002. Commits 3–4 are mirrored in livekit/python-sdks#866; server-sdk-go does not have them yet.
Testing
pnpm exec vitest --environment node runinpackages/livekit-server-sdk: 8 files, 121 tests passing, including thetest/apiintegration suite against a locallivekit/test-servercontainer (LK_TEST_SERVER_URL).New tests, each confirmed failing before its implementation commit:
TwirpRPC.test.ts: "without a fallback origin, a transport error retries the same host", "without a fallback origin, a 5xx retries the same host", and "a Cloud API host retries without consulting region discovery" (asserts zero/settings/regionsfetches).TwirpRPC.test.ts: "a 503 without Retry-After is not retried" and "a 503 with Retry-After is retried".failover.test.ts:backoffMsandparseRetryAfterunit tests.test/api/failover.test.tshelper now asks it for a 502. Those tests exercise region failover, which a 502 still triggers.failover.test.ts:cloud-api.livekit.io,cloud-api.staging.livekit.io, andCLOUD-API.LIVEKIT.IOget max attempts;cloud-api.example.comgets one.pnpm lintandprettier --checkare clean on the changed files (the 3 existing lint warnings are pre-existing and untouched).