doc(BA-6925): add BEP-1063 DB record retention management#12940
Merged
Conversation
Propose a general-purpose DB retention layer: a super-admin declares a retention period per data category, and the manager periodically purges older records via a batched, referentially-safe LeaderCron sweep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds BEP-1063, proposing centralized database record-retention policies and leader-driven cleanup.
Changes:
- Defines retention storage, category catalog, sweeping, APIs, and migration.
- Registers BEP-1063 as Draft.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
proposals/README.md |
Registers BEP-1063. |
proposals/BEP-1063-db-record-retention.md |
Documents the retention-management design. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | ✅ | `clear-history` deletes terminated sessions/kernels + `error_logs` + Redis stats, then runs `VACUUM` / `VACUUM FULL` | | ||
| | ➕ | Recurring deletes move to policies; the CLI is kept as a manual force-sweep + `VACUUM FULL` owner; the sweep relies on autovacuum | | ||
|
|
||
| ## 3. Implementation Design |
| |---|---|---| | ||
| | `event_logs` | `created_at` | append-only, no FKs | | ||
| | `audit_logs` | `created_at` | has a `status` transition (running→success) but **no `updated_at` column** → `created_at` is the only boundary | | ||
| | `error_logs` | `created_at` | **deletes all rows past the boundary** (no `is_read`/`is_cleared` filter). Has mutable flags but no `updated_at`. Outbound `user` FK only. Generalizes the current etcd timer | |
| | Category | Table(s) | Terminal signal | Safe-delete procedure | | ||
| |---|---|---|---| | ||
| | `terminated_sessions` | `sessions` (+ `kernels`) | session `status ∈ DEAD_SESSION_STATUSES {TERMINATED, CANCELLED}` + `terminated_at`; kernel `status ∈ DEAD_KERNEL_STATUSES {TERMINATED, CANCELLED}` + `terminated_at`. Reuse both canonical frozensets (`models/session/row.py`, `models/kernel/row.py`) rather than hard-coding | Delete `kernels` first (`sessions.id` has no cascade from kernels), then `sessions`. `session_dependencies` / `vfolder_attachment` cascade. `routings.session` is `ON DELETE RESTRICT`, so filter to sessions with no surviving routing. Also purge the deleted kernels' Redis statistics (absorbs a `clear-history` duty) | | ||
| | `destroyed_endpoints` (deployment) | `endpoints` | `lifecycle_stage = DESTROYED` + `destroyed_at` | Children cascade (`replica_groups`, `routings`, `endpoint_auto_scaling_rules`, `deployment_policies`). FK-less GUID children (`endpoint_tokens`, `deployment_revisions`) are deleted by the handler via endpoint id | |
| | Category | Table | Terminal / expiry signal | Safe delete | | ||
| |---|---|---|---| | ||
| | `invalidated_login_sessions` | `login_sessions` | `status ∈ {INVALIDATED, REVOKED}` + `invalidated_at` (dedicated) | no inbound FK → direct batched delete | | ||
| | `deleted_roles` | `roles` (RBAC) | `status = DELETED` + `deleted_at` (dedicated) | all inbound FKs CASCADE (`user_roles`, `permissions`, `role_invitations`) | |
| | Category | Table(s) | Boundary | Note | | ||
| |---|---|---|---| | ||
| | `kernel_usage_records` | `kernel_usage_records` | `period_end` | Highest-volume; append-only raw, no FK | | ||
| | `usage_buckets` | `domain/project/user_usage_buckets` (+ `usage_bucket_entries`) | `period_end` / `period_start` | `usage_bucket_entries` has no timestamp and no FK — delete entries by the purged bucket ids, then the buckets | |
|
|
||
| The sweep runs as one `PeriodicTask` on the manager's **`LeaderCron`**, **not** the `GlobalTimer` + distributed-lock path and **not** a sokovan reconciler. | ||
|
|
||
| - **Leadership replaces the distributed lock.** `LeaderCron` runs its tasks only on the elected leader (`ValkeyLeaderElection`), so single-execution is already guaranteed; `LOCKID_LOG_CLEANUP_TIMER` is no longer needed. This is why the sweep does not reuse `GlobalTimer`. |
| - **Leadership replaces the distributed lock.** `LeaderCron` runs its tasks only on the elected leader (`ValkeyLeaderElection`), so single-execution is already guaranteed; `LOCKID_LOG_CLEANUP_TIMER` is no longer needed. This is why the sweep does not reuse `GlobalTimer`. | ||
| - **Not a reconciler.** Retention has no per-entity convergence, retryable outcomes, or cross-tick state; it is periodic batched deletion. The reconciler pattern (BEP-1054) would be pure overhead. | ||
|
|
||
| The tick cadence is the single `retention.sweep_interval`. Each tick: load **every enabled policy** → resolve its catalog handler, compute `threshold = now − retention_period`, delete in `retention.batch_size` batches (chunk by primary key / `LIMIT`) until no older rows remain or `retention.per_tick_budget` is hit → record `last_swept_at` (observability). Whether the task deletes inline or fires a `DoRetentionSweepEvent` for a handler (keeping delete logic in a testable event handler, as `log_cleanup` does) is an implementation choice. Space is reclaimed by **autovacuum**; the sweep never runs `VACUUM`. |
|
|
||
| ### (d) Admin API — super-admin v2 CRUD | ||
|
|
||
| `retention_policies` is a v2 entity exposed through the full stack (REST v2 + Strawberry GraphQL + shared adapter + SDK v2 + CLI v2), **super-admin only**, with the standard 6 operations. `category` is validated against the catalog on create/update. Because the category set is fixed by the catalog, the common operation is `update` (tune period / enable) on seeded rows; reads expose `last_swept_at` for sweep visibility. |
| |---|---| | ||
| | Recurring `error_logs` deletion (old etcd timer) | `error_logs` policy — auto-swept; the `GlobalTimer` + `DoLogCleanupEvent` path is retired and the etcd value is absorbed as its seed | | ||
| | Recurring terminated session/kernel + Redis-stat deletion | `terminated_sessions` policy — auto-swept, Redis stats included | | ||
| | One-shot forced cleanup | CLI retained — trigger an immediate sweep of a category | |
|
|
||
| | Category | Table(s) | Terminal signal | Safe-delete procedure | | ||
| |---|---|---|---| | ||
| | `terminated_sessions` | `sessions` (+ `kernels`) | session `status ∈ DEAD_SESSION_STATUSES {TERMINATED, CANCELLED}` + `terminated_at`; kernel `status ∈ DEAD_KERNEL_STATUSES {TERMINATED, CANCELLED}` + `terminated_at`. Reuse both canonical frozensets (`models/session/row.py`, `models/kernel/row.py`) rather than hard-coding | Delete `kernels` first (`sessions.id` has no cascade from kernels), then `sessions`. `session_dependencies` / `vfolder_attachment` cascade. `routings.session` is `ON DELETE RESTRICT`, so filter to sessions with no surviving routing. Also purge the deleted kernels' Redis statistics (absorbs a `clear-history` duty) | |
…logic Restructure the retention catalog into eight domain-based categories, drop the idle-checker analogy in favor of code-fixed per-table cleanup, scope the layer to DB records only (Redis out of scope), reduce policy knobs to retention_period/enabled with cadence in server config, and authorize via a super-admin role gate rather than RBAC scope resolution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
retention_policiesDB table (per-category, global scope; per-scope is a non-goal), a code-side category catalog that owns each category's referentially-safe delete procedure, a LeaderCron batched sweep (replacing theerror_logsGlobalTimerand absorbingclear-history's recurring duties), sweep cadence/batch size in server config, andUserRole.SUPERADMINrole-gate authorization (not RBAC scope resolution).created_at; in-place reconcile/scheduling history →updated_at; lifecycle: terminated sessions/kernels, destroyed endpoints, auxiliary terminal/expiry records; usage/billing) with delete-safety notes.Test plan
Resolves BA-6925