Skip to content

The module map refresh groups procedure_stats before its DISTINCT ON (#5583) - #5584

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/5583-module-map-group-first
Oct 8, 2026
Merged

erikdarlingdata merged 1 commit into
devfrom
fix/5583-module-map-group-first

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

Claude posting for Erik Darling.

Fixes #5583

Why

The daily module map refresh read 10.6 M procedure_stats rows 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:

  • the DISTINCT ON sort spilled 2.19 GB (about 21.5 s)
  • the src CTE was written to disk (2.18 GB), because up and st both read it
  • st's max() 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: both src heads (RefreshSql, RefreshSinceSql) now GROUP BY server_name, sql_handle, database_name, schema_name, object_name and select max(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).
  • UpsertAndAdvanceTail is untouched. Its DISTINCT ON ... ORDER BY server_name, sql_handle, collection_time DESC still 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 say src is already one row per handle and name set.
  • The result is unchanged: for each handle the names on its newest row, the same last_seen, the same watermark. The newest group is the newest row, so its names win and max(collection_time) is that row's time. NULL names group together. Ties at the same newest collection_time with different names stay as arbitrary as before (the sort has no tie-break).
  • Caveat, also in the code comment: the planner estimated 1,302,615 groups against 21,636 real ones (about 60x high). A store with other statistics could still plan a Sort + GroupAggregate. That is no worse than today's plan. A HashAggregate spills in partitions on PostgreSQL 13+ rather than failing.

Measured on the copy (#5583 issuecomment-6066741175, same copy, same window, same rows)

Before After
Total 35,524 ms 8,328 ms
Aggregate none (Sort, then Unique) Partial HashAggregate per chunk, then Finalize HashAggregate. Groups 1,302,615 estimated, 21,636 actual. 1 batch, no spill, 16 MB peak
Rows into the sort 10,595,501 21,636
Sort external merge, 2,188,744 kB on disk quicksort, 5,372 kB
CTE src on disk, 2,184,336 kB in memory, 5,029 kB
st max() 2,014 ms 2.6 ms
Upsert 21,636 conflicting 21,636 conflicting
Temp blocks read / written 546,635 / 546,674 0 / 0

Test plan

  • New DarlingModuleMapGroupedSourceLiveTests (4 facts, own scratch store each), on a PostgreSQL 18 + TimescaleDB rig:
    • result facts (both statements) cover a handle whose names change, a handle on two servers, all-NULL names, NULL-then-named and named-then-NULL. They also cover a row exactly at the bound (read) and one a tick before it (not read). Run against the OLD SQL first, both pass. Against the new SQL, both pass.
    • plan facts (both statements) seed 1.2 M rows over 3,000 handles. 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.
  • DarlingModuleMapTests gets pins for the max(...) select and the GROUP BY text in both statements.
  • Targeted run: DarlingModuleMap*, DailyRefreshBatching*, BaselineSupply*, StoreSessionTimeZone*, StoreSqlClockDiscipline*, TimescaleContinuousAggregate*, FleetPartialDatabaseFailureBand*, *Census*, *Ratchet*, *DocCommentHygiene*: Total 697, Failed 0, Skipped 0.
  • -trait Stage=Guard: Total 1411, Failed 0.
  • Build: 0 Warning(s), 0 Error(s).
  • Not run: the full suite (not run on this laptop) and 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

  • UpsertAndAdvanceTail is byte-for-byte unchanged apart from one doc-comment sentence.
  • Lite has no module map, so there is no parity site.

…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.
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