runtime-next: bound status trailers so connector errors aren't masked by ResourceExhausted - #3356
Open
jacobmarble wants to merge 4 commits into
Open
runtime-next: bound status trailers so connector errors aren't masked by ResourceExhausted#3356jacobmarble wants to merge 4 commits into
jacobmarble wants to merge 4 commits into
Conversation
`MAX_STATUS_MESSAGE_LEN` and its helpers live in `runtime-next`, but three other crates map errors onto the same wire and need the same ceiling. Move them to `proto-grpc`, which each of those crates already depends on, and re-export from `runtime-next` so callers are unchanged. No behavior change: `runtime-next` bounds exactly what it bounded before.
Both crates carried their own copy of `anyhow_to_status` which formatted an error chain into a status message of unbounded length. A gRPC status rides in an HTTP/2 trailer, and an oversized trailer is not merely truncated: it spills across more CONTINUATION frames than `h2` allows, which tears down the connection and replaces the status text with an opaque transport error. Use the shared bounded implementation, as `runtime-next` already did.
When a connector exits non-zero we report its final stderr log as the RPC's terminal Status: the log's message as `grpc-message`, and the whole encoded `ops::Log` again as `last-log-bin` metadata. Neither was bounded. That status is the sole carrier of the connector's error, and it travels entirely in an HTTP/2 trailer. hyper pins `max_header_list_size` to 16 KiB, which gives `h2` a budget of five CONTINUATION frames; past that it trips `too_many_continuations` and answers with a connection-level GOAWAY(ENHANCE_YOUR_CALM). Every stream on the connection then fails with an opaque `ResourceExhausted: h2 protocol error` — so a connector which prints a large diagnostic as it dies destroys the message explaining why it died. Bound the log before it reaches the wire. The budget is small enough that even worst-case encoding (3x percent-encoding of the message, 4/3 base64 of the metadata) fits a single frame, needing no CONTINUATION frame at all. Part of #3353.
…ilure A shard which fails takes its leader's session down with it, and the leader broadcasts that failure to every shard of the task. Two things then went wrong with what an operator sees. The leader forwarded the failed peer's `Status` verbatim. A `Status` describes the stream it arrived on, not the streams it's relayed to, so one shard's transport error became every shard's reported cause — and its metadata rode along, so an oversized trailer that had already broken one connection was relayed onto every other. Format the failure into a message instead, which names the peer that failed and carries no metadata. The broadcast also returns to the shard whose connector caused it. The `biased` select prefers the connector arm, but only if the loop polls again; if the leader's echo lands first, a shard reports the echo of its own failure rather than the connector error behind it. Prefer a connector error which is already ready. Part of #3353.
jacobmarble
force-pushed
the
jgm-resource-exhausted
branch
from
August 13, 2026 15:44
5ed95d3 to
ea39819
Compare
jacobmarble
marked this pull request as ready for review
August 13, 2026 15:51
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
1 participant
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.
Description:
Fixes #3353. When a materialization connector died, shards often reported
ResourceExhausted: h2 protocol error: http2 errorinstead of the connector'sactual error — which for a recent data-loss bug meant an operator had to know to
go read the connector's own logs to find the real cause.
Here's the mechanism. A gRPC error status travels entirely in an HTTP/2 trailer
(a header block sent after the response body). hyper caps how big a header block
can be at 16 KiB, which lets h2 spread one across at most 5 "continuation"
frames. Go past that and h2 assumes it's under attack: it kills the whole TCP
connection with an
ENHANCE_YOUR_CALMerror, which tonic reports asResourceExhausted. Every stream on that connection dies, and the status textthat was in flight is lost.
connector-init— which runs inside the connector container and reports theconnector's exit — built that status with no size limit at all: the connector's
last stderr line as the message, plus the whole encoded log again as binary
metadata. A connector that prints a big diagnostic as it dies therefore destroys
the very message explaining why it died.
Three changes:
connector-init). Truncate the finallog message, drop oversized structured fields, and bound the metadata copy.
Budget is 2 KiB raw, so even worst-case encoding fits one 16 KiB frame with no
continuation frame at all.
session fails, the leader used to forward the failed peer's status object
verbatim to every shard — so a single shard's transport error became every
shard's reported cause, and an oversized trailer got relayed onto more
connections. It now sends a formatted
leader session failed: …message,which names the peer that actually failed and carries no metadata.
connector fails, the leader's broadcast comes back to that same shard. If the
leader's copy lands first, the shard now checks whether its connector already
has an error waiting and reports that instead.
Also bounded the two remaining copies of
anyhow_to_status(inruntimeandshuffle) that had the same gap. The bounding helpers moved toproto-grpcsoall four crates share one copy instead of three drifting ones.
Workflow steps:
No user-facing workflow change. Shard failures in
flowctl catalog statusand intask alerts should now name the connector's own error rather than a transport
error.
Documentation links affected:
None.
Notes for reviewers:
connector-inithalf ships in an image. It needs aflow-connector-initrelease plus a bump of the inline pin before it takeseffect in production. The runtime-side changes here protect tasks still running
older images, since they stop the oversized trailer from being relayed onward.
last-log-binmetadata — only connector-init'sown test. I bounded it rather than deleting it, per discussion.
prefer_connector_errordiscards a ready connector response on the errorpath. The session is failing either way; the only question is which error
describes why.
alone: an asymmetric h2 window setting in
gazette(connection window raised,stream window at default, which disables adaptive windowing); the full built
spec crossing five gRPC hops inline at session start; several clients still at
tonic's 4 MB decode default; and vendored gazette's
MaxGRPCRecvSizesitting atits 4 MiB default. Happy to open issues for these.
arithmetic, the truncation, an end-to-end pass through
bidiwith a largestderr blob, and the connector-vs-leader error preference.