mikebridge opened a new pull request, #43490: URL: https://github.com/apache/superset/pull/43490
### SUMMARY The `purge_audit_log` table grew without bound. Every scheduled purge run that hit the same blocked entity wrote another identical record (until #42863 deduped the write path), and nothing ever aged out operational noise. This adds the retention policy defined by [SC-116701](https://app.shortcut.com/preset/story/116701) and the scheduled task that applies it. Deletion-only — no schema change, no migration. Three categories, drained in priority order under one shared per-run budget: 1. **Blocked duplicates.** Within an entity's *current* blockage streak, only the earliest row survives — it carries the "blocked since" fact and is never deleted while the streak is current. Later duplicates go regardless of age; that is the rule that actually bounds growth. 2. **Operational expiry.** `failed` rows and `blocked` rows from *resolved* streaks age out past `PURGE_AUDIT_OPERATIONAL_RETENTION_DAYS` (default 90). 3. **Evidence expiry.** `confirmed` / `target_absent` — the only surviving trace of a destroyed object — are **never** touched unless `PURGE_AUDIT_EVIDENCE_RETENTION_DAYS` is explicitly set. Setting it is the operator's assertion that an approved compliance policy permits expiring destruction evidence. `pending` rows belong to `reconcile_pending()` and are structurally unreachable. Invalid retention values fail closed: the run logs a warning and skips that category rather than widening removal. `PURGE_AUDIT_PRUNING_ENABLED = False` restores the previous behaviour exactly, and a disabled run says so rather than silently doing nothing. **Design notes worth a reviewer's attention:** - **A `failed` attempt does not end a blockage streak.** A failed purge is an infrastructure outcome — the cascade raised — not evidence the block cleared; the blocking policy is untouched. Treating it as a boundary would let one transient error demote the blocked-since survivor and restate the blockage as beginning *after* the failure. - **Concurrency.** Deletes are conditional on the expected status and counted from statement rowcounts, so overlapping runs cannot double-remove or double-report. Beyond that, a blocked row whose streak classification is *unstable* is never deleted by either category: finalizing a `pending` row resolves it in place, keeping its original timestamp, so an unresolved attempt is a boundary that can appear mid-history and turn the row after it into a survivor. Evidence expiry likewise refuses to delete a row that still bounds surviving blocked rows, which is what stops a boundary receding — and makes an evidence window shorter than the operational one safe rather than corrupting. - **Known limitation, documented in the module docstring.** These guards are evaluated when candidates are selected, not when they are deleted. `created_on` is stamped by the writing process rather than the database, so a row can become visible with a timestamp in the past; one appearing in the window between the SELECT and the DELETE would evade them. Closing it completely needs candidacy evaluated *inside* the DELETE (dialect-specific: MySQL rejects self-referencing DELETE subqueries), serialization against the audit writers, or a database-assigned `created_on`. Tracked as follow-up; the exposure is bounded by writer clock spread and by how closely spaced an entity's blocked rows are. - Each category is guaranteed at least one batch of the shared budget. Strict priority would let a permanent duplicate backlog starve age-out forever — reintroducing the unbounded growth this feature exists to stop. - The startup warning that already covers the other retention tasks now covers this beat entry too, so an operator who replaced `CELERY_CONFIG` is told pruning is not running. This PR was reviewed by a nine-lens panel plus three adversarial data-systems passes before opening; the survivor-invariant findings from those passes are fixed here, each with a red-first control run. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF No UI. Operator-visible surface is configuration, metrics (`deletion_retention.prune_purge_audit.*` — per-category removal counts plus `carried_over`), and one structured completion log line per run. ### TESTING INSTRUCTIONS ```bash # Unit pytest tests/unit_tests/commands/deletion_retention/test_prune_audit.py # Integration (needs a metadata DB; run against Postgres) pytest tests/integration_tests/deletion_retention/prune_audit_tests.py ``` 26 unit tests (classification and the exhaustiveness contract against the model's `ALL_STATUSES`, fail-closed config validation, task reporting/isolation, clock parity with the audit writer) and 20 integration tests against the real table: age-independent dedup, bounded convergence with carryover, cross-entity isolation, resolved streaks, `failed` streak-transparency, evidence protection by count-and-identity, opt-in expiry and its disable-again edge, pending/future-row immunity, skew-resistant classification, unstable-block deferral in both categories, boundary-guard deferral including the inverted-window case, uuid-less block retention, starvation resistance, rerun idempotence, and the status re-check. Manual: enable the beat entry, seed duplicate `blocked` rows for one entity, run `deletion_retention.prune_purge_audit`, and confirm the earliest row survives and the counts appear in logs/metrics. Full recipe in `specs/sc-116701-purge-audit-pruning/quickstart.md`. ### ADDITIONAL INFORMATION - [x] Has associated issue: SC-116701 (follow-up to SC-115343 / #42863, part of the soft-delete work under the approved [SIP-208](https://github.com/apache/superset/issues/39286)) - [x] Required feature flags: none — pruning is independent of `SOFT_DELETE` because audit rows outlive the flag. The beat-entry startup warning is gated on it. - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [x] Introduces new feature or API — new Celery task, three config keys, no REST surface - [ ] Removes existing feature or API **Behaviour change on upgrade** (recorded in `UPDATING.md`): operational audit records older than 90 days begin pruning automatically. Completed-destruction evidence is untouched. Opt out with `PURGE_AUDIT_PRUNING_ENABLED = False`; deployments overriding `CELERY_CONFIG` must carry the new beat entry forward. This PR was developed with AI assistance (Claude Code), including the implementation, tests, and review remediation; a human (@mikebridge) reviews before merge. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
