chore(datastore): npm remote cache reap (S20-A plan: 10/21)

Pawel Rozlachrequested to merge
prozlach/s20a-lifecycle-closed-beta-step-10 into main

What this delivers

Step 10 of the S20-A plan: the reap logic for the npm remote cache. Two Reaper implementations land in a new file, internal/datastore/lifecycle_reap_npm_remote.go.

  • NpmRemoteVersionReaper reaps the subtree of one tombstoned npm_remote_versions row. It removes the version's npm_remote_files rows and the blob_storage_attachments row each file holds, then the version row.
  • NpmRemotePackageReaper reaps the subtree of one tombstoned npm_remote_packages row. It removes the package's npm_remote_metadata_files rows and their attachments, then the package row.

Both reach their cache rows through the bounded holder delete step 8 declares, deleteNpmAttachmentHolderBatch over an npmAttachmentHolder. That helper removes the holder rows and returns the attachments they held. Each Reap then releases those attachments itself, in one call on every path, through step 8's BlobStorageAttachmentStore.releaseNpmAttachments and the DeleteIfUnreferencedByNpm guard behind it. Both take the children first and the parent last. The release sits before the parent delete, which is where NpmPackageReaper puts its own. Both remote parents are the row a cache fill locks first. Taking the parent last is each reap's one leg out of the fill's order. A chunk that runs out of budget mid-subtree leaves the parent standing for the next chunk.

npm_remote_metadata_files carries no soft_deleted_at column, so it holds no marked state for a scan to find. Walking the parent package is the only route to those rows and to the attachments behind them. There is no tag leg, and that absence is schema rather than omission. No npm_remote_tags table exists, so nothing of that shape can pin a version row.

Neither reaper has a production caller. Plan step 15 wires the chunk driver that calls a Reaper, and plan step 6 adds the npm_remote_versions discovery scan. Today the tests are the only callers, so no request and no job in a booted service reaches this code.

The change also appends four query-name constants to internal/datastore/query_names.go, one per new instrumented statement. The suite is 12 integration tests against a real Postgres, plus an argument-guard table of 8 cases.

The reviewable diff is git diff origin/main...HEAD, which is what GitLab shows against this target. It carries only step 10's own change set, because !1705 (merged) merged and this merge request now targets main.

Spec coverage

Spec: docs/specs/S20-a-lifecycle-closed-beta.md

Step 10 owns the npm remote reap. Rows owned by another step of docs/plans/2026-08-11-s20a-lifecycle-closed-beta.md name that step.

Acceptance criteria

Purger and discovery

# Criterion Tests
AC-P1 Eligible repositories tombstone discovered, newer one not; window zero and non-zero Step 5. Not tested here
AC-P2 A soft_deleted_at IS NULL row is never discovered by any scan or walk, every table in the reap inventory Scan half: Steps 6 and 7. Walk half for this family: TestNpmRemoteVersionReaper_Reap_RemovesTheVersionSubtree, TestNpmRemoteVersionReaper_Reap_IsIdempotent (both assert the live sibling version, its file, its attachment and the parent package's cache row survive), TestNpmRemotePackageReaper_Reap_RemovesTheMarkedPackageAndItsCache, TestNpmRemotePackageReaper_Reap_IsIdempotent, TestNpmRemotePackageReaper_Reap_BoundsEachChunk (all assert the live sibling package survives)
AC-P3 EXPLAIN on the repositories, npm_packages, container_images scans Steps 5 and 7. Not tested here
AC-P4 EXPLAIN on the five version-level scans Step 6. Not tested here
AC-P5 Re-run on a drained subtree is a success no-op; a mid-walk abort leaves fewer rows and no per-artifact marker Re-run half: TestNpmRemoteVersionReaper_Reap_IsIdempotent, TestNpmRemotePackageReaper_Reap_IsIdempotent. No-marker half: TestNpmRemotePackageReaper_Reap_StallsWhileAVersionSurvives, TestNpmRemoteVersionReaper_Reap_RunsEveryStatementOnTheCallersHandle (an aborted chunk rolls back whole and the retry converges). The gofail mid-walk abort is Step 14
AC-P6 The repository walk reaps live and tombstoned rows in one pass; a marked row is not read as another worker's claim Walk itself: Step 14. Reaper side for this family: TestNpmRemoteVersionReaper_Reap_IsStateBlindOnTheRowItIsHanded, plus the unmarked-file assertion in TestNpmRemoteVersionReaper_Reap_RemovesTheVersionSubtree
AC-P7 Every purge transaction is bounded; more artifacts than one chunk yields more than one transaction TestNpmRemoteVersionReaper_Reap_BoundsEachChunk, TestNpmRemotePackageReaper_Reap_BoundsEachChunk, and TestNpmRemotePackageReaper_Reap_TakesTheRowOnceTheBudgetOutrunsTheCache for the other side of the bound: one call whose budget outruns the cache takes the parent row too
AC-P8 A purge job in a namespace that stopped serving writes refuses retryably, per lifecycle column Step 16. Not tested here

Per-format reap logic

# Criterion Tests
AC-F1 Tombstoned npm version reaped with files and attachments, through npm's own guard Step 8 (TestNpmVersionReaper_Reap_RemovesTheVersionSubtree)
AC-F2 Unpublish-then-republish leaves no orphan at the shared coordinate Step 8 (TestNpmVersionReaper_Reap_LeavesTheRepublishedRowUntouched)
AC-F3 Tombstoned npm package reaped with tags, metadata files and attachments, no 23503 Step 8 (TestNpmPackageReaper_Reap_RemovesAPackageTombstonedBySingleVersionUnpublish)
AC-F4 DeleteNpmMetadataFiles removes each attachment in the same transaction Step 8
AC-F5 Tombstoned Maven version and package reaped with files and attachments Step 9. Not tested here
AC-F6 Tombstoned container repository reaped through the existing per-artifact deleters Steps 13 and 14. Not tested here
AC-F7 Container index and its children reaped parent-first, no MANIFEST_REFERENCED Step 13. Not tested here
AC-F8 Tombstoned container_images row under a live repository reaped through the same walk Step 13. Not tested here
AC-F9 Container repository walk reaps live and tombstoned images in one pass Steps 13 and 14. Not tested here
AC-F10 A tombstoned remote-cache subtree is reaped for every remote table carrying the column, plus npm_remote_metadata_files and their attachments walked from the parent. A positive hit per table This step, for the npm remote family. npm_remote_versions and npm_remote_files: TestNpmRemoteVersionReaper_Reap_RemovesTheVersionSubtree, TestNpmRemoteVersionReaper_Reap_ReapsAVersionTheEvictorMarked. npm_remote_packages and npm_remote_metadata_files: TestNpmRemotePackageReaper_Reap_RemovesTheMarkedPackageAndItsCache, which walks the parent for the cache rows because that table carries no soft_deleted_at. Maven remote is Step 11 and container_remote_images is Step 12
AC-F11 container_remote_manifests, container_remote_blobs, container_remote_tags, container_remote_manifest_relationships Step 12. Not tested here
AC-F12 After a subtree is reaped every blob it referenced has zero blob_storage_attachments rows, counted per sha256 This step, for the npm remote family: the countAttachments assertions in assertNpmRemoteVersionSubtreeGone, TestNpmRemoteVersionReaper_Reap_ReapsAVersionTheEvictorMarked, TestNpmRemoteVersionReaper_Reap_BoundsEachChunk, TestNpmRemotePackageReaper_Reap_RemovesTheMarkedPackageAndItsCache and TestNpmRemotePackageReaper_Reap_BoundsEachChunk. The negative half, an attachment a surviving npm row still holds, is TestNpmRemoteVersionReaper_Reap_LeavesAnAttachmentAnotherNpmRowHolds

Repository entry point

# Criterion Tests
AC-R1 Missing or non-boolean destructive returns 400 Step 15. Not tested here
AC-R2 destructive=false on an empty repository returns 204, name reusable Step 15. Not tested here
AC-R3 destructive=false on a non-empty repository returns 409 Step 15. Not tested here
AC-R4 The 409 message names the remedy the caller has Step 15. Not tested here
AC-R5 destructive=true on a non-empty repository returns 202, tombstones, enqueues in the same transaction Steps 15 and 17. Not tested here
AC-R6 destructive=true on an empty repository returns 204, no tombstone, no job Step 15. Not tested here
AC-R7 Every client route naming the repository returns 404 after the 202 Steps 3, 4 and 15. Not tested here
AC-R8 Create at a tombstoned name returns 409, 201 after the finalizer Steps 14 and 15. Not tested here
AC-R9 The finalizer cannot succeed early with an artifact left in place Step 14. Not tested here
AC-R10 The finalizer removes the repository row, format-child row and collection links Step 14. Not tested here

Sweep

# Criterion Tests
AC-S1 A tombstoned repository with no live purge job is re-queued within one interval Step 18. Not tested here
AC-S2 Oldest-first page ordering, and EXPLAIN shows no Sort node Steps 5 and 18. Not tested here
AC-S3 An orphaned npm_packages row is enumerated on the added index and purged without 23503 Steps 7 and 18. Not tested here
AC-S4 A repository with a live purge job is not re-queued Step 18. Not tested here
AC-S5 A terminal prior job is re-queued, pinning UniqueByArgsWhileLive() Step 18. Not tested here
AC-S6 A permanently failing purge stays counted and re-queued at the bounded cadence Steps 17b and 18. Not tested here
AC-S7 The sweep registers a worker for its periodic kind Step 18. Not tested here
AC-S8 A frozen namespace has nothing enqueued, and is re-queued once the condition clears Step 18. Not tested here

Tombstone visibility

# Criterion Tests
AC-V1 ContainerRepositoryStore.FindByNameInNamespace hides a tombstoned repository, docker and oci Step 3. Not tested here
AC-V2 RepositoryStore.FindByID returns ErrNotFound Step 3. Not tested here
AC-V3 RepositoryStore.Update returns ErrNotFound Step 3. Not tested here
AC-V4 Delete, the three counter writes and Missing still act on a tombstoned row Step 3. Not tested here
AC-V5 parentRepositoryIsActive refuses a parent tombstoned mid-transaction Step 4. Not tested here
AC-V6 The four container_images gates hide a tombstoned image, plus FindByID S17 Phase 4. Not tested here
AC-V7 Every /v2/ route resolving through FindByName answers 404 NAME_UNKNOWN S17 Phase 4. Not tested here
AC-V8 Blob mount answers 202 when every source image is tombstoned S17 Phase 4. Not tested here
AC-V9 A mount whose from= names a marked image still answers 201 S17 Phase 4. Not tested here
AC-V10 A mount into a tombstoned destination name answers 201 S17 Phase 4. Not tested here
AC-V11 UpsertContainerImage does not resurrect a tombstoned image S17 Phase 4. Not tested here
AC-V12 ContainerImageStore.Delete still acts on a tombstoned row S17 Phase 4. Not tested here

Accounting call sites

# Criterion Tests
AC-A1 The destructive repository DELETE emits no counter delta at the tombstone write Step 19. Not tested here
AC-A2 Δsize and Δcomponents fire at the purger, delta set per level Step 19 for the emit. This step pins the reaper's inputs to it: Components is 1 per reaped npm_remote_versions row and 0 for a package reap (TestNpmRemoteVersionReaper_Reap_RemovesTheVersionSubtree, TestNpmRemotePackageReaper_Reap_RemovesTheMarkedPackageAndItsCache), and SizeBytes is zero (see Resolved spec ambiguities)
AC-A3 Every emit site commits its source rows before issuing the delta Step 19. The reaper-side half, that a rolled-back chunk moves nothing, is TestNpmRemoteVersionReaper_Reap_RunsEveryStatementOnTheCallersHandle
AC-A4 A purge chunk emits from its own committed affected-row aggregate Step 19. This step pins that ReapTotals counts the chunk's own affected rows: a repeat chunk reports ReapTotals{} (TestNpmRemoteVersionReaper_Reap_IsIdempotent, TestNpmRemotePackageReaper_Reap_IsIdempotent) and the version is counted exactly once across chunks (TestNpmRemoteVersionReaper_Reap_BoundsEachChunk)
AC-A5 A raw foreign-key cascade emits nothing and is not used Steps 12 and 13. Not tested here; no npm remote foreign key carries an ON DELETE action, which the stall case exercises directly

Schema and configuration

# Criterion Tests
AC-C1 The repositories and npm_packages discovery index migrations apply and roll back cleanly Step 1. Not tested here
AC-C2 No migration adds or drops a column, no CHECK pins soft_deleted_at to NULL Step 1. Not tested here
AC-C3 unique_container_images_ns_id_cr_id_name is partial on every partition S17 Phase 4. Not tested here
AC-C4 Config load accepts, defaults and rejects the lifecycle window and interval Step 2. Not tested here

Error cases

# Condition Tests
E-1 DELETE omits destructive: 400 Step 15. Not tested here
E-2 destructive carries a non-boolean: 400 Step 15. Not tested here
E-3 destructive=false on a non-empty repository: 409 with the right message Step 15. Not tested here
E-4 An artifact published between the DELETE and its DELETE FROM repositories Step 15. Not tested here
E-5 DELETE on a missing or already-tombstoned repository: 404 Step 15. Not tested here
E-6 Create at a tombstoned repository's name: 409 Step 15. Not tested here
E-7 Any client route naming a tombstoned repository: 404 in its own vocabulary Steps 3, 4 and 15. Not tested here
E-8 Blob mount whose from= names a tombstoned repository: 202 S17 Phase 4. Not tested here
E-9 Purge job: 23503 on a parent delete aborts the transaction and fails the job; not mapped to ErrRepositoryNotEmpty TestNpmRemotePackageReaper_Reap_StallsWhileAVersionSurvives, which asserts the stall reaches the caller as ErrReapParentPinned, that the chunk rolled back whole (package row, every cache row and every attachment back), and that the same call converges once the version is reaped
E-10 Purge job: ContainerManifestDeleter refuses a still-referenced manifest Step 13. Not tested here
E-11 Purge job: the finalizer aborts because an artifact remains Step 14. Not tested here
E-12 Purge job: the target is already hard-deleted; the walk finds nothing and reports success TestNpmRemoteVersionReaper_Reap_IsIdempotent, TestNpmRemotePackageReaper_Reap_IsIdempotent (reaper half; the job's own success reporting is Step 16)
E-13 Purge job: the retention window changed between enqueue and run Steps 5 and 16. Not tested here
E-14 Sweep: the enumeration query fails and River retries Step 18. Not tested here
E-15 Sweep: the interval elapses before enumeration completes Step 18. Not tested here
E-16 Emit API unavailable at a purge or tombstone site Step 19. Not tested here
E-17 Parent gate: parent tombstoned between the gate and the child INSERT Step 4. Not tested here
E-18 A tombstoned artifact's coordinate is reoccupied before the reap; both rows exist and the purger keys on the tombstone TestNpmRemoteVersionReaper_Reap_LeavesTheRefilledRowUntouched, which pins that unique_npm_remote_versions_ns_id_pkg_id_version admits the pair and that the reap takes only the marked row

Security considerations

# Concern Tests
S-1 Authorization is unchanged; the purger performs no authorization decision of its own Not asserted here and nothing to assert: neither reaper takes a caller identity, and its whole input is a TombstoneRow and a limit. The job's own absence of a request context is Step 16
S-2 destructive is a confirmation, not an authorization control Step 15. Not tested here
S-3 Deletion is irreversible in closed beta and the API says so Step 15 (OpenAPI). Not tested here
S-4 Input validation: no purger input is user-controlled; the purge job carries UUID identifiers TestNpmRemoteReapers_Reap_Guards pins that a zero-UUID namespace or row id is rejected before any statement reaches the handle, and that a well-formed call is admitted. The reap takes no name, version string or digest at all
S-5 No injection surface is added; all purger SQL is jet-built or parameterized Not asserted directly. The suites constrain behavior, not statement construction; the one raw statement this step drives is deleteUnreferencedNpmAttachmentStmt, already fully parameterized and covered in Step 8
S-6 Error messages must not become an existence oracle Step 15. Not tested here; the reap answers no caller
S-7 Purge failures are operationally visible; a purge that cannot complete must never report success TestNpmRemotePackageReaper_Reap_StallsWhileAVersionSurvives asserts the stall surfaces as an error rather than a silent skip. The gauge that makes it visible is Step 17b

e2e scenario catalogs

No scenario is added, and none is affected. The step type settles it. CLAUDE.md's Revisit the e2e scenario catalogs before marking behavior-changing work complete guardrail binds feat and fix work, and step 10 is chore. The diff adds no route, no handler and no request path, so no catalog scenario changes behavior. docs/testing/e2e/ holds README.md, docker.md, maven.md, npm.md and oci.md, and this merge request touches none of them.

One correction belongs here, because a reader who opens the plan finds the opposite claim. The plan's e2e scenario impact paragraph says that no npm or Maven catalog exists. That sentence was true when the plan merged and went stale a day later, when 571b4e55e added docs/testing/e2e/npm.md and docs/testing/e2e/maven.md. Step 10 did not falsify it, and CLAUDE.md's Step MRs do not edit the plan file, the Status table included rule leaves a docs(plans) merge request as the only destination for the correction.

Size

CLAUDE.md's Justify a diff past 500 reviewable LOC in the MR description guardrail applies. The diff against the merge base is 1,966 insertions and 20 deletions over eight files.

File group Paths Insertions Deletions
Non-test Go internal/datastore/lifecycle_reap_npm_remote.go (509), internal/datastore/lifecycle_reap_npm.go (14), internal/datastore/blob_storage_attachments.go (10), internal/datastore/npm_remote_eviction.go (4), internal/datastore/query_names.go (4) 541 18
Test Go internal/datastore/lifecycle_reap_npm_remote_integration_test.go (1,266), internal/datastore/lifecycle_reap_npm_remote_test.go (156) 1,422 1
Spec docs/specs/S20-a-lifecycle-closed-beta.md 3 1

Review round two added 279 of those insertions: two regression pins the hosted suite carries and this one had dropped, TestNpmRemoteReapers_StatementOrder and TestNpmRemotePackageReaper_Reap_IsStateBlindOnTheRowItIsHanded, with the statement-audit rig the first one needs, plus comment corrections in five paragraphs.

The plan forecasts about 200 source lines and about 520 test lines for step 10. Both halves run at more than twice their forecast. In absolute lines the test half carries about seven tenths of the overrun, 658 lines against 306. The total also passes the ~900 working ceiling the plan adopts for this workstream. The plan's Size forecast and the working ceiling section states that ceiling and asks a reviewer to read these steps against it.

Splitting does not help, and the plan already made the split its own seam analysis found. Its Seam interrogation outcomes section records that one reap step per format kind forecast 1200 to 1600 lines each. It records the reason for the smaller unit as well: hosted and remote share no table and no store. The outcome was six reap steps over the seam step 5 extracted, and this merge request is one of the six.

The two reapers inside it do not separate either. They compose the same helper over two tables, and one argument-guard table covers both. The package reap's foreign-key stall case also needs a surviving version under a marked package. Separating the tests from the code breaks the test-first authorship order and lands production code with no cover.

Merge order

!1705 (merged), step 8

While !1705 (merged) was open, this merge request targeted prozlach/s20a-lifecycle-closed-beta-step-8 rather than main. !1705 (merged) merged as d6a7a30b0, GitLab deleted its branch, and GitLab retargeted this merge request to main.

The branch carried !1705 (merged)'s pre-squash commits underneath its own, and a plain rebase onto main replays those commits against their own squash. The rebase used git rebase --onto main 4c2e9c631 instead, where 4c2e9c631 is the pre-squash step 8 tip of this branch, so only the step 10 commits replayed. One check came first: 4c2e9c631 and d6a7a30b0 have the same tree object, so main carries step 8 byte for byte. The rebase changed no file content, and every replayed commit kept its original tree object.

Five symbols this step names came from step 8, and main now carries all five: npmAttachmentHolder, npmAttachmentRef, deleteNpmAttachmentHolderBatch, BlobStorageAttachmentStore.releaseNpmAttachments and BlobStorageAttachmentStore.DeleteIfUnreferencedByNpm. The first four are compiled references, in the reap file and in its integration suite. DeleteIfUnreferencedByNpm is the guard behind the release, and this step names it in doc comments only. The required order was !1705 (merged) first, then this merge request, and the merge order followed it. If this merge request retargets to main before !1705 (merged) merges, main gains code that names symbols it does not carry.

Two step 8 files have a second author

internal/datastore/lifecycle_reap_npm.go and internal/datastore/blob_storage_attachments.go both belong to step 8, which merged as !1705 (merged). This merge request carries two comment-only hunks in the first and one in the second, under an override the operator granted for those hunks. The override was granted once for a single hunk and extended in review to cover the rest; it is not a standing licence to edit step 8 files. Each hunk corrects a claim that step 10's two new callers make stale, and none of them changes a statement. blob_storage_attachments.go also takes additive edits from step 12 in !1704 (merged), whose only hunk there ends well above the paragraph corrected here, so the two merge in any order.

!1702 (merged), step 9

The Reaper interface doc on main, in internal/datastore/lifecycle_scan.go, reads At most limit rows are deleted per call. Both reapers here document up to 2*limit, because Rows counts a freed attachment and the budget does not. The same wording stands in step 8's lifecycle_reap_npm.go, so this step adds no new deviation.

!1702 (merged) rewrites that interface doc, its bound included. If !1702 (merged) merges first, the contract text is its to state. If this merge request merges first, main keeps the current wording. Nothing in this diff changes on either order.

!1740 (merged), spec correction

!1740 (merged), on prozlach/s20a-spec-remote-tombstone-discovery, edits the same region of docs/specs/S20-a-lifecycle-closed-beta.md. It corrects the same table-header defect by a different route, in the lines that follow the table. It also rewrites both sections that the new spec sentence in this merge request links to. git merge-tree reports no conflict, and the two hunks do not overlap. If !1740 (merged) merges first, the spec then carries the renamed header and a paragraph that states why the old header was wrong. The result is redundant, not contradictory. The sentence this merge request adds is count-free, so it stays correct in either merge order. This note records that redundancy, and it names no required merge order.

Plan Status row 10

This merge request does not fill it, and the branch touches no plan file. CLAUDE.md's Step MRs do not edit the plan file, the Status table included rule routes the reference to a batch or standing docs(plans) merge request instead. That same rule defines an empty cell as not yet recorded rather than not started.

Files, against the plan's list

The plan's Files: entry for step 10 names two paths, and the branch carries eight. Every path is listed here in the shape the plan uses for step 2. A reviewer who diffs the change set against the plan can then tell an authorized addition from scope creep.

  • internal/datastore/lifecycle_reap_npm_remote.go (Create). Named by the plan.
  • internal/datastore/lifecycle_reap_npm_remote_integration_test.go (Create). Named by the plan.
  • internal/datastore/lifecycle_reap_npm_remote_test.go (Create). The argument-guard suite. Step 8 shipped the same companion, lifecycle_reap_npm_test.go, against a Tests: entry that also named only its integration file. The plan's Testing Strategy section names <x>_test.go as one leg of this package's three-way split.
  • internal/datastore/query_names.go (Modify, 4 lines). Four query-name constants, appended to the end of their existing npm_remote_* groups, which is the discipline the plan's Sibling arms in shared files section prescribes. internal/datastore/queries_test.go requires one constant per instrumented statement, each used at exactly one call site, so the four new statements cannot borrow step 8's names.
  • internal/datastore/lifecycle_reap_npm.go (Modify, comments only). The count claims described under Corrections carried below, plus the lock-order clause on deleteNpmAttachmentHolderBatch, under the override the operator granted for those hunks.
  • internal/datastore/blob_storage_attachments.go (Modify, comments only). The bound paragraph on releaseNpmAttachments named two of its five callers and one of the two package-arm indexes. Step 10 adds two of the callers it omits, so the correction lands here under the same override. Step 12's arm in !1704 (merged) is additive and sits well above this paragraph.
  • internal/datastore/npm_remote_eviction.go (Modify, comments only). This is a review correction, and it is the seventh path in the change set. The doc comment on NpmRemoteEvictor.EvictNpmRemotePackage said that the package's versions, files and npm_remote_metadata_files rows stay live for the purger's subtree reap. NpmRemotePackageReaper, which this merge request adds, takes the npm_remote_metadata_files rows with the package row and takes no version. It aborts with SQLSTATE 23503 while one version survives. The two doc comments therefore stated opposite contracts, and this hunk corrects the evictor's half. No open merge request touches this file.
  • docs/specs/S20-a-lifecycle-closed-beta.md (Modify, 3 insertions and 1 deletion). The header of the table What a tombstone scan cannot reach, plus two prose lines above it. The old header read Reachable by tombstone scan, which is not what the column enumerates. A table is scan-reachable only when it carries soft_deleted_at and a tombstone-discovery index. The column lists the tables that carry the column, whether or not an index reaches them. The new header states that column-presence predicate, and the two prose lines name the difference. Those lines point at internal/datastore/migrations/structure.sql for the discovery indexes, and at the inventory for the column list only. No row cell moved, and no format's reap routing is settled here.

Corrections carried

Step 10 adds two callers to deleteNpmAttachmentHolderBatch, so two count claims in internal/datastore/lifecycle_reap_npm.go stop being true. Both are now count-free rather than raised from two to four. the two legs write two tables becomes each leg writes its own table, and the two bounded reap legs becomes every caller of deleteNpmAttachmentHolderBatch. A count-free claim stays true for step 11 and for every format after it, so it ends the class of defect rather than resetting it.

This merge request also corrects a conflation it did not introduce. The three tables count in internal/datastore/lifecycle_reap_npm.go tracked statements, not tables, and a76fe4918 wrote it. A bump from three to four makes the sentence accidentally right as a table count and leaves the statement arithmetic wrong. The go-jet reason needs no count at all, so the sentence now reads more than one table.

ADR-007 divergence, tracked in #739

Two facts this step's code states disagree with ADR-007 as published. Neither divergence is introduced here, and in both the code states the schema the tree ships.

  • ADR-007 defines an npm_remote_tags table. S15 dropped it. structure.sql has no such table, and internal/datastore/migrations/npm_remote_schema_integration_test.go pins the drop. The version reaper's doc gives that as the reason it carries no tag leg.
  • ADR-007 gives npm_remote_metadata_files.kind two values. The shipped check_npm_remote_metadata_files_kind admits three. The package reaper's limit + 3 bound rests on three, and the integration suite covers all three kinds as positive hits.

docs/specs/S15-npm-remote.md already lists both, in its own words as divergences pending an ADR-007 amendment. CLAUDE.md's Do not edit files in docs/adr/ directly guardrail means nothing inside this repository closes either one: the amendment goes to the handbook.

Both divergences are now tracked in #739, with the sites, the evidence, and the size of the amendment. That issue is where an amendment gets written; this merge request needs no change either way, and the code above is correct under the schema the tree ships.

Obligations recorded for plan step 15

The three obligations in this section have no change available in this merge request. Each one binds the step that wires the chunk driver, which is plan step 15. They are recorded here because merged policy forbids a step merge request from editing the plan file. CLAUDE.md states that rule as Step MRs do not edit the plan file, the Status table included, and a docs(plans) merge request is the destination that carries them into the plan.

  • The reap metrics belong to the driver. The chunk driver owns the rows-deleted signal, the per-component signal and the per-format error signal. metrics.InstrumentQuery carries no error dimension today. A chunk that fails and a chunk that succeeds therefore produce the same series. The driver also needs an npm_remote_packages level, which is what makes a stalled package reap visible.
  • One deadlock note expires when the driver lands. internal/datastore/npm_remote_metadata_files.go states that only one resource is locked today, so nothing can deadlock yet. It adds that a flow that needs both resources takes the package first. That first claim reads false once both reapers have a caller. The file is outside this change set, so the correction belongs to the step that adds the caller.
  • The driver's page belongs far below the ceiling. validateNpmRemoteReapArgs refuses a limit above MaxLifecycleReapPageSize, so the high end is bounded rather than left to the caller. That constant is a refusal boundary and not a suggested page size. releaseNpmAttachments issues one DeleteIfUnreferencedByNpm per attachment freed, and deleteNpmAttachmentHolderBatch renders LIMIT(int64(budget)). Both the row count and the statement count in one transaction still scale with what the driver passes, up to that ceiling. The spec's chosen option names the chunk bound as the mitigation for the accepted lock-order inversion. The driver's own page size is therefore what that design rests on.

Related issue

This merge request carries no Related to line, and the step type is the reason. Step 10 is chore, which CLAUDE.md's Link a related issue on every feature MR guardrail exempts alongside the other maintenance types.

Work item #507 is not the link. It covers the hosted npm whole-package unpublish, in npm_metadata_files and npm_package_unpublish_deleter.go. That is step 8's subject, and !1705 (merged) carries the link to it. This step reaps the npm remote cache instead.

Database Review Evidence

Note

Collected with the db-review-prep skill against 9ea6229af, which was the merge base on prozlach/s20a-lifecycle-closed-beta-step-8 while this merge request targeted that branch. The evidence predates the rebase onto main, and nothing in the rebase or in the later guard commit changes the four statements it covers. Two of the four are built by a step 8 helper, and each block states its own provenance. Migration mode did not run: the change set adds no file under internal/datastore/migrations/sql/ and no edit to internal/datastore/migrations/structure.sql, against the step 8 base and against main.

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17.10 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), with synthesized seed data rolled back per query and the container torn down at the end of the run. Numbers reflect moderate cardinality and do not capture production-scale effects. See Database review evidence for seed sizing, methodology, and the anomalies the skill flags. Expand each row's details for the seed shape, rendered SQL, bound args, and raw plan.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.NpmRemoteVersionReaper.Reap Delete → Index Scan npm_remote_versions_pNN_pkey 1 / 1 8.30 6.334ms 11 / 0 1/64
datastore.NpmRemotePackageReaper.Reap Delete → Index Scan npm_remote_packages_pNN_pkey 1 / 1 8.30 11.256ms 17 / 0 1/64
datastore.npmRemoteVersionFilesHolder.deleteNpmAttachmentHolderBatch Delete → Hash Semi Join npm_remote_files_pNN_namespace_id_npm_remote_version_id_idx 100 / 100 224.82 5.698ms 296 / 0 1/64
datastore.npmRemotePackageMetadataFilesHolder.deleteNpmAttachmentHolderBatch Delete → Nested Loop npm_remote_metadata_files_pNN_namespace_id_npm_remote_packa_idx 3 / 3 36.85 0.778ms 20 / 0 1/64

Three readings of that table need a word each.

  • The index names carry pNN here and a number in the raw plans. Every seed transaction generates a fresh namespace_id, so the hash routes each run to a different partition of the same 64. The index is the partition's copy of the parent index, pk_npm_remote_versions, pk_npm_remote_packages, index_npm_remote_files_on_ns_id_ver_id, and unique_npm_remote_metadata_files_ns_id_pkg_id_kind.
  • Rows for the first two statements come from the driving Index Scan. A DELETE with no RETURNING reports rows=0 at its root node, so the root count says nothing about what matched. The two holder legs do carry RETURNING, and their root counts are the ones in the table.
  • Time is the plan's Execution Time footer. For the first two statements that number is dominated by a one-off referential-integrity plan compilation, which the first query note covers.

Every one of the four tables is hash-partitioned on namespace_id with a modulus of 64, and every statement binds namespace_id, so all four prune to one partition. No statement in this change set shows partition fan-out.

Query notes:

  • datastore.npmRemoteVersionFilesHolder.deleteNpmAttachmentHolderBatch: Seq Scan over the 5000-row partition, under a Hash Semi Join against the page of 100 ids. This is a cost crossover at the skill's seed size and not a missing index. A control run with 100,000 rows in the same partition, seeded identically in every other respect, moves the planner to Nested Loop over npm_remote_files_pNN_pkey, and execution decreases from 5.698ms to 0.717ms. Both plans are in the details block below. The inner page select uses index_npm_remote_files_on_ns_id_ver_id at both sizes.
  • datastore.NpmRemoteVersionReaper.Reap and datastore.NpmRemotePackageReaper.Reap: each parent delete pays a referential-integrity check against every child table that names it, and each check plans across that child's 64 partitions. The version delete fires one check, on npm_remote_files. The package delete fires two, on npm_remote_versions and npm_remote_metadata_files. The first call in a backend pays the plan compilation: 5.904ms for the version delete and 6.757ms plus 4.122ms for the package delete, all against empty child sets. The plan is then cached per backend, and repeat calls measured in the same transaction cost 0.401ms and 0.231ms. A chunk driver that reaps many subtrees on one pooled connection pays this once, not once per row.
  • No statement returns rows whose count grows with the table. The two parent deletes match at most one row on a primary key. Both holder legs are bounded by the caller's LIMIT $4, which deleteNpmAttachmentHolderBatch binds from the reap's remaining budget, and both reapers reject a limit below one before any statement runs.

Two schema facts the reap logic rests on were read back from the migrated container rather than taken from the code.

  • npm_remote_metadata_files carries no soft_deleted_at column. index_npm_remote_versions_on_ns_id_soft_deleted_at is the npm remote family's only WHERE soft_deleted_at IS NOT NULL index. Walking the parent package is therefore the only route to the cache rows, which is what the package reaper does.
  • check_npm_remote_metadata_files_kind admits 0, 1, and 2, and unique_npm_remote_metadata_files_ns_id_pkg_id_kind admits one row per kind. Three rows per package is the ceiling, so the limit + 3 bound in NpmRemotePackageReaper.Reap's doc holds.
datastore.NpmRemoteVersionReaper.Reap

Summary: The plan matches the method's intent. The bound namespace_id prunes to one of 64 partitions and the composite primary key answers the row lookup, with estimate and actual both at one row. No anomalies in the statement itself. The Trigger for constraint line is the foreign key from npm_remote_files, and the second query note above gives its cost profile.

Seed shape: namespaces=1, repositories=1, npm_remote_repositories=1, npm_remote_packages=1, npm_remote_versions=5000

Rendered SQL:

DELETE FROM public.npm_remote_versions
WHERE (npm_remote_versions.namespace_id = $1::uuid) AND (npm_remote_versions.id = $2::uuid);

Bound args: [123f0aa6-5844-48bd-9181-d04f10ee424a, db20e6da-bfe6-4e33-93cc-3f74847b0500]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_remote_versions  (cost=0.28..8.30 rows=0 width=0) (actual time=0.096..0.097 rows=0 loops=1)
   Delete on npm_remote_versions_p13 npm_remote_versions_1
   Buffers: shared hit=11
   ->  Index Scan using npm_remote_versions_p13_pkey on npm_remote_versions_p13 npm_remote_versions_1  (cost=0.28..8.30 rows=1 width=10) (actual time=0.025..0.026 rows=1 loops=1)
         Index Cond: ((id = 'db20e6da-bfe6-4e33-93cc-3f74847b0500'::uuid) AND (namespace_id = '123f0aa6-5844-48bd-9181-d04f10ee424a'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=190
 Planning Time: 1.444 ms
 Trigger for constraint npm_remote_files_npm_remote_version_id_namespace_id_fkey13 on npm_remote_versions_p13: time=5.904 calls=1
 Execution Time: 6.334 ms

Repeat-call control (same transaction, three tombstoned versions deleted in sequence, showing the referential-integrity plan cache):

--- call 1 ---
 Planning Time: 1.535 ms
 Trigger for constraint npm_remote_files_npm_remote_version_id_namespace_id_fkey49 on npm_remote_versions_p49: time=8.170 calls=1
 Execution Time: 8.652 ms
--- call 2 ---
 Planning Time: 0.207 ms
 Trigger for constraint npm_remote_files_npm_remote_version_id_namespace_id_fkey49 on npm_remote_versions_p49: time=0.401 calls=1
 Execution Time: 0.490 ms
--- call 3 ---
 Planning Time: 0.127 ms
 Trigger for constraint npm_remote_files_npm_remote_version_id_namespace_id_fkey49 on npm_remote_versions_p49: time=0.231 calls=1
 Execution Time: 0.283 ms

Timings: planning 1.444ms, execution 6.334ms, total 7.778ms.

datastore.NpmRemotePackageReaper.Reap

Summary: The plan matches the method's intent, and it is the same shape as the version delete. The bound namespace_id prunes to one of 64 partitions and the composite primary key answers the row lookup. The two Trigger for constraint lines are the foreign keys from npm_remote_versions and npm_remote_metadata_files, both empty for this seed. Those two keys are what turns a package delete run before its children are gone into SQLSTATE 23503, which the method's doc describes as the stall.

Seed shape: namespaces=1, repositories=1, npm_remote_repositories=1, npm_remote_packages=5000

Rendered SQL:

DELETE FROM public.npm_remote_packages
WHERE (npm_remote_packages.namespace_id = $1::uuid) AND (npm_remote_packages.id = $2::uuid);

Bound args: [d220f6b4-bd4f-4116-a2d8-2f74e1f24239, a4a33138-df29-4407-b068-05c85ea9291b]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_remote_packages  (cost=0.28..8.30 rows=0 width=0) (actual time=0.109..0.109 rows=0 loops=1)
   Delete on npm_remote_packages_p60 npm_remote_packages_1
   Buffers: shared hit=17
   ->  Index Scan using npm_remote_packages_p60_pkey on npm_remote_packages_p60 npm_remote_packages_1  (cost=0.28..8.30 rows=1 width=10) (actual time=0.017..0.018 rows=1 loops=1)
         Index Cond: ((id = 'a4a33138-df29-4407-b068-05c85ea9291b'::uuid) AND (namespace_id = 'd220f6b4-bd4f-4116-a2d8-2f74e1f24239'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=116
 Planning Time: 0.918 ms
 Trigger for constraint npm_remote_versions_npm_remote_package_id_namespace_id_fkey60 on npm_remote_packages_p60: time=6.757 calls=1
 Trigger for constraint npm_remote_metadata_files_npm_remote_package_id_namespac_fkey60 on npm_remote_packages_p60: time=4.122 calls=1
 Execution Time: 11.256 ms

Timings: planning 0.918ms, execution 11.256ms, total 12.174ms.

datastore.npmRemoteVersionFilesHolder.deleteNpmAttachmentHolderBatch

Summary: The inner page select does what the method intends at both seed sizes. It prunes to one partition and reads exactly LIMIT $4 ids from index_npm_remote_files_on_ns_id_ver_id, and the target version holds 200 files against a budget of 100, so the page bound is what stops it. The outer delete's join method moves with table size, and the 5000-row plan's Seq Scan is the planner picking a cheaper path over a small partition rather than a missing index. The 100,000-row control below settles that: the same statement moves to Nested Loop over the primary key and execution decreases from 5.698ms to 0.717ms.

The statement is built by deleteNpmAttachmentHolderBatch, which lands in !1705 (merged). The holder it is built from, npmRemoteVersionFilesHolder, and its query name, npm_remote_files_delete_reap_batch, land here.

Seed shape (primary): namespaces=1, repositories=1, npm_remote_repositories=1, npm_remote_packages=1, blob_storage_blobs=1, blob_storage_attachments=1, npm_remote_versions=97, npm_remote_files=5000 (200 under the reaped version, 4800 under 96 live siblings)

Seed shape (control): the same, with npm_remote_versions=1997 and npm_remote_files=100000 (200 under the reaped version, 99,800 under 1996 live siblings)

Rendered SQL:

DELETE FROM public.npm_remote_files
WHERE (npm_remote_files.namespace_id = $1::uuid) AND (npm_remote_files.id IN ((
           SELECT npm_remote_files.id AS "npm_remote_files.id"
           FROM public.npm_remote_files
           WHERE (npm_remote_files.namespace_id = $2::uuid) AND (npm_remote_files.npm_remote_version_id = $3::uuid)
           LIMIT $4
      )))
RETURNING npm_remote_files.blob_storage_attachment_id AS "npm_reaped_attachment.attachment_id",
          npm_remote_files.blob_sha256 AS "npm_reaped_attachment.sha256";

Bound args: [0e02b473-b2c9-45f5-b260-27c33ff4522a, 0e02b473-b2c9-45f5-b260-27c33ff4522a, 7e6348b5-42b7-4440-bee1-283c99a4008b, 100]

Plan (EXPLAIN (ANALYZE, BUFFERS) output, 5000 rows in the partition):

 Delete on npm_remote_files  (cost=56.08..224.82 rows=100 width=50) (actual time=0.893..2.953 rows=100 loops=1)
   Delete on npm_remote_files_p20 npm_remote_files_1
   Buffers: shared hit=296
   ->  Hash Semi Join  (cost=56.08..224.82 rows=100 width=50) (actual time=0.380..2.309 rows=100 loops=1)
         Hash Cond: (npm_remote_files_1.id = "ANY_subquery"."npm_remote_files.id")
         Buffers: shared hit=96
         ->  Seq Scan on npm_remote_files_p20 npm_remote_files_1  (cost=0.00..154.50 rows=5000 width=26) (actual time=0.025..1.512 rows=5000 loops=1)
               Filter: (namespace_id = '0e02b473-b2c9-45f5-b260-27c33ff4522a'::uuid)
               Buffers: shared hit=92
         ->  Hash  (cost=54.83..54.83 rows=100 width=56) (actual time=0.342..0.345 rows=100 loops=1)
               Buckets: 1024  Batches: 1  Memory Usage: 17kB
               Buffers: shared hit=4
               ->  Subquery Scan on "ANY_subquery"  (cost=6.33..54.83 rows=100 width=56) (actual time=0.084..0.292 rows=100 loops=1)
                     Buffers: shared hit=4
                     ->  Limit  (cost=6.33..53.83 rows=100 width=16) (actual time=0.072..0.250 rows=100 loops=1)
                           Buffers: shared hit=4
                           ->  Bitmap Heap Scan on npm_remote_files_p20 npm_remote_files_2  (cost=6.33..101.33 rows=200 width=16) (actual time=0.070..0.084 rows=100 loops=1)
                                 Recheck Cond: ((namespace_id = '0e02b473-b2c9-45f5-b260-27c33ff4522a'::uuid) AND (npm_remote_version_id = '7e6348b5-42b7-4440-bee1-283c99a4008b'::uuid))
                                 Heap Blocks: exact=2
                                 Buffers: shared hit=4
                                 ->  Bitmap Index Scan on npm_remote_files_p20_namespace_id_npm_remote_version_id_idx  (cost=0.00..6.28 rows=200 width=0) (actual time=0.062..0.063 rows=200 loops=1)
                                       Index Cond: ((namespace_id = '0e02b473-b2c9-45f5-b260-27c33ff4522a'::uuid) AND (npm_remote_version_id = '7e6348b5-42b7-4440-bee1-283c99a4008b'::uuid))
                                       Buffers: shared hit=2
 Planning:
   Buffers: shared hit=49 read=1
 Planning Time: 3.148 ms
 Execution Time: 5.698 ms

Control plan (EXPLAIN (ANALYZE, BUFFERS) output, 100,000 rows in the partition):

 Delete on npm_remote_files  (cost=175.65..988.98 rows=100 width=50) (actual time=0.102..0.394 rows=100 loops=1)
   Delete on npm_remote_files_p04 npm_remote_files_1
   Buffers: shared hit=604
   ->  Nested Loop  (cost=175.65..988.98 rows=100 width=50) (actual time=0.090..0.341 rows=100 loops=1)
         Buffers: shared hit=404
         ->  HashAggregate  (cost=175.23..176.23 rows=100 width=56) (actual time=0.071..0.079 rows=100 loops=1)
               Group Key: "ANY_subquery"."npm_remote_files.id"
               Batches: 1  Memory Usage: 32kB
               Buffers: shared hit=4
               ->  Subquery Scan on "ANY_subquery"  (cost=0.29..174.98 rows=100 width=56) (actual time=0.026..0.049 rows=100 loops=1)
                     Buffers: shared hit=4
                     ->  Limit  (cost=0.29..173.98 rows=100 width=16) (actual time=0.016..0.030 rows=100 loops=1)
                           Buffers: shared hit=4
                           ->  Index Scan using npm_remote_files_p04_namespace_id_npm_remote_version_id_idx on npm_remote_files_p04 npm_remote_files_2  (cost=0.29..318.14 rows=183 width=16) (actual time=0.015..0.025 rows=100 loops=1)
                                 Index Cond: ((namespace_id = '11c664ee-5b98-412a-a050-fd7e7e3be254'::uuid) AND (npm_remote_version_id = '6dd7966f-398d-4a15-b07f-4c7b5d393cd4'::uuid))
                                 Buffers: shared hit=4
         ->  Index Scan using npm_remote_files_p04_pkey on npm_remote_files_p04 npm_remote_files_1  (cost=0.42..8.12 rows=1 width=26) (actual time=0.002..0.002 rows=1 loops=100)
               Index Cond: ((id = "ANY_subquery"."npm_remote_files.id") AND (namespace_id = '11c664ee-5b98-412a-a050-fd7e7e3be254'::uuid))
               Buffers: shared hit=400
 Planning:
   Buffers: shared hit=50
 Planning Time: 0.687 ms
 Execution Time: 0.717 ms

Timings: planning 3.148ms, execution 5.698ms, total 8.846ms. Control: planning 0.687ms, execution 0.717ms, total 1.404ms.

datastore.npmRemotePackageMetadataFilesHolder.deleteNpmAttachmentHolderBatch

Summary: The plan matches the method's intent. The inner page select prunes to one partition and reads the package's three cache rows from the (namespace_id, npm_remote_package_id, kind) prefix of unique_npm_remote_metadata_files_ns_id_pkg_id_kind, and the outer delete drives a Nested Loop over the primary key. The page never fills here: the unique index caps a package at three cache rows, which is below any budget the guard admits. That is why this leg keeps the index path at 5000 rows where the files leg does not.

The statement is built by deleteNpmAttachmentHolderBatch, which lands in !1705 (merged). The holder it is built from, npmRemotePackageMetadataFilesHolder, and its query name, npm_remote_metadata_files_delete_reap_batch, land here.

Seed shape: namespaces=1, repositories=1, npm_remote_repositories=1, blob_storage_blobs=1, blob_storage_attachments=1, npm_remote_packages=1666, npm_remote_metadata_files=4998 (3 under the reaped package, 4995 under 1665 live siblings)

Rendered SQL:

DELETE FROM public.npm_remote_metadata_files
WHERE (npm_remote_metadata_files.namespace_id = $1::uuid) AND (npm_remote_metadata_files.id IN ((
           SELECT npm_remote_metadata_files.id AS "npm_remote_metadata_files.id"
           FROM public.npm_remote_metadata_files
           WHERE (npm_remote_metadata_files.namespace_id = $2::uuid) AND (npm_remote_metadata_files.npm_remote_package_id = $3::uuid)
           LIMIT $4
      )))
RETURNING npm_remote_metadata_files.blob_storage_attachment_id AS "npm_reaped_attachment.attachment_id",
          npm_remote_metadata_files.blob_sha256 AS "npm_reaped_attachment.sha256";

Bound args: [eabf33c6-cecb-4874-aea7-2560ab430fe0, eabf33c6-cecb-4874-aea7-2560ab430fe0, 4eeeb29f-a064-46af-a2db-f1d21b6452d5, 100]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_remote_metadata_files  (cost=12.16..36.85 rows=3 width=50) (actual time=0.101..0.145 rows=3 loops=1)
   Delete on npm_remote_metadata_files_p58 npm_remote_metadata_files_1
   Buffers: shared hit=20
   ->  Nested Loop  (cost=12.16..36.85 rows=3 width=50) (actual time=0.082..0.116 rows=3 loops=1)
         Buffers: shared hit=14
         ->  HashAggregate  (cost=11.88..11.91 rows=3 width=56) (actual time=0.053..0.055 rows=3 loops=1)
               Group Key: "ANY_subquery"."npm_remote_metadata_files.id"
               Batches: 1  Memory Usage: 24kB
               Buffers: shared hit=5
               ->  Subquery Scan on "ANY_subquery"  (cost=0.28..11.87 rows=3 width=56) (actual time=0.042..0.048 rows=3 loops=1)
                     Buffers: shared hit=5
                     ->  Limit  (cost=0.28..11.84 rows=3 width=16) (actual time=0.031..0.036 rows=3 loops=1)
                           Buffers: shared hit=5
                           ->  Index Scan using npm_remote_metadata_files_p58_namespace_id_npm_remote_packa_idx on npm_remote_metadata_files_p58 npm_remote_metadata_files_2  (cost=0.28..11.84 rows=3 width=16) (actual time=0.030..0.034 rows=3 loops=1)
                                 Index Cond: ((namespace_id = 'eabf33c6-cecb-4874-aea7-2560ab430fe0'::uuid) AND (npm_remote_package_id = '4eeeb29f-a064-46af-a2db-f1d21b6452d5'::uuid))
                                 Buffers: shared hit=5
         ->  Index Scan using npm_remote_metadata_files_p58_pkey on npm_remote_metadata_files_p58 npm_remote_metadata_files_1  (cost=0.28..8.30 rows=1 width=26) (actual time=0.018..0.018 rows=1 loops=3)
               Index Cond: ((id = "ANY_subquery"."npm_remote_metadata_files.id") AND (namespace_id = 'eabf33c6-cecb-4874-aea7-2560ab430fe0'::uuid))
               Buffers: shared hit=9
 Planning:
   Buffers: shared hit=52
 Planning Time: 1.695 ms
 Execution Time: 0.778 ms

Timings: planning 1.695ms, execution 0.778ms, total 2.473ms.

What this evidence does not cover

BlobStorageAttachmentStore.releaseNpmAttachments and the DeleteIfUnreferencedByNpm statement behind it run in the same transaction and the same chunk as the four statements above. Both land in !1705 (merged), and neither is in this change set, so neither is planned here. A reviewer reading the chunk end to end needs !1705 (merged)'s own evidence for that leg.

Review decisions recorded on this merge request

Round 2 of the review raised two findings that this merge request does not act on. Both would reverse a documentation change this branch already landed, and both stay as written.

  • The lock-order paragraph at internal/datastore/lifecycle_reap_npm_remote.go:163-169 stays as f2f16e6b9 wrote it. The finding argues that no production path hands a fill a tombstoned parent id, which reverses that commit's recorded position.
  • The account of namespace_id in the two parent DELETEs, at internal/datastore/lifecycle_reap_npm_remote.go:227-229 and :394-395, stays as 25f3df1bc wrote it. The finding argues that namespace_id is half the row's identity rather than a pruning hint, which reverses that commit's recorded position.

This is a bot message 🤖 — /smurfit

Edited by Pawel Rozlach

Merge request reports