Skip to content

Commit 7b761b0

Browse files
ggevayclaude
andcommitted
persist: record the parameter-tuning discussions in the design doc
Capture the warm-interval analysis (the purge-window mechanism, why abort-and-restart warming probes better than a patient handshake, and why the interval stays at 20 seconds pending evidence), the tuning trade-offs of the remaining parameters (the delay's rescue floor vs its false-fire population, the bucket-capacity sizing rationale, the dead-socket slot-pinning interaction), and open Alternatives with the naive options (the primary's own pool, just lowering the timeouts), each alternative in its own subsection. Calibrate the pool-isolation motivation to the evidence along the way, in the doc and the hedge.rs module doc: same-pool traffic mostly survived the observed events, so the case for isolation is the correlated residual plus what a shared pool forecloses, not blanket fate sharing. Also retire the informal "retry ladder" term in favor of naming retry_external. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d0c0bd4 commit 7b761b0

2 files changed

Lines changed: 112 additions & 14 deletions

File tree

doc/developer/design/20260806_hedged_blob_gets.md

Lines changed: 108 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ succeeds first.
1313
- Cap the freshness impact of the dead-connection failure class (described
1414
below) at roughly the hedge delay plus one normal get, whenever a healthy
1515
connection is reachable. When it is not (both requests stall), behavior
16-
falls back to today's, with the existing retry ladder as the backstop.
16+
falls back to today's, with persist's existing retry-with-backoff
17+
machinery (`retry_external`, described in The Problem) as the backstop.
1718
- Change nothing about persist's semantics, error surface, or write paths.
1819

1920
## Non-Goals
@@ -112,8 +113,8 @@ starts immediately and races a timer set to the configured delay. If the
112113
primary completes first (the full blob, not just its first byte: `Blob::get`
113114
returns a complete result, so partial progress is invisible to the
114115
wrapper), its result is returned verbatim, success or error.
115-
A fast error therefore still takes today's path into the `retry_external`
116-
ladder. Hedging targets hangs, not failures.
116+
A fast error therefore still takes today's path into `retry_external`'s
117+
retries. Hedging targets hangs, not failures.
117118

118119
If the delay elapses, the hedge leg fires on the sibling, subject to the
119120
two admission guards described under Bounding amplification below. If the
@@ -128,7 +129,7 @@ error. The two error cases are asymmetric. If the hedge leg errors, the
128129
get keeps waiting on the primary alone. If the primary errors after the
129130
hedge fired, the get waits a delay-sized grace window for the hedge and
130131
then returns the primary's error, so a slow hedge cannot hold the get past
131-
the point where handing the error to the retry ladder is the better move.
132+
the point where handing the error to `retry_external` is the better move.
132133
Either way, when the get does fail, the caller sees the primary's error
133134
object, unchanged. This matters because `ExternalError::is_timeout`
134135
matches on the error string, so even attaching the hedge's error as context
@@ -170,9 +171,10 @@ timer that fires late is a hedge that does not fire.
170171

171172
### Pool isolation
172173

173-
The hedge is only useful if it cannot be assigned the same dying
174-
connection, so the sibling must not share connection state with the
175-
primary. Three facts make that work.
174+
The sibling shares no connection state with the primary, so a hedge
175+
cannot be handed a connection dying in the same event that stalled the
176+
primary. The full case for isolation, including what a shared pool
177+
forecloses, is under Alternatives. Three facts make the isolation work.
176178

177179
First, isolation cannot come from cloning: the AWS SDK client embedded in
178180
`S3BlobConfig` is reference-counted, so cloning the config shares the HTTP
@@ -230,6 +232,40 @@ counter and leaves the gauge alone, so a fast-failing sibling (for
230232
example, one whose credentials rotted) cannot masquerade as a fast healthy
231233
one.
232234

235+
Because the killed ping closes its socket, the warm interval also bounds
236+
how long a dead sibling socket can linger in the pool undetected: about
237+
two intervals (up to one until a ping lands on it, one more until the
238+
cycle timeout kills it). That window is what decides whether hedging
239+
survives a correlated event that hits the sibling's own sockets. Until
240+
the purge, a hedge can check out the dead socket and hang until the read
241+
timeout while contributing nothing, so at the current interval hedging
242+
protects against such an event only if the sibling happened to escape
243+
it. Such a hedge also holds its concurrency slot for the whole hang, so
244+
the event's first hedges can pin both default slots for up to the read
245+
timeout, leaving later gets unhedged with
246+
`hedges_skipped{reason="concurrency"}` climbing. A single-digit interval would shrink the exposure to a few seconds
247+
and let the pool heal within the event itself. The healing loop is also
248+
a better prober than one patient handshake: each restarted attempt
249+
resets TCP's exponential SYN backoff, probing the path about once a
250+
second instead of ever more sparsely, and performs a fresh DNS
251+
resolution, which can retarget a rotated S3 front-end address once the
252+
record's short TTL lapses. The 2023-era caution against aggressive
253+
connect timeouts does not apply here, because a warming handshake has no
254+
request waiting on it, so aborting a viable-but-slow attempt costs
255+
nothing.
256+
257+
The interval nevertheless stays at 20 seconds, as a cost decision. At 20
258+
seconds the warmer's requests are a modest fraction of the fleet's
259+
organic blob gets and their request cost is negligible. At a few seconds
260+
the warmer would rival or exceed the fleet's entire organic blob-get
261+
request volume, a spend increase that deserves a deliberate decision
262+
backed by evidence rather than a default. If enabled hedging still
263+
leaves correlated events visible in freshness, with hedges erroring on
264+
dead sibling sockets, concurrency skips clustering at event times, and
265+
the warm-error counter spiking alongside, that is the evidence that the
266+
interval is too coarse, and it is a dyncfg, so it can be lowered at
267+
runtime without a release.
268+
233269
The warmer runs only while hedging is enabled, re-checking the flag at its
234270
normal cadence while idle. While hedging is disabled the sibling is
235271
therefore fully idle: no requests, no credential refreshes (SDK providers
@@ -263,7 +299,11 @@ request rate onto an already-degraded dependency. The same guard keeps
263299
large gets that legitimately exceed the delay (a 128 MiB part on a
264300
bandwidth-constrained pod) from settling into permanent double egress.
265301
The bucket starts full so that low-traffic processes can still hedge the
266-
rare event that motivates the feature. The known blind spot is the mirror
302+
rare event that motivates the feature. The capacity is also sized
303+
against the events themselves: the observed damage mechanism is a few
304+
hung gets on hot paths starving everything downstream, so one event
305+
needs a handful of rescues per process, and a full bucket of 32 is an
306+
order of magnitude above that. The known blind spot is the mirror
267307
of the protection: in a window where more than one percent of gets are
268308
legitimately slow, the drained bucket will also refuse the occasional
269309
genuine dead-connection hang, visible as `hedges_skipped{reason="budget"}`.
@@ -285,6 +325,20 @@ than an operational lever, and fewer knobs means less to review and fewer
285325
LaunchDarkly entries. Note that `budget_ratio = 0` is not a kill switch,
286326
because the bucket starts full. `enabled` is the kill switch for hedging.
287327

328+
The delay is the knob most tempting to retune, so the trade is worth
329+
recording. Lowering it moves the rescue floor (a hedge win arrives no
330+
earlier than the delay plus one round trip) but grows the false-fire
331+
population: the race triggers on full-blob completion, not first byte,
332+
so the gets that legitimately outlast the delay are large parts on
333+
constrained bandwidth, and each false fire both duplicates that part's
334+
egress and spends a budget token, so a lower delay can drain the bucket
335+
with healthy traffic and leave a real event refused. Raising it has a
336+
wide plateau in the other direction: a rescue stays useful as long as
337+
the delay plus one normal get fits the freshness target, which holds up
338+
to several seconds. Any retune should start by re-measuring the
339+
would-be fire rate at the candidate delay (the rate in Rollout is
340+
workload-dependent, dominated by part sizes and pod bandwidth).
341+
288342
Honesty about what the kill switch covers: `enabled = false` stops both
289343
hedge requests and the warmer, leaving the sibling fully idle, but the
290344
sibling client and its credential chain are constructed at process start
@@ -395,9 +449,44 @@ process start.
395449

396450
## Alternatives
397451

398-
Lowering `persist_blob_read_timeout` does not cover the class: as The
399-
Problem notes, it is first-byte-only, and the characterized connections
400-
died before any timeout fired at all.
452+
### Hedging on the primary's pool
453+
454+
Hedging on the primary's own connection pool would need no sibling
455+
machinery at all, and in the dominant event shape (one connection dies
456+
while its pool-mates stay healthy, which is why the same-pool retry
457+
succeeds promptly today) it would usually work. The case for isolation
458+
is the residual minority of correlated, path-scoped events, where we
459+
observed kills clustering within a second on one pod and fresh
460+
connects stalling fleet-wide: there, a shared pool is most likely to
461+
hand the hedge a sick connection exactly when the hedge matters most.
462+
A shared pool also forecloses levers that matter regardless of fate
463+
sharing: it pins the hedge to the pool's cached DNS answer and
464+
front-end address, lets a checkout queue behind the very jam the hung
465+
get is part of, and cannot be kept warm without distorting the
466+
primary's pool. Isolation makes the hedge's independence structural in
467+
precisely the worst events, and the mid-event evidence in The Problem
468+
(most collections on the same pod stayed fast while one hung) shows
469+
independent healthy paths exist to be reached.
470+
471+
### Just lowering the client timeouts
472+
473+
Lowering the client timeouts instead of hedging was the 2023 answer to
474+
this class (the connect and read timeouts were cut from 30 and 60
475+
seconds to today's 7 and 10), and the residual hang is what that lever
476+
left behind. Tightening further runs into structural limits. The read
477+
timeout is first-byte-only and the characterized connections died
478+
before any timeout fired at all, and a mid-body hang is bounded only by
479+
the 90 second attempt timeout, which cannot come down to freshness
480+
timescales without killing every legitimately long large-part fetch. A
481+
timeout is also a sequential remedy with a forced trade: it converts a
482+
possibly-about-to-succeed request into an error, pays the backoff and
483+
the restart, and retries on the same pool with no fresh connection or
484+
DNS resolution guaranteed, so its threshold must stay conservative. A
485+
hedge is concurrent: the primary keeps running, so firing early costs a
486+
bounded duplicate rather than a lost success, which is why hedging can
487+
afford a 2 second trigger where a 2 second timeout could not.
488+
489+
### Per-chunk hedging
401490

402491
Hedging per multipart chunk inside `S3Blob::get` is a possible refinement:
403492
`S3Blob::get` already fetches large blobs in 8 MiB parts, so a per-chunk
@@ -406,21 +495,29 @@ on missing first bytes rather than wall clock. It is S3-only, duplicates
406495
the race logic per backend, and is a natural follow-up once the generic
407496
version has proven itself, not the first version.
408497

498+
### A pool-less sibling
499+
409500
Disabling pooling on the sibling client (max idle connections of zero)
410501
would guarantee isolation and fresh DNS with no warmer machinery, but it
411502
puts a full TCP and TLS handshake on the critical path of every hedge,
412503
which is up to 7 seconds during exactly the correlated events the feature
413504
targets.
414505

506+
### A both-legs debug mode
507+
415508
A debug mode that awaits both legs and asserts they agree was rejected as
416509
unsound, not merely unnecessary: as Correctness explains, two gets of the
417510
same key are only required to agree for keys protected by a live lease, so
418511
the assertion would misfire on legitimate delete races.
419512

513+
### A hedging-aware Blob trait
514+
420515
Extending the `Blob` trait with a hedging-aware method was rejected because
421516
it pushes a transport concern into a six-method correctness-critical trait
422517
that most implementations have no answer for.
423518

519+
### Hedging in maelstrom
520+
424521
A configuration for maelstrom (persist's Jepsen-style linearizability test
425522
harness) was considered and deferred. Maelstrom's blob is neither S3 nor
426523
Azure, so its sibling would be the same instance, and `UnreliableBlob`

src/persist/src/hedge.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@
2525
//! streaming interface. Extending hedging to any of them is forbidden.
2626
//!
2727
//! The hedge handle must not share a connection pool (or DNS state) with the
28-
//! primary, otherwise the hedge can be assigned the very connection that is
29-
//! dying. See [crate::cfg::open_hedge_sibling] for how that isolation is
30-
//! constructed per backend.
28+
//! primary, otherwise the hedge can be handed a connection dying in the same
29+
//! event that stalled the primary, exactly when a hedge matters most. See
30+
//! [crate::cfg::open_hedge_sibling] for how that isolation is constructed
31+
//! per backend.
3132
//!
3233
//! Hedging operates within a single `retry_external` attempt, before any
3334
//! failure surfaces. The retrying in `retry_external`, which is what

0 commit comments

Comments
 (0)