bazel l1 [fork 5/5]: MinIO edge — retry cap, read deadline, per-backend breaker by shreyas-blacksmith · Pull Request #31 · useblacksmith/bazel-remote

shreyas-blacksmith · 2026-07-30T17:55:00Z

Piece 5 of 5 peeled off shreyas/bazel-l1-goal-state (design record: FastActions/fa#4779, ledger 25). Base: piece 4. Three commits (275141e, 5052a57, 51fe73e).

Summary

3-attempt cap on minio-go's internal retries — surfaces backend failures in seconds instead of ~62s of SDK retrying per call.
5s context deadline on miss fall-through GETs — Bazel treats any read error as a miss, so the honest answer from a sick MinIO is a fast miss, not a patient stall (pre-fix: 60–220s per cold miss under black-hole).
Hand-rolled three-state circuit breaker, one per backend (~200 lines, no new dependency — sony/gobreaker was tried and rejected for module-graph ripple into the FA agent): trips on 5 consecutive failures, open 15s, single half-open probe. Open ⇒ reads return not-found immediately; queued uploads fail fast with a distinct breaker_open outcome (no FD held through retries). Timeouts count as failures, clean misses as successes, client cancellations as neither. State gauge + transition counter + log lines.

Evidence

Live black-hole drill on staging 2026-07-30 (this exact head): trip at exactly 5 × ~5.01s-bounded misses, ~7ms fail-fast while open, one-probe cycle under sustained outage, closed ~100ms after egress restore; the sibling backend on the same node never left closed — isolation is per-backend, even intra-node.

Made with Cursor

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled. (Staging)}

piob-io · 2026-07-31T18:38:41Z

The 5s deadline feels too short for me. At least let's make it configurable via live config, so we can raise it quickly (we have been using etcd for that for non-agent services). But maybe the default should be higher. 30 seconds? 60 seconds?

shreyas-blacksmith · 2026-07-31T19:23:35Z

@piob-io split the two semantics rather than raising the one number: connection establishment is a constant 10s at the transport layer; that's the black-hole fail-fast and what feeds the breaker. The overall deadline is 5m and configurable via s3_proxy.read_timeout (changed to support large CAS objects).

Live config is declined for now since bazel-remote carries no etcd machinery and an L1 restart is cheap. This is recorded as a fast-follow candidate in ledger 28. Happy to discuss if we want live-reconfig for v1.

piob-io · 2026-07-31T20:30:41Z

breakerReadOutcome is recorded when GetObject returns — i.e. at TTFB. The body then streams under the readDeadline via cancelReadCloser, but its terminal state never reaches the breaker. In the dominant brownout mode (connections open fine, bytes don't flow), every fall-through read records outcomeSuccess, gets reclaimed at the 5s mark, and degrades to a miss in the disk layer — so the breaker never trips and the fleet pays a full deadline per read indefinitely, instead of flipping to fast-miss mode. The fix is small and the pieces already exist: extend cancelReadCloser to report the body's terminal state to the breaker (error/deadline expiry before clean EOF → one outcomeFailure; clean completion → success or nothing). The breaker already handles straggler outcomes gracefully, so a late body-failure just increments the closed-state streak — exactly what's wanted.

piob-io · 2026-07-31T20:31:01Z

The breaker's metrics name is spec.Endpoint+"/"+spec.Bucket, but the earlier piece deliberately settled the backend label as the backends-map key (c.key, with endpoint fallback) across upload outcomes, hits/misses, and queue metrics. As-is, bazel_remote_s3_breaker_state{backend="10.x.y.z:9000/bucket"} won't join with ..._upload_outcomes_total{backend="http://staging-minio…:9000"} on dashboards. Suggest naming the breaker with the same key (bucket isn't a backend identity anyway — it's per-request since the multi-backend piece).

piob-io

Neat!

minio-go's library default is MaxRetry = 10 attempts with exponential backoff, and bazel-remote configures no retry policy of its own on the S3 path — against a sick MinIO backend that turned every failing queued PUT into ~62s of retention (worker pinned, FD held) before the failure surfaced. Cap the constructed client at 3 attempts so a blip is still ridden out but a genuine failure surfaces in seconds. Applies to every backend (multi-backend map mode and single-backend mode both construct clients through newBackend). Co-authored-by: Cursor <cursoragent@cursor.com>

We measured reads against a wedged MinIO node pinned for ~220s, each retaining a file descriptor and a pooled connection the whole time: nothing bounded how long a fall-through Get or a Contains probe could wait on a sick backend. Give every read-path MinIO call a 5s context deadline — a healthy LAN backend answers in milliseconds; one that cannot answer in 5s should fail the call, and the disk cache layer treats that like any other backend error. The deadline context also governs the streamed GET body: the timer is released when the caller closes the body (cancelReadCloser), so a transfer stalled mid-stream is reclaimed too instead of holding its connection open indefinitely. The deadline is a package-level var only so tests can shrink it, mirroring uploadTimeout. Co-authored-by: Cursor <cursoragent@cursor.com>

Even with capped retries and a read deadline, every request against a known-sick MinIO backend still pays the full failure price (and holds an FD while doing so). Put a circuit breaker on every MinIO call, one breaker per backend so in multi-backend map mode a sick shard trips only its own breaker and cannot blind healthy shards. The breaker is hand-rolled (cache/s3proxy/breaker.go: a mutex, a counter, a timestamp — no goroutines, no timers), modeled on the FA agent's failureBreaker precedent rather than pulling in a library: this module is embedded in the FA agent, and an extra module in the fork's go.mod ripples into the agent's module graph. Policy: 5 consecutive failures open the breaker — consecutive-only, no failure-ratio window, since the 5s read deadline already bounds what a brownout costs per request and consecutive-only matches the agent precedent. Open lasts 15s; then half-open admits exactly ONE probe: success closes and resets, failure re-opens for another fixed 15s (no exponential backoff — a cache edge wants fast re-probe, and a wasted probe costs one request, not a job). Read path (miss fall-through Get, Contains): when the breaker is open the proxy answers with the existing miss convention immediately — a miss is the honest answer the disk cache layer can act on; an error would surface to the client. A genuine not-found counts as breaker success (a miss means MinIO answered), a read-deadline expiry counts as failure, and a parent-context cancellation (the client went away) is counted neither way. Write path (queued upload workers): PUTs are breaker-wrapped with no extra deadline (large CAS blobs may legitimately be slow; the existing 10m uploadTimeout still reclaims workers). When open, items fail fast without dialing MinIO and are classified in bazel_remote_s3_upload_outcomes_total and the OperationObserver as status="error", reason="breaker_open". A precondition failure (already_exists) counts as breaker success. New metrics: bazel_remote_s3_breaker_state{backend} (0=closed, 1=half-open, 2=open) and bazel_remote_s3_breaker_transitions_total{backend,to}; every transition also logs a line through the proxy's errorLogger. Co-authored-by: Cursor <cursoragent@cursor.com>

… bound Review (fork 5/5, Piotr): the single 5s deadline conflated two things — failing fast on a dead backend, and bounding a healthy backend's body stream. A large CAS fall-through read could be guillotined mid-stream on a perfectly healthy MinIO; raising the one knob to 60s would instead have re-introduced the 60-220s miss stalls the drills measured. Now: connection establishment (dial, TLS, response headers) is bounded by a constant 10s connectTimeout at the transport layer — that is where the black-hole fail-fast lives — while the overall per-call deadline (which the streamed body rides) defaults to 5m, mirroring uploadTimeout's large-blob reasoning, and is configurable via s3_proxy.read_timeout / WithReadDeadline. Co-authored-by: Cursor <cursoragent@cursor.com>

… metrics Post-approval review feedback on the MinIO-edge piece (Piotr + bugbot): - Streamed GETs now defer their breaker outcome to the body's terminal state (bodyOutcomeReadCloser). Recording at time-to-first-byte meant the dominant brownout mode -- connections open fine, bytes don't flow -- could never trip the breaker: every read recorded a header success that reset the streak, then degraded to a miss at the deadline, so the fleet paid a full deadline per read indefinitely. Clean EOF is a success, a mid-stream error (including our own deadline) with the caller still interested is a failure, caller-side abandonment counts neither way. The half-open probe slot is held until the probe's body terminates. - Write-through uploads use ExecuteNoProbe: a PutObject bounded only by the 10m uploadTimeout must not become the half-open recovery probe, or a recovered MinIO could keep serving artificial misses for the whole upload. Uploads fail fast unless the breaker is closed; probing is the read path's job (bounded in seconds, arrives constantly). - The breaker's "backend" metric label is now the backends-map key (endpoint fallback) instead of endpoint/bucket, matching every other backend-labeled series so they join on dashboards. Bucket is per-request state since the multi-backend piece, not backend identity. Co-authored-by: Cursor <cursoragent@cursor.com>

On the CAS/v2 path Get returned casblob.ExtractLogicalSize(rc) directly: on failure that yields a nil reader, so the caller never closes the bodyOutcomeReadCloser — leaking the deadline timer and, when the call was the half-open probe, wedging probeInFlight forever (a >=16-byte header with a non-positive size errors without any Read ever seeing EOF, so no outcome fires). Close both releases the timer and files the body outcome: an un-expired early close counts neither way, because a malformed stored object says nothing about backend health (bugbot, PR #31). Co-authored-by: Cursor <cursoragent@cursor.com>

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit ecb52e6. Configure here.}

cursor · 2026-08-01T00:33:11Z

+		breaker:    c.breaker,
+		parent:     ctx,
+		rctx:       rctx,
+	}


Probe holds backend for full deadline

High Severity

A half-open Get keeps probeInFlight until the streamed body finishes, and that body is governed by readDeadline (default 5m). During a brownout stall or a large CAS fill, every other read becomes an artificial miss and uploads fail with breaker_open for up to the full deadline, contradicting ExecuteNoProbe's premise that read probes are bounded in seconds.

Additional Locations (2)

cache/s3proxy/s3proxy.go#L182-L183

cache/s3proxy/breaker.go#L134-L143

^{Reviewed by Cursor Bugbot for commit ecb52e6. Configure here.}

shreyas-blacksmith force-pushed the shreyas/bazel-l1-piece-5-minio-edge branch from 51fe73e to 07237dc Compare July 30, 2026 21:22

shreyas-blacksmith force-pushed the shreyas/bazel-l1-piece-4-backend-map branch 2 times, most recently from d2c394b to 8e73e82 Compare July 31, 2026 13:55

shreyas-blacksmith force-pushed the shreyas/bazel-l1-piece-5-minio-edge branch from 07237dc to e41fea8 Compare July 31, 2026 13:55

shreyas-blacksmith marked this pull request as ready for review July 31, 2026 14:54