Repository navigation
The module map refresh groups procedure_stats before its DISTINCT ON (#5583) - #5584
Merged
Merged
Conversation
…5583) src now groups by (server_name, sql_handle, database_name, schema_name, object_name) with max(collection_time), so the tail sorts one row per handle and name set instead of the raw read. Adds live result and plan pins.
erikdarlingdata
marked this pull request as ready for review
October 8, 2026 19:03
This was referenced Oct 8, 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.
Claude posting for Erik Darling.
Fixes #5583
Why
The daily module map refresh read 10.6 M
procedure_statsrows to land on 21,636 handles and took 35.5 s on a large store (#5578 issuecomment-6066615850). Three costs came from handing the raw rows to the statement's tail:DISTINCT ONsort spilled 2.19 GB (about 21.5 s)srcCTE was written to disk (2.18 GB), becauseupandstboth read itst'smax()made a second pass over it (2.0 s).The scan (5.3 s) and the upsert (under 1 s) were not the problem.
What changes
DarlingModuleMap.cs: bothsrcheads (RefreshSql,RefreshSinceSql) nowGROUP BY server_name, sql_handle, database_name, schema_name, object_nameand selectmax(collection_time) AS collection_time. The two heads share the select list and group-by text through two private constants, which carry the doc comment (cost, numbers, caveat).UpsertAndAdvanceTailis untouched. ItsDISTINCT ON ... ORDER BY server_name, sql_handle, collection_time DESCstill keeps the newest row's names per handle and the conflict-key order (Payload-dim batch upserts deadlock under concurrent collection cycles: unnest arrays carry no cross-session lock order #1801). I fixed its doc comment to saysrcis already one row per handle and name set.last_seen, the same watermark. The newest group is the newest row, so its names win andmax(collection_time)is that row's time. NULL names group together. Ties at the same newestcollection_timewith different names stay as arbitrary as before (the sort has no tie-break).Measured on the copy (#5583 issuecomment-6066741175, same copy, same window, same rows)
srcstmax()Test plan
DarlingModuleMapGroupedSourceLiveTests(4 facts, own scratch store each), on a PostgreSQL 18 + TimescaleDB rig:EXPLAIN (FORMAT JSON)of the shipped text must show an aggregate over the relation scan and no Sort whose input is the raw scan. Against the OLD SQL both fail. Against the new, both pass. That is the red plant.DarlingModuleMapTestsgets pins for themax(...)select and theGROUP BYtext in both statements.DarlingModuleMap*,DailyRefreshBatching*,BaselineSupply*,StoreSessionTimeZone*,StoreSqlClockDiscipline*,TimescaleContinuousAggregate*,FleetPartialDatabaseFailureBand*,*Census*,*Ratchet*,*DocCommentHygiene*: Total 697, Failed 0, Skipped 0.-trait Stage=Guard: Total 1411, Failed 0.Installer.Tests. The 35.5 s to 8.3 s figures come from the copy's profile in The module map's daily refresh sorts 10.6 M rows on disk to find 21,636 handles #5583. I did not re-measure them. My 1.2 M-row scratch volume asserts the plan shape only.For the coordinator to check
UpsertAndAdvanceTailis byte-for-byte unchanged apart from one doc-comment sentence.