fix(rpc): Recover the congestion window after an overload backs it off - #18603
Open
zhichenxu-meta wants to merge 1 commit into
Open
fix(rpc): Recover the congestion window after an overload backs it off#18603zhichenxu-meta wants to merge 1 commit into
zhichenxu-meta wants to merge 1 commit into
Conversation
Summary:
After `onError()` halves the window down to the floor, it can get stuck there
until the backend returns to full health -- even though the control law says
it should climb back.
The update is
newWindow = w * gradient + stepCoef * sqrt(w)
so one recomputation changes the window by
`stepCoef * sqrt(w) - w * (1 - gradient)`, and the window was stored as an
`int64_t`. Decrements are large and survive truncation. Increments are often a
fraction of a unit, and truncation discarded them every window, forever.
**The case where this bites.** BATCH starts at a window of 2. A rate-limit
storm halves it to the floor. If the backend is still partially degraded when
the storm passes -- gradient 0.7, say -- then
`sqrt(2) - 2 * (1 - 0.7) = 0.81`, under one unit, so the window stays at 2
instead of recovering toward its equilibrium of 11. Throughput stays pinned at
the floor for as long as the backend stays short of healthy.
Carry the window as a `double` and report `limit()` as its floor. No new
parameters, no change to the law, no change to `limit()`'s signature or its
`[minWindow, maxWindow]` range.
**Scope, stated precisely.** Growth stalls exactly when
`stepCoef * sqrt(w) < w * (1 - gradient) + 1`.
- PER_ROW constructs the window with `start == max` (default ceiling 100). It
begins at its ceiling and never grows. Unaffected.
- BATCH on a healthy backend has gradient ~1.0, where the step is `sqrt(w)`,
always at least one unit. It already climbs to the 256 ceiling. Unaffected.
- Descending into congestion, decrements are large until the window nears
equilibrium, where truncation strands it within about one unit. Marginal.
- Climbing from the floor at a gradient between 0.5 and 0.79 is the case that
breaks, and overload recovery is how a live system gets there.
**What is not claimed.** I could not reproduce the stalling regime on live
traffic. There is no ODS series for the window -- `RpcMetrics` exports
requests, errors, retries, latency and row counts, but no controller state --
and the runtime stats are per-query only. A scaling experiment on a verifier
cluster, BATCH mode with 4, 8 and 16 batches, gave flat wall times of 50s, 55s
and 53s: healthy traffic grows normally there, as the analysis predicts. The
stalling regime needs a backend held at moderate congestion, which that
cluster does not reproduce.
Lands alone: two files plus tests, no callers affected.
Differential Revision: D116798431
✅ Deploy Preview for meta-velox canceled.
|
Contributor
|
@zhichenxu-meta has exported this pull request. If you are a Meta employee, you can view the originating Diff in D116798431. |
Selective Build Plan
Affected targets (7)Directly changed (6)
Transitively affected (1)
Fast path • Graph from main@ffd016a0fa4c |
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.
Summary:
After
onError()halves the window down to the floor, it can get stuck thereuntil the backend returns to full health -- even though the control law says
it should climb back.
The update is
so one recomputation changes the window by
stepCoef * sqrt(w) - w * (1 - gradient), and the window was stored as anint64_t. Decrements are large and survive truncation. Increments are often afraction of a unit, and truncation discarded them every window, forever.
The case where this bites. BATCH starts at a window of 2. A rate-limit
storm halves it to the floor. If the backend is still partially degraded when
the storm passes -- gradient 0.7, say -- then
sqrt(2) - 2 * (1 - 0.7) = 0.81, under one unit, so the window stays at 2instead of recovering toward its equilibrium of 11. Throughput stays pinned at
the floor for as long as the backend stays short of healthy.
Carry the window as a
doubleand reportlimit()as its floor. No newparameters, no change to the law, no change to
limit()'s signature or its[minWindow, maxWindow]range.Scope, stated precisely. Growth stalls exactly when
stepCoef * sqrt(w) < w * (1 - gradient) + 1.start == max(default ceiling 100). Itbegins at its ceiling and never grows. Unaffected.
sqrt(w),always at least one unit. It already climbs to the 256 ceiling. Unaffected.
equilibrium, where truncation strands it within about one unit. Marginal.
breaks, and overload recovery is how a live system gets there.
What is not claimed. I could not reproduce the stalling regime on live
traffic. There is no ODS series for the window --
RpcMetricsexportsrequests, errors, retries, latency and row counts, but no controller state --
and the runtime stats are per-query only. A scaling experiment on a verifier
cluster, BATCH mode with 4, 8 and 16 batches, gave flat wall times of 50s, 55s
and 53s: healthy traffic grows normally there, as the analysis predicts. The
stalling regime needs a backend held at moderate congestion, which that
cluster does not reproduce.
Lands alone: two files plus tests, no callers affected.
Differential Revision: D116798431