Skip to content

A PostgreSQL analysis pass now names the statement: PG_BAD_ACTOR facts from pg_statement_stats carry one shape's share of the window's execution time, its calls/sec and mean, and a drill-down with the normalised text — the query-shaped leaf every PostgreSQL story needs (#3542 lane 7) - #3670

Merged
erikdarlingdata merged 1 commit into
devfrom
feat/3542-pg-bad-actor
Sep 19, 2026

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

What does this PR do?

Headline mechanism. A PostgreSQL target's analysis pass now names the statement. PgTargetFactCollector.CollectQueryFactsAsync reads the window's top five statement shapes from pg_statement_stats and emits one PG_BAD_ACTOR_<queryid> fact each (pg_queries), whose value is the statement's share of the window's total execution time. PgTargetScorer.Queries.cs grades that share between two unmeasured bars behind an idle-window gate and stamps threshold_lineage = 0; PgTargetAdvice.Queries.cs states the numbers it read and both levers with their counter-objectives; PgTargetDrillDownCollector.Queries.cs joins the normalised text and its hash. Through the real analyze_server, a planted window where one statement holds 60% of the time yields a severity-1.0 card whose drill-down carries the statement text. This is #3542 v1 step 7 (lane 7) — partial, do not close.

The lie it stops telling

Until now a PostgreSQL pass could say the server was busy and never say with what: every story ended at a server-level symptom, and the operator's next step — "which statement?" — was a manual get_pg_top_queries. The design (#3542 §2a, §3) puts PG_BAD_ACTOR in v1 for exactly that reason: stories need a query-shaped leaf from day one. After this PR the pass concludes "statement queryid N held 60% of the window's total statement execution time (10 min of 16.7 min), across 2,448 calls (0.17/s), averaging 245 ms per call" — and the drill-down has the text.

What the family concludes, and from which table

  • Source: pg_statement_stats — the collector's STORED deltas (delta_calls, delta_total_exec_time_ms), never a re-differencing of the cumulative columns (DarlingPgTrendReader.QueryDurationTrendSql / DarlingPgStatementReader.PgTopQueriesSql discipline: the collector's delta spans the gap it observed; a LAG here would span the gap in the stored rows). The one unstored counter the family needs, temp_blks_written (lane 6's per-statement spill evidence), is differenced the reference reader's way — GREATEST(x − LAG(x), 0) over the FULL series identity (queryid, database_id, user_id, toplevel).
  • Decision variable: share of the window's total, with the denominator taken by SUM(SUM(…)) OVER () over the grouped result — evaluated before LIMIT, so it is every shape that ran, not the five returned (MCP payload-contract campaign: the agent-facing API tells the truth (~30 defects + an 11-rule contract) #3541 A7 / MCP percents name their denominator: shares are of the window or say they are of the page, so a three-row page stops summing to 100% of everything (#3541 A7) #3613). A page-scoped share would sum to 100% by construction.
  • Rate: calls_per_sec over the statement's known accrual span via the three-state interval idiom (V128, Measurement-layer campaign: the delta honesty contract (11 findings, one keystone) #3540): stored sample_interval_seconds → NULLIF(MAX(…), 0); NULL (pre-V128 row) → LAG(collection_time) over the queryid's snapshots; no ELSE 0. Calls are summed only over snapshots whose span is known (FILTER), so a first-sighting snapshot contributes 0 calls and 0 span, and the rate is absent (not 0/s) when no span is known. Both paths exercised on the rig: stored-interval series → 2,448/14,400 s; NULL-interval series → 235 calls over 47 known gaps (first snapshot excluded from both sides).
  • database_id is the collector's key and stays so (Measurement-layer campaign: the delta honesty contract (11 findings, one keystone) #3540 A11c): the datid OID is reused across DROP/CREATE and can splice two databases' histories — a measurement fact recorded in the collector and the delta seeder, reproduced here as written, not "fixed". The fact rolls up to queryid (the grain the key, the text store and pg_plan_capture share) and carries database_count; the drill-down lists the OIDs with per-database calls/time.
  • Units: total_exec_time_ms is ms on every row — the collector's SELECT names the ≥ 13 column (total_exec_time; pre-13 total_time), so a pre-13 target never populates the table and there is no unit branch to write.
  • Metadata: share_of_window_time, window_total_exec_ms, window_busy_fraction, calls, total_exec_ms, mean_exec_ms (absent at 0 calls), max_exec_ms, calls_per_sec (absent when unknowable), temp_blks_written, database_count, threshold_lineage. queryid is in the KEY and nowhere else — metadata is doubles-only, a 64-bit id is exact in a double only to 2^53, and the rig showed -1234567890123456789 rendering as -1.2345678901234568E+18; a lossy copy a reader could paste into get_pg_top_queries and match nothing is worse than none.

The lineage shape of every bar (#3538 A5/A7 via #3605)

All unmeasured, each with the marker within six lines and the calibrating read named; PgTargetThresholdLineageTests green:

Bar Value Calibrating read
BadActorShareConcerning (severity 0.5) 0.25 per-server distribution of the top-1 statement's share of SUM(delta_total_exec_time_ms) per window over pg_statement_stats (p50/p95 across the dogfood PostgreSQL fleet)
BadActorShareCritical (1.0) 0.60 same read
BadActorBusyFloor (idle gate) 0.05 Σ delta_total_exec_time_ms / observed seconds per server-window
BadActorCoFireBoost (both amplifiers) 0.25 analysis_findings co-fire rates

No SQL Server constant reused by value: the SQL Server family's execution-count tiers (1,000 / 10,000 / 100,000) and per-execution impact bars (50 / 2,000 ms CPU; 5,000 / 250,000 reads) are a different instrument on a different source — pg_stat_statements has no CPU and its block counts are cache-tier counters — and the class summary says why share is the honest PostgreSQL question. Window scaling: the share is a fraction of the same rows' total; the gate is a fraction of ObservedDurationMs (never nominal — pinned by DoesNotContain("PeriodDurationMs") on the partial). No fact is emitted when the coverage witness stamped nothing.

Co-fire gating

Two Layer-2 amplifiers on the bad actor, each asking only whether the sibling FIRED (its own scorer's bar put BaseSeverity > 0) and, where the statement can corroborate, whether it does: PG_TEMP_SPILL fired AND this statement's temp_blks_written > 0 (the server is spilling and this is one of the spillers — lane 6's edge, seen from the leaf); PG_CPU_PERCENT fired (Aurora/PI only; inert on stock where the fact is absent; the advice says elapsed ≠ CPU, so this is corroboration not attribution). Both are wired and inert today because the sibling families are stubs — pinned both ways (predicates true/false on arranged facts; through the real ScoreAll, base 1.0, two evaluated amplifiers, none matched).

Why this shape (and what was rejected)

  • Share, not the SQL Server tier × impact pair — argued in PgTargetScorer.Queries.cs's summary.
  • Top-5 as a page size, not a threshold (TopStatementCount): it bounds how many facts one pass emits; the share bar decides which fire. Same cut as PgFactCollector.BadActorSql's LIMIT 5, for the engine-independent reason (a story needs one leaf; a handful is what greedy traversal can choose among).
  • Idle gate over window_busy_fraction rather than a minimum-calls or minimum-ms floor: 60% of 100 ms on a dev box would root a card without it; an absolute floor would not scale with hours_back.
  • No text join in the collector. pg_statement_text is dimension-shaped (V73: "not a collector table") and PgTargetFactCollectorTests pins every FROM/JOIN of this collector to a collector table / servers / CTE. The text and its hashtext() (the breadcrumb that survives the queryid re-key) are the drill-down's, which has no such pin and is where the text is read for display anyway. Rejected: widening the census's allowed-table set for one join.
  • Advice permutes on nothing. The SQL Server composer splits frequent-cheap vs rare-heavy on exec >= 1000. A constant in prose is a threshold the lineage scanner cannot see, so this one states the arithmetic (total = calls × mean: 2,448 × 245.1 ms) and both levers with figures beside them and their counter-objectives (fewer calls trades result freshness; a cheaper call trades write amplification and vacuum work on the indexed table). No DDL (D8); pinned DoesNotContain("CREATE INDEX").
  • queryid instability is stated everywhere it matters: the advice (occurrence history restarts at one across a major upgrade or a compute_query_id change), the drill-down's queryid_note, and text_hash + text_first_seen as what survives the re-key.
  • PgTargetRelationshipGraph.Query.cs stays empty, and now says why: a bad actor is a leaf (nothing v1 is downstream of it; PG_QUERY_REGRESSION is v2), the edges INTO it are lanes 6/9's, and — recorded so the decision is made once — AddEdge takes an exact destination key while this family's keys are dynamic, so the first lane to write an edge into a bad actor must either resolve the destination at story-build time or declare a stable alias key. Until then a bad actor clearing 0.5 roots its own card, as BAD_ACTOR_* does on SQL Server (whose graph declares no edge for it either).

What it does NOT do

No PG_QUERY_REGRESSION, no plan-hash corroboration, no pg_plan_capture read (v2/v3). No edge into or out of a bad actor (lanes 6/9 own the incoming ones). No per-query CPU (pg_stat_kcache is not read). No PG_TEMP_SPILL-rooted temp offenders in the drill-down — the seam is marked /* filled by lane 6 */ in CollectTopStatementsAsync, and a spill-rooted story gets no statement detail until lane 6 defines "offender" for its family. No schema, no rung, no shared-switch edit, no PgTargetFactKeys edit, no PgTargetToolRecommendations edit (verified: the PG_BAD_ACTOR_ arm already names get_pg_top_queries and get_pg_plans; pinned).

Two plumbing pins every content lane necessarily moves — moved here, minimally

  1. PgTargetFactCollectorTests.AllSql_IsEveryPublicSqlConst_AndEveryExecutedQueryIsOne: Assert.Equal(2, sites) → Assert.True(sites >= 2, …). The equality was the proof the scan saw the plumbing's two files; every family adds a command site, and an equality that broke on each is the "tax on writing one" FactCollectorFailureReportingTests already refuses to be. The per-site closure (every new NpgsqlCommand( names a *Sql const) is untouched and is the real pin.
  2. PgTargetSharedSwitchRoutingTests.FactAdvice_DelegatesEveryPgKey_AndNeverReachesASqlServerComposer: the loop asserted Assert.Null for every key because every composer was a stub. It now asserts DELEGATION — FactAdvice.Compose(key) == PgTargetAdvice.Compose(key) and FactAdvice.GetForFactKey(key) == PgTargetAdvice.Static(key) (record value equality), which is the test's stated purpose and holds as each lane lands — plus one explicit null (a stub family) and one explicit non-null (this family). The census (V1CollectSurface), the emission order and the exit-criterion e2e (Assert.Single(facts) — this collector emits nothing without statement rows) are untouched.

Blast radius

SQL Server pass: untouched — no shared file changed; the only edits outside *.Queries.cs / *.Query.cs are the two test pins above. A PostgreSQL target with no pg_statement_stats rows (no pg_stat_statements, pre-V64 store) degrades through ReportCollectionFailure (42P01 quiet) or returns no rows, contributing no facts — the plumbing e2e still sees exactly one fact. compare_analysis: PG_BAD_ACTOR_* keys do not start with ComparisonBanding.PlanCacheIdentityPrefix ("BAD_ACTOR_") and so take the presence path, not the churn path — noted for the coordinator below.

Test plan

New Darling/Darling.Tests/PgTargetQueriesTests.cs (nine tests; reads repo files through the raw RepoFile.ReadRepoFile, so no s_lfReaders entry):

  • Scorer: 0.60 → 1.0, 0.25 → 0.5, 0.10 → 0.2, 0.30 → 0.571; threshold_lineage = 0 stamped on every graded fact including the one the idle gate zeroes; busy 0.04 → 0, busy 0.05 → opens; no share → 0 and no stamp; PG_PROBE under pg_queries → 0 (the routing test's probe still holds).
  • Amplifiers (reflection over the internal definitions + the real ScoreAll): spill arm true only when PG_TEMP_SPILL fired AND the statement spilled; CPU arm on PG_CPU_PERCENT fired; inert through ScoreAll today.
  • Advice: every figure stated (60%, 10 min of 16.7 min, 2,640 calls (0.18/s, 227.4 ms, 900.5 ms, 2 databases, 1,912 temp blocks), threshold_lineage = 0, the re-key caveat, both counter-objectives, both tool names, no CREATE INDEX, RemediationTsql null; omissions when metadata is absent; static block claims no figure and is the same object from both entry points.
  • Source pins on both reads (stored deltas / full-identity LAG / OVER () denominator / three-state interval / FILTER / no ELSE 0 / no pre-13 branch / no text join in the collector; text + hashtext + first_seen + array_agg OIDs in the drill-down; no DeSkew in code; timeout const; long.TryParse of the key; lane 6 seam marked).
  • Gated e2e (the exit criterion): three statements at 0.600 / 0.300 / 0.100 share (one with the pre-V128 NULL-interval shape, one running against two databases), 25 h span, 4 h window: fact shape and every metadata value asserted exactly (window total 1,000,032 ms; calls_per_sec on both interval paths; temp_blks_written 1,880 after the first-row clamp); severities 1.0 / 0.571 / 0.2; analyze_server → findings, the heavy card at 1.0 with headline …held 60%…, drill_down.pg_bad_actor_statements[key].query_text = the planted text, text_hash a number, queryid a string, two database rows heaviest-first, next_tools both; the medium card's text null (never ""); get_analysis_facts source=pg_queries → total_facts 4, shown 3, all stamped.

Executed here, not only compiled: a net10.0 harness (AssemblyName=Darling.Tests) compiling the new class plus PgTargetFactCollectorTests, PgTargetSharedSwitchRoutingTests, PgTargetThresholdLineageTests, PgTargetMcpSurfaceTests, FactCollectorCommandTimeoutTests, FactCollectorFailureReportingTests, AnalysisPassCommandTimeoutTests, AnalysisPassTokenThreadingTests, StoreSqlClockDisciplineTests, FactSourceRegistryTests, LiveCleanupConversionRatchetTests, DocCommentHygieneTests, RepoFileAdoptionTests, CommentFilterAdoptionTests, AnalysisShutdownResidueTests, CommandDeadlineScannerAdoptionTests, CSharpSourceWalkerTests — 239 / 239 green against a throwaway timescale/timescaledb:2.28.1-pg18 (Docker context desktop-linux, migrated to the current ladder, torn down after), including both live e2es. First execution under the Windows job is CI's.

Builds, zero warnings of ours: PerformanceMonitor.Analysis, PerformanceMonitor.Darling.Analysis, PerformanceMonitor.Darling.Service, Darling.Tests, Lite.Tests, deprecated/Dashboard.Tests (-p:EnableWindowsTargeting=true for the test projects). The one Lite.Tests warning (LogTailOverlapThresholdPinTests.cs:95 xUnit2000) is pre-existing on dev.

Which component(s) does this affect?

  • Lite
  • Darling
  • Lite Tests
  • Darling Tests
  • SQL collection scripts
  • Documentation
  • Full Dashboard (deprecated)
  • CLI Installer (deprecated)

How was this tested?

Above. No production store or fleet server was read. No SQL Server surface is touched; the SQL Server pass reads none of the changed files.

Checklist

  • I have read the contributing guide
  • My code builds with zero warnings (dotnet build -c Debug)
  • I have tested my changes against at least one SQL Server version (not applicable — a PostgreSQL-target-only family; the SQL Server pass is untouched and no shared switch changed)
  • I have not introduced any hardcoded credentials or server names

Partial for #3542 (v1 step 7, lane 7). CHANGELOG deliberately untouched — the coordinator splices this wave's entries.

…comes the query-shaped leaf of a PostgreSQL analysis pass — top statements from pg_statement_stats stored deltas (window denominator via SUM() OVER (), three-state interval for calls/sec), unmeasured share bars with threshold_lineage = 0 and an idle-window gate, value-stated advice naming both levers and the queryid re-key caveat, and a drill-down joining the normalised text and its hash (#3542 lane 7)
@erikdarlingdata
erikdarlingdata merged commit e0041b5 into dev Sep 19, 2026
5 of 7 checks passed
@erikdarlingdata
erikdarlingdata deleted the feat/3542-pg-bad-actor branch September 19, 2026 00:51
@erikdarlingdata

Copy link
Copy Markdown
Owner Author

Coordinator review (review-dark window): family-only files plus the two plumbing pins that assumed stubs forever — both edits are correct (sites >= 2 keeps the per-site closure; the advice pin becomes delegation equality, which is the stronger property). The calls_per_sec rate drops calls whose interval is unknown from the numerator as well as the denominator (FILTER (WHERE interval_seconds IS NOT NULL)) — the three-state idiom done right. Every bar declared unmeasured with its calibrating read named; SUM(SUM()) OVER () window denominator; no shared file touched. Two shared-vocabulary decisions recorded for the between-waves PR: ComparisonBanding.PlanCacheIdentityPrefix vs PG_BAD_ACTOR_, and a stable alias key so lanes 6/9 can edge INTO a dynamic-key leaf. Arming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant