133 NpgsqlCommand sites still inherit Npgsql's undocumented 30s default timeout

Summary

133 NpgsqlCommand sites still set no CommandTimeout, so each silently inherits Npgsql's undocumented 30 s default — a value nobody chose. Split: 111 in .Service, 20 in .Storage, 2 in .Viewer.

This is a tracking issue for a characterized defect class, not a speculative cleanup. It has already produced three distinct production failures.

Why this is worth tracking rather than discovering one outage at a time

#2810 — all 31 PgFactCollector commands inherited the default. Measured on the production store with the shipped query across the six busiest servers, two crossed 30 s cold (34.7 s, 32.4 s) and a third sat at 28.2 s, while all were comfortably under it warm (10.6-21.2 s). It failed only on the cold-plus-large combination, which is exactly why it read as intermittent, and why measuring by hand looked fine — the second run is always warm. Two earlier PRs had aimed at it and missed: one made the query 2.4x faster, the other made the failure visible, and neither touched the ceiling.

#2871 — the analysis pass, 32 sites. All 19 production failures landed between 30.1 s and 31.4 s: a fixed wall, not a variable-duration fault. The shipped query runs in 1.4-1.6 s and 0.2-0.3 s on the two stores, so it only crosses when the store stalls. Its blast radius was worse than one pass: a null baseline result was cached unconditionally, silencing that (server, metric) pair's anomaly detection for the full 1-hour cache TTL.

#2796 — the same shape one layer down. An unbounded server-scoped watermark read exceeded the 30 s default on a large store; the cancelled read returned null, which is indistinguishable from a first run, so the collector silently downgraded to its 60-minute fallback window and re-collected what it already held.

The common defect is not slowness. It is that the deadline was never a decision, and that exceeding it fails in a way that looks like a legitimate result.

What a fix looks like

#2810 and #2871 both set an explicit value with two-sided reasoning: above the measured worst case, and below the budget the caller shares. In both cases the upper bound was the binding one — a deadline near the shared pass budget lets a single stalled command consume the pass and cost that server every other read in it.

The value does not transfer between groups. The 60 s used in both closed passes was derived from the 120 s s_analysisTimeout. The 133 remaining sites run under the collection sweep, which has a different budget and a different sharing structure, so each group needs its own reasoning rather than the number copied across.

Suggested shape

Group by the budget each site runs under, not by file. For each group: measure the real worst case (cold, on the largest store — a warm sample will look fine), pick a bound with both sides stated, apply by shape rather than by the reported site, and pin structurally so a new untimed command cannot be added silently. Prove the pin red more than one way; #2810 and #2871 each proved theirs three ways, with each variant failing a different assertion.

Worth doing in separate PRs per group rather than one sweep, since the reasoning differs and a wrong bound is worse than no bound.

Scope boundary, established while investigating #2800: this class does not apply to Lite.

DuckDBCommand.CommandTimeout defaults to 0 — no limit. The defect being tracked here is specifically Npgsql's undocumented 30 s default being inherited by a command nobody set a deadline on, and then a cancellation at that ceiling failing in a way that looks like a legitimate result. With no ceiling there is nothing to exceed and nothing to cancel, so the mechanism cannot occur on the DuckDB side.

The 133 remaining sites are therefore all Npgsql: 111 .Service, 20 .Storage, 2 .Viewer. A sweep should not be extended to Lite's collectors on the grounds that the shapes look alike.

Related and worth noting for whoever picks this up: #2800 measured Lite's equivalent read at 0.34 ms unbounded against Darling's 40,743-50,560 ms for the same logical query. Cost structures diverge sharply between a columnar engine and chunked Postgres, so a fix justified by measurements on one side should not be assumed to transfer to the other in either direction.

Narrow the group to the alert pass proper: drop PgPlanForceActionStore

Merge pull request #2882 from erikdarlingdata/fix/2874-service-timeouts

Group 1 done: the alert evaluation pass — 45 sites, merged in #2882 (`8e046fa3`)

The census in this issue undercounts by roughly 3.5x, and the reason matters

The 133 figure came from scanning new NpgsqlCommand(. That is not the only construction shape: NpgsqlDataSource.CreateCommand(sql) returns a command carrying the same inherited 30 s default, and the census misses every one of them. Recounted on current dev across both shapes:

project	sites	untimed
`.Service`	260	198
`.Viewer`	192	192
`.Storage`	124	67
`.Analysis`	71	0 (done — #2810, #2871)
total	647	457

By shape: new NpgsqlCommand( 276 sites / 90 untimed; .CreateCommand( 371 sites / 367 untimed.

Note .Viewer in particular — this issue records it as 2 sites; it is 192, essentially all _dataSource.CreateCommand(...). Four of Group 1's own sites were that shape too, in DarlingPostgresAlertReadAdapter, and a pin keyed to one spelling would have declared the family clean while they stayed on the default. That is the #2786 shape, so the guard added in #2882 matches both.

What Group 1 established that does not transfer

The alert pass runs sequentially inside the per-server sweep body. It has no enclosing budget — EvaluateAlertsAsync is called with the plain stopping token, no CancelAfter anywhere. That is structurally unlike #2810 and #2871, which both sit under DarlingWorker.s_analysisTimeout's 120 s bound.

So for this group the per-command deadline is the pass budget, multiplied by the number of sequential reads, while the body holds one of only four fleet sweep permits and that server's relaunch is skipped throughout.

The value therefore went down, not up: 10 s, bounded below by the measured worst case (1,744.9 ms cold, the forced-plan read over ~6.0 GB; every other read in the family under 3 ms) and above by the 30 s s_alertSweepInterval the pass runs on. The asymmetry is what justifies erring short — an exceeded deadline skips one alert check and retries in 30 s, while a long-running read starves fleet-wide collection.

This was live, not theoretical: the forced-plan read failed five times on the production store on 2026-09-04, each surfacing as Exception while reading from stream — how Npgsql renders its own deadline, and the exact misdiagnosis #2826 exists to prevent.

One site set was deliberately excluded

PgPlanForceActionStore (4 sites) looks like a member of this family and is not one — review caught this. Its only caller is PlanForceBot.RunAfterAnalysisAsync, built at DarlingWorker.cs:4084 closing over the plain stoppingToken, so it runs outside the analysis pass's CancelAfter despite being invoked lexically inside it. It shares the unbudgeted shape but runs on the analysis interval (clamped 5-360 min), not a 30 s cadence, so the upper bound derived here does not apply. Left for its own group; the exclusion is documented in both the constant and the pin so it is not re-added by resemblance.

Suggested remaining groups

Grouped by the budget each runs under, not by file:

.Viewer (~192) — interactive reads on a user-facing window. Different bound entirely: a human is waiting, and there is no sweep permit at stake.
MCP readers (~45+, Mcp/DarlingDataReader.cs 26, DarlingFleetReader 11, DarlingTrendReader 8) — also interactive, but agent-driven and potentially long-running by design.
The post-analysis force-plan pass — PgPlanForceActionStore's 4, above.
Startup / migration (DarlingStoreUpgrade 7, TimescaleSupport 8, DarlingManagedPostgres) — long-running by nature; PgMigrations already carries its own 300 s.
The collection sweep proper (DarlingCollectorRunner 8, DarlingWorker 7, StoreConfigProvider 13).

The 60 s from #2810/#2871 and the 10 s from #2882 are all derived from their own pass's budget and cadence. Neither transfers. Each group needs its own measurement and its own two-sided reasoning; a wrong bound is worse than no bound.

Give the migration lock wait its own derived budget, not one statemen…

Correct the storage census to the number the shipped scanner actually…

Merge pull request #2888 from erikdarlingdata/fix/2874-storage-timeouts

Group 2 landed: `PerformanceMonitor.Darling.Storage`

#2888 — merged as ef6dfe36310408dff112e0f299e67d04f5bb283a.

Census, corrected. The shipped doc comment said 69 untimed; the real number is 70 of the project's 124 command sites. Re-derived empirically by lifting the pin's own regexes and its depth <= 0 statement-span walker into a net10.0 harness and running it over the merge-base tree (8e046fa3) — the only tree the claim describes, since post-fix the count is zero by construction:

	total	`new NpgsqlCommand(`	`.CreateCommand(`
sites	124	74	50
untimed	70	20	50

Every one of the project's 50 .CreateCommand( sites was untimed. The stale 69 traces to a partly-corrected scan: the same walker with its depth counter clamped at zero reports 68, missing exactly PgMigrations.cs:3981 and QueryStoreSliceRepair.cs:321; 69 was written when only one of those two had been found, and the "49 of 69" split followed by subtracting the correct 20 from the stale total. Corrected in the pin's doc comment, the CHANGELOG and the PR body, which now all agree.

The 367 of 371 figure verified and is unchanged — it is the .CreateCommand( shape Darling-wide, not per-project: .Service 131 + .Viewer 190 + .Storage 50 = 371 sites, 127 + 190 + 50 = 367 untimed.

Five budget regimes, each binding itself rather than sharing one blanket value: the MCP read surface (48 sites) at a new StorageCommandDeadlines.McpReadSeconds = 30; the migration session's six scaffolding statements at the existing MigrationCommandTimeoutSeconds (300); the advisory-lock wait at a new MigrationLockWaitTimeoutSeconds; TimescaleDB setup/rearm (3) and its catalog/coverage reads (5) at SetupTimeoutSeconds / JobCatalogReadTimeoutSeconds; the startup self-test (3) at its own per-layer 8 s; plan-dim maintenance (3) at a hoisted 300 s, with VACUUM FULL keeping its deliberate CommandTimeout = 0; and the slice-repair rail-lift (1) at its session's 900 s.

New constant: MigrationLockWaitTimeoutSeconds = 5 * MigrationCommandTimeoutSeconds (1500 s). Review caught that the first pass gave the pg_advisory_lock acquire one statement's budget, while the lock is taken once and held while every pending rung applies in the same session. Floored on the rung census — exactly four of the 108 rungs touch pre-existing data (V22, V23, V39, V104), each one command already capped at the statement bound, so four multiples is the floor and the fifth is margin. Capped from above by nothing: MigrateAsync has one production caller on the plain stopping token, with no CancelAfter, no configured HostOptions.StartupTimeout, no health check, no container HEALTHCHECK and no orchestrator probe, so the value comes from the failure asymmetry instead.

It narrows the mid-wait death rather than closing it. A fifth data-moving rung, or one rung needing longer than its own bound, still outlasts the wait — tracked separately in #2894. The CHANGELOG no longer claims to be "closing" that startup failure.

For whoever takes `.Viewer` (~192 sites) or `.Service`

The budget-regime reasoning from this group does NOT transfer. Do not copy McpReadSeconds = 30, the 300 s statement bound, or the 1500 s lock wait into another project. Every one of those numbers was derived from what specifically shares that budget:

McpReadSeconds is bounded below 60 s because those reads have no enclosing budget and nothing restarts them — a stalled read holds a pooled store connection and hangs an interactive MCP tool call. A read that sits under an enclosing CancelAfter, or that a sweep restarts, has a different upper bound.
MigrationLockWaitTimeoutSeconds is 5 x the statement bound only because four specific rungs can each legitimately consume a statement bound. That multiplier is meaningless anywhere else.
The self-test's 8 s comes from its own per-layer network probe budget, not from anything about PostgreSQL.

Per this issue's own rule: group by the budget each site runs under, measure the real worst case cold on the largest store, state both bounds, apply by shape rather than by the reported site, and pin structurally. And re-derive the site count rather than starting from this issue's split — the .Service figure of 111 does not reconcile against an empirical recount (#2895).

This issue stays open. .Viewer (~192 sites) and .Service remain.

Sweep the viewer pin recursively, minus build output (#2874)

Correct the export-surface count in the viewer deadline entry (#2874)

Record the pin's recursive sweep and the sixth red-first variant (#2874)

Merge pull request #2901 from erikdarlingdata/fix/2874-viewer-timeouts

`.Viewer` group landed — and its budget came out at 15 s, not the 30 s the MCP family uses

PR #2901, merged as part of this tracking issue. This issue stays open: .Service is still outstanding.

Three regimes, and the notable result is how few

.Viewer is a WPF WinExe and nothing encloses any read — zero CancelAfter, SemaphoreSlim, .WaitAsync, Task.WhenAny, CreateLinkedTokenSource, no ASP.NET/Kestrel, verified against the pre-change tree. So as in #2882 and #2888, the per-command deadline IS the budget.

regime	sites	constant	measured floor (cold)	ceiling derived from
interactive / refresh	188	`InteractiveReadSeconds = 15`	3.01 s	a connection permit, not a time budget
command plane	3	`CommandPlaneSeconds = 5`	3.9 ms	the 45 s poll loop it sits inside
connect gate	2	`ConnectGateSeconds = 10`	79 ms	the 60 s connect-timeout clamp

Interactive and background refresh are NOT separable, which was checked rather than assumed: the fleet timer and per-tab timer call the same methods a user gesture calls (RefreshActiveInnerTabAsync is the tab-activation method), and the set of reads reachable only by the unattended fan-out is empty — all seven candidates have gesture or visible-tab callers. Splitting them would need a budget threaded per call site, not a constant. Export is likewise not a regime: DataGridExport is entirely static, no async, no Npgsql — all ten surfaces format in-memory grid rows.

Why 15 and not 30

The floor is measured, not asserted: a store built by the product's own PgMigrations.MigrateAsync (V109 + TimescaleDB), seeded to exact production per-server density (derived from get_collector_cost: 189,414 query_stats rows/server/day) across the full 30-day retention horizon — 5.68 M + 1.98 M rows. TopQueriesSql, dumped from the built DLL, measured 588 ms at 1 h, 1.12 s at 7 d, 3.01 s at 30 d. 30 days is the widest window reachable, because retention drops the data behind it.

The ceiling is a permit, not a clock — this is the part that makes the number specific to this surface. MaxPoolSize = 10, and CorrelatedTimelineLanesControl awaits a single Task.WhenAll over exactly ten reads: one panel can hold every permit in the pool. An eleventh read then waits ConnectionTimeoutSeconds (default 5) and throws a connect error — misreporting a slow store as a network fault. Ten concurrent 30-day reads measured 24.4–64.1 s, six of them past that silent default.

So the derivation lands below the MCP family's 30 s rather than at it: reads 4.4× slower than the MCP surface's 685 ms, against a permit pool 10× scarcer. Not a copied number — the two surfaces share a store, not a budget.

CommandPlaneSeconds is pinned relationally against DefaultCommandTimeout rather than by value, and it narrows rather than closes its overshoot: PollCommandResultAsync checks its budget only between iterations, so worst case drops from ≤30 s to ≤5 s but is not eliminated. ConnectGateSeconds is separate because both probes swallow failures (fail-open / fail-safe), so an overshoot there silently mis-classifies the store instead of reporting.

Census correction: 193 sites, not 192

The census I briefed was one short, and the missed shape is worth recording for whoever takes .Service:

ViewerDataService.Blocking.cs passed connection.CreateCommand as a bare method group into a helper in .Analysis, which built the command and set only CommandText. No ( follows the method name, so neither of #2874's two census regexes can see it. Fixed with a factory lambda that stamps the deadline; the new pin scans for that shape explicitly.

That makes three distinct construction shapes now known to this sweep: new NpgsqlCommand(, .CreateCommand(, and a bare .CreateCommand method group passed as a delegate.

The pin

ViewerCommandTimeoutTests.cs, directory-scoped like .Storage's but recursive rather than TopDirectoryOnly — that change was forced by a red-first variant which planted an untimed command in Themes/ and found it invisible to a top-level-only walk. It asserts bands, not equalities, following .Storage's precedent of not freezing every deliberate value in one test.

Proven red six ways, each failing a different assertion: remove one deadline; revert the method group; untimed command in a subdirectory; 15→30 (ceiling); 15→3 (floor); 5→30 (relational ceiling). Test count moved 6913 → 6928, exactly +15 — 5 [Fact] plus 10 [InlineData].

What remains on this issue

.Service is the last group. Current per-project counts on dev after this merge — crude construction-site counts against timeout assignments, so read them as direction not gospel:

project	construction sites	timeout assignments
`.Service`	260	90
`.Storage`	124	125
`.Viewer`	193	194
`.Analysis`	71	76

Note #2895 is open on this exact point: this issue's stated split (111 .Service / 20 .Storage / 2 .Viewer) does not reconcile with a re-derived count, and .Service is where the discrepancy sits. Re-derive .Service's census before working it, and derive its budget from its own callers — the four numbers this sweep has produced (60 s, 10 s, 30 s, and now 15/5/10) came from four different enclosing constraints, and none of them transferred.

Not verified

The Windows suites ran in CI only. All timings are from a local seeded store, not production — the AWS CLI was unavailable in that lane's session, so no production timing was taken. No live viewer run: the connection-pool argument is derived from source rather than observed. And no user-reported viewer failure is closed by this — it removes an inherited default, it does not fix a reported symptom.

Re-measure the sweep floor the widened regex moved (#2874)

Catch the method-group hand-off through a local, not just inlined (#2874

Give every Darling storage command an explicit deadline (Part of erik…

Give every Darling viewer command an explicit deadline (erikdarlingda…

Stamp the analysis pass's two method-group command sites (erikdarling…

Part of erikdarlingdata#2874: explicit deadlines on the collection sw…

Give the alert pass's CPU read its explicit deadline (Part of erikdar…

CHANGELOG: the alert pass's CPU read deadline and the computed pin sc…

Give the startup path's store commands an explicit deadline (erikdarl…

Part of erikdarlingdata#2874: derived deadlines for the three straggl…

Claim the serial collection-loop thread's nine commands too (erikdarl…

Hold the Query Store backfill's per-server gate past an abandonment (e…

Judge the alert-pass deadline on stripped source, so a comment cannot…

Stamp the reload watermark from the version actually applied (erikdar…

Describe the current call shape in the watermark pin's doc comment (e…

Scope the store-connection question to the nearest binding (Part of e…

Adopt the shared command-deadline judgement in the collection-sweep p…

Navigation Menu

Repository navigation

133 NpgsqlCommand sites still inherit Npgsql's undocumented 30s default timeout #2874

Summary

Why this is worth tracking rather than discovering one outage at a time

What a fix looks like

Suggested shape

Group 1 done: the alert evaluation pass — 45 sites, merged in #2882 (`8e046fa3`)

The census in this issue undercounts by roughly 3.5x, and the reason matters

What Group 1 established that does not transfer

One site set was deliberately excluded

Suggested remaining groups

Group 2 landed: `PerformanceMonitor.Darling.Storage`

For whoever takes `.Viewer` (~192 sites) or `.Service`

`.Viewer` group landed — and its budget came out at 15 s, not the 30 s the MCP family uses

Three regimes, and the notable result is how few

Why 15 and not 30

Census correction: 193 sites, not 192

The pin

What remains on this issue

Not verified

54 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

133 NpgsqlCommand sites still inherit Npgsql's undocumented 30s default timeout #2874

Description

Summary

Why this is worth tracking rather than discovering one outage at a time

What a fix looks like

Suggested shape

Activity

erikdarlingdata commented on Sep 4, 2026

erikdarlingdata commented on Sep 4, 2026

Group 1 done: the alert evaluation pass — 45 sites, merged in #2882 (8e046fa3)

The census in this issue undercounts by roughly 3.5x, and the reason matters

What Group 1 established that does not transfer

One site set was deliberately excluded

Suggested remaining groups

erikdarlingdata commented on Sep 4, 2026

Group 2 landed: PerformanceMonitor.Darling.Storage

For whoever takes .Viewer (~192 sites) or .Service

erikdarlingdata commented on Sep 4, 2026

.Viewer group landed — and its budget came out at 15 s, not the 30 s the MCP family uses

Three regimes, and the notable result is how few

Why 15 and not 30

Census correction: 193 sites, not 192

The pin

What remains on this issue

Not verified

54 remaining items

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Group 1 done: the alert evaluation pass — 45 sites, merged in #2882 (`8e046fa3`)

Group 2 landed: `PerformanceMonitor.Darling.Storage`

For whoever takes `.Viewer` (~192 sites) or `.Service`

`.Viewer` group landed — and its budget came out at 15 s, not the 30 s the MCP family uses