fix(client): ignore frames on forgotten streams by zaidoon1 · Pull Request #983 · hyperium/h2

zaidoon1 · 2026-10-13T16:28:27Z

When a client cancels a request, the server has usually already sent part of the response, sometimes all of it. h2 keeps locally reset streams around for a bit (50 streams for 1s by default) and silently drops their frames. Once a stream is out of that window, though, every late HEADERS or DATA frame for it gets a RST_STREAM(STREAM_CLOSED) back. Each of those counts against max_local_error_reset_streams, and that counter never goes down, so a long-lived client connection eventually sends itself GOAWAY(ENHANCE_YOUR_CALM, "too_many_internal_resets") and every request in flight on it fails.

We ran into this on the client side of a proxy with long-lived upstream connections. A downstream client gives up, we drop the upstream request, and with a slow upstream the response is usually already on the way. It's the same thing as the uv logs in #856 (44 cancelled streams turned into 1024 resets), and what bobrik pointed out there about the counter only ever going up.

With this change the client just drops HEADERS and DATA on streams it no longer tracks, like it already does for RST_STREAM and WINDOW_UPDATE. DATA still gives back its connection window, and the codec has already decoded the header block, so HPACK state stays in sync. Frames on stream IDs the client never opened are still a connection error, every other local reset still counts toward the limit, and the server side isn't touched.

RFC 9113 §5.1, on the closed state:

An endpoint MUST minimally process and then discard any frames it receives in this state. [...] An endpoint can perform this minimal processing for all streams that are in the "closed" state.

The same paragraph says "Endpoints SHOULD NOT use timers for this purpose", and §5.4.2 says frames the peer sent before it saw the RST_STREAM "can be ignored, except where they modify connection state".

Other clients I looked at:

Go's x/net/http2 drops DATA for unknown streams below nextStreamID (still giving back the flow control) and HEADERS for any stream it doesn't know. There's a TODO there about only doing it for streams it actually cancelled, which they left alone: "Tough without accumulating too much state."
nghttp2 (and so curl) ignores DATA for streams it no longer has, while still counting it toward the connection window, and as a client it ignores HEADERS on any stream that isn't idle.
Chromium ignores DATA after doing the session flow control, and ignores HEADERS ("it may just be that the stream was cancelled").
Netty does reply with STREAM_CLOSED, but its limit on sent RST_STREAMs is off by default for clients and per time window when it's on, so a Netty client doesn't end up closing its own connection over this.

On the concern from #856 about a server that keeps sending DATA on cancelled streams forever: after this change that costs the client the same as DATA on a stream it reset a moment ago. Nothing gets allocated or queued, and each discarded frame counts against the DATA frame budget the way it would on an open stream, so a flood of small frames still ends in GOAWAY(ENHANCE_YOUR_CALM, "too_many_data_frames"). As on an open stream, a final DATA frame doesn't count. A lot of servers end a response with an empty one, and counting those would hit MAX_RECV_EMPTY_DATA_FRAMES after 100 cancelled streams. The reset limit still covers everything where h2 actually has to send a reset.

I didn't try to tell streams we reset apart from streams that closed normally. That needs memory per stream, and the bounded memory h2 already has is exactly what runs out here. Go made the same call. On the client this does mean max_concurrent_reset_streams and reset_stream_duration no longer change what happens to late frames; they still matter on the server.

When a client cancels a request, the server has often already sent some or all of the response. h2 remembers locally reset streams for a while and drops their frames, but once a stream falls out of that window, each late HEADERS or DATA frame got a RST_STREAM(STREAM_CLOSED) back. Every one of those counts against max_local_error_reset_streams, which never goes down, so a long-lived client connection eventually sends itself GOAWAY(ENHANCE_YOUR_CALM, "too_many_internal_resets") and fails every request in flight on it. RFC 9113 section 5.1 lets an endpoint minimally process and discard frames on any closed stream, so the client now does that for HEADERS and DATA, like it already did for RST_STREAM and WINDOW_UPDATE. DATA still gives back its connection window and counts against the DATA frame budget the way it would on an open stream, and the codec has already decoded the header block. Frames on streams the client never opened are still a connection error, and the server side is unchanged. New test client_discards_late_frames_on_forgotten_streams fails on master with GoAway(too_many_internal_resets). Five tests that expected the STREAM_CLOSED reply now check that nothing is sent instead. client_counts_late_data_frames_on_forgotten_streams checks the budget.

nickvikeras · 2026-10-18T14:10:18Z

I need this fix as well. My use case is streaming multiple range scans back from bigtable, doing a merge-join across them, and cancelling the streams when the join has produced enough rows to fill a page. I can't know how many rows I need from the database until I have joined them, so every request ends in a cancellation. Cancellation is not an edge case.

Since I cancel every bigtable stream, the behavior this PR fixes in h2 is causing spurious fault spikes when the connection resets because the other range scans that were in-flight on those reset connections fail. This is ultimately causing my service to breach its availability SLA.

…P/2 client connections

…P/2 client connections (#28359) ## Description kv-rpc cancels Bigtable range scans as soon as a page is full. It can't know how many rows it needs until the merge-join has run, so nearly every List request ends in cancelled streams ([details](hyperium/h2#983 (comment))). h2 0.4.19 clients answer every frame that arrives late for a stream they reset and have since forgotten with another `RST_STREAM`, and count each one toward a per-connection limit (1024) that never goes down. At the limit the client closes the connection with `GOAWAY(ENHANCE_YOUR_CALM, "too_many_internal_resets")`, and every other read in flight on it fails. On testnet this caused nearly all of kv-rpc's remaining server errors after the List-scan refactor from #28308; hyperium/h2#856 is the same report from uv. hyperium/h2#983 makes clients discard HEADERS and DATA on those streams, as RFC 9113 §5.1 requires ("minimally process and then discard") and as the Go, nghttp2 and Chromium clients do. Flow control and HPACK state stay in sync, servers are unchanged, and every other reset still counts toward the limit. This pins h2 to that PR's commit (`44260b15`): h2 `master` (0.4.20 plus #977 and #978) with the fix on top. The commit is fetched from branch `ignore-late-frames-983` of a fork, `nickvikeras/h2`, so builds keep working if the PR branch is force-pushed during review. Remove the pin once an h2 release includes it. The pin changes h2 for every HTTP/2 client and server in the workspace, not only kv-rpc's Bigtable client. 0.4.20 also brings server-side changes, such as refused streams counting toward the reset limit and stricter `:status` and GOAWAY checks. ## Test plan Deployed on 1.82 (`nickv/kv-rpc-h2-983-1.82`, `f9cbebd35f`) to kv-rpc testnet and mainnet on 2026-10-18, and compared three ~23-hour windows: before the List-scan refactor from #28308 was deployed (2026-10-16), with the refactor only (the day before the pin), and with the refactor and the pin. The pinned build also picked up #28348 from the 1.82 branch (`sui-rpc-api` rendering, subscriptions, and `OUT_OF_RANGE` for pruned transaction boundaries); nothing else differs from the refactor-only build. **Testnet** | | Before refactor | Refactor only | Refactor + pin | |---|---|---|---| | Requests | 20.2M | 20.1M | 18.4M | | Server errors | 3,452 | 372 | 4 | | Availability | 99.98289% | 99.99815% | 99.99997% | | 5-minute windows with a method below 99.9% | 41 of 276 | 2 of 278 | 0 of 278 | | h2 `locally-reset streams reached limit` warnings | 1,465 | 459 | 0 | **Mainnet** | | Before refactor | Refactor only | Refactor + pin | |---|---|---|---| | Requests | 105.9M | 109.8M | 103.6M | | Server errors | 23,203 | 1,035 | 381 | | Availability | 99.97808% | 99.99905% | 99.99963% | | 5-minute windows with a method below 99.9% | 110 of 276 | 2 of 276 | 0 of 276 | | h2 `locally-reset streams reached limit` warnings | 1,285 | 7 | 0 | - Server errors are `rpc_requests` with status `internal`, `unknown`, `unavailable`, `resource-exhausted`, `deadline-exceeded` or `data-loss`, excluding health checks. A breach is a method whose availability falls below 99.9% in a non-overlapping 5-minute window. - Both refactor-only breaches on testnet were h2 resets (88 and 11 `too_many_internal_resets` errors in those windows). The two on mainnet were Bigtable closing connections (`connection closed`, `load_shed`), which this pin doesn't address, and most of mainnet's remaining errors are of that kind. - Since the deploy, neither network has logged `too_many_internal_resets`, the reset-limit warning, or `too_many_data_frames` (0.4.20's new DATA frame limit), and no pod has restarted. - Locally, the Bigtable regression test from #28352 (`cancelled_read_bursts_do_not_fail_concurrent_reads`) fails on h2 0.4.19 with 445 failed reads and passes 3 of 3 runs on the pin. h2's own test suite gives the same results on the pin as on h2 `master`. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] Indexing Framework:

…ose HTTP/2 client connections (#28361) ## Description Cherry-pick of #28359 to the 1.82 release branch. Original commit: d8ad4c5 ## Test plan This is the build running on kv-rpc testnet and mainnet since 2026-10-18 (`f9cbebd35f`, from `nickv/kv-rpc-h2-983-1.82`), except that it fetches the same h2 commit (`44260b15`) from `nickvikeras/h2` instead of `hyperium/h2`. Results are in #28359. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] Indexing Framework:

When the h2 client cancels a stream, the server may already have sent the response. Once h2 forgot the reset stream, it answered each late frame with RST_STREAM(STREAM_CLOSED), and those resets counted against a per-connection lifetime limit of 1024. A long-lived upstream connection that cancels many in-flight requests therefore closed itself with GOAWAY(ENHANCE_YOUR_CALM, "too_many_internal_resets") and failed every request in flight on it. An h2 client that discards those frames, as RFC 9113 section 5.1 allows, keeps the connection (hyperium/h2#983). The new test keeps one request in flight while the client cancels 21 rounds of 100 requests that a raw server had already answered; with released h2 the in-flight request fails with too_many_internal_resets.

…n streams The test needs the h2 client to drop frames that arrive on streams it no longer tracks (hyperium/h2#983). Released h2 answers each with RST_STREAM(STREAM_CLOSED), so the test fails against crates.io h2.

## Description kv-rpc (mainnet `sui-kv-rpc`, which serves GraphQL's ledger reads from BigTable) had two client-side problems talking to BigTable. Both could leave many reads waiting on one stuck HTTP/2 connection: 1. **The channel pool collapsed to one connection.** It counted a request as finished when response headers arrived, so it never saw open ReadRows streams and shrank to its minimum. On mainnet a pod had a single channel 78% of the time, with 170–700 reads in flight. Every read on the pod shared that connection and queued behind the others, and when the connection stalled, every read on the pod stalled: the served watermark froze for up to 30 s and lag alerts fired. 2. **hyper's default HTTP/2 receive windows (2 MiB per stream, 5 MiB per connection) let unread responses block a connection.** `ListEvents`/`ListTransactions` open bitmap-index scans and read them lazily, and data BigTable has sent but kv-rpc hasn't read counts against the connection's window. A few such scans fill it; after that nothing on the connection, the watermark refresh included, receives data until those requests hit their 30 s deadline. This is the likely mechanism of the 30 s freezes before this PR: they began with list requests and ended at the 30 s list deadline. Spreading reads over more connections already shrank the worst lag to 7.7 s; the larger windows remove the starvation itself. What this PR changes: - **The pool sizes itself on load.** At least 4 channels, resized each minute on peak in-flight requests (1–25 per channel), with a request counted until its response body ends. Each request goes to the least-loaded channel; a channel that just failed counts as busier for 5 s. - **Stalled channels are detected and routed around.** When a caller waits 1 s on a channel while other channels keep receiving responses, a background check probes it. If the probe gets no answer, the channel is quarantined: its open requests fail locally with a retryable error and retry on other channels. A channel that keeps getting quarantined is replaced. - **ReadRows retries** on another channel, resuming after the last row received, when a stream goes silent for 5 s or fails in a way that points at its channel or connection (3 attempts; the last has no silence limit). - **HTTP/2 receive windows default to 256 KiB per stream and 256 MiB per connection**, so a connection holds 1,024 streams' worth of unread data before it blocks, instead of 2.5. Windows are limits, not allocations: pod memory did not change. The design follows Google's own BigTable clients. Pool sizing and its defaults (10 initial channels, 1–25 requests per channel, resize by at most 2 channels a minute, at most 200) match the Java client's channel pool. Least-loaded selection with a 5 s penalty after `Unavailable`, `ResourceExhausted` or `Internal` comes from the Go client, with a weight of 25 instead of 10 (see design details). The health check reuses ideas from the Java client's `ChannelPoolHealthChecker`: a `PingAndWarm` probe, replacing only a channel that stands out from the rest, doing nothing when most of the pool looks bad, and rate-limited replacement. The difference is speed. Java probes each channel every 30 s, evicts one only after at least 60% of its probes fail within 5 minutes, and lets its open requests run on. That can't end a 30 s freeze, so this pool probes as soon as a caller has waited 1 s and fails the stuck requests so they retry on other channels. ```mermaid flowchart TB subgraph before["Before: one connection per pod, most of the time"] direction LR r1["170–700 reads in flight"] --> c1["1 HTTP/2 connection 5 MiB receive window"] c1 -. "connection stalls, or unread list scans fill the window" .-> x1["every read on the pod waits, up to 30 s"] end subgraph after["After"] direction LR r2["reads"] --> p2["pool picks the least-loaded healthy channel"] p2 --> c2["10–22 connections per pod 256 MiB receive window each"] h2["health check: silent channel → probe → quarantine; its reads retry elsewhere"] -.-> p2 end before ~~~ after ``` ### Mainnet results The 1.82 backport (#28445) runs on mainnet `sui-kv-rpc` (5 pods, ~1,340 requests/s). The pool changes went out 2026-10-24 20:29Z; the window sizes (set through config, the same values this PR makes the default) went out 2026-10-26 04:02Z. Apart from this change, the "before" build lacks only a version bump, a consensus change and a `rustls-webpki`/`webpki-roots` lockfile bump for cargo-deny advisories. | | Before | Pool fix | Pool fix + windows | |---|---|---|---| | Period | 3.9 days | 31 h | 8.5 h | | Requests/s | 1,320 | 1,351 | 1,341 | | Latency p50 | 15.9 ms | 8.1 ms | 8.2 ms | | Latency p99 | 236 ms | 75 ms | 77 ms | | Latency p99.9 | 857 ms | 207 ms | 205 ms | | Requests slower than 1 s | 1 in 1,900 | 1 in 46,000 | 1 in 60,000 | | Requests slower than 10 s | 24,146 (6,248/day) | 0 | 0 | | Worst watermark lag | 29.6 s | 7.7 s | 0.9 s | | Pod-minutes with lag over 10 s (sampled once a minute) | 14 (3.6/day) | 0 | 0 | | Pod-minutes with lag over 3 s | 38 (9.8/day) | 8 (6.2/day) | 0 | | `Unavailable` responses per million | 614 | 2 | 21 | | `Internal` responses per million | 5.1 | 0.04 | 0.05 | | `request deadline exceeded` log lines | 572 | 0 | 0 | | BigTable reads over 10 s (server-side) | 1,058 | 0 | 0 | | BigTable channels per pod | 1 for 78% of the time | 10–22 | 10–12 | The strong comparison is before vs pool fix: at the old rate of 3.6 pod-minutes per day with lag over 10 s, 31 h should have produced about 5, and there were none. The windows column covers only 8.5 h, too short to confirm its zeros on its own (the pool-fix rate predicts about 2 pod-minutes with lag over 3 s in that time). The periods also fall on different days (weekday, weekend, Monday morning), but the request rate was the same in all three, and p99 dropped within ten minutes of the pool deploy: 189–442 ms in 10-minute windows over the 3 h before, 50–88 ms over the 3 h after. - Every major method got faster at the same request rate: p99 dropped 2–5× and p99.9 2–7× (before → with windows): | Method | Requests/s | p50 ms | p99 ms | p99.9 ms | Requests over 10 s | |---|---|---|---|---|---| | `BatchGetObjects` | 359 | 16.9 → 8.7 | 184 → 85 | 441 → 175 | 4,219 → 0 | | `BatchGetTransactions` | 279 | 9.2 → 6.8 | 188 → 49 | 454 → 95 | 2,674 → 0 | | `GetCheckpoint` | 250 | 7.3 → 3.6 | 193 → 40 | 463 → 201 | 4,718 → 0 | | `ListEvents` | 227 | 25.3 → 11.4 | 395 → 92 | 1,502 → 249 | 7,477 → 0 | | `ListTransactions` | 202 | 30.2 → 23.1 | 407 → 75 | 1,407 → 188 | 4,974 → 0 | - BigTable itself did not get faster; kv-rpc stopped waiting on its own connection. Server-side p99 per table is unchanged, while the time kv-rpc waits for the same reads dropped to near it: | Table | BigTable server p99 | kv-rpc's wait for the read, p99 | |---|---|---| | `checkpoints` | 4.7 → 4.5 ms | 133 → 8.8 ms | | `transactions` | 22.6 → 20.9 ms | 154 → 24.7 ms | | `tx_seq_digest` | 23.2 → 21.8 ms | 140 → 25.5 ms | | `objects` | 68.5 → 64.5 ms | 147 → 67.6 ms | | `watermark_alt` | 9.8 → 10.1 ms | 44.9 → 15.0 ms | - "BigTable reads over 10 s" are server-side durations, which include time BigTable spent waiting for kv-rpc to accept data. Before the fix, point reads whose server p99 is 20–70 ms (`transactions`: 438, `objects`: 112) took over 10 s on the server, consistent with BigTable waiting for kv-rpc to read. - `Unavailable` responses are monotonic-read misses ("monotonic read at checkpoint N not available on this replica; retry"): the caller has seen a newer checkpoint than this pod serves. Before the fix they came from frozen watermarks. Afterwards they came in a few bursts of a few seconds: one in the pool-fix build, on a pod where the health check quarantined a channel within two minutes, and two in the windows build, on one or two pods, with no channel suspicions, gap timeouts or slow watermark reads at those times. - In 40 h with the new pool, 4 channels were quarantined; all 4 passed a later probe and were restored, so none needed replacing. 17 reads cancelled by quarantine and 5 that hit the gap timeout were retried on other channels. No quarantines or gap timeouts in the 8.5 h since the window change. ```mermaid %%{init: {"xyChart": {"width": 620, "height": 300}, "themeVariables": {"xyChart": {"plotColorPalette": "#d62728"}}}}%% xychart-beta title "Worst kv-rpc watermark lag in each hour" x-axis "October 2026, UTC" 21 --> 26.5 y-axis "seconds" 0 --> 30 line [1.3, 1.2, 1.7, 1.0, 1.3, 25.7, 5.7, 28.6, 1.3, 17.4, 2.1, 10.6, 2.1, 1.6, 26.5, 3.4, 8.2, 1.0, 1.8, 8.5, 1.1, 1.1, 1.6, 1.3, 19.2, 1.2, 1.9, 0.8, 0.9, 0.8, 4.6, 1.1, 0.8, 6.1, 0.9, 0.9, 1.2, 2.0, 9.4, 3.1, 1.1, 1.0, 0.8, 0.9, 0.9, 0.8, 0.9, 1.0, 1.4, 1.1, 0.9, 0.8, 0.8, 1.2, 1.0, 0.9, 1.2, 0.9, 0.9, 1.0, 0.8, 1.2, 2.3, 0.9, 5.0, 0.9, 15.6, 2.2, 1.1, 0.6, 6.8, 0.7, 1.1, 1.0, 0.7, 1.6, 2.0, 1.0, 1.2, 0.5, 0.9, 0.6, 0.7, 0.7, 0.8, 10.9, 7.4, 29.6, 20.3, 1.1, 0.9, 23.2, 15.8, 4.0, 1.0, 3.1, 7.7, 1.1, 0.9, 0.7, 0.8, 7.7, 1.1, 0.7, 1.1, 1.1, 0.7, 4.9, 0.7, 1.4, 0.7, 0.9, 1.9, 1.0, 0.7, 0.8, 1.2, 1.9, 1.0, 2.0, 7.6, 2.9, 0.8, 0.8, 0.9, 0.9, 0.9, 0.8, 0.8, 0.7, 0.5, 0.9] ``` ```mermaid %%{init: {"xyChart": {"width": 620, "height": 300}, "themeVariables": {"xyChart": {"plotColorPalette": "#d62728"}}}}%% xychart-beta title "kv-rpc p99 request latency, hourly" x-axis "October 2026, UTC" 21 --> 26.5 y-axis "ms" 0 --> 450 line [227, 235, 344, 377, 217, 375, 186, 236, 249, 152, 110, 192, 182, 266, 240, 200, 319, 229, 192, 204, 224, 196, 210, 210, 291, 245, 138, 116, 173, 214, 214, 237, 198, 172, 196, 202, 239, 355, 350, 297, 246, 222, 248, 232, 298, 352, 235, 308, 275, 292, 356, 215, 212, 246, 166, 199, 215, 206, 209, 196, 234, 243, 337, 229, 256, 217, 224, 334, 305, 249, 247, 231, 335, 238, 228, 228, 228, 182, 211, 402, 242, 243, 175, 249, 193, 225, 221, 289, 248, 338, 247, 245, 186, 74, 61, 74, 64, 74, 63, 77, 64, 64, 63, 76, 75, 54, 60, 69, 57, 77, 73, 73, 71, 95, 70, 79, 81, 81, 82, 87, 78, 68, 78, 77, 72, 75, 72, 74, 83, 74, 68, 85] ``` Deploys on the x-axis: pool fix at 24.85 (10-24 20:29Z), windows at 26.17 (10-26 04:02Z). ### Cost and risk - CPU across the 5 pods went from 5.2 to 6.0 cores (+15%; 6.4 in the pool-fix build) at the same request rate, likely the cost of 10–22 connections and their health probes per pod instead of 1 (not profiled). Memory is unchanged at about 1 GiB per pod. - Each pod holds 10–22 BigTable connections instead of usually 1. - Safeguards: the pool never quarantines more than half its channels (a wider stall points at BigTable or the network). It replaces at most one unhealthy channel per minute, and the replacement must pass a probe before the old channel is dropped. A probe that can't get credentials neither quarantines nor restores a channel. The final ReadRows attempt has no gap timeout, so reads that are slow on the server still complete. - Each part can be turned off or tuned through config without a release (below). - Size: about 2,050 added non-test lines and 2,600 test lines. ### Settings New settings (kv-rpc / kvstore indexer): - `bigtable-channel-health-check` / `bigtable-pool.channel-health-check` (default `true`) - `bigtable-read-rows-gap-timeout-ms` (default `5000`, must be above 0) and `bigtable-read-rows-max-attempts` (default `3`, at least 1; `1` turns off the gap timeout and retries), same names in both. `bigtable-channel-timeout-ms` (default `60000`) still ends every attempt's wait for response headers, so a stall before the headers is retried only while it is longer than the gap timeout; startup logs a warning when it isn't. - `bigtable-http2-stream-window-bytes` / `bigtable-http2-connection-window-bytes` (kv-rpc) and `bigtable-pool.http2-stream-window-bytes` / `bigtable-pool.http2-connection-window-bytes` (indexer), defaults `262144` (256 KiB) and `268435456` (256 MiB). Startup rejects values outside 1..=2^31-1, which h2 would otherwise panic on. Retired setting: `bigtable-pool.downscale-threshold` is ignored and logs a deprecation warning, the way `checkpoint-buffer-size` does. <details> <summary>Design details: selection penalty, quarantine and retry rules</summary> The pool now keeps at least 4 channels and sizes itself on the interval's peak in-flight count, with a request counted until its response body ends. It picks the least-loaded channel. For 5s after `Unavailable`, `ResourceExhausted`, `Internal` (a gRPC status, or a bare HTTP 400, 429, 502, 503 or 504), a connection failure, or a ReadRows gap timeout, a channel counts as `max-rpcs-per-channel` (25) requests busier. Those are the Go client's penalty codes and duration with a larger weight: a channel that fails fast never has anything in flight, so Go's flat +10 still lets it win most requests once the other channels each have more than 10 in flight, which the 1–25 load band allows. With +25, inside the band it gets requests only once every other channel carries 25 more; above 25 per channel, until a resize adds channels, it can still take more than round-robin's 1/N. (A steady-state model with 4 channels, 50ms successes, 2ms failures and a fixed arrival rate gives the failing channel 62% of requests with +10 and 22% with +25 at 25 in flight per channel, and 45% with +25 at 37.5 per channel, against 25% under main's round-robin. Other assumptions move these figures a lot; holding the healthy channels at 25 in flight gives about 84% with +10.) It quarantines a channel where a waiting caller gets nothing for 1s while other channels keep receiving responses. One waiting caller is enough, not the 2 first planned: with fewer requests in flight than channels, a stalled channel rarely collects a second, and the probe is what keeps a merely slow request from quarantining its channel. Callers busy with data they already have don't count, so a slow consumer doesn't by itself make a channel look stalled (its unread data can still fill the connection's flow-control window and starve the callers that are waiting). A quarantine needs a `PingAndWarm` probe that times out or fails on the connection; any status BigTable sends back, even `PermissionDenied`, shows the connection works. Quarantine fails the channel's open requests locally with a retryable `Unavailable`. ReadRows applies a per-message gap timeout and resumes on another channel after the last delivered row. It also retries, after a short jittered backoff, `Unavailable`, streams BigTable refuses with a GOAWAY before response headers, streams whose connection fails underneath them (h2 I/O errors, which tonic reports as `Unknown`), and requests hyper drops unsent because their connection closed (tonic reports those as `Cancelled`; this is the "not sent" case of gRFC A6 transparent retries, which the Rust SDK client handles the same way). A GOAWAY after response headers is still not retried: hyper ends the body cleanly, and tonic reports `Unknown: missing grpc-status trailer` with nothing that identifies the GOAWAY. ReadRows had no retries before this change. The Java and Go clients also retry `DeadlineExceeded`, `Aborted`, and an `Internal` from RST_STREAM; this hotfix doesn't, so it retries only failures that point at a channel or its connection. Mass cancellation relies on the `nickvikeras/h2` pin (hyperium/h2#983). Safeguards: the pool never quarantines more than half its channels (a wider stall points at BigTable or the network). It replaces at most one unhealthy channel per minute, and the replacement must pass a probe before the old channel is dropped. A repeatedly quarantined channel is only replaced when it stands out from the rest of the pool. A channel that a resize or refresh drops while its probe runs neither counts toward the half-pool cap nor takes the replacement slot, though a failed probe still fails the requests left on it. A probe fetches credentials before its deadline for the connection starts; one whose token fetch fails or times out (say, the metadata server is down at token refresh) never reaches the connection, so it neither quarantines nor restores a channel. The final ReadRows attempt has no gap timeout, so reads that are slow on the server still complete. A quarantine cancellation is not reported to batch-write flow control as overload. </details> <details> <summary>How the mainnet numbers were measured</summary> - Periods: before = 10-20 23:30Z to 10-24 20:15Z (build `e10772f4`); pool fix = 10-24 20:45Z to 10-26 03:55Z (`24e593ec`); windows = 10-26 04:10Z to 12:30Z (`f9d8bfec`). Rollouts are excluded. Error counts use each build's full lifetime from raw counters, so they include pod startup. - Latency: `rpc_request_latency` histogram over the whole period; quantiles are interpolated within histogram buckets. - Watermark lag is kv-rpc's own lag: the age of the pod's served checkpoint (`kv_rpc_served_checkpoint_timestamp_ms`) minus the age of the indexer's watermark (lowest pipeline, highest writer), sampled once a minute. Freezes shorter than a minute can be missed. - BigTable server latency: Cloud Monitoring `bigtable.googleapis.com/server/latencies` for `ReadRows`. kv-rpc's wait: `kv_get_latency_ms` for `checkpoints`, `objects` and `watermark_alt`, and `kv_get_stream_poll_wait_ms` for `transactions` and `tx_seq_digest`. - Log lines: Loki, `{namespace="rpc-kv-mainnet"}` restricted to each build's pods, over the periods above. </details> ## Test plan New tests use a mock BigTable server and a hand-rolled HTTP/2 server; the list is collapsed below. Mainnet behaviour is under "Mainnet results". <details> <summary>Test list</summary> - 1,500 ReadRows streams on one connection, each with its reader waiting on the body, cancelled by quarantine, then the server keeps sending DATA on them (and, in a second test, sends late HEADERS and DATA on streams cancelled before their headers): no client GOAWAY, and a probe and a new ReadRows on the same connection succeed. A variant drops 1,200 of 1,500 streams directly and the other 300 complete. All three fail against stock h2 0.4.20 with `GOAWAY(ENHANCE_YOUR_CALM, "too_many_internal_resets")`. To check, delete the `h2` line under `[patch.crates-io]` and run `cargo nextest run -p sui-kvstore -E 'test(mass_cancel) | test(late_headers) | test(dropping_most)'`. - A single read stalled before headers, the only caller waiting on its connection, ends within 2s of the background health checker quarantining the channel (nothing else could end it for an hour) and returns exactly its rows from another channel. - Resume after an interruption for key sets, forward and reversed ranges, with and without `rows_limit` (asserts the retried request shape, plus unit cases for open, closed and empty bounds, several ranges, keys mixed with ranges); reads already fully delivered send no retry, so no empty row set reaches BigTable. A header stall and a body stall each trip the gap timeout; the retry avoids the failed channel even when it is the least loaded, the abandoned attempt leaves nothing in flight or waiting, and the channel carries the error penalty. The final attempt waits out a read slower than the gap timeout. A graceful GOAWAY on an unprocessed stream is retried elsewhere, and so are reads cut off when their connection closes, open (h2 I/O error) or queued in hyper (never sent). - Open response streams count as in flight and steer selection; a channel failing with `Unavailable` gets 1 of 10 sequential requests, not 5, and the penalty ends after 5s. A penalized channel gets requests only once the other channels each carry `max_rpcs_per_channel` more, and a bare HTTP 503 penalizes a channel like `Unavailable`. Peak-based resize holds inside the band and at the floor, and the next interval's peak starts from the requests still open. The circuit breaker blocks quarantine when 3 of 4 channels stall, and a silent channel isn't probed while no other channel is receiving responses. Unpolled streams never make a channel suspect, while waiting ones do, and a probe that answers keeps it in service; an error status counts as an answer, a probe cut off by its connection closing does not. While the client's token fetch fails, or hangs past the probe timeout, a stalled suspect stays in service and a healthy quarantined channel stays out; once credentials work, the next probes quarantine the first and restore the second. A quarantined channel gets no requests until a probe restores it. A repeat offender is replaced. Replacement is rate-limited and keeps the old channel if the new one can't connect or never answers its probe. A channel dropped from the pool during its probe doesn't take the replacement slot, and a suspect dropped during its probe still has its stalled read failed. A quarantine-cancelled write leaves the batch-write rate alone. - Waiting-only silence, one waiting caller being enough to probe a channel, the error penalty (on failures, on gap timeouts and on bare HTTP errors), its weight and its expiry, the probe answer rule, a probe without credentials changing nothing (counted either as no answer or as an answer, and with the token fetch inside the probe deadline), the I/O-error retry, the "another channel is progressing" condition, both rules for channels that left the pool mid-probe, and waking body readers on quarantine each have a test that fails when that behaviour is removed. - With hyper's 2 MiB / 5 MiB windows, a small read on a one-channel pool stalls while three 3 MiB responses on the channel sit unread; with the pool's default windows it finishes. </details> --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] Indexing Framework:

adriangb · 2026-10-28T05:25:59Z

+1, we are hitting this talking to GCS. Our upstream client drops, that cascade drops everything in the request stack and then we end up with reset connections.

zaidoon1 force-pushed the client-ignore-late-frames branch from e0a6c16 to 44260b1 Compare October 13, 2026 16:57

nickvikeras mentioned this pull request Oct 19, 2026

deps: pin h2 to hyperium/h2#983 so cancelled requests don't close HTTP/2 client connections MystenLabs/sui#28359

Merged

8 tasks

nickvikeras added a commit to MystenLabs/sui that referenced this pull request Oct 19, 2026

deps: pin h2 to hyperium/h2#983 so cancelled requests don't close HTT…

d8ad4c5

…P/2 client connections

nickvikeras mentioned this pull request Oct 24, 2026

[kvstore] Detect and route around stalled BigTable channels MystenLabs/sui#28441

Merged

8 tasks

adriangb mentioned this pull request Oct 28, 2026

feat(transport): expose http2_max_local_error_reset_streams on Endpoint grpc/grpc-rust#2708

Closed

theduke mentioned this pull request Oct 28, 2026

chore(deps): merge upstream Pingora 0.9 and patch zstd advisories wasmerio/pingora#15

Merged

Navigation Menu

Repository navigation

fix(client): ignore frames on forgotten streams - #983

fix(client): ignore frames on forgotten streams#983
zaidoon1 wants to merge 1 commit into
hyperium:masterfrom
zaidoon1:client-ignore-late-frames

zaidoon1 commented Oct 13, 2026 •

edited

Loading

Uh oh!

nickvikeras commented Oct 18, 2026 •

edited

Loading

Uh oh!

adriangb commented Oct 28, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

zaidoon1 commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nickvikeras commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriangb commented Oct 28, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zaidoon1 commented Oct 13, 2026 •

edited

Loading

nickvikeras commented Oct 18, 2026 •

edited

Loading