You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
The per-server collection-health memo keys on the window START and its caller passes a fresh UtcNow, so it never hits in production and its dictionary never evicts (defect in #3856/#3880) #3894
Found while reading for #3893 arm 1, verified in source at d21bbc9a2. This is a defect in my own merged work (#3856 → c0eb0046e, stamped by #3880 → d5868c67d), and it has two consequences, one of which means that fix is currently inert.
The defect
DarlingDataReader.PerServerCollectionHealthMemo keys its dictionary on the window START:
:1891 — var key = (serverId, windowStartUtc);, no rounding.
Its only production caller hands it a fresh clock on every call:
Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs:1007-1009 — var nowUtc = DateTime.UtcNow; then GetCollectionHealthMemoizedAsync(postgres, resolved.ServerId, nowUtc.AddDays(-7), nowUtc).
DateTime.UtcNow carries sub-microsecond precision, so nowUtc.AddDays(-7) is a different key on literally every call.
Consequence 1 — the memo never hits, so #3856 is inert
Every get_collection_health call misses the memo and starts its own 7-day scan of v_collection_log. That is precisely the race #3856 existed to stop: the per-server read crossing the mcp role's 15 s statement_timeout during the store's write bands (57014, first observed 2026-09-21 21:3xZ). The fix has been merged and reporting a collection_health_age_seconds stamp of 0 on every call — which is honest, and is also the tell nobody read: a memo that always reports age 0 is a memo that never hit.
Consequence 2 — unbounded growth in a long-lived host
_byKey is never pruned. Reading the whole class: TryGetValue, an assignment, and a second TryGetValue — no Remove, no Clear, no cap, no lifetime sweep. One KeyState (holding its rows list) accumulates per call, for the life of the MCP host process. The fleet twin does not have this exposure because it keeps a single _latest slot rather than a dictionary.
Why the pins passed
Both suites hold the key constant by construction, so they prove the mechanism works when handed the same key and never that production hands it the same key:
PerServerCollectionHealthMemoTests — unit arms reuse a fixed WindowFrom(T0) while advancing only nowUtc.
its live class (:456-466) — reuses now.AddDays(-7) as the window start while nowUtc advances by a second.
The missing pin is the one that exercises the production entry point twice with two fresh clocks and asserts ScansStarted == 1.
Bound the dictionary — evict on lifetime expiry, or cap with an LRU. A per-server memo on a fleet-sized host should hold at most one entry per server per live window.
Pin what the old pins could not see — a test that calls the production path twice with two real clocks and asserts one scan, plus an eviction/bound assertion. The existing key-constant arms stay; they are not wrong, only insufficient.
Ships with the next nightly regardless of this fix, so the inert-memo half is live on .458+ until it lands.
Claimed at filing (pm-user-worker seat) — this is my own defect from #3856/#3880 and I am fixing it now, ahead of #3893's arm 1, because the inert half is live on .458+ and the growth half runs in a long-lived host. Fix as described: key on window length, bound the dictionary, and add the pin that exercises the production entry point with two real clocks (the existing key-constant arms stay; they were insufficient, not wrong).
Activity
erikdarlingdata commentedon Sep 22, 2026
Claimed at filing (pm-user-worker seat) — this is my own defect from #3856/#3880 and I am fixing it now, ahead of #3893's arm 1, because the inert half is live on .458+ and the growth half runs in a long-lived host. Fix as described: key on window length, bound the dictionary, and add the pin that exercises the production entry point with two real clocks (the existing key-constant arms stay; they were insufficient, not wrong).
The per-server collection-health memo keys on the window's length at …
erikdarlingdata commentedon Sep 22, 2026
Closed by the watcher: delivered in PR #3900, merged to dev.