Skip to content

Attach Aurora per-query peak memory as a context fact (#3691) - #4370

Merged
erikdarlingdata merged 5 commits into
devfrom
feature/3691-aurora-peakmem-fact
Sep 26, 2026
Merged

erikdarlingdata merged 5 commits into
devfrom
feature/3691-aurora-peakmem-fact

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Refs #3691.

Why

Aurora's aurora_stat_statements() already collects max_exec_peakmem_bytes (per-query peak executor memory) into pg_statement_stats; it reaches the get_pg_top_queries MCP tool but was never carried into the per-query PostgreSQL-target fact/advice pipeline (PgTargetFactCollector -> PgTargetAdvice), so an operator reading a PG_BAD_ACTOR finding had no visibility into a statement's peak memory even when the collector already had it. Per the coordinator's ruling (2026-09-26 00:15Z, inside #3691's scope), this attaches the value as a CONTEXT fact only: no bar, no threshold, never a factor in severity, rank, or whether a finding fires. A threshold waits for a fleet distribution read the coordinator owns.

What changes

  • PgTargetFactCollector.Queries.cs: PgTargetTopStatementsSql now selects MAX(max_exec_peakmem_bytes) alongside the existing MAX(max_exec_time_ms) high-water mark, carried through the same per-snapshot -> per-statement aggregation. The C# reader stores it on the fact's Metadata["max_exec_peakmem_bytes"] only when the column is non-null (the same absent-not-zero pattern already used for max_exec_ms, calls_per_sec and mean_exec_ms). Off Aurora the column is NULL and the key is simply absent from the fact.
  • PgTargetAdvice.Queries.cs: ComposeQueries states the value in the card's investigation text when present ("Peak executor memory for a single execution reached X (Aurora only, context, not a threshold)."), using a new local FormatBytes helper (KB/MB/GB, display-only). The scorer (PgTargetScorer.Queries.cs) is untouched — it still reads only share_of_window_time / window_busy_fraction — so this fact cannot move severity, rank, or firing.

Test plan

  • macOS build 0/0: dotnet build Darling/Darling.Tests/Darling.Tests.csproj -p:EnableWindowsTargeting=true and same for Lite.Tests/Lite.Tests.csproj.
  • PgTargetQueriesTests.ComposeQueries_CarriesAuroraPeakMemAsContext_AbsentOffAurora_NeverChangesTheGrade (new unit test, no DB needed) — Windows-only suite, cannot run here. Fails on old code because the old ComposeQueries has no peak-mem branch at all: Assert.Contains("Peak executor memory for a single execution reached 8.0 MB ...") finds nothing.
  • APlantedWindowWhereOneStatementJumpsToSixtyPercent_... (existing e2e, extended) — asserts the heavy (Aurora fixture, max_exec_peakmem_bytes: 8_388_608 planted on every snapshot) fact carries Metadata["max_exec_peakmem_bytes"] == 8_388_608, and the medium (stock-PostgreSQL fixture, nothing planted) fact has NO such key at all (absent, never 0). Gated on DARLING_TEST_PG; Windows-only suite. Fails on old code because the old collector never reads the column, so the heavy assertion finds no key.
  • Lite parity: N/A. Lite has no PostgreSQL/Aurora collection or PG-target findings path (checked: no PgTarget, Npgsql, or Aurora references in Lite/); nothing to mirror.

CHANGELOG entry

SECTION: Added
ENTRY:

For the coordinator

  • A threshold/bar on this fact deliberately was not added; that waits on the fleet distribution read the coordinator owns per the ruling.
  • The two new/extended tests target net10.0-windows and cannot run on this macOS lane; they build clean (0/0) and are listed unchecked above with the assertion that fails on the pre-fix code. CI decides them.
  • McpPayloadContractCensusTests was checked and does not reference get_pg_top_queries's payload or max_exec_peakmem_bytes, so no update was needed there.

pm-pr lane report (pin fix)

Root cause: wording only, not a broken feature. The new pin
ComposeQueries_CarriesAuroraPeakMemAsContext_AbsentOffAurora_NeverChangesTheGrade
expected the literal string "8.0 MB" for 8,388,608 bytes. PgTargetAdvice.Queries.cs's
FormatBytes renders with the format specifier {0:0.#}, which drops a trailing .0 on
an exact whole number — so it emits "8 MB", matching the same rounding convention
RollupBackfillPlan.FormatBytes already uses elsewhere ("1 KB", "2 TB", but "1.5 GB"
only when the value isn't whole). The collector (PgTargetFactCollector.Queries.cs) and the
advice builder (PgTargetAdvice.Queries.cs) already agree on the same metadata key string,
max_exec_peakmem_bytes — no drift there, and the feature composes the sentence correctly
on the Aurora path and omits it correctly off Aurora.

Fix: changed the test's expected substring from "8.0 MB" to "8 MB". No product code
touched.

Harness evidence: macOS can't run the xUnit suite, so verified with a throwaway
net10.0 console project referencing PerformanceMonitor.Analysis and calling
PgTargetAdvice.Compose with the test's own fixture (same metadata, same queryid, same
8,388,608-byte value): with max_exec_peakmem_bytes present, the investigation text reads
"...reached 8 MB (Aurora only, context, not a threshold)." verbatim; without the key, the
text contains no "Peak executor memory" substring at all. Headline unchanged in both
cases (checked by inspection of the composed block). dotnet build Darling/Darling.Tests/Darling.Tests.csproj after the merge with origin/dev: Build
succeeded, 0 Warning(s), 0 Error(s).

New head: 3cb769a0 (merge of origin/dev onto 0aa96750f6 plus the one-line test fix).

pm-pr lane report (ordinal-swap regression, found by the temp-spill live test)

Verdict: (a) the PR's change causes it — fixed

Traced path

  • Darling.Tests/PgTargetTempTests.cs:568 asserts spill.Severity == 0.8881 after FactScorer().ScoreAll(facts) (line 561).
  • The amplifier that supplies the missing 0.3: PerformanceMonitor.Analysis/PgTargetScorer.Temp.cs (TempAmplifiers, PgTargetFactKeys.TempSpill case) fires TempCauseBoost (0.3) when any PG_BAD_ACTOR_* fact has Metadata["temp_blks_written"] > 0. That's the "named offender in the drill-down" — the bad-actor fact is emitted by PgTargetFactCollector.Queries.cs's CollectQueryFactsAsync, driven by PgTargetTopStatementsSql.
  • The other amplifier (KnobCoFireBoost, 0.4×1.25=0.5 baseline → the knob co-fire) always fired; only the bad-actor amplifier was silently lost.
  • 0.5551 (BaseSeverity) × 1.3 (knob co-fire only) = 0.72163 = the CI's actual. × 1.6 (both amplifiers) = 0.88816 ≈ expected 0.8881.

SQL diff finding

git diff origin/dev...3cb769a0 -- Darling/PerformanceMonitor.Darling.Analysis/PgTargetFactCollector.Queries.cs: the PR inserted max_exec_peakmem_bytes as SQL SELECT column index 4 (0-based), pushing temp_blks_written to index 5, calls_per_sec to 6, database_count to 7, window_total_exec_ms to 8. The reader loop's ordinal reads (reader.GetValue(n)) were updated for indices 0–3 and 6–8, but 4 and 5 were swapped: reader.GetValue(4) (SQL's max_exec_peakmem_bytes) was assigned to tempBlksWritten, and reader.GetValue(5) (SQL's temp_blks_written, a bigint) was assigned to maxExecPeakmemBytes.

Off Aurora (every non-Aurora target, including this test's planted rows), max_exec_peakmem_bytes is SQL NULL. Read at ordinal 4 as tempBlksWritten, IsDBNull(4) was true → tempBlksWritten = 0 on every bad-actor fact, always, off Aurora — silently dropping the amplifier for any non-Aurora story with a temp-writing offender. temp_blks_written's real value (500,000 in the test) landed in maxExecPeakmemBytes metadata instead, harmlessly absent-guarded (if (maxExecPeakmemBytes is { } peakMem)), so nothing else broke and no other assertion caught it — only this one severity check, which is why the census found it failing only here.

Fix

One-line swap at PgTargetFactCollector.Queries.cs (now lines 205–206): read ordinal 4 into maxExecPeakmemBytes, ordinal 5 into tempBlksWritten, matching the SQL's actual column order. New head: 27b08bc6 on feature/3691-aurora-peakmem-fact (pushed; PR #4370 unchanged otherwise, merged with origin/dev — already up to date, no new dev commits).

Verification

  • dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug: Build succeeded, 0 Warning(s), 0 Error(s).
  • Live verification: Darling.Tests/Lite.Tests target net10.0-windows and cannot run (dotnet test) on macOS. Per repo convention, stood up a throwaway net10.0 console harness (/tmp/lane4370-harness, deleted after use) referencing the real built PerformanceMonitor.Darling.Analysis / .Storage / .Service / PerformanceMonitor.Analysis assemblies, reproducing the test's exact plant + collect + ScoreAll sequence against timescale/timescaledb:2.30.1-pg18 in Docker (container pmpr-4370, host port 55463, -c timezone=UTC, removed after use):
    • Before the fix (verified analytically: 0.5551 × 1.3 = 0.721625, matches CI's reported 0.72162037037037041 to the shown digits): the bad-actor's temp_blks_written metadata came back 0 (bug reproduced by inspection of the swapped read; not separately re-run post-fix-revert to save time).
    • After the fix: bad actor 777 temp_blks_written metadata = 500000 (PASS), spill.BaseSeverity = 0.5550925925925926 (PASS, rounds to 0.5551), spill.Severity = 0.8881481481481481 (PASS, rounds to 0.8881) — exact match to the test's asserted values.

Notes for pm-pr

  • Docker container pmpr-4370 was removed; role darling created transiently inside the throwaway container only (not a shared/persistent instance).
  • No other files touched; git merge origin/dev was a no-op (branch already current).
  • CI should now pass this test; no other assertions in PgTargetQueriesTests.cs/PgTargetTempTests.cs depend on this ordinal (checked: only temp_blks_written's zero-vs-nonzero mattered for the amplifier predicate; max_exec_peakmem_bytes's own assertions in PgTargetQueriesTests.cs read the correct SQL-side value regardless of the swap, since that test doesn't plant Aurora peakmem and gets NULL either way — not independently re-verified against CI due to context budget, but the swap fix strictly improves correctness in both directions).

…wn pin (#3691)

The new ComposeQueries_CarriesAuroraPeakMemAsContext_AbsentOffAurora_NeverChangesTheGrade
pin expected "8.0 MB"; FormatBytes's {0:0.#} format specifier drops a
trailing .0 on a whole number, the same convention RollupBackfillPlan.FormatBytes
already uses ("1 KB", not "1.0 KB"). Verified with a throwaway net10.0 console
harness against PgTargetAdvice.Compose with the test's own fixture: the sentence
is present with the Aurora value ("...reached 8 MB..."), absent without it, and
the collector/advice metadata key (max_exec_peakmem_bytes) is the same string on
both sides. No product-code change.
max_exec_peakmem_bytes (col 4) and temp_blks_written (col 5) were read
at each other's ordinal, so a NULL peakmem (every non-Aurora target,
including all local/CI tests) zeroed temp_blks_written on every
PG_BAD_ACTOR_* fact. That dropped lane 6's 'named offender' amplifier
(TempCauseBoost, +0.3) on PG_TEMP_SPILL, which is why
APlantedSpillWithAMidWindowReset_ProducesTheSpillToWorkMemStory_WithTheOffenderInTheDrillDown
computed severity 0.7216 (× 1.3, only the knob co-fire) instead of the
expected 0.8881 (× 1.6, both amplifiers).
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 26, 2026 01:57
@erikdarlingdata
erikdarlingdata merged commit 9861eb8 into dev Sep 26, 2026
15 of 16 checks passed
@erikdarlingdata
erikdarlingdata deleted the feature/3691-aurora-peakmem-fact branch September 26, 2026 01:57
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