Repository navigation
feat(BA-8874): purge dangling personal projects after the retention period - #16450
HyeockJinKim wants to merge 1 commit into
Conversation
…eriod Co-Authored-By: Claude Code <noreply@anthropic.com>
91d001b to
a323dc8
Compare
lablup-octodog
left a comment
There was a problem hiding this comment.
Summary
This PR fixes personal projects outliving a purged user by collecting them through the retention path, and the direction fits the existing retention structure. The user purge marks the personal project deleted so the FK empties creator_id, and the leader sweep then purges dangling personal projects past the projects policy period through the project repository, with backfill and seed migrations and tests. However, the new policy ships with enabled: false, so no collection happens until an admin enables it.
Findings
Should fix
seeds/manager/retention_policy/defaults.yaml:39: The newprojectspolicy is seeded withenabled: falsein the migration, the seed file and both fixtures, so until an admin enables it a purged user's personal project is never collected after the period and stays forever, and the staging 1-day period the issue expects is absent from the seed tree.src/ai/backend/manager/leader/tasks/retention_sweep.py:68: The leader task passes onlyretention.batch_sizeto the project purge and never consultsretention.per_tick_budget, so a configured per-tick budget does not bound project cleanup: each tick may hard-purge up to batch_size projects, each deleting its sessions, kernels and vfolders.src/ai/backend/manager/models/alembic/versions/b7d2a91c4e58_seed_projects_retention_policy.py:28: The seed migration inserts the row withoutidand lets the database generate one, while the seed file and both fixtures pin the same row tob1e5f4a0-0009-...; applying the seed files to a database that already ran this migration inserts a secondprojectsrow and collides onuq_retention_policies_categoryunless the seeder dedupes by category.
Checks
Of 40 checks, 29 passed, 4 were skipped and 7 are still running (typecheck and scenario shards 0-5); none failed.
Coverage
All 38 changed files' patches were retrieved, and the project, retention and user repositories, the project/retention rows, the migrations and the new tests were read in full at the head commit. The truncated patches for test_composer, test_leader_election, test_retention_policy_row and test_retention_repository were not read, the seed loader and install path were not read so the seed-id mismatch could not be confirmed as a real failure, and the 20-call read_file cap prevented comparing against the base commit.
| - id: b1e5f4a0-0009-4c10-8a01-000000000009 | ||
| category: projects | ||
| retention_period_days: 30 | ||
| enabled: false |
There was a problem hiding this comment.
The projects policy ships with enabled: false here, in the install fixtures and in seed migration b7d2a91c4e58, and RetentionSweepTask._sweep_dangling_personal_projects() returns as soon as enabled_policy() finds nothing. On a fresh or upgraded deployment a purged user's personal project is therefore still kept forever until an admin turns the policy on, and the issue's staging 1-day period has no override in the seed tree. If collection is meant to take effect out of the box, the policy needs a seeded enabled state (and a staging override).
| result = await self._project_repository.purge_dangling_personal_projects( | ||
| threshold, | ||
| self._config_provider.config.retention.batch_size, | ||
| ) |
There was a problem hiding this comment.
Only batch_size is passed here; retention.per_tick_budget, which RetentionDBSource.sweep() uses to stop a tick once enough rows were deleted, is not consulted. With a budget set, one tick can still purge up to batch_size dangling projects, each deleting its sessions, kernels and vfolders (with storage RPCs), so the budget no longer bounds the leader's work. Applying the remaining budget here, or stopping after it is spent, would keep the tick bounded.
| depends_on = None | ||
|
|
||
| _INSERT_POLICY = sa.text(""" | ||
| INSERT INTO retention_policies (category, retention_period, enabled) |
There was a problem hiding this comment.
The migration inserts the row without id, so uuid_generate_v7() assigns a fresh one, while seeds/manager/retention_policy/defaults.yaml and both fixtures pin this row to b1e5f4a0-0009-4c10-8a01-000000000009. If the seed files are applied to a database that already ran this migration, the pinned-id insert adds a second projects row and hits uq_retention_policies_category (unless the seeder dedupes by category). Passing the same fixed id would keep the migration row and the seed row identical.
Summary
projectsretention category. A backfill migration marks projects already dangling, and a seed migration adds the policy row on upgraded databases.Test plan
pants test --changed-since=origin/main: backfill and seed migrations, dangling project purge (sessions, kernels, vfolders, per-project failure isolation), user purge marking, leader task and wiring.Resolves BA-8874
🤖 Generated with Claude Code
📚 Documentation preview 📚: https://sorna--16450.org.readthedocs.build/en/16450/
📚 Documentation preview 📚: https://sorna-ko--16450.org.readthedocs.build/ko/16450/