Stop calling the startup warning the only sign of an accept stall - #370
Merged
diolektor merged 1 commit intoSep 19, 2026
Merged
Conversation
Docs: - The architecture overview ended the connection-budget paragraph on the startup warning, so a reader planning for overload concluded the state cannot be watched at runtime — the last public page still reading that way. It now names both signals: a WARN when the loop parks and an INFO with the duration when it resumes, rate-limited so a server flapping at the ceiling does not turn its own log into the outage, plus the oxphp_accept_stalled gauge and the oxphp_accept_stalls_total counter, which are updated on every park including the ones the rate limit passes over. - The sentence beside it treated the startup warning and the stall as one mechanism. The check reads configuration only, so a park caused by slow clients or idle keep-alives is one it cannot predict — which is exactly when the runtime signals are all an operator has.
diolektor
deleted the
fix/architecture-overview-omits-runtime-accept-stall-signals
branch
September 19, 2026 21:52
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.
Problem
docs/architecture/overview.mdended the connection-budget paragraph with "The server warns about that at startup." That was true when it was written — the startup check was the only signal there was. It is not true now. An accept that has to wait for aMAX_CONNECTIONSpermit logs aWARNnaming the budget, the permit arriving logs anINFOwith how long the loop was parked, and two metrics export the state:oxphp_accept_stalledandoxphp_accept_stalls_total.The configuration reference, the metrics reference and the health-checks page all describe them. The architecture overview was the last public page where the state still read as unobservable at runtime — and it is the page a reader consults when sizing a deployment for overload, so the conclusion it invited was that an exhausted connection budget can only be anticipated, never watched.
The sentence beside it made a second, quieter mistake. "Clearing it is necessary rather than sufficient: connections that never reach PHP hold permits too" left the startup check and the stall itself looking like one mechanism. They are not. The startup check reads configuration only —
PHP_WORKERS+QUEUE_CAPACITY+QUEUE_MAX_WAITINGagainstMAX_CONNECTIONS, evaluated once — and its own doc comment says what follows from that: "A configuration this clears can still exhaust the budget." A park caused by slow clients, idle keep-alives or long h2 streams is one it cannot predict, and that is exactly the case where the runtime signals are all an operator has. Naming the signals without separating the two mechanisms would have left the reader with the more useful half still attached to the wrong one.What changed
accept_stall_begin()runs as the first statement of the no-permits branch, before the suppression window is computed, so the two series are updated on every park including the ones the log passes over in silence. The paragraph says so, and links to the metrics reference for the rest./metricsis routed only by the internal listener, whose accept loop takes noMAX_CONNECTIONSpermit at all — which is why the series can be read during a stall, when the main listener is answering nothing.Clearing itbecameClearing the startup warning. The pronoun was ambiguous between the warning and the condition. The sentence gains a tail saying that a park caused by connections which never reach PHP is one the startup check cannot predict, which is the distinction above stated where the reader meets it.llms-full.txtregenerated.Two wordings this change carried in its own first draft were dropped before review, both in the direction of promising more than the code does:
oxphp_active_connectionsreaching its ceiling remains an indirect indication; it is awkward to use, because reading it requires knowing the configured budget and comparing two sources, but it exists. The sentence now makes a claim about the startup check instead, which is the claim that actually holds.No behaviour changes — documentation only.
Verification
scripts/gen-llms-txt.sh --check— up to date, 58 pages.scripts/check-links.sh— 649 in-repo links, none broken (the one this change adds included).cargo fmt --check,cargo clippy --no-default-features -D warnings,cargo test --no-default-features— 1170 passed, 0 failed, 3 ignored. No Rust or C source is touched on this branch, so these cannot be affected by it; CI runs them regardless.src/config/mod.rs; theWARN, theINFOcarryingstalled_secs, the five-second suppression window and the unconditional counter increment ahead of it insrc/main.rs; the gauge and counter definitions and their exposition types insrc/metrics.rs; and the absence of any connection semaphore on the internal listener insrc/server/internal.rs, which is what makes the metrics readable through a stall.docs/,README.mdandCHANGELOG.mdfinds one page that was incomplete, and this is it. The configuration reference, the metrics reference, the health-checks page and the released changelog entry describing these signals are all accurate and are left alone. The releasedQUEUE_MAX_WAITINGchangelog entry mentions the startup check without claiming it is the only signal, so it is correct as history and is also left alone.