Repository navigation
refactor: use an outbox for lifecycle artifact deletion - #1683
Conversation
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
cd7ec36 to
4579720
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It redesigns how lifecycle deletes backend artifacts, and a mistake there could leak or wrongly delete stored data, so a human look is still worthwhile.
What was reviewed:
armLifecycleAttemptin pg.ts: the metadata delete, the outbox insert and the cursor/claim update run in one transaction, and the renumbered SQL parameters match the values arrays.- I searched for remaining callers of the removed methods (
commitLifecycleAttempt,revalidateLifecycleRecoveryAttempt,inFlightOnly,LifecycleClaimedState) and found none. - The outbox table and its
attempted_atindex already exist in migration 0079, so no new migration is needed. decodeLifecycleContinuationnow rejects any continuation that still has abatch. I ruled this out as a bug on the assumption that the old journal format never reached production. A human should confirm that, because a bucket saved mid-batch would fail to decode on every claim.
Extended reasoning...
The change rewrites the lifecycle metadata-deletion path in src/storage/database/pg.ts, continuation.ts and the lifecycle schemas. The in-flight journal kept in the continuation is replaced with outbox rows in storage.lifecycle_abandoned_artifacts, with about 1,300 net lines removed. It doesn't touch auth or injection: the queries are fully parameterized and run as the service role. It does control when stored objects get permanently deleted. I deferred because the change is large and changes the design of data-deletion code, and because its safety depends on deployment history (whether any old-format continuation was ever persisted), which the code alone can't settle.
This review covers commit cd7ec36, which is no longer the latest commit on this pull request; later commits are not covered by it.
Coverage Report for CI Build 42643103949Coverage decreased (-0.09%) to 87.475%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
## What kind of change does this PR introduce? feat ## What is the current behavior? #1627 and #1683 added the database operations and outbox needed for noncurrent lifecycle deletion but executor to run was missing. ## What is the new behavior? Add a bucket executor that evaluates archived versions, deletes eligible metadata in bounded batches and removes the corresponding backend artifacts. Failed artifact deletions remain in the outbox for retry. ## Additional context Scheduling the executor and retry helper is a next separate change. Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
What kind of change does this PR introduce?
Refactor
What is the current behavior?
The lifecycle worker deletes eligible metadata and stores an in-flight batch in the bucket continuation. It records physical deletion outcomes in that journal and advances the cursor after cleanup completes. Pending artifacts can later be moved to the abandoned artifact table for retry.
What is the new behavior?
The worker deletes eligible metadata, writes the corresponding physical keys to the outbox and advances the cursor in one transaction. Physical deletion happens afterward; unresolved keys remain in the outbox for retry. The continuation holds only the cursor and counters.
Additional context
Related to #1627