Collect PostgreSQL deadlock reports from the server log (#2661)

[erikdarlingdata]

Closes #2661.

What was missing

pg_stat_database.deadlocks — a number that goes up. SQL Server has three deadlock reads and a graph
viewer; PostgreSQL had a counter.

PostgreSQL writes the whole thing to its server log:

Process 1549 waits for ShareLock on transaction 809; blocked by process 1556.
Process 1556 waits for ShareLock on transaction 808; blocked by process 1549.
Process 1549:
BEGIN; UPDATE dl SET v=v+1 WHERE id=1; ... UPDATE dl SET v=v+1 WHERE id=2; COMMIT;
Process 1556:
BEGIN; UPDATE dl SET v=v+1 WHERE id=2; ... UPDATE dl SET v=v+1 WHERE id=1; COMMIT;

The wait graph, the victim, and every participant's full statement. In that last respect it beats the
SQL Server graph, which names the victim's SQL and frequently leaves the other side as a handle.

Why it works where plan capture does not

auto_explain needs a preload and a restart — on a managed fleet that is a parameter-group change and a
reboot, which is exactly why #2538's Aurora half is closed as unreachable. A deadlock report is
unconditional: no setting suppresses it, and log_lock_waits governs ordinary lock waits rather than
this. The only precondition is reading the log, which plan capture already established and
pg_plan_capture_readiness already reports on. So this ships value on the fleet in front of us today.

Two traps, both measured rather than assumed

%Q writes the query id with no separator before the severity. The captured line really reads
[1549] 322048460535975151ERROR: deadlock detected. A pattern requiring whitespace there matches nothing —
and matches nothing in the way that looks like "this server has no deadlocks", which is the worst possible
failure for this collector.

The DETAIL block ends at the next line carrying a log prefix, not at a blank line or after N lines. A
participant's statement is arbitrary user SQL that can contain newlines, each arriving tab-indented. A
line-count rule truncates every multi-line statement to its first, and the row still looks fine.

Both are pinned by tests built from real captured output, not a plausible-looking approximation — a fake
would not have had either.

Identity, because the window overlaps on purpose

Both transports read a bounded tail on a schedule, and the window overlaps deliberately so a report cut in
half at one edge is whole in the next. Every row carries a hash of its graph text and the reads group on it.

The hash is over the graph, not over (timestamp, victim_pid): two reports in the same millisecond with
the same victim pid are vanishingly unlikely, but the graph is what actually distinguishes them, and hashing
the thing itself needs no argument about how unlikely a collision is.

Participants are counted from the wait edges, not the Process N: headers — the server omits a header
when it could not recover the statement text, and a participant with no statement is still in the cycle.

Verification

A real deadlock induced on the rig, through the real service:

• the collector stored it with the same hash the parser produced offline
• get_pg_deadlocks returned the victim, participants, lock modes and resources; get_pg_deadlock_detail
  returned the graph with both participants' SQL
• a second induced deadlock then proved both halves of the identity design: the store held three rows
  which the read collapsed to two deadlocks — the first with times_seen: 2 from the overlapping
  window, and the repeat as its own row, because the process IDs differed and so did the hash

Both read statements were also checked with PREPARE against a real PostgreSQL with their actual parameter
signatures — the same thing DarlingPgReadSqlParsesLiveTests does — and all 20 parser assertions were run
against the product assembly locally before pushing.

Empty is ambiguous, and the reads say so

No deadlocks is the healthy answer and the shape of an unreadable log. Every empty envelope here names the
independent check rather than leaving it: pg_stat_database's cumulative counter. If that moved and this is
empty, the log is the problem rather than the server.

[claude]

occurred_at is parsed assuming the log timestamp is already UTC — it isn't, unless log_timezone = 'UTC'.

%m (and %t) render in whatever log_timezone the target has configured, and the zone abbreviation is captured right here (s_deadlockBlock's (?<zone>\w+) group at line 61) but then discarded — Extract() only forwards match.Groups[1] (the bare digits) and match.Groups["pid"]/["detail"] into FromBlock. FromBlock then does:

DateTime.TryParse(timestampText, CultureInfo.InvariantCulture,
    DateTimeStyles.AdjustToUniversal | DateTimeStyles.AssumeUniversal, out var occurredAt)

AssumeUniversal treats the parsed wall-clock digits as already being UTC. That's only correct when the source server's log_timezone is UTC. This code path is explicitly for the self-hosted/filesystem transport (the RDS-API path is separate and Aurora conventionally logs in UTC), and on a self-hosted target where an admin has set log_timezone to something else (a common thing to do, e.g. to match local business hours), every stored occurred_at will be silently off by the zone offset.

That's not just cosmetic — DarlingPgDeadlockReader.GetDeadlocksAsync windows on occurred_at >= $2 AND <= $3 (built from hours_back), so a shifted timestamp can push a real deadlock outside the requested window and it just won't show up in get_pg_deadlocks — the same "silently looks like no deadlocks happened" failure mode the PR description calls out for the %Q-separator trap, just from a different cause that isn't covered by a test here (all the fixtures use UTC as the zone).

Worth either converting using the captured zone abbreviation (best-effort TimeZoneInfo lookup) or, more simply, reading the session's actual log_timezone alongside the tail (like the readiness collector already reads other GUCs) and applying that offset before storing.

[claude]

Reviewed the diff for correctness, Lite/Darling parity, and security.

Findings:

• Inline comment on PgDeadlockLogParser.cs: occurred_at is parsed with DateTimeStyles.AssumeUniversal, treating the raw %m timestamp digits as UTC. The captured timezone abbreviation is discarded. On a self-hosted target where log_timezone isn't UTC, stored timestamps will be off by the zone offset, which can push real deadlocks outside a hours_back window and make get_pg_deadlocks silently report nothing — the same failure shape the PR explicitly designed around for the %Q and multi-line-DETAIL traps, just untested for a non-UTC zone.

Checked and found OK:

• Lite/Darling parity: get_pg_deadlocks/get_pg_deadlock_detail are correctly added to the Darling-only ratchet list in CrossAppMcpToolInventoryPinTests (Lite has no PostgreSQL target), and the new table is naturally excluded from Lite's DuckDB schema via TargetEngine filtering — no drift.
• Storing the victim/participant statement text verbatim (no literal redaction) matches the existing precedent of the SQL Server DeadlocksCollector (VictimSqlText/GraphXml verbatim), so this isn't a new deviation from PgPlanCaptureCollector's redaction (which exists because literals recur throughout the plan tree, a different problem).
• The C# regex (PgDeadlockLogParser) and the embedded PostgreSQL ARE regex (PgDeadlocksCollector.QueryText) are kept textually identical for the two documented traps (%Q no-separator, tab-indented multi-line DETAIL) — good, since these are two independent implementations that both transports depend on agreeing.
• Migration schema-qualifies collect.pg_deadlocks, column order matches PayloadColumns/WritePayload, schema version and hypertable worker-count bumps (67→68 collectors, 70→71/81→82 workers) are all consistent with the new table.
• Grid row wiring in ViewerServerTab.xaml (rows 16/17) lines up correctly with the appended RowDefinitions, despite some stray indentation in that block.

[claude]

Correctness: this collector will actually run — and fail — on Aurora/RDS, contrary to the doc comments and the PR's own claims.

AppliesTo returns true for every target and the comment above it says "The route is chosen at dispatch, so this definition never executes against a managed target." But unlike pg_plan_capture, there is no dispatch branch for pg_deadlocks:

["pg_deadlocks"] = (r, s, ct) => r.RunAsync(PgDeadlocksCollector.Instance, s, ct),

(DarlingWorker.cs:4823)

compare with the real managed-target routing that exists for plan capture:

["pg_plan_capture"] = (r, s, ct) =>
    s.Target.IsAurora || s.Target.IsAwsRds
        ? r.IngestRdsPlansAsync(s, ct)
        : r.RunAsync(PgPlanCaptureCollector.Instance, s, ct),

(DarlingWorker.cs:4845)

There's no IngestRdsDeadlocksAsync/RdsDeadlockIngestor anywhere in the diff. So on Aurora/RDS this collector's raw query (pg_catalog.pg_ls_logdir() + pg_catalog.pg_read_file(...)) runs directly against a target where, per this codebase's own RdsLogUnavailableTests.cs, that function only returns a PERMISSIONS failure (pg_read_server_files isn't grantable there). Every 5-minute cycle will log a permission failure and never collect a row.

This directly contradicts the CHANGELOG entry ("It needs nothing configured on the target... so it works on a managed fleet today") and the MCP tool descriptions/doc comments claiming parity with pg_plan_capture's dual-transport design. Either the RDS ingestion route needs to be built (mirroring RdsPlanIngestor), or the claims need to be walked back and AppliesTo should exclude managed targets until it exists.

[claude]

Correctness bug: "omit the hash → most recent graphs" doesn't hold.

DISTINCT ON requires its result set to follow the ORDER BY it's paired with, and here that's ORDER BY d.deadlock_hash, d.collection_time (required, since the DISTINCT ON key must lead the sort). So after dedup the rows are in hash order, not recency order, and LIMIT $3 then takes the first N in that hash order.

When deadlock_hash is supplied this doesn't matter (the WHERE clause narrows to at most one group). But the no-hash case — which is exactly how both callers use it, GetPgDeadlockDetail with deadlock_hash: null and the "Deadlock Graphs" JS panel ({ server, limit: 5 }, no hash) — is documented as returning "the most recent graphs" (see the XML doc above GetDeadlockDetailAsync and the MCP tool description in DarlingMcpPgDeadlockTools.cs). As written it instead returns an arbitrary, hash-ordered sample of up to limit distinct deadlocks, which for a busy server means the same 5 (whichever hash sorts first) forever, never the newest.

Fixing this needs the recency ordering applied after the per-hash dedup, e.g. wrap in a subquery:

SELECT * FROM (
    SELECT DISTINCT ON (d.deadlock_hash) ...
    ORDER BY d.deadlock_hash, d.collection_time
) x
ORDER BY x.occurred_at DESC
LIMIT $3

No test exercises this query's ordering (unlike DeadlocksSql, which correctly does GROUP BY ... ORDER BY MIN(occurred_at) DESC), so it shipped unverified.

[claude]

Reviewed the deadlock-collector addition. Overall the log parsing is careful and well tested (the two documented traps — %Q's missing separator and the tab-indented multi-line DETAIL block — are backed by real captured fixtures), and the Lite/Darling shared-library split, schema migrations, MCP tool inventory, worker-sizing math, and cross-app pin tests are all kept in sync correctly.

Left two inline comments on concrete bugs, both high-confidence and independently verifiable from the diff:

1. PerformanceMonitor.Collectors/PgDeadlocksCollector.cs — AppliesTo returns true for every target and claims "the route is chosen at dispatch, so this definition never executes against a managed target," but DarlingWorker.cs's dispatch table has no Aurora/RDS branch for pg_deadlocks (unlike pg_plan_capture, which has an explicit IsAurora || IsAwsRds branch to IngestRdsPlansAsync). No RdsDeadlockIngestor exists. So on managed targets this will run pg_read_file/pg_ls_logdir directly and fail with a PERMISSIONS error every cycle — contradicting the CHANGELOG's and MCP tool docs' claim that this "works on a managed fleet today."

2. Darling/PerformanceMonitor.Darling.Storage/DarlingPgDeadlockReader.cs — DeadlockDetailSql's DISTINCT ON (deadlock_hash) ... ORDER BY deadlock_hash, collection_time LIMIT $3 returns rows in hash order, not recency order, once DISTINCT ON collapses the dupes. The no-hash call path (both get_pg_deadlock_detail with no hash and the "Deadlock Graphs" panel) is documented to return "the most recent graphs" but will instead return the same hash-ordered slice regardless of when deadlocks actually occurred.

No missing-index DMV suggestions, nothing security-sensitive stood out (parameterized queries throughout, no string-built SQL from user input), and I didn't spot other Lite/Darling parity gaps beyond the two above.