Held retention policies only arm on service restart - re-evaluate them on a periodic cadence too #3812

Summary

A retention policy held by the coverage gate can only be armed by TimescaleSupport.EnsureRetentionPoliciesAsync (Darling/PerformanceMonitor.Darling.Storage/TimescaleSupport.cs:4773), and that method has exactly one call site: the service startup path, Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs:1397.

So once coverage catches up, nothing arms the policy until someone restarts the service. That is a bad dependency for a service that is designed to run for weeks or months untouched. The only operators who get a timely release are the ones who happen to update often.

Add a periodic re-evaluation on the running service. Keep the startup check — this is an additional cadence, not a replacement.

Why this is safe to run repeatedly (it already is)

The arming path was written to be re-runnable and is already exercised that way:

• Every step is idempotent: add_retention_policy(if_not_exists => true) returns -1 for an existing policy, the horizon converge is named-config-only and a no-op once converged, and arm/hold are direct alter_job state sets.
• The verdict is a measurement, not an inference — MeasureRetentionCoverageAsync (TimescaleSupport.cs:4694) returns three-valued Covered / Short / Unknown and is fail-closed. Unknown touches nothing in either direction (TimescaleSupport.cs:4868-4875), so a probe timeout on a periodic run cannot stop purging fleet-wide.
• It is failure-isolated per policy.
• TimescaleSupportTests.cs:1487 / :1496 already call it twice in a row and assert convergence, and :1627 / :1666 / :1696 walk a hold through to an arm across repeated calls. Repeat invocation is a tested property, not a hope.

The cost is small: one coverage query per policy (RetentionArmSafetySql — a chunk-pruned min(time) over the source plus each consumer), 20 policies on a current store.

Suggested shape

Mirror the fleet-level periodic-maintenance pattern that already sits a few lines away in the same loop — the compression-job self-heal (#1581):

• s_compressionCheckInterval = TimeSpan.FromHours(1) (DarlingWorker.cs:118)
• _nextCompressionCheckUtc field, MinValue-seeded (DarlingWorker.cs:480)
• the tick at DarlingWorker.cs:2159-2162, gated on _timescaleAvailable

A s_retentionPolicyCheckInterval / _nextRetentionPolicyCheckUtc pair in the same block, calling the same EnsureRetentionPoliciesAsync, is the whole change. Hourly matches the sibling cadences and is far tighter than "next restart"; anything from 1-6h is defensible since a hold costs disk slowly.

One thing to get right: the periodic call needs its own store connection from the worker's pool rather than reusing the startup-scoped timescaleConnection, which is disposed after the setup block.

Log noise

EnsureRetentionPoliciesAsync always logs a summary line, and logs one HELD PAUSED warning per held policy. Hourly, a store with a long-lived hold would emit the same warnings 24x/day. Either downgrade the periodic pass's per-policy log to Debug and keep the summary, or log only on state change (held->armed, armed->held, converged). State-change-only is the better answer: an arm is exactly the event an operator wants in the log, and it is currently the one thing that never gets a distinct line.

Docs and strings that become wrong

Several places assert the restart requirement in prose, and all of them are load-bearing (one is the remediation text an operator reads inside the alert itself):

• docs/retention-hold-runbook.md — step 3 of the sequence and the whole "Why step 3 is not optional" section.
• DarlingSelfAlertEvaluator.cs:3789 — the Retention Held alert's remediation text, verbatim: "arms at STARTUP rather than the moment coverage catches up - EnsureRetentionPoliciesAsync runs on the service start path and nowhere else."
• TimescaleSupport.cs:4762 — the EnsureRetentionPoliciesAsync doc comment: "this gate arms the held policy by itself on the next start."
• TimescaleSupport.cs:4864 — the HELD PAUSED warning text: "the policy arms itself on the next start."
• DarlingSelfAlertTests.cs:3741 and TimescaleSupportTests.cs:1377 pin some of this wording.

The --backfill-rollups runbook should end up saying: run the backfill, and the hold clears on its own within the check interval; restart if you want it immediately.

Guard to watch

PayloadDimensionTests.cs:1453 string-scans for MethodBody(source, "public static async Task<int> EnsureRetentionPoliciesAsync") and fails loudly on a parse miss. Adding a call site is fine; changing the signature is not free.

Field evidence (DARLING01, 2026-09-20)

Startup logs from both 2026-09-19 and 2026-09-20 report the identical line:

20/20 retention policies in place, 16 armed, 4 held paused pending backfill

Two restarts, no change — because coverage is genuinely Short, which is the gate working correctly. The four holds and their measured shortfalls:

That store is restarted often (nightly installs). A field store on a stable build would sit held indefinitely after a backfill that actually worked, with the Retention Held alert continuing to fire — which is precisely the "reads like the backfill failed" failure the runbook warns about, except with no restart coming to end it.