Repository navigation
feat: add db support for noncurrent lifecycle execution - #1627
Conversation
Coverage Report for CI Build 42202464917Coverage increased (+0.3%) to 87.542%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
src/storage/database/pg.ts— A worker that finishes a run without agenerationkey in its result gets its bucket rescheduled at once, so lifecycle scans of the whole bucket repeat back to back.completeLifecycleRunreads the policy generation from$4::jsonb ->> 'generation', which is the free-formlastResultpayload. A missing key gives NULL, soIS NOT DISTINCT FROMis false andnext_run_atbecomesclock_timestamp(). Fix: pass the claimed generation as its own typed parameter, compare it withbucket.lifecycle_configuration_generation, and drop the dependency onlastResult.Why this was flagged
The trigger is any caller of
completeLifecycleRun(input, lastResult, nextRunAt)(adapter.ts:252) whoselastResulthas no stringgenerationfield. The signature types that argument asRecord<string, unknown>, and nothing documents that the field is required. At pg.ts:806-810 the CASE comparesbucket.lifecycle_configuration_generation::textwith$4::jsonb ->> 'generation'. When the key is missing that side is NULL, so the ELSE branch setsnext_run_at = clock_timestamp()and the caller'snextRunAtis ignored. The bucket is then due immediately and gets reclaimed. A full noncurrent evaluation runs again after every completion instead of waiting for the next schedule. That burns database load on every lifecycle-enabled bucket. Only one caller exists today: the integration tests at noncurrent-lifecycle-database.test.ts:731, :775 and :1360. One of them (:731) already passes{}. The worker that will call this is not in this PR, so nothing enforces the hidden contract.Verification: nit. Trigger: any future caller that passes a
lastResultwithout agenerationkey. In pg.ts:3926-3931,next_run_atcompares the bucket generation with$4::jsonb ->> 'generation', and $4 isJSON.stringify(lastResult)(line 3944). When the key is missing,->>returns NULL, sonext_run_atis set toclock_timestamp()and the caller'snextRunAtis silently ignored.
b455d29 to
23f7f76
Compare
|
@claude review always |
a555fd9 to
e5c47b6
Compare
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
5538ee8 to
0dc265a
Compare
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
0dc265a to
a2af09d
Compare
## 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 --------- 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?
feature
What is the current behavior?
It can store lifecycle policies and evaluate pages of noncurrent versions. There is no database API to coordinate a lifecycle deletion run or recover its unfinished backend cleanup.
What is the new behavior?
Add the database layer for lifecycle execution: leased bucket claims, bounded deletion batches, eligibility rechecks under locks and a durable journal committed with metadata deletion itself. It also tracks cleanup outcomes and abandoned artifacts for a later retry.
Additional context
This is a standalone database layer and a worker is still needed to schedule runs and delete backend artifacts.
Related to #1380