Repository navigation
collect.plan_force_actions has no retention path — the force-plan ledger grows without bound once #2138 is enabled #2948
Description
Activity
Closed by #2986 (
7a3e9c75).PlanForceLedgerRetentionDays = 365atDarlingRetention.cs:119, purged by a time-predicatedDELETE.This issue's premise was wrong, and that is worth recording rather than quietly fixing. V107's doc comment did not merely omit a horizon — it argued against one: "Deliberately NOT enrolled in retention: an audit of writes to production servers is the one series that should outlive the metrics that motivated it, and its volume is bounded by the bot's own cooldowns." This issue quotes that comment and stops one clause short of the part that disagrees with it.
The change proceeded because the stated justification does not hold: cooldowns bound the rate, and a bounded rate over unbounded time is unbounded size. The property the comment was protecting — an audit outliving the metrics that motivated it — is preserved by a long horizon rather than by none. At 365 days it is the longest horizon in the store: 4x
AlertHistoryRetentionDays, 12x the metric base, and comfortably clear of the longest window the bot itself reads back for eligibility (a week, for the two-taken-back-forces cooldown), so retention cannot make the bot forget a decision it is still bound by. The comment now states the horizon that does the bounding instead of denying there is one.Predicated
DELETE, not a hypertable — and the reason is stronger than cost.TimescaleSupportalready excludes tables of this shape and says why: "registries keep their PRIMARY KEYs, which TimescaleDB would reject or force onto the partition column." This journal hasaction_id bigint GENERATED ALWAYS AS IDENTITY PRIMARY KEY, andrelated_action_idpoints back to it — that self-reference is how a review row references the force row it re-judges. Forcing the key onto(action_id, action_time)breaks it. So conversion is the thing the repo declines to do to PK-bearing tables, before the data-moving rung cost even arises. Verified on a real store:plan_force_actions0 hypertables,collection_log1 as a positive control.One claim in this issue was false and is corrected in the code. The index
idx_plan_force_actions_timeleads withserver_id, and the purgeDELETEcarries noserver_idpredicate — so the index is not seekable for it, and the note claiming it serves both themin(action_time)probes and the purge range scan was wrong. The one-day slice is what bounds the work, and the comment now says that.Lite has no equivalent table — measured, not assumed. The force-plan bot exists only under
Darling/(zero files underLite/), and none of the 12 tables in Lite'sSchema.GetAllTableStatements()is a plan-force journal. TheForcePlanFailure*hits underLite/are a different concept: reading a monitored server's own Query Store forcing failures. Not the one-SKU half of a shared seam — the subsystem is not there.Evidence
The constant/SQL pin passes against a no-op, so it is not the evidence. The behavioural assertion is a live-PostgreSQL end-to-end with ages chosen to discriminate: a 400-day row goes while a 100-day row survives, though that row is past every other horizon in the store (90/60/30).
Three mutations against real PostgreSQL 17 + TimescaleDB 2.28.1, each anchored and hashed: deleting the purge block entirely, and re-wiring the purge to
AlertHistoryRetentionDayswhile leaving the constant at 365, are both invisible to the pin and caught only by the end-to-end. Two of the three also leaveDarling.Tests.dllbyte-identical, becauseDarlingRetention.cscompiles intoPerformanceMonitor.Darling.Service.dll— so hashing only the test assembly reports a real mutation as vacuous.Latent when fixed: zero rows in production, since the bot ships disabled. That is the argument for doing it now rather than after the writer ships, not against.
collect.plan_force_actionshas no retention path — the force-plan bot's ledger grows without boundFound while working #2874 group E and verified independently here. Latent today, because the bot ships disabled; it becomes real the moment #2138 is enabled.
The gap
collect.plan_force_actionsis created byPgMigrations.cs:2318as a plainCREATE TABLE— not a hypertable — and its own doc comment calls it "the bot's audit trail and ledger, and it is a first-class" record. It is append-only by design.Nothing purges it. Two independent checks, and the second is the one that matters because the first alone proves nothing:
git grep -c plan_force_actionsinDarling/PerformanceMonitor.Darling.Service/DarlingRetention.cs→ 0."collection_log"and"file_io_stats"both appear — so the grep form works and the absence is real rather than an artifact of tables being named indirectly.The retention sweep's own structure confirms it: it iterates
CollectorCatalog.All(:168,:276), andplan_force_actionsis not a collector target table, so it is not in that catalog. The file already contains precedent for exactly this situation — a comment at:499notes another object is "NOT inCollectorCatalog.All, so the loop above skips it" and handles it explicitly. This table has no such handling.So the growth is unbounded in both senses: no time-based purge, and no hypertable chunk dropping either, because it was never converted.
Severity, honestly
Zero rows in production today — the force-plan bot ships off, so nothing writes to it. That is why this has gone unnoticed and why it is not urgent.
It stops being latent when the bot is enabled. The write rate is one row per force/unforce decision per server, which is small per event but unbounded over time, and this is a monitoring store whose whole purpose is bounded retention — every other collected table has a horizon. An audit ledger that grows forever on a store sized for rolling windows is the kind of thing that surfaces as a disk alert years later with no obvious cause.
Relevant that #2138 is open and PR #2731 is in flight on the bot's write path — this is the right moment to decide the horizon, while the writer is still being built, rather than after data exists.
The decision
query_statsis real, and the argument for keeping it forever is also coherent for a ledger that records automated changes to a production server's plans. Someone has to choose, and the choice should be recorded rather than defaulted.DELETE? It already hasidx_plan_force_actions_time ON (server_id, action_time), so a predicated delete is cheap. Conversion would need a data-moving migration rung, which this repo tracks deliberately — see the rung census pin, which asserts the set of data-moving rungs against a declared register, so adding one is not silent but is not free either.CollectorCatalog.Allloop is the wrong home since this is not a collector table; the:499precedent shows how a non-catalog object is handled today.Not verified
I did not measure a growth rate, because there is nothing to measure — the table is empty wherever the bot is off. The "one row per decision per server" characterisation is from reading the writer, not from observed traffic. I also have not checked whether the Lite SKU has an equivalent table with the same gap; that is worth confirming before fixing, since a one-SKU fix is a recurring source of drift here.