flux-network: harden HTTP failures and remove hot-path allocations - #144
Closed
Bronek wants to merge 15 commits into
Closed
flux-network: harden HTTP failures and remove hot-path allocations#144Bronek wants to merge 15 commits into
Bronek wants to merge 15 commits into
Conversation
Bronek
marked this pull request as draft
August 28, 2026 14:28
A client that meets a reset before it has read the answer to its upload reports a node that is down rather than the status it was sent. A response that closes a connection now shuts the write side alone whenever the request stream behind it was cut short — every status the service raises itself, and a connection driven over its buffer limit — and reads its peer out under an idle and a total cap until the peer stops, the caps run out, or it closes. The bytes over the limit are no verdict on a request already delivered: a connection holding one keeps it, and the answer the caller is producing carries the close. Lingering connections answer to the linger's caps rather than the idle sweep, and hold their place in the group meanwhile. Assisted-by: Claude Code:claude-opus-5
A request to an endpoint that answers nothing used to wait as long as the connection stood, and a peer that broke the protocol dropped the connection with nothing to say for it. An outbound request now carries a deadline from the moment its bytes are queued — a request waiting behind a backlog is already waiting — and every request that will not be answered names its reason: the deadline, an answer the service cannot frame, an answer over the caps the operator sets, or an endpoint that closed with the request in flight. The failure reaches the caller before the disconnect it causes, so a puller learns why the connection went before learning that it did. Assisted-by: Claude Code:claude-opus-5
An endpoint that echoes a status chosen elsewhere needs the whole range and the phrase-free status line RFC 9112 §4.1 permits, so a status the service has no phrase for is framed as `HTTP/1.1 250 ` rather than carrying a phrase nobody chose. A caller-chosen 1xx is final: it completes the request, framed with no length and no body. `reason_phrase` is public, and the phrase it returns for a status it does not name is now the empty string rather than `Unknown`. Assisted-by: Claude Code:claude-opus-5
The caps were timing the wrong thing. They ran from the moment the answer was queued, so a large answer over a small socket was cut off mid-body — the idle cap ending a connection that had delivered a fraction of the length it announced, which is the reset lingering exists to avoid. They now time what they name: the reading and discarding that begins where the answer ends. A connection still writing is bounded by its transport, exactly as a draining one is. This is the one network-side addition PR B needs: `write_side_shut` reports that the peer has the whole of what was queued and the end of the stream after it, which is where the clock starts. A linger whose clock has yet to start folds no deadline of its own, since the write it is waiting on is what wakes the poll. A connection over its buffer limit with nothing pending is now answered with the 431 every other head too large for the service gets, rather than dropped where it stands: an over-limit that lands exactly on a request boundary closes the way one that lands mid-request already did, however the request before it was answered. Assisted-by: Claude Code:claude-opus-5
The reason an outbound request carries is what tells an operator to raise a cap apart from what tells them an endpoint is broken, so each path that picks one is pinned: a head over its cap, a chunked body over its cap and one the close delimits over its cap all report `TooLarge`, while a chunk size the service cannot read and an answer framed twice over report `Malformed`. Each asserts the failure reaches the caller ahead of the disconnect that follows it. A request deadline also has to be the deadline a blocking drive waits for, which is what the last test holds it to. Assisted-by: Claude Code:claude-opus-5
The caps time the reading and discarding, so a connection whose answer has yet to reach its peer runs under none of them. A peer that stops reading held such a connection open — and its place in the group — against nothing but the transport, which bounds a TCP peer late and a Unix-domain one never. The sweep now lets go of a lingering connection only once its own clock has taken over. Until then it is held to the same idle bound as any other, a draining connection included, and folds that bound into the deadline rather than none. Assisted-by: Claude Code:claude-opus-5
Every parse borrowed a fresh header vector, every outbound request owned a copy of its method and of the host it addressed, and every response carried its body through the caller's slice. A warm connection needs none of it: the scratch a parse borrows is bounded, all that is asked of a request in flight is whether it was HEAD, and a body composed into a buffer the service keeps reaches the wire in one copy. Parsing borrows a stack array of MAX_HEADERS entries sliced to what the configuration asks for, and with_max_headers refuses a larger cap. An outbound connection records the request in flight rather than its method. request() writes the Host it defaults to straight into the send buffer. respond_with lends a closure the service's body scratch and frames the Content-Length it wrote, HEAD included; respond is that call with the body it was handed. Framing stays in one place, now the Responder the request carries, and the wire bytes are unchanged. Assisted-by: Claude Code:claude-opus-5
A warm keep-alive connection in each direction runs a thousand round trips under a global allocator that counts allocation events on the thread driving them, and each direction is asserted to have made none. The peers are plain sockets driven from the same thread between iterations of the network, so nothing else runs while a count is open and nothing the peers do grows a buffer of their own. The bytes each side reads are checked against the message it expects, so a direction cannot pass by falling silent. flux-profiler's CountingAllocator tallies bytes rather than events and keeps its counters to the profiler, so the test brings its own. Assisted-by: Claude Code:claude-opus-5
A lingering connection arms its caps once the transport has taken the whole answer, and finding that out walks the network's connections. The linger loop asked for every lingering connection on every tick; it now asks only for one whose clock has yet to start, so a connection whose caps are already running costs nothing further. `write_side_shut` says what it means: the transport has taken everything queued for the connection and the end of the stream after it, with up to a socket buffer of that still on its way, not that the peer has read it. The test pins that the caps run from the moment the answer has left, which is the time the ticks are given, rather than from the start of the poll wait that preceded it. Assisted-by: Claude Code:claude-opus-5
A head that never ends is caught by the cap while the parse is still incomplete, which is a path of its own: an answer whose open head runs past the cap now pins `TooLarge` there, where before only the head that parsed whole was held to it. A request ending exactly at the buffer limit has two correct closes, and which one the peer reads is settled by whether a tick falls between the request and its answer. The pair that answer first are named for that, and the third holds the other outcome: an answer that carries the close itself, with no rejection behind it. Assisted-by: Claude Code:claude-opus-5
A response the service refuses never reaches its body: the status range, the header names and the pending request are all settled before the closure that would compose one runs. Nothing said so, and nothing would have noticed the check moving below the render. The header count had the same gap on the other side. Its three tests covered the builder alone, so what the count decides on the wire --- a request at the count served, one over it answered 400, and a count written past the scratch held to what the scratch holds --- went unpinned, as did the answer an endpoint sending too many headers gets. Both new server-side tests run over a socket file: what a header count decides is the parse, and the loopback ports the rest of the suite shares are contended enough already. The zero-alloc harness takes the connection its service dialled straight from the listener rather than opening a placeholder first, and records what one round trip already settles. Assisted-by: Claude Code:claude-opus-5
`HttpConfig`'s fields are public, so a configuration is as likely written whole as built call by call. The builder refuses a header count outside `1..=MAX_HEADERS`; a field written straight into the struct was quietly held to `MAX_HEADERS` at every parse instead, so the same configuration meant two different things. `HttpService::new` validates the configuration it is handed, sharing the check with the builder so the rule and its message cannot drift, and the parse borrows the scratch the count names. Assisted-by: Claude Code:claude-opus-5
`write_side_shut` answers a question the lingering close asks of its own transport, and nothing outside the crate has cause to ask it. It is `pub(crate)`, and the linger test that reached for it measures the same instant from where a peer can see it: the client reads the whole answer and then the end of the stream, which is the answer having left. The test keeps its point by timing that instant against both ends. The peer takes nothing for half a second, so the answer leaves only after the pause -- and the caps end the connection an idle cap after that, rather than an idle cap after it was queued. Assisted-by: Claude Code:claude-opus-5
`buffered` handed the tests a connection's byte count, which is the service's own representation and nothing a caller has business reading. It goes, and what it stood in for is measured where it belongs. The lingering close's discard is a state transition, so it is pinned by a unit test that drives a connection through it: bytes read while lingering leave the buffer empty, leave the total cap where the answer put it, and move the idle cap to the read. The wire-level test keeps what a peer can see -- the answer, then the end of the stream, and a client that goes on uploading uncut. The gates that make the over-limit-while-pending path deterministic ask the network instead. An accepted connection is polled for reads alone until the service writes, and a raw read empties the socket, so between a request and its answer the one iteration reporting work is the one that took the overrun whole. Assisted-by: Claude Code:claude-opus-5
Bronek
force-pushed
the
bronek/api_server-b
branch
from
September 3, 2026 13:53
f9db1f1 to
755f23d
Compare
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.
This PR builds on #142. It adds lingering close for rejected requests, outbound request deadlines with explicit failure reasons, support for the full HTTP status range, and allocation-free processing on warm connections.
Lingering close
A response that ends an incomplete request stream shuts only the write side once the answer drains, then reads and discards the peer's remaining upload. This lets the peer receive the HTTP error and EOF instead of a connection reset.
HttpConfig::linger: Option<Linger>bounds this phase by idle time since the last inbound byte (5 seconds by default) and total elapsed time (30 seconds by default). The clocks start only after the transport has taken the complete answer; until then, the ordinary idle timeout still bounds a peer that stops reading.without_linger()restores immediate close after the response drains.If a connection exceeds its buffer limit while a request is pending, the service preserves the application's response, adds
Connection: close, and then lingers. If no request is pending, it responds with 431 and lingers. A complete request answered withConnection: closecontinues to drain and close normally; lingering is reserved for lost request framing.Request deadlines and failure reasons
HttpConfig::request_timeout: Option<Duration>gives each outbound request a deadline starting when its bytes are queued. It defaults toNone.An outbound request that can no longer be answered produces
HttpEvent::RequestFailed { token, reason }before the correspondingDisconnectedevent:TooLargedistinguishes operator-configured limits from malformed responses, whileDisconnecteddistinguishes a lost in-flight request from an idle endpoint closing.Status framing
Responses may use any status from 100 through 599. Common statuses use their conventional reason phrases; other statuses use an empty phrase, as RFC 9112 permits. A caller-chosen 1xx is final: it completes the request without a content length or body.
Invalid responses—such as an out-of-range status, invalid header, or response without a pending request—are refused before the body closure runs.
Allocation discipline
After connection warm-up, HTTP request serving and outbound response parsing allocate nothing. Parsing uses a fixed stack header scratch, an in-flight request records only whether it was
HEAD, andrespond_withrenders into a service-owned reusable buffer.The allocation test warms both directions, runs 1,000 round trips each way under a thread-local counting allocator, and requires zero allocation events.
HttpConfig::max_headersmust be in1..=MAX_HEADERS(128).HttpService::newvalidates direct struct construction under the same rule aswith_max_headers, so both configuration paths behave identically.Verification
The new test suites cover lingering over TCP and Unix-domain sockets, cap timing, over-limit pending responses, all request-failure reasons and their event ordering, status boundaries, and allocation-free warm paths. Additional HTTP and in-module tests pin header-count boundaries, rejected response composition, incomplete response heads, and internal discard behaviour without exposing representation through the public API.
Workspace tests, Clippy with warnings denied, the nightly formatting check, and
cargo docpass without new warnings.The existing loopback tests still select unused TCP ports before binding them, so they retain a known race. A following change makes
listenreturn its bound endpoint and converts the tests to bind port zero; until then, a job that loses this race can be rerun.Scope
This PR does not change
listen, which still returnsio::Result<()>. Reporting the bound endpoint, binding tests directly to ephemeral ports, and avoiding the scratch copy in the slice-basedrespondpath remain separate follow-up changes.Base: #142.
Refer: #143
Assisted-by: Claude Code:claude-fable-5
Assisted-by: Codex:gpt-5