Skip to content

fix(hyperlane): stop dropping pipelined requests - #46

Merged
vshengbro merged 1 commit into
masterfrom
fix/http-pipelining-read-ahead
Oct 13, 2026
Merged

vshengbro merged 1 commit into
masterfrom
fix/http-pipelining-read-ahead

Conversation

@vshengbro

Copy link
Copy Markdown
Member

Summary

HTTP pipelining was completely broken: a client that wrote several requests
in one call received a response for only the first, and every later request
was silently discarded. This restores it, which is also where the large
throughput gain comes from.

A single socket read can deliver bytes belonging to the next pipelined
request. fill_http_from_stream created a fresh PooledReader per request,
so those read-ahead bytes were dropped along with the buffer when the reader
was dropped and returned to the thread-local pool. The next request took a
recycled buffer with start/end reset to zero, and the data was gone.

Stream now carries the unconsumed bytes across requests.

Changes

  • type/src/stream/struct.rs — Stream gains leftover plus its
    start/end offsets, holding bytes already read from the socket but not
    yet parsed into a request
  • type/src/stream/impl.rs — fill_http_from_stream seeds the reader with
    the carried bytes before parsing and stores back whatever the reader read
    past the current request
  • core/tests/server/fn.rs — regression test: writes 8 pipelined requests on
    one connection and asserts all 8 responses return
  • core/tests/mod.rs — import AsyncReadExt for the test client

All self access is sequenced before the reader borrows the socket or after
the reader is dropped, so the mutable borrows of Stream and TcpStream
never overlap.

Verification

Measured on an 18-core Mac, release build, closed-loop keep-alive client,
before and after on the same machine in the same session:

Scenario Before After
depth 64 0 QPS (32/32 conns failed) 863,653 QPS
depth 16 0 QPS 836,190 QPS
depth 4 0 QPS 523,048 QPS
depth 1 (serial) 189,065 QPS 189,812 QPS

Serial QPS is flat by design: this fixes correctness, not the single-request
path. The pipelined figures are what the fix unlocks.

  • The regression test fails against the unfixed parser with
    "expected 8 pipelined responses, got 1", and passes once restored
  • cargo test --workspace — 725 passed, 0 failed
  • cargo clippy --all-targets — 0 warnings
  • crate fmt --check — clean
  • audit_rust_standards.py . — 51/51 PASS

Notes

Two hypotheses were tested and rejected before landing this, so they are not
in the diff:

  • Disabling the per-request read timeout (the default is 6000 ms, so every
    request goes through tokio::time::timeout) changed nothing: 190.7k
    versus 194k QPS
  • Lowercasing header keys into a stack buffer was a no-op, because the value
    is still needed as an owned String for the header map

Profiling on this machine could not produce usable leaf attribution, so the
19 allocations per request that remain were left alone rather than optimized
blind.

🤖 Generated with Hermes

HTTP pipelining was completely broken: a client that sent several
requests in a single write received a response for only the first
one, and every later request was silently discarded.

`fill_http_from_stream` created a fresh `PooledReader` per request. A
single socket read can deliver bytes belonging to the next pipelined
request, and those bytes were held in the reader's `start..end` window.
Dropping the reader returned the whole buffer to the thread-local pool
via `mem::take`, discarding the read-ahead bytes. The next request took
a recycled buffer with `start`/`end` reset to zero, so the read-ahead
data was gone for good.

`Stream` now carries the unconsumed bytes across requests: `leftover`
plus its `start`/`end` offsets. They are seeded into the reader before
parsing and stored back after the request is parsed. All `self` access
happens before the reader borrows the socket or after it is dropped, so
the mutable borrows of `Stream` and `TcpStream` never overlap.

Measured on an 18-core Mac, release build, closed-loop keep-alive
client (128 connections, 10s runs, three repetitions each):

- pipelined depth 64: 0 QPS (32/32 connections failed) -> 863,653 QPS
- pipelined depth 16: 0 QPS -> 836,190 QPS
- pipelined depth 4: 0 QPS -> 523,048 QPS
- serial (depth 1): 189,065 -> 189,812 QPS, unchanged in run variance

The serial number is flat by design: this restores correctness rather
than speeding up the single-request path.

Adds a regression test that writes 8 pipelined requests on one
connection and asserts all 8 responses come back. It fails against the
unfixed parser with "expected 8 pipelined responses, got 1".

🤖 Generated with [Hermes](https://hermes-agent.nousresearch.com)
@ghfind-review ghfind-review Bot added the review: high ghfind author score; see https://ghfind.com label Oct 13, 2026
@vshengbro
vshengbro merged commit 9be6c74 into master Oct 13, 2026
8 checks passed
@vshengbro
vshengbro deleted the fix/http-pipelining-read-ahead branch October 13, 2026 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review: high ghfind author score; see https://ghfind.com

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant