Repository navigation
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
Conversation
…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)
|
Coordinator review (review-dark window): family-only files plus the two plumbing pins that assumed stubs forever — both edits are correct ( |
What does this PR do?
Headline mechanism. A PostgreSQL target's analysis pass now names the statement.
PgTargetFactCollector.CollectQueryFactsAsyncreads the window's top five statement shapes frompg_statement_statsand emits onePG_BAD_ACTOR_<queryid>fact each (pg_queries), whose value is the statement's share of the window's total execution time.PgTargetScorer.Queries.csgrades that share between two unmeasured bars behind an idle-window gate and stampsthreshold_lineage = 0;PgTargetAdvice.Queries.csstates the numbers it read and both levers with their counter-objectives;PgTargetDrillDownCollector.Queries.csjoins the normalised text and its hash. Through the realanalyze_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) putsPG_BAD_ACTORin 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
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.PgTopQueriesSqldiscipline: the collector's delta spans the gap it observed; aLAGhere 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).SUM(SUM(…)) OVER ()over the grouped result — evaluated beforeLIMIT, 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.calls_per_secover 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): storedsample_interval_seconds→NULLIF(MAX(…), 0); NULL (pre-V128 row) →LAG(collection_time)over the queryid's snapshots; noELSE 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_idis the collector's key and stays so (Measurement-layer campaign: the delta honesty contract (11 findings, one keystone) #3540 A11c): thedatidOID 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 toqueryid(the grain the key, the text store andpg_plan_captureshare) and carriesdatabase_count; the drill-down lists the OIDs with per-database calls/time.total_exec_time_msis ms on every row — the collector's SELECT names the ≥ 13 column (total_exec_time; pre-13total_time), so a pre-13 target never populates the table and there is no unit branch to write.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.queryidis 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-1234567890123456789rendering as-1.2345678901234568E+18; a lossy copy a reader could paste intoget_pg_top_queriesand 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;
PgTargetThresholdLineageTestsgreen:BadActorShareConcerning(severity 0.5)SUM(delta_total_exec_time_ms)per window overpg_statement_stats(p50/p95 across the dogfood PostgreSQL fleet)BadActorShareCritical(1.0)BadActorBusyFloor(idle gate)Σ delta_total_exec_time_ms/ observed seconds per server-windowBadActorCoFireBoost(both amplifiers)analysis_findingsco-fire ratesNo 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_statementshas 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 ofObservedDurationMs(never nominal — pinned byDoesNotContain("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_SPILLfired AND this statement'stemp_blks_written > 0(the server is spilling and this is one of the spillers — lane 6's edge, seen from the leaf);PG_CPU_PERCENTfired (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 realScoreAll, base 1.0, two evaluated amplifiers, none matched).Why this shape (and what was rejected)
PgTargetScorer.Queries.cs's summary.TopStatementCount): it bounds how many facts one pass emits; the share bar decides which fire. Same cut asPgFactCollector.BadActorSql'sLIMIT 5, for the engine-independent reason (a story needs one leaf; a handful is what greedy traversal can choose among).window_busy_fractionrather 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 withhours_back.pg_statement_textis dimension-shaped (V73: "not a collector table") andPgTargetFactCollectorTestspins everyFROM/JOINof this collector to a collector table /servers/ CTE. The text and itshashtext()(the breadcrumb that survives thequeryidre-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.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); pinnedDoesNotContain("CREATE INDEX").queryidinstability is stated everywhere it matters: the advice (occurrence history restarts at one across a major upgrade or acompute_query_idchange), the drill-down'squeryid_note, andtext_hash+text_first_seenas what survives the re-key.PgTargetRelationshipGraph.Query.csstays empty, and now says why: a bad actor is a leaf (nothing v1 is downstream of it;PG_QUERY_REGRESSIONis v2), the edges INTO it are lanes 6/9's, and — recorded so the decision is made once —AddEdgetakes 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, asBAD_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, nopg_plan_captureread (v2/v3). No edge into or out of a bad actor (lanes 6/9 own the incoming ones). No per-query CPU (pg_stat_kcacheis not read). NoPG_TEMP_SPILL-rooted temp offenders in the drill-down — the seam is marked/* filled by lane 6 */inCollectTopStatementsAsync, 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, noPgTargetFactKeysedit, noPgTargetToolRecommendationsedit (verified: thePG_BAD_ACTOR_arm already namesget_pg_top_queriesandget_pg_plans; pinned).Two plumbing pins every content lane necessarily moves — moved here, minimally
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"FactCollectorFailureReportingTestsalready refuses to be. The per-site closure (everynew NpgsqlCommand(names a*Sqlconst) is untouched and is the real pin.PgTargetSharedSwitchRoutingTests.FactAdvice_DelegatesEveryPgKey_AndNeverReachesASqlServerComposer: the loop assertedAssert.Nullfor every key because every composer was a stub. It now asserts DELEGATION —FactAdvice.Compose(key) == PgTargetAdvice.Compose(key)andFactAdvice.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.csare the two test pins above. A PostgreSQL target with nopg_statement_statsrows (nopg_stat_statements, pre-V64 store) degrades throughReportCollectionFailure(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 withComparisonBanding.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 rawRepoFile.ReadRepoFile, so nos_lfReadersentry):threshold_lineage = 0stamped 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_PROBEunderpg_queries→ 0 (the routing test's probe still holds).ScoreAll): spill arm true only whenPG_TEMP_SPILLfired AND the statement spilled; CPU arm onPG_CPU_PERCENTfired; inert throughScoreAlltoday.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, noCREATE INDEX,RemediationTsqlnull; omissions when metadata is absent; static block claims no figure and is the same object from both entry points.LAG/OVER ()denominator / three-state interval /FILTER/ noELSE 0/ no pre-13 branch / no text join in the collector; text +hashtext+first_seen+array_aggOIDs in the drill-down; noDeSkewin code; timeout const;long.TryParseof the key; lane 6 seam marked).calls_per_secon both interval paths;temp_blks_written1,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_hasha number,queryida string, two database rows heaviest-first,next_toolsboth; the medium card's textnull(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 plusPgTargetFactCollectorTests,PgTargetSharedSwitchRoutingTests,PgTargetThresholdLineageTests,PgTargetMcpSurfaceTests,FactCollectorCommandTimeoutTests,FactCollectorFailureReportingTests,AnalysisPassCommandTimeoutTests,AnalysisPassTokenThreadingTests,StoreSqlClockDisciplineTests,FactSourceRegistryTests,LiveCleanupConversionRatchetTests,DocCommentHygieneTests,RepoFileAdoptionTests,CommentFilterAdoptionTests,AnalysisShutdownResidueTests,CommandDeadlineScannerAdoptionTests,CSharpSourceWalkerTests— 239 / 239 green against a throwawaytimescale/timescaledb:2.28.1-pg18(Docker contextdesktop-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=truefor the test projects). The oneLite.Testswarning (LogTailOverlapThresholdPinTests.cs:95xUnit2000) is pre-existing on dev.Which component(s) does this affect?
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
dotnet build -c Debug)Partial for #3542 (v1 step 7, lane 7). CHANGELOG deliberately untouched — the coordinator splices this wave's entries.