fix(limits): Bound idle probing and preserve partial results - #45
Merged
Conversation
Add a whole-phase idle timeout so a stalled response cannot prevent load measurements. Preserve completed samples when idle expires or the caller cancels, and distinguish caller deadlines from per-direction caps. Close early upload responses on cancellation to wake HTTP/2 flow-control waits, joining the cleanup callback before transport teardown. Document the combined phase budget and caller cooperation contract, with offline library and CLI regression coverage. Closes #39 Co-Authored-By: GPT-6 <noreply@openai.com>
korya
marked this pull request as ready for review
September 10, 2026 02:17
Document that the fixed idle budget can truncate healthy slow paths and larger sample sets. Preserve the preceding probe error when a later probe times out, and avoid timeout warnings after every requested sample has completed. Align the custom-TLS socket assertion with the existing eventual cleanup contract, while preserving the immediate assertion for ordinary dials. Exercise phase-event cancellation and elapsed return time alongside exact phase budgets. Co-Authored-By: GPT-6 <noreply@openai.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.
Problem
A stalled idle probe can keep a measurement running indefinitely, even when discovery and load phases have finite time budgets.
The CLI supplies no parent deadline, so neither stalled headers nor an unfinished idle response body is bounded. Caller cancellation also discards completed idle samples, and a caller deadline during load is reported as the direction's own duration cap.
Solution
Bound the whole idle phase with a configurable timeout, retain completed samples, and continue selected load measurements when only idle expires.
Options.IdleTimeoutandnq --idle-timeoutdefault to 10 seconds. Non-positive values select that default; negativeIdleProbesstill skips idle measurement.The cap does not scale with
IdleProbes: healthy slow paths and larger sample sets may require a largerIdleTimeoutto reach the requested percentile thresholds. Ten seconds is a configurable product default, not a network-health threshold. Timeout warnings retain the preceding probe error when available, and a deadline observed after every requested sample completed does not produce an idle-timeout warning.reason=duration_capDiscovery, idle, and load deadlines use the existing injectable clock. The combined default phase budget is 44 seconds for both directions, 32 seconds for one, or 34 seconds for both with idle skipped. Return time also includes local orchestration and prompt cleanup; caller-supplied transports, dialers, body closers, sinks, and log handlers must cooperate.
MaxDurationretains its per-direction meaning, and the result JSON shape is unchanged.The budget regression also covers an HTTP/2 upload peer that sends an early response and stops reading. Cancellation explicitly closes that response body to wake the stream writer, and joins the close callback before transport teardown. This avoids waiting indefinitely on HTTP/2 flow control while preserving the existing socket cleanup order.
Known teardown limitation: Go's HTTP/2 response-body close can still wait on the connection write mutex with unread response data and a peer that stops reading its socket entirely. This pre-existing case needs a separate transport-abort investigation; abandoning the close callback would violate the worker ownership contract.
Actual CLI transcript rendered for review: one idle sample survives a stalled response, the warning reports the 500 ms timeout, download completes, and the process exits successfully.
When an earlier probe failed, the CLI now retains that error in the timeout warning:
Validated with
go build ./...,go vet ./...,golangci-lint run ./..., andgo test -race -count=3 ./.... Offline tests cover HTTP/1.1 and HTTP/2 header/body stalls, zero and partial samples, preceding errors, expiry after the final sample, cancellation from the phase event sink, caller deadlines, selected/skipped phases, deadline cleanup, owned sockets, and the built CLI binary. Phase-budget tests also assert elapsed return time with a generous scheduling/teardown allowance. The custom-TLS socket assertion follows the existing eventual-cleanup contract and passes 30 repetitions under the race detector. Specs, changelog, and the test matrix are updated together.Related: