feat(server): add --statement-timeout to bound wedged statements

[fuziontech]

Why

Nothing in the stack bounds a single statement today. DuckDB has no statement timeout, a client SET statement_timeout is an ignored SET (transform.SetShowTransform.IgnoredParams), the cnpg shards run idle_in_transaction_session_timeout=0, and pgwire clients wait forever.

On 2026-09-16 a DuckLake worker deadlocked in-process mid-INSERT — duckdb/duckdb#24961 class: all 62 threads parked in futex/nanosleep, CPU→0, ~380 B/s (TCP keepalive only) for five hours, and interrupt() never honored. The statement ran 19,994,187 ms (5h33m) and ended only when the tenant's CI job hit its own 6h wall. Their refresh workflow uses a concurrency group with cancel-in-progress: false, so five consecutive hourly refreshes queued behind it and were cancelled.

A server-side bound would have turned that into a bounded failure and freed the concurrency group.

What

--statement-timeout / DUCKGRES_STATEMENT_TIMEOUT / statement_timeout: in YAML.

0 is the default and preserves today's unbounded behaviour — this changes nothing until a deployment opts in.

The deadline is applied in queryContextInner, the single point every execution path (simple, batched, extended, COPY, cursor) passes through on its way to an engine. It rides on the statement context, never the connection context: a timeout ends one statement, not the session.

The subtle part

That placement is exactly why classification needed care. When a statement deadline fires the connection context stays healthy, so c.ctx.Err() is nil and isCallerCancellation alone would have misclassified the timeout as an infra failure — surfacing a bare 42000 carrying context deadline exceeded instead of a clean cancellation.

• statementTimedOut() answers from the statement context and is checked first at every 57014 site.
• The message is PostgreSQL's exact wording, canceling statement due to statement timeout, because drivers string-match it to decide retryability. A user cancel keeps due to user request.
• isQueryCancelled had to learn context deadline exceeded — it previously matched only context canceled.

Honest limits

This is blast-radius containment, not a cure. An engine that ignores cancellation will not stop working when this fires: the client is freed with a clear error, but the worker can stay wedged. Retiring a timed-out worker is a named follow-up — it has to respect the destroy-before-reuse ordering in SessionManager.DestroySession.

Honoring a client-supplied statement_timeout GUC is also a follow-up; this PR leaves it an ignored SET so behaviour is unchanged for clients.

Testing

• server/statement_timeout_test.go: deadline lands on the statement context; 0 leaves statements unbounded; an expired deadline classifies as 57014 with the timeout wording while the connection context stays healthy; a user cancel keeps the user-request wording; isQueryCancelled matches both sentinel and wrapped forms of deadline/cancel.
• Full go test ./... green.
• Not asserted in the mw-dev e2e, deliberately — the knob is server-global, so an in-Job assertion would either start killing the harness's own legitimately slow assertions (reshard copies, DuckLake round-trips, concurrent writers) or burn minutes on a deliberately slow query. Reasoned in tests/mw-dev/README.md per the testing contract; it becomes cheaply assertable if the knob ever goes per-connection.

Suggested rollout

Set it well above the slowest legitimate statement and well below whatever external wall is otherwise the only bound. The affected tenant's real INSERTs peak near 10 minutes, so something like 60m in mw-prod-us bounds a wedge at ~1h instead of 6h while leaving real work untouched.

Also adds docs/plans/2026-09-16-warehouse-reliability-plan.md recording the three root causes found this session and what remains for each.

🤖 Generated with Claude Code

[github-actions]

Test Impact Plan

Deterministic summary of how this PR changes tests, CI runners, and coverage-risk signals.

Summary

Area	Added	Changed	Deleted
Test files	1	5	0
E2E/journey files	0	0	0
Workflow files	0	0	0

Signals

• Test cases: +6 / -0
• Assertions: +27 / -10
• Skips or known failures added: 0
• Workflow continue-on-error added: 0
• Workflow path filters added: 0
• Test commands removed from justfile: 0
• E2E/journey retry lines added: 0

Coverage risk: neutral or increased

No coverage-reduction warnings detected.

[bill-ph]

Found one P0 blocker: the new timeout context is not applied to extended-protocol non-cursor statements. In server/conn_extended_query.go, handleExecute creates queryCtx only for tracing and then calls c.executor.Exec(...) / c.executor.Query(...) (the executor implementations use background contexts); there is no queryContext()/queryContextInner() call on this path. Thus prepared/bound statements can still run indefinitely, even though the PR promises the timeout covers simple, batched, extended, COPY, and cursor execution. Please create/use a statement context for extended Execute (and pass it through the context-aware executor methods), then classify the resulting timeout with the PostgreSQL timeout wording.

— Robo Bill

[benben]

Automated review generated on behalf of @benben.

Approve. Adds an opt-in server-global per-statement deadline on the context built in queryContextInner and teaches the 57014 classification to recognize it. The knob is correctly wired through config for standalone and control plane, but the extended-protocol Execute/Describe paths never go through queryContextInner and use context-less executor calls, so the timeout does not reach them, and the sticky stmtCtx plus many hardcoded "due to user request" strings make the classification claims only partially true.

• major server/conn_extended_query.go:926 Extended-protocol Execute/Describe never get the statement deadline. handleExecute and the Describe probes call context-less executor.Query/Exec and never call queryContext(), so JDBC/pgx/psycopg3 statements stay unbounded despite the PR's every-path claim.
• major server/conn.go:538 Sticky timed-out stmtCtx misclassifies later unrelated errors as timeouts. A simple-protocol statement times out, then an extended Execute (which never overwrites stmtCtx) hits any rows.Err(); line 1085/1171 and conn_results.go report 57014 timeout and skip logging.
• major server/conn.go:1885 Most 57014 sites still hardcode 'due to user request' on timeout. A timed-out simple-protocol INSERT (the incident shape) surfaces the user-request wording here and at conn_query_exec.go:73/249/365/868/920, conn_extended_query.go:902/970, conn_cursor.go:309/384/450/509.
• minor server/conn_cursor.go:382 Cursor lifetime timeout classifies as 42000 after another statement ran. DECLARE ctx expires but stmtCtx points at a later statement, so isCallerCancellation is false and the client gets a bare 'context deadline exceeded'.
• minor server/conn_errors.go:26 isQueryCancelled now maps every deadline error to 57014 even with timeout=0. Internal execTimeout/attach deadlines or worker-side gRPC DeadlineExceeded now surface as 57014 and count as cancelled in query_metrics.go:187.
• minor server/statement_timeout_test.go:20 Tests never drive a real execution path; extended gap passes green. All tests call queryContextInner directly; none executes a statement through handleExecute or executeSelectQuery with a deadline.
• nit tests/mw-dev/README.md:529 New bullet inserted mid-sentence inside the PostHog analytics bullet. The preceding bullet's text 'to read them back, so ingestion cannot...' now dangles after the new paragraph.

Model: fable, PR authored with opus.

[fuziontech]

Thanks — the P0 was real, and two of the findings were design errors on my part rather than gaps. Pushed f59661c5.

P0 — extended Execute never got the deadline. Confirmed: RegisterQuery is only ever called from queryContextInner, and server/conn_extended_query.go had zero call sites, so handleExecute ran on the context-less executor.Query/Exec (i.e. context.Background() in PinnedExecutor). My "single point every execution path passes through" claim came from an existing comment about cancellation registration; I didn't verify the call graph, and it was wrong.

The gap was wider than the timeout — prepared statements weren't reachable by a pgwire CancelRequest either. Routing this path through queryContextInner fixes both. Regression test drives a real Parse/Bind/Execute and asserts the executor saw a deadline; verified it fails without the fix (extended Execute never reached the executor's context-aware path).

Two consequences I handled rather than leave to be found later:

• queryContextInner(false) — no disconnect monitor. Its bufio Peek would race the message loop's reads, since the extended protocol pipelines Parse/Bind/Execute/Sync. Same reasoning the cursor path documents. (It also panicked on nil-reader test conns, which is how I noticed.)
• A suspended portal keeps its RowSet open for the next Execute, so cancelling at the opening Execute's return tore down the rowset the next one resumes from — caught by TestPortalSuspensionPaging. Release ownership now moves to the portal and closeExec runs it, mirroring how finishProfiling is already retained across legs.

Sticky stmtCtx (major) — agreed, and removed entirely. Classification is now driven by the error and gated on the timeout being configured, so there's no stored state to go stale.

isQueryCancelled widening (minor) — agreed, reverted. That gate also restores byte-for-byte behaviour when the feature is off, so internal deadlines (attach, exec, worker gRPC) don't become 57014 for deployments that never enable this.

Hardcoded wording (major) — fixed at all 13 sites via cancellationMessage(err).

Tests never drove a real path (minor) — agreed, and it's exactly why the extended gap passed green. The new test goes through the real protocol handlers.

README nit — fixed, the bullet had orphaned the preceding sentence.

One behaviour change worth flagging: extended Execute now emits a QueryStart query-log entry (via markExecStarted) where it didn't before — the intended consequence of becoming cancellable. Six fixtures encoded the old behaviour (three read the first channel entry, three recorded on the context-less methods) and were updated.

Full go test ./... matches the clean-tree baseline exactly; the three remaining failures (cache-proxy, opa, tests/integration) reproduce with my changes stashed.

[bill-ph]

Re-reviewed at commit f59661c. The follow-up correctly routes extended Execute through queryContextInner(false) and ExecContext/QueryContext, keeps suspended portal ownership of the statement cleanup, and adds a regression test for the previously unbounded prepared-statement path. No remaining P0 blockers found.

Non-blocking note: the extended Describe metadata probes still use context-less executor calls, so a pathological Describe can outlive the configured statement timeout; consider covering those probes if the timeout is intended to bound planning/metadata work too.

— Robo Bill

[benben]

Automated review generated on behalf of @benben.

Approve (re-review of new commits). The new commit routes extended-protocol Execute through queryContextInner with portal-owned release, and rewrites timeout classification to be error-driven and feature-gated; both hold up, but the newly documented "every path" invariant is still violated by the writable-CTE rewrite and COPY paths, and the suspended-portal ownership collides with the single-slot cancel registry.

Extended Execute deadline, sticky stmtCtx, hardcoded wording and the README bullet are fixed; the Describe probes (conn_extended_query.go:402/552) still call context-less executor.Query.

• major server/conn_query_exec.go:1180 Writable-CTE rewrite and COPY paths still run context-less, timeout unreachable. An extended-protocol INSERT rewritten via WITH-CTE (executeMultiStatementExtended) or a COPY TO (conn_copy.go:298/377) calls executor.Exec/Query without stmtCtx, contradicting the new CLAUDE.md invariant.
• minor server/conn_extended_query.go:880 closeCursorsAtTxEnd unregisters the in-flight statement's cancel entry. Extended COMMIT/ROLLBACK with a suspended portal open: closeExec→releaseStmtCtx→UnregisterQuery deletes the COMMIT's own single-slot registration, so a CancelRequest during it finds nothing; likewise any later statement clobbers a suspended portal's registration.
• minor server/conn_extended_query.go:871 Suspended-portal deadline spans all Execute legs; undocumented like cursors. JDBC/pgx fetchSize paging whose client-side processing exceeds the timeout gets 57014 on the next Execute while the server is idle; CLAUDE.md documents this only for DECLARE cursors.
• nit server/conn_errors.go:85 Duplicate statementTimedOut check is dead code. Already returned true at line 77.
• nit server/conn_extended_query.go:865 Extended Execute now emits QueryStart entries; query-log volume change undocumented. markExecStarted inside queryContextInner adds a QueryStart row per prepared statement and moves post-start failures to ExceptionWhileProcessing; not mentioned in PR description.

Model: fable, PR authored with opus.