fix: enforce a 24 hour floor on the deletion age window - #9
Conversation
The age window is the only thing standing between a run and live data. ClickHouse uploads a part's blobs to S3 and registers them in system.remote_data_paths a moment later; in that window a live blob is absent from the reference table and looks orphaned, and there is no per-object re-check before the S3 delete. `--useage 0` silently removed that protection -- `if args.useage else ""` emits no clause at all -- the default WAS 0, and render.py accepted it. So the dangerous configuration was also the out-of-the-box one for anyone who did not set it explicitly. --useage now defaults to 24 and is refused below 24, in both s3gc.py and render.py: the renderer catches it before a Job is applied, the tool catches a direct CLI run that never passes through the renderer. Refused for --dry-run too, deliberately: a preview computed over a wider set than the delete would honour is worse than no preview, because the reviewed number is the one the customer approves. A floor rather than a hardcoded constant, because the parameter is only dangerous downward. Upward it is the "be more careful" lever -- a cluster with slow merges or long mutations may legitimately want 72 hours or a week. Removing it would forfeit that and buy nothing the floor does not give. Development escape hatch, scoped to the one non-production phase. PHASE=dev-automation seeds and deletes its own fixtures within minutes, so a 24 hour window would make it find nothing and "succeed" vacuously -- worse than failing. That phase, and only that phase, passes --dev-allow-short-useage; the collect, dry-run and delete branches never do, and a test asserts it. A run that uses it logs a warning and writes a warning row to the durable run log, so it cannot be mistaken for a normal one. Replaces the test that documented the hazard with tests that enforce it. All 11 scope mutations remain caught. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review recordDecision taken while reviewing #6, where Decided: hard floor of 24, default 24, enforced in both Rejected: removing the parameter and hardcoding 24. It is only dangerous downward; upward it is the "be more careful" lever for clusters with slow merges or long mutations. Escape hatch scoped to Verified: 107 passed / 1 xfailed, render + kubeconform clean, #6's eleven scope mutations 11/11 caught. |
1 participant
Summary
Sets a 24-hour minimum deletion-age window in both the CLI and Job renderer.
dev-automationmay use a short window only with its explicit development override.Fix
The entrypoint now passes
--dev-allow-short-useage=true, matching the parser requirement. Regression tests pin the exact arguments.Validation
Stacked on #7; #10 follows.