Repository navigation
fix(hyperlane): stop dropping pipelined requests - #46
Merged
Merged
Conversation
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)
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
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.
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_streamcreated a freshPooledReaderper 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/endreset to zero, and the data was gone.Streamnow carries the unconsumed bytes across requests.Changes
type/src/stream/struct.rs—Streamgainsleftoverplus itsstart/endoffsets, holding bytes already read from the socket but notyet parsed into a request
type/src/stream/impl.rs—fill_http_from_streamseeds the reader withthe 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 onone connection and asserts all 8 responses return
core/tests/mod.rs— importAsyncReadExtfor the test clientAll
selfaccess is sequenced before the reader borrows the socket or afterthe reader is dropped, so the mutable borrows of
StreamandTcpStreamnever 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:
Serial QPS is flat by design: this fixes correctness, not the single-request
path. The pipelined figures are what the fix unlocks.
"expected 8 pipelined responses, got 1", and passes once restored
cargo test --workspace— 725 passed, 0 failedcargo clippy --all-targets— 0 warningscrate fmt --check— cleanaudit_rust_standards.py .— 51/51 PASSNotes
Two hypotheses were tested and rejected before landing this, so they are not
in the diff:
request goes through
tokio::time::timeout) changed nothing: 190.7kversus 194k QPS
is still needed as an owned
Stringfor the header mapProfiling 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