Repository navigation
Attach Aurora per-query peak memory as a context fact (#3691) - #4370
Merged
Merged
Conversation
…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
marked this pull request as ready for review
September 26, 2026 01:57
This was referenced Sep 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #3691.
Why
Aurora's
aurora_stat_statements()already collectsmax_exec_peakmem_bytes(per-query peak executor memory) intopg_statement_stats; it reaches theget_pg_top_queriesMCP tool but was never carried into the per-query PostgreSQL-target fact/advice pipeline (PgTargetFactCollector->PgTargetAdvice), so an operator reading aPG_BAD_ACTORfinding 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:PgTargetTopStatementsSqlnow selectsMAX(max_exec_peakmem_bytes)alongside the existingMAX(max_exec_time_ms)high-water mark, carried through the same per-snapshot -> per-statement aggregation. The C# reader stores it on the fact'sMetadata["max_exec_peakmem_bytes"]only when the column is non-null (the same absent-not-zero pattern already used formax_exec_ms,calls_per_secandmean_exec_ms). Off Aurora the column is NULL and the key is simply absent from the fact.PgTargetAdvice.Queries.cs:ComposeQueriesstates 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 localFormatByteshelper (KB/MB/GB, display-only). The scorer (PgTargetScorer.Queries.cs) is untouched — it still reads onlyshare_of_window_time/window_busy_fraction— so this fact cannot move severity, rank, or firing.Test plan
dotnet build Darling/Darling.Tests/Darling.Tests.csproj -p:EnableWindowsTargeting=trueand same forLite.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 oldComposeQuerieshas 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_608planted on every snapshot) fact carriesMetadata["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 onDARLING_TEST_PG; Windows-only suite. Fails on old code because the old collector never reads the column, so the heavy assertion finds no key.PgTarget,Npgsql, or Aurora references inLite/); nothing to mirror.CHANGELOG entry
SECTION: Added
ENTRY:
aurora_stat_statements()only) as context when available, alongside its share of window time; it never changes a finding's severity, rank, or whether it fires, and reads NULL rather than 0 off Aurora.REF:
[Attach Aurora per-query peak memory as a context fact (#3691) #4370]: Attach Aurora per-query peak memory as a context fact (#3691) #4370
For the coordinator
McpPayloadContractCensusTestswas checked and does not referenceget_pg_top_queries's payload ormax_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_NeverChangesTheGradeexpected the literal string
"8.0 MB"for 8,388,608 bytes.PgTargetAdvice.Queries.cs'sFormatBytesrenders with the format specifier{0:0.#}, which drops a trailing.0onan exact whole number — so it emits
"8 MB", matching the same rounding conventionRollupBackfillPlan.FormatBytesalready uses elsewhere ("1 KB","2 TB", but"1.5 GB"only when the value isn't whole). The collector (
PgTargetFactCollector.Queries.cs) and theadvice 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 correctlyon 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 codetouched.
Harness evidence: macOS can't run the xUnit suite, so verified with a throwaway
net10.0 console project referencing
PerformanceMonitor.Analysisand callingPgTargetAdvice.Composewith the test's own fixture (same metadata, samequeryid, same8,388,608-byte value): with
max_exec_peakmem_bytespresent, the investigation text reads"...reached 8 MB (Aurora only, context, not a threshold)."verbatim; without the key, thetext contains no
"Peak executor memory"substring at all. Headline unchanged in bothcases (checked by inspection of the composed block).
dotnet build Darling/Darling.Tests/Darling.Tests.csprojafter the merge withorigin/dev: Buildsucceeded, 0 Warning(s), 0 Error(s).
New head:
3cb769a0(merge oforigin/devonto0aa96750f6plus 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:568assertsspill.Severity == 0.8881afterFactScorer().ScoreAll(facts)(line 561).PerformanceMonitor.Analysis/PgTargetScorer.Temp.cs(TempAmplifiers,PgTargetFactKeys.TempSpillcase) firesTempCauseBoost(0.3) when anyPG_BAD_ACTOR_*fact hasMetadata["temp_blks_written"] > 0. That's the "named offender in the drill-down" — the bad-actor fact is emitted byPgTargetFactCollector.Queries.cs'sCollectQueryFactsAsync, driven byPgTargetTopStatementsSql.KnobCoFireBoost, 0.4×1.25=0.5 baseline → the knob co-fire) always fired; only the bad-actor amplifier was silently lost.SQL diff finding
git diff origin/dev...3cb769a0 -- Darling/PerformanceMonitor.Darling.Analysis/PgTargetFactCollector.Queries.cs: the PR insertedmax_exec_peakmem_bytesas SQL SELECT column index 4 (0-based), pushingtemp_blks_writtento index 5,calls_per_secto 6,database_countto 7,window_total_exec_msto 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'smax_exec_peakmem_bytes) was assigned totempBlksWritten, andreader.GetValue(5)(SQL'stemp_blks_written, abigint) was assigned tomaxExecPeakmemBytes.Off Aurora (every non-Aurora target, including this test's planted rows),
max_exec_peakmem_bytesis SQL NULL. Read at ordinal 4 astempBlksWritten,IsDBNull(4)was true →tempBlksWritten = 0on 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 inmaxExecPeakmemBytesmetadata 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 intomaxExecPeakmemBytes, ordinal 5 intotempBlksWritten, matching the SQL's actual column order. New head:27b08bc6onfeature/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).Darling.Tests/Lite.Teststargetnet10.0-windowsand 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 builtPerformanceMonitor.Darling.Analysis/.Storage/.Service/PerformanceMonitor.Analysisassemblies, reproducing the test's exact plant + collect +ScoreAllsequence againsttimescale/timescaledb:2.30.1-pg18in Docker (containerpmpr-4370, host port 55463,-c timezone=UTC, removed after use):temp_blks_writtenmetadata came back 0 (bug reproduced by inspection of the swapped read; not separately re-run post-fix-revert to save time).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
pmpr-4370was removed; roledarlingcreated transiently inside the throwaway container only (not a shared/persistent instance).git merge origin/devwas a no-op (branch already current).PgTargetQueriesTests.cs/PgTargetTempTests.csdepend on this ordinal (checked: onlytemp_blks_written's zero-vs-nonzero mattered for the amplifier predicate;max_exec_peakmem_bytes's own assertions inPgTargetQueriesTests.csread 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).