feat(retention): bound update_history and backup_runs, surface retention in the UI - #38
Merged
Merged
Conversation
…ion in the UI
Three tables grew without bound. Only sessions (hourly) and action_log (daily)
were ever pruned; cached_updates is replaced wholesale on each scan so it is
bounded by container count. update_history, backup_runs and backup_run_items
were not pruned at all -- one row per image update, one per backup run, and one
per stack per run, kept forever.
The growth is slow, silent, and only shows up on exactly the deployments that
matter: long-lived ones with automation switched on. Nothing surfaced it -- no
size metric, no warning, no bound. The consequences arrive in the order that
makes them hardest to diagnose: history views get slow, then the SQLite file
gets awkward, then -- now that capstan.db is itself in the backup set
(agent-os-36o) -- every backup run drags an ever-growing database with it.
internal/database/retention.go holds one retention concept for all four history
tables: DeleteOldUpdateHistory and DeleteOldBackupRuns alongside the existing
DeleteOldActionLogs, a RetentionDays accessor that defaults to 90 and clamps to
a floor of 7, and PruneHistory running every policy in one pass. Each table is
pruned independently, since one failing must not skip the others. The floor
lives in code rather than the schema so editing the settings row by hand cannot
bypass it either -- a prune is irreversible.
main.go's daily ticker now calls PruneHistory, replacing twenty lines of inline
parsing, and a startup pass runs at T+2min. The ticker's first fire is 24 hours
away, so an instance that restarts daily -- a container on a nightly compose
pull -- never reached it and the cleanup never ran at all.
Migration 11 seeds the two new settings keys and indexes
update_history.completed_at, previously reachable only by full scan.
Two corrections to the issue as filed:
DeleteUpdateHistoryOlderThan is described as having no callers at all. It has
one -- handlers/updates.go:731, a manual "clear history older than" endpoint.
Nothing invoked it automatically, which is what this fixes; the manual
endpoint is untouched.
The new controls were to go "next to the existing log retention control".
There is no existing control: grep for log-retention across frontend/src
returns nothing, so the endpoint had no caller outside backend tests and the
only way to change retention was to edit the settings row by hand. This adds
a History retention section to Settings -> Audit Log covering all three,
which is the first time any of it is reachable from the UI.
The endpoint now takes all three fields as optional pointers so a client can
update one without knowing the others; retentionDays alone still behaves
exactly as before and the pre-existing handler tests pass unchanged.
Verified with the manual check the issue asks for -- backdate rows, restart,
confirm the startup pass removes them and logs how many:
SEEDED (update_history, backup_runs, backup_run_items, action_log): (2,2,2,1)
level=INFO msg="History retention pass complete" update_history_deleted=1
backup_runs_deleted=1
AFTER: (1, 1, 1, 0) SURVIVORS: [u-new, b-new, i-new]
backup_run_items going 2 -> 1 with no explicit delete is the cascade working;
it is covered by a test rather than assumed, because ON DELETE CASCADE only
fires when foreign_keys enforcement is genuinely on for the connection
(agent-os-94t). The frontend floor test was confirmed to fail with the guard
removed.
agent-os-0jp
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
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.
Closes
agent-os-0jp. Second of the fourmain.gobeads (7li→0jp→r7e→69a).The bug
Three tables grew without bound. Only
sessions(hourly) andaction_log(daily) were ever pruned;cached_updatesis replaced wholesale each scan so it is bounded by container count. The remaining three were not:update_history— one row per container image update, forever.backup_runs— one row per backup run, forever.backup_run_items— one row per stack per run, forever.This is slow, silent, and only manifests on exactly the deployments that matter: long-lived ones with automation switched on. Nothing surfaces the growth — no size metric, no warning, no bound. The consequences arrive in the order that makes them hardest to diagnose: history views get slow, then the SQLite file gets awkward, then — now that
capstan.dbis itself in the backup set (agent-os-36o) — every backup run drags an ever-growing database along with it.What changed
internal/database/retention.goholds one retention concept for all four history tables:DeleteOldUpdateHistoryandDeleteOldBackupRuns, matchingDeleteOldActionLogs.RetentionDays(key)reads a setting, defaults to 90 and clamps to a floor of 7. A prune is irreversible, so an absurdly low setting must not wipe the history someone is in the middle of reading. Enforcing the floor in code rather than in the schema means editing the settings row by hand cannot bypass it either.PruneHistory()runs every policy in one pass. Each table is pruned independently — one failing must not skip the others, since the whole point is bounding growth.main.go's daily ticker now callsPruneHistory(), replacing twenty lines of inline parsing, and a startup pass runs at T+2min. The ticker's first fire is 24 hours away, so an instance that restarts daily — a container on a nightly compose pull — never reached it and the cleanup never ran at all. The delay keeps the prune off the startup critical path.Migration 11 seeds the two new settings keys and adds an index on
update_history.completed_at, which was previously only reachable by full scan.A correction to the bead
The bead says
DeleteUpdateHistoryOlderThanhas "no callers at all, in production code or tests". That is no longer true —handlers/updates.go:731calls it from a manual "clear history older than" endpoint. The substance stands: nothing invoked it automatically, which is what this fixes. The manual endpoint is untouched.The Settings UI: the premise was wrong
The bead asks for the new controls "in the Settings UI next to the existing log retention control". There is no existing control.
GET/PUT /settings/log-retentionhad no caller in the frontend at all:Only backend tests hit it. The only way to change retention was to edit the settings row by hand. So rather than adding two controls beside one, this adds a History retention section to Settings → Audit Log covering all three, which is the first time any of it is reachable from the UI.
The endpoint now takes all three fields as optional pointers, so a client can update one without knowing the others; at least one must be present.
retentionDaysalone still works exactly as before — the pre-existing handler tests pass unchanged.Verified
Unit and component tests, plus the manual check the bead asks for: backdate rows, restart the server, confirm the startup pass removes them and logs how many.
The
backup_run_itemscount going 2 → 1 with no explicit delete is the cascade doing its work —i-oldwent with its parent run. That is covered by a test rather than assumed, becauseON DELETE CASCADEonly fires whenforeign_keysenforcement is genuinely on for the connection (the failure mode behindagent-os-94t).The frontend floor test was confirmed to fail with the guard removed, per the standing rule that a test which would also pass against the broken code proves nothing.
Gates
Deliberately not done
The bead's step 5 ("consider a periodic
VACUUMorPRAGMA incremental_vacuum") is not in the acceptance criteria. Deleting rows frees pages for reuse but does not shrink the file, so a large purge leaves it at its high-water mark. That is a separate decision with its own cost —VACUUMrewrites the whole database and takes a write lock — and is better made deliberately than folded in here.