Repository navigation
Tags
Tags
fix: give every upstream transport one gross per-attempt timeout budget timeout was documented as "the maximum time for the entire request" but only ever worked downwards: every transport carried its own hard per-attempt cap out of unexported constants, and raising timeout above that cap changed nothing. The caps differed by a factor of 4.5 with no intent behind it - roughly 2s for UDP, 6s for TCP/TLS, 4s for DoH/DoH3, and up to 9s for DoQ (quic-go's default 5s HandshakeIdleTimeout, never set by us, stacked on top of a 2s write and a 2s read). An upstream whose round trip exceeded its transport's hidden cap could never be used, whatever timeout said, and with attempt-count 0 fanout retried it for the full configured timeout with every single attempt still dying at that same hidden cap. processClient now narrows the context once per attempt to min(4s, timeout/attempt-count) - or a flat 4s when attempt-count is 0, since there is no attempt count to divide by - and every client lost its own timing policy in favour of that single clock: no more Client.Timeout on the DoH/DoH3 http.Client, no more net.Dialer.Timeout, and DoQ/DoH3 now set QUICConfig.HandshakeIdleTimeout from the same budget instead of letting quic-go default it to 5s. The 4s ceiling is not a new value; it is what DoH and DoH3 already ran under, chosen here because those two have the most expensive handshake of the bunch. Gross is the fairer measure across transports too: the difference between UDP, TCP and QUIC is a one-off handshake cost of at most a few hundred milliseconds, not something an operator can act on, whereas the same timeout 5s previously meant 2s against one upstream and 5s against another. This also removes the root cause of the DoH3 half of #45: the old 4s http.Client cap fired before quic-go's 5s HandshakeIdleTimeout, severing a stalled QUIC dial mid-flight and leaving the transport holding a connection that was neither usable nor idle. With a single clock derived from the same context, there is no longer a race between two independently-owned timers. Two tests relied on a client-side readTimeoutOverride to keep deliberately silent upstreams fast in a suite that otherwise ran at the 30s production default; that knob is gone along with the rest of the per-client timing policy it stood in for. TestWorkerCountLessThenServers now has its stub upstreams answer immediately with SERVFAIL instead of staying silent, sidestepping the budget entirely. TestBusyServer switches from Attempts=0 to a large bounded Attempts (paired with a Timeout scaled to match), since attempt-count 0 gives every attempt the flat 4s ceiling regardless of Timeout; attempt-count 0's own contract - retry until Timeout, never hang past it - stays covered by TestServeDNS_InfiniteRetryWithContextTimeout. README documents the new min(4s, timeout/attempt-count) budget under timeout, since the 4s figure is not obvious from the option alone. Fixes #47 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PreviousNext