fix(datastore): npm hosted reap and the unpublish attachment fix (S20-A plan: 8/21)

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

What this delivers

Three things, all inside step 8's scope in the plan.

  • Reap logic for the five hosted npm tables and their attachments. NpmVersionReaper and NpmPackageReaper land in a new internal/datastore/lifecycle_reap_npm.go and implement the merged Reaper interface.
  • An npm-family attachment guard, BlobStorageAttachmentStore.DeleteIfUnreferencedByNpm, over the four npm tables that reference blob_storage_attachments.
  • The orphaned-attachment fix. NpmMetadataFileStore.DeleteNpmMetadataFiles now frees the attachment of each row its own DELETE ... RETURNING removed.

Before the fix, a whole-package unpublish left up to three blob_storage_attachments rows behind, one per packument-cache kind. No row referenced them and nothing ever found them. Reclamation reads blob rows, and the attachment row is what holds a blob's reference count above zero. So each stranded row kept its blob out of reach of every later pass.

Why this opens as fix and not chore

The plan types step 8 chore. The diff changes behavior at whole-package unpublish, where those three attachment rows now go with their cache rows. The operator ruled that the diff governs the type, not the plan. The work item #507 carries type::bug and bug::functional, so the work item agrees with the ruling, and the fix title needs no second argument.

Two live call sites, not one

The plan's Scope: line names one call site for the orphaned-attachment fix, the whole-package unpublish path. The change reaches two.

  1. Whole-package unpublish, through NpmPackageUnpublishDeleter.
  2. The management-API package delete. NpmPackageManagementDeleter.removeCacheRows now frees its cache attachments through DeleteNpmMetadataFiles. It previously ran DeleteIfUnreferenced over all ten referencing tables, where the npm-family guard covers four. That path is wired in production at cmd/artifact-registry/wire_management.go:123 and :454, and the bulk npm delete worker holds the same deleter. The guard narrowing on this second path is a behavior change that the singular Scope: line does not name.

Both live paths ran end to end against a booted service. Each package held three packument-cache attachment rows before its delete and none after. Nothing client-observable moved: unpublish answers 200 {"ok":true}, the packument then answers 404, and the management delete answers 202. Both paths end in identical row state.

The metrics confirm that the redundant guard calls are gone, by measurement rather than by reading the diff. After two package deletes, gitlab_artifact_registry_database_queries_total{name="blob_storage_attachments_delete_if_unreferenced_by_npm"} carried 6 observations. The shared blob_storage_attachments_delete_if_unreferenced series carried none. Six is three attachments across two deletes, so the new guard fires once per freed row on both paths.

The reap half is not reachable in production. Nothing constructs either reaper outside tests, and no worker kind or periodic kind exists to enqueue one. Plan step 15 wires the purge worker and the chunk driver, and that is the first caller.

The dead guard loop, and the proof that it is dead

removeCacheRows used to read its cache rows FOR UPDATE, call DeleteNpmMetadataFiles, then call DeleteIfUnreferenced once per row. This merge request removes the read, the loop, and the orphaned queryNpmMetadataFilesSelectLockCacheRows constant. The wrapper stays for its doc.

The loop is dead by set inclusion rather than by caller tracing, which is what makes the removal a proof and not a guess. deleteUnreferencedNpmAttachmentStmt carries four NOT EXISTS arms: npm_files, npm_metadata_files, npm_remote_files and npm_remote_metadata_files. deleteUnreferencedAttachmentStmt carries ten: those four plus container_blobs, container_manifests, container_remote_blobs, container_remote_manifests, maven_files and maven_remote_files. The four are a strict subset of the ten, enumerated rather than asserted. So one of two things happens on every state a writer can produce. Either the npm guard removed the row and the loop matches nothing. Or the npm guard refused because an npm table still references the row, and the superset guard refuses too.

A third outcome exists outside that pair, and it needs a state no writer produces: a non-npm holder of the same attachment id. All four NOT EXISTS arms pass and the DELETE reaches the database. PostgreSQL's no-action referential check over the other six referencing tables then raises 23503, and the caller's transaction aborts. DeleteIfUnreferenced returns 0 on that same state and lets the transaction commit. On the two live paths that is a 500 on whole-package unpublish and on the management package delete, in a state the previous code tolerated. Nothing reaches it today, and the reason is the same one the set-inclusion proof rests on: Create is a bare INSERT ... RETURNING with no ON CONFLICT, so every artifact writer mints an attachment of its own and no id is shared across formats. It also fails closed rather than losing data. DeleteIfUnreferencedByNpm's doc is hedged for exactly this, and this description was not. The doc claims that the two guards agree on every state a writer can produce. Two paragraphs later it states that a caller that lets two rows share one id must still handle 23503.

The loop is dead today. It becomes a narrow second-chance free under a future writer that shares one attachment id across two npm rows. No production writer does that.

The removed FOR UPDATE read took no lock that anything depends on. Three checks a reviewer can repeat:

  • The read sat at the third rung of the lock order documented at internal/datastore/npm_metadata_files.go:578-584.
  • DeleteNpmMetadataFiles runs a DELETE with the identical WHERE on namespace_id and npm_package_id, so it takes exclusive locks on the same rows at the same point. Only an error check sat between the read and the store call.
  • The two tests that pin locking on this path pin npm_packages instead: TestNpmPackageRowLockStmt and TestNpmPackageManagementDeleter_DeletePackage_LocksThePackageRowFirst.

The failure mode those checks guard against is an intermittent deadlock under concurrent unpublish, not a red test. This path already carries two documented inversions, tracked as #490.

NpmPackageManagementDeleter.attachments and its constructor line stay, although no code reads the field once the loop is gone. unused does not fire on it, because the type is exported. A linter run over the package confirms that, untagged and with the integration tags. The operator ruled that the field stays, and the field's own doc comment now carries that record. A later cleanup pass that reads the package doc therefore finds that record. It does not take the file's silence as proof that the field is dead.

npm_packages.versions_count

ADR-007 puts this counter's decrement at exactly the event the reap performs. The counter includes the package's soft-deleted versions, so an unpublish leaves it where it is, and only a hard delete moves it. The reap now issues that decrement.

The chunk that deletes an npm_versions row decrements its parent package by one, through NpmPackageStore.DecrementNpmPackageVersionsCount on the handle the reap was passed. That handle is the chunk driver's transaction, so the pair is exactly once per row actually deleted. A rolled-back chunk takes the decrement back with the delete, and the retry that at-least-once delivery guarantees deletes and decrements once. On the store's own pool the decrement stands while the delete rolls back, and the retry then decrements a second time. That drift is permanent, because no recompute exists for this column.

This gives DecrementNpmPackageVersionsCount the caller the note that kept it caller-less asked for. That note was on #549 (closed), which closed as Complete on 2026-08-19. The purger-side obligation it left behind moved to #686. Whether the per-row decrement here discharges #686 is that issue's call rather than this description's. One in-tree pointer still names the closed issue, and this branch does not touch it. NpmPackageTagsCountMaintainer's doc in internal/format/npm/unpublish_version.go cites #549 (closed) for the version-cap mirror. origin/main rewrote that paragraph after this branch's merge base, so the correction to #686 belongs to the rebase and not to a hunk here. Three checks prove that the widening breaks no compile. git grep finds no production call site. No interface in the tree names the method. And go vet with the integration tags exits 0, which compiles both fakes.

Three tests cover the new behavior: TestNpmVersionReaper_Reap_DecrementsThePackageVersionsCount, TestNpmVersionReaper_Reap_ClampsThePackageVersionsCountAtZero, and TestNpmVersionReaper_Reap_LeavesThePackageVersionsCountOnARolledBackChunk.

Accepted smell, named here rather than left for each reviewer to rediscover. Two functions now take a qrm.DB and trade a compile-time transaction guarantee for a documented one. DecrementNpmPackageVersionsCount and decrementNpmPackageColumn previously always ran on the store's pool. The two alternatives were a runtime type assertion on the handle and a duplicate of the statement sequence inside the reap. The widening moves toward ADR-007's same-transaction rule, because the unwidened function cannot run inside the chunk transaction at all.

internal/managementapi/package_resources.go is deliberately unchanged, and the disclosure runs the other way from what a reviewer expects. Its doc says that the count "can sit permanently above the number of versions the version list returns, until a hard-delete removes those rows". That until clause was false before this change, because no hard delete moved the counter. It is true after it, so this fix repaired a merged user-facing API doc without editing the file.

ReapTotals.SizeBytes reports zero

Both hosted npm reaps report zero for SizeBytes, and that is this arm's contract rather than an omission.

The merged ReapTotals doc routes the field to repositories.size_bytes, which ADR-007 deduplicates within the repository. npm_versions.size_bytes deduplicates within the version instead. A digest that a reaped version shares with a live version is still attached in the repository. The reaped row's own column therefore decrements a counter that must not move at all. A version-scoped column cannot produce a repository-deduplicated delta. Reporting nothing is better than reporting a number in the wrong scope, and all four reap arms report zero.

The repository-scoped delta is derivable, and this is recorded so step 19's owner does not rediscover it as unsolvable. The repository is reachable from the rows the reap deletes, through npm_files.npm_version_id, then npm_versions.npm_package_id, then npm_packages.npm_repository_id, then npm_repositories.repository_id. So the derivation needs no change to the merged TombstoneRow and no change to the Reaper contract. S20-A step 9 records the same design for its Maven half, and files it under #611. This merge request files no second item.

Step 19 cannot compute the number from where it sits. docs/dev/storage-accounting.md:119 puts the delta on the format layer, because blob_storage_attachments carries no repository_id. Step 19's own acceptance test passes under either scope. One version pushed into an empty repository has one digest, so its version-scoped number and its repository-scoped number are equal.

The lock order the reapers run on

This merge request introduces both reapers, so it introduces their lock order. internal/datastore/lifecycle_reap_npm.go does not exist at the merge base. No path on main reaches npm_packages for the first time after npm_versions, npm_files, npm_tags or blob_storage_attachments. Every npm write path on main takes npm_packages first, through RotatePackumentRebuildTokenTx as its first mutating statement.

Two inversions against the order documented at internal/datastore/npm_metadata_files.go:578-589, and both are named in the code:

  1. npm_packages last. NpmVersionReaper.Reap acquires npm_files, then blob_storage_attachments, then npm_tags, then npm_versions, then npm_packages. NpmPackageReaper.Reap is out of order on both of its legs too.
  2. blob_storage_attachments ahead of npm_tags and npm_versions. The documented order takes attachments last of all.

PostgreSQL picks the deadlock victim, and it is not obliged to pick the reap. A chunk that loses aborts with 40P01 and rolls back whole, delete and decrement together, and the retry redoes both. A counterparty that loses is covered by nothing here, and one route to that is constructible. The reap deletes npm_files rows regardless of their own soft_deleted_at, while CascadeSoftDeleteNpmPackage's legs carry soft_deleted_at IS NULL progress predicates. A cascade that holds npm_packages from its first statement therefore wants the active child rows a reap holds, and the reap wants npm_packages. Both cascades run inline in the request's own transaction with no 40P01 retry wrapper, so a cascade that loses surfaces as a 500.

Neither the code nor this description characterizes the attachments leg in either direction. Whether a shared attachment closes a cycle with the packument rebuild is new analysis that this merge request does not do.

RotatePackumentRebuildTokenTx's own routing sentence tracks this class at #490. It forbids a fix by re-ordering one call site in isolation, and the code cites it as the class's tracker. The reap is not on that issue yet. The hazard is latent rather than shipping, because the reap has no production caller until plan step 15 wires the chunk driver.

Files outside the plan's Files: list

Twelve files, and the reasons differ per file. They are not one carve-out.

File Why it is here
internal/datastore/query_names.go Additive query-name declarations. The operator authorized this file for the wave, on the precedent merged step 5 set by adding names to it.
internal/datastore/queries_test.go One line registering DeleteIfUnreferencedByNpm in the pinned rawSQLTimedFunctions fixture. Forced rather than chosen. TestEveryStatementIsInstrumented gates on any call to a statement verb, and this method calls db.ExecContext directly. instrumentQuery and instrumentExec both take a jet statement that hand-written raw SQL cannot give them. The fixture fails in both directions, so an unpinned raw-SQL function and a pinned function that runs no statement both go red. Step 5's precedent does not cover this file, because its squash never touched it. A citation of that precedent here is the inverted borrow the repository rules name.
internal/datastore/npm_package_management_deleter.go The dead guard loop above, plus the two comments in this file that the same change falsifies. It is a shipping management-API path.
internal/datastore/npm_write_token_rotation_integration_test.go One comment that the change falsifies. It said that the cascade performs no attachment delete of its own.
internal/datastore/npm_packages.go Three reasons. execAffected's doc described the cascade legs only, and the reap adds two callers with a different purpose. DecrementNpmPackageVersionsCount takes the caller's handle now. And the argument guards behind both decrement wrappers moved into npmPackageDecrementGuard, so the wrapper that reads the store's pool can run them before it does.
internal/format/npm/unpublish_version.go One comment in a second package that the change falsifies. NpmPackageTagsCountMaintainer's doc said that the lifecycle purger owns the decrement once its reap lands. The reap lands here and owns it.
internal/datastore/lifecycle_reap_npm_test.go (new file) A new untagged suite for the reapers' argument guards, TestNpmReapers_Reap_Guards. The existing reap suite is //go:build integration and holds no error assertion at all. The merged sibling TestLifecycleScanStore_ScanTombstonedRepositories_Guards uses a zero-value store and needs no database.
internal/datastore/blob_storage_attachments_stmt_test.go The parser helpers that pin the shared guard's shape now run over the npm guard, for the four properties no behavioral test reaches. Each table is enumerated once, and the outer DELETE carries the partition key. Every clause joins the WHERE with AND, and each clause correlates on both ref.namespace_id and ref.blob_storage_attachment_id. The last two are behaviorally invisible because every npm case seeds one namespace. The shared statement's remaining two text pins need no copy, and the const's own doc now names what covers each instead.
internal/datastore/blob_storage_attachments_refcheck_integration_test.go One line. attachmentGuardedTables takes a statement parameter for the reuse above, and this file holds its other call site.
internal/datastore/blob_storage_attachments_test.go DeleteIfUnreferencedByNpm's three argument-guard arms had no test. This file holds the Create guard table they mirror, so it is where the package already covers that shape.
internal/datastore/npm_packages_test.go Three cases: the by-N tags_count decrement's guards on a client-less store, its zero-count no-op, and the shared decrement body's nil-handle arm. The first two pin that the guards run before the store's pool is read. This order is a property of the exported type's zero value rather than of today's wiring.
internal/format/npm/unpublish_version_test.go Two fakes declared DecrementNpmPackageVersionsCount with the pre-widening signature and still compiled, because the seam interface does not declare that method at all. This merge request widens both, so each recorder keeps the shape of the method a handler must dispatch to.

Spec coverage

Purger and discovery:

# Criterion Tests
AC-P1 Tombstoned repositories row discovered, fresh one not Step 5 (discovery scan). Not reachable from a reaper
AC-P2 A row with soft_deleted_at IS NULL is never discovered by any scan or walk Root half: Steps 5-7 (scans) and Step 14 (walk root). Reaper half pinned here as its complement: TestNpmVersionReaper_Reap_IsStateBlindOnTheRowItIsHanded
AC-P3 EXPLAIN on the repositories, npm_packages, container_images scans Steps 5 and 7
AC-P4 EXPLAIN on the five version-level scans Step 6
AC-P5 Re-run on a reaped subtree is a no-op that reports success. An interrupted walk leaves fewer rows No-op half: TestNpmVersionReaper_Reap_IsIdempotent, TestNpmPackageReaper_Reap_IsIdempotent. gofail abort half: Step 14
AC-P6 Repository walk reaps live and already-tombstoned rows in one pass Step 14. Its reaper-side precondition is pinned here: TestNpmVersionReaper_Reap_IsStateBlindOnTheRowItIsHanded
AC-P7 Every purge transaction is bounded TestNpmVersionReaper_Reap_BoundsEachChunk, TestNpmPackageReaper_Reap_BoundsEachChunk
AC-P8 Frozen namespace refuses retryably Step 15a

Per-format reap logic:

# Criterion Tests
AC-R1 Tombstoned npm version reaped with its files and each file's attachment. A shared attachment survives, through npm's own guard TestNpmVersionReaper_Reap_RemovesTheVersionSubtree, TestNpmVersionReaper_Reap_LeavesAnAttachmentAnotherNpmRowHolds, TestBlobStorageAttachmentStore_DeleteIfUnreferencedByNpm_EveryNpmReferencingTable, TestBlobStorageAttachmentStore_DeleteIfUnreferencedByNpm_MarkedRowStillCounts, TestBlobStorageAttachmentStore_DeleteIfUnreferencedByNpm_AnotherAttachmentDoesNotHold, TestNpmAttachmentReferenceCases_CoverEveryNpmReferencingTable
AC-R2 Republish before reap: tombstoned row and its blob references go, live row untouched TestNpmVersionReaper_Reap_LeavesTheRepublishedRowUntouched
AC-R3 Package tombstoned by the single-version path reaped with tags, metadata rows and their attachments, without a 23503 TestNpmPackageReaper_Reap_RemovesAPackageTombstonedBySingleVersionUnpublish. Partial: the npm_tags clause has no reachable state, see Gaps
AC-R4 DeleteNpmMetadataFiles frees each row's attachment, at the unpublish call site and in the purger Call site: TestUnpublishPackageIntegration_CacheAttachmentsFreed. Purger: TestNpmPackageReaper_Reap_RemovesAPackageTombstonedBySingleVersionUnpublish
AC-R5 Maven version and package reap Step 9
AC-R6 Container repository reaped through the existing per-artifact deleters Step 14
AC-R7 Image index and its children reaped parent-first Step 13
AC-R8 Tombstoned container_images row under a live repository Step 13
AC-R9 Container repository walk reaps live and tombstoned images Step 14
AC-R10 Remote-cache subtree reaped per table Steps 10-12
AC-R11 container_remote_manifests and container_remote_blobs reaped Step 12
AC-R12 After a reap every blob has zero attachment rows, counted per sha256 TestNpmVersionReaper_Reap_RemovesTheVersionSubtree, TestNpmVersionReaper_Reap_BoundsEachChunk, TestNpmPackageReaper_Reap_RemovesAPackageTombstonedBySingleVersionUnpublish, TestUnpublishPackageIntegration_CacheAttachmentsFreed

Repository entry point (11), Sweep (8), Tombstone visibility (10), Accounting call sites (5), and Schema and configuration (4) are not reachable from this step. They belong to Steps 18, 16, 3, 19, and 1-2. The single-version-path test covers error case E-9 (23503 on a parent delete). It asserts that the collateral is cleared, so the violation never fires. If the violation does fire, requireNoFKViolation names the constraint. The job-level abort-and-requeue is Step 15.

Fifteen tests joined the suite after that table was written, and the table does not list them. Three cover the versions_count decrement and are named above. Four pin the npm guard statement's shape: TestDeleteUnreferencedNpmAttachmentStmt_EnumeratesEachTableOnce, _OuterDeleteCarriesThePartitionKey, _CorrelatesOnNamespaceAndAttachmentID and _EveryClauseJoinsWithAnd. Four cover argument guards this arm introduced: TestBlobStorageAttachmentStore_DeleteIfUnreferencedByNpm_ArgumentGuards and _NilDB, plus TestNpmPackageStore_DecrementNpmPackageTagsCountBy_ArgumentGuards and _ZeroCountWritesNothing. The twelfth, TestNpmPackageStore_DecrementNpmPackageVersionsCount_NilDB, covers the shared decrement body's nil-handle arm, which nothing exercised before. The thirteenth, TestNpmPackageReaper_Reap_IsStateBlindOnTheRowItIsHanded, covers both marker states at package level as positive hits. It was added in review, because NpmPackageReaper.Reap's doc claims state-blindness by pointing at the version reaper and nothing checked it at this level. Both of its subtests pass unchanged, so it is a regression pin rather than a fix: every leg keys on row.ID and not on the marker. The fourteenth and fifteenth cover the parent-delete refusal: TestNpmPackageReaper_Reap_RefusesWhileAVersionSurvives and TestNpmPackageReaper_Reap_RefusesWhenASiblingChunkShortensThePage. The reapers' own argument-guard suite TestNpmReapers_Reap_Guards is new as well.

Declared gaps

The AC-R3 row above points at this list, so it travels with the table.

  • The same-name package coordinate collision has no test, at package level, in any reap arm. A republish after whole-package unpublish leaves a marked row and a live row at one (namespace_id, npm_repository_id, name), which unique_npm_packages_ns_id_repo_id_name permits because it is partial on soft_deleted_at IS NULL. The version-level equivalent is pinned by TestNpmVersionReaper_Reap_LeavesTheRepublishedRowUntouched. Raised in review and deferred to #746, which carries why: no acceptance criterion asks for it, no reap arm has the case at package level, and the fixture needs a variant rather than a rename. Every leg keys on the row's UUID, so the reap is correct in that state while that holds.
  • AC-R3's npm_tags clause at package level has no reachable state. fk_npm_tags_npm_version_id_npm_versions ties a tag's lifetime to its version's, and a package reap runs after its versions are reaped. Both unpublish verbs also clear tags at write time. Covered where reachable, at version level. The package test pins the resulting zero count. Plan-vs-tree observation, not a blocker. Both arms' docs now carry the counter half of the same gap. ADR-007 makes tags_count a buffered count of a package's npm_tags rows, and it accepts a small over-cap at the buffer boundary because the cap is a product limit. What has no backstop is a delta never emitted, which #632 records as a permanent drift rather than a bounded lag. Neither leg settles the column, and #686 carries the obligation for the caller that will. Each doc gives why neither leg owes it today, and which caller takes it on once one exists.
  • "npm's own guard, not DeleteIfUnreferenced" is not observable from row state, by design. An attachment id is never shared across formats, so the two guards agree on every reachable state. The tests assert that agreement directly.
  • npm_remote_files and npm_remote_metadata_files arms are covered through shared fixtures. The reap that produces those rows is Step 10.

The 23503 parent-delete stall now carries a typed signal

Earlier rounds of this merge request declined a family sentinel for the stall and recorded the absence instead. That answer rested on both sibling reaps declining one, and S20-A step 9 inverted it: f79239807 merged ErrReapParentPinned and mapReapParentDeleteError, and rewrote the Reaper contract to oblige a caller to "re-queue a parent delete refused with ErrReapParentPinned". This branch is rebased onto that merge, so the obligation was unsatisfiable against the npm arms and npm was the one reap family whose refusal a chunk driver could not classify.

Both npm parent deletes now route through mapReapParentDeleteError. Two subtests cover the refusal, mirroring the merged Maven arm rather than adding one case: a surviving version pins the package permanently, and a sibling chunk that commits while this page waits on its locks shortens the page and pins it transiently. Both reach the caller as the same sentinel, which is the contract, and Reaper's caller obligations carry what separates them.

They assert the sentinel and not the SQLSTATE, and no constraint name. The child tables are hash-partitioned, so PostgreSQL reports the declared name on PG 18 and the auto-generated per-partition name on PG 16 and 17, and .gitlab-ci.yml runs test:integration on all three.

requireNoFKViolation gains a sentinel arm, and both arms are load-bearing. mapReapParentDeleteError returns ErrReapParentPinned bare, with no *pgconn.PgError inside it, so the existing errors.As arm cannot see a parent-delete stall at all, while the child legs still wrap the driver error. This was measured rather than reasoned about: with the sentinel arm the helper names the stall, and with that arm removed the same state reports only the generic "the reap chunk must succeed".

40P01 still has no sentinel anywhere in internal/, so errors.As for a *pgconn.PgError stays in the contract for the deadlock case. Step 17a still owns the purge-outcome counter that separates "nothing left to reap" from a foreign-key stall and from an ordering refusal.

The second gap is about what a test can distinguish, and not about whether the criterion holds. The criterion holds by construction and a reader can check it: DeleteNpmMetadataFiles and both reapers call DeleteIfUnreferencedByNpm, and the call sites answer to grep.

e2e scenario impact

Guardrail 12 binds, because this opens as fix. No file under docs/testing/ changes, and the evidence follows rather than an intuition.

The plan's own e2e paragraph is stale, and this description does not repeat it. Line 964 of the plan says that no npm or Maven catalog exists, and that docs/testing/e2e/ holds only README.md, docker.md and oci.md. origin/main holds five files there: README.md, docker.md, maven.md, npm.md and oci.md. npm.md already carries whole-package unpublish scenarios. So "no catalog to update" is not available as the answer, and the answer is a split by reachability.

  • The #507 fix ships today and moves nothing a scenario asserts. The change sits below the protocol, in which rows the transaction removes. No status code, no response body, and no counter moves, and a live exercise of both paths measured that on the wire.
  • The reap has no production caller until step 15, so no scenario can exercise it.

docs/testing/e2e/npm.md:129 carries the other scenario a guardrail-12 check reaches for, e2e.npm.lifecycle.unpublish-package-counters, whose text reads "versions_count counts soft-deleted versions per ADR-007 and only a hard delete lowers it". That is the exact property this change makes reachable in code, and the scenario stays true unedited. deleteNpmVersionRow is the only hard delete of npm_versions in the tree, and nothing in production constructs a reaper. Nothing lowers the counter, so the sentence asserts what it always did. A scenario that exercises the counter falling needs a production caller for the reap, which is what wiring the purge worker supplies.

docs/testing/e2e/npm.md:127 says that blob storage is reclaimed by garbage collection after the grace period. That sentence is forward-looking for every blob today, and this fix does not change its present truth. PgBlobStore.DeleteBlob at internal/storage/pg_blobstore.go:340 is the only code that deletes blob_storage_blobs rows, and it is wired in production through storage.NewBlobStoreStack. No production caller invokes it. internal/format/oci/store.go:285-290 records that intent explicitly: it hard-deletes the container_blobs row and its attachment row, and it leaves the CAS row in place for later reclamation. So this fix removes a blocker for packument-cache blobs rather than making a false expectation true.

Reviewable size

Guardrail 18 asks for a split or a justification past 500 reviewable lines. Measured against the merge base 76316b30, the diff is 18 files, +2911 and -169.

Group Files +/-
New reap logic lifecycle_reap_npm.go +738 / -0
npm attachment guard blob_storage_attachments.go +181 / -0
The fix and its two call sites npm_metadata_files.go, npm_package_unpublish_deleter.go, npm_package_management_deleter.go +108 / -95
The versions_count decrement npm_packages.go, internal/format/npm/unpublish_version.go +102 / -42
Query names query_names.go +10 / -4
Tests ten files +1772 / -28

Non-test code is +1139 and -141 of that total. Tests are +1772 of the +2911 insertions, so they are about three fifths of what a reviewer reads.

A split does not help here. The plan defines step 8 as one step, and both halves of it free attachments through the same new guard and the same statement. A split puts the guard in one merge request with no caller, and the caller in the other with no guard. The one clean cut is the test suite, and a step MR without its tests is what guardrail 6 forbids.

The versions_count increment rode along, and this says why rather than leaving it to be inferred from "added after the first pass". That increment is npm_packages.go and internal/format/npm/unpublish_version.go (+102 / -42) plus its three reap tests and the guard cases in npm_packages_test.go (+107 / -0). It is separable in the mechanical sense, because it depends on the reaper existing rather than the reverse. It is not separable in three others. ADR-007 puts this counter's decrement at exactly the hard delete this arm introduces. deleteNpmVersionRow is the only hard delete of npm_versions in the tree. A merge request that lands the reaper without the decrement therefore lands the first code that removes the rows the counter counts. It leaves the counter where ADR-007 says it must not stay. The decrement runs on the chunk's own transaction, and that transaction does not exist until the reaper does. A follow-up merge request therefore widens a signature for a caller that merged one merge request earlier. And its three tests reap a real subtree and re-read the counter, so a follow-up must duplicate this arm's fixtures rather than reuse them.

The plan's own forecast for step 8 was wrong, and this states the correction rather than substituting it silently. The plan forecast about 900 lines for this step, against a declared working ceiling of about 900. The delivered diff is +2911. Part of the difference is work added after the first pass. That work is the versions_count decrement with its three tests, plus the four statement-shape tests. It also includes the argument-guard cases on the three methods this arm adds or rewires, and the new reaper argument-guard suite.

Merge order and coordination

Each note below is written so that it stays true on both sides of the merge it names.

Base and target. This branch targets main. Step 5, its dependency, merged as !1651, so there is no stack to retarget.

The query-name ceiling. internal/metrics/cardinality.go caps labelName at 350, and TestNameBudget_CoversEveryDeclaredQueryName fails the moment the two catalogs declare more than that. S20-A step 6 raises the cap to 400, and it is the only open merge request that raises it.

The counts, re-measured with grep -cE '^\s+query[A-Za-z0-9_]*\s+=\s+"' rather than carried over from when this merge request opened. At the merge base 76316b30: 303 names in internal/datastore/query_names.go and 12 in internal/storage/queries.go, so 315. On this branch: 309 and 12, so 321, because the arm adds seven names and removes one. This section previously gave 304 and 316 for the base, and those do not reconcile with that +7 -1. A reader who adds the delta to 316 lands on 322.

The exposure is not confined to the first two of steps 8, 9 and 12 to merge. origin/main moves under this branch, so each figure here is a measurement with a date on it. At a16f895dc on 2026-08-19 it declared 309 + 12 = 321. A sweep of the open merge requests that day counted 41 further names across twelve of them, this branch's net +6 among them. That total is over the cap of 350 before the last of those merge requests lands. The rule outlives those numbers, and it is the part to read. While step 6 is unmerged, whichever merge request takes the running total past 350 is the one whose pipeline goes red. Every merge request after it stays red until step 6 lands. Rebased onto origin/main as measured, this branch alone declares 315 + 12 = 327.

The blob_storage_attachments const block. This step, step 9 and step 12 each add one long constant to that block. gofmt then re-aligns the whole block to a different width in each. The planned merge order was 8, then 9, then 12, and step 9 merged first instead, as f79239807. This branch is rebased onto that merge, so it takes main's realignment rather than producing its own, and its own hunk is the one added constant. Step 12 still rebases deliberately rather than mechanically, so a reviewer who sees a realigned block reads it as coordinated and not accidental. This replaces the promise of a mechanical rebase in the plan's ### Sibling arms in shared files section, and that plan text travels in a separate merge request. Step 12 also rewrites 13 entries in queries_test.go, and this step's one-line addition falls inside that rewritten hunk.

The merged Reaper doc. The interface once said "At most limit rows are deleted per call", and this arm's 2*limit bound contradicted it. S20-A step 9 owned the interface text and rewrote it, and that merged as f79239807. The obligation now puts the ceiling where this arm's doc and its tests already put it: "Two conditions put one call's ceiling at 2*limit rows". This branch is rebased onto that merge, so the interface text, this arm's doc and the integration assertion agree, and the deferral this section once recorded is discharged.

The reap page ceiling. The guard clamps limit at both ends, refusing anything above MaxLifecycleReapPageSize. No handler fronts a reap, both reapers and their constructors are exported, and Reap is on the exported Reaper interface, so this guard is the only bound these calls have. docs/dev/database-query-patterns.md states the rule: a caller-fed LIMIT $n with an uncapped n is not bounded.

The ceiling is the reap-side one rather than the scan's. S20-A step 9 merged MaxLifecycleReapPageSize as the constant every reap arm clamps against, and this arm reads it. MaxLifecycleScanPageSize bounds a discovery scan's page instead, which is one read-path slice, where this limit also drives one guarded DELETE per freed attachment. The two carry the same value today, so no bound a caller sees moved, and a reap that read the scan constant would still be reading the bound of a different operation.

The plan's Status row

This merge request fills step 8's row in the plan's Status table, at docs/plans/2026-08-11-s20a-lifecycle-closed-beta.md.

Guardrail 4 asks for that row, and the plan reads it twice. Step 10 and step 14 both name step 8 in their Depends on line. An empty cell left a reader of either line with no merge request to reach.

An earlier revision of this section deferred the row to a separate batching merge request. That merge request does not exist for this plan. Across this plan and the S22 plan, 17 step merge requests have merged and 16 filled their own row.

Rows 8 and 9 are the only adjacent pair in this wave, and step 9's merge request fills row 9 in the same round. Whichever of the two lands second rebases through a one-line hunk and keeps both rows.

Database Review Evidence

Migration mode did not run. The branch changes no file under internal/datastore/migrations/sql/, so there is no apply or rollback timing to collect.

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17 container (postgres:17-alpine, server 17.10, matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), migrated to the branch head with goose, 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.BlobStorageAttachmentStore.DeleteIfUnreferencedByNpm Delete → 4× Nested Loop Anti Join index_blob_storage_attachments_on_namespace_id_and_sha256, plus index_npm_files_on_ns_id_bsa_id, index_npm_metadata_files_on_ns_id_bsa_id, index_npm_remote_files_on_ns_id_bsa_id, index_npm_remote_metadata_files_on_ns_id_bsa_id 1 / 1 41.55 0.297 ms 57 / 4 1/64 each for blob_storage_attachments, npm_files, npm_metadata_files, npm_remote_files, npm_remote_metadata_files
datastore.deleteNpmVersionRow Delete → Index Scan pk_npm_versions 1 / 1 8.30 0.122 ms 18 / 0 1/64 npm_versions
datastore.NpmMetadataFileStore.DeleteNpmMetadataFiles Delete → Index Scan unique_npm_metadata_files_ns_id_pkg_id_kind 3 / 3 11.84 0.036 ms 11 / 0 1/64 npm_metadata_files
datastore.NpmPackageReaper.Reap Delete → Index Scan pk_npm_packages 1 / 1 8.30 0.230 ms 23 / 0 1/64 npm_packages
datastore.reapNpmAttachmentHolder.PackageMetadataFiles Delete → Nested Loop unique_npm_metadata_files_ns_id_pkg_id_kind (batch), pk_npm_metadata_files (delete) 3 / 3 36.85 0.050 ms 20 / 0 1/64 npm_metadata_files
datastore.reapNpmAttachmentHolder.VersionFiles Delete → Hash Semi Join index_npm_files_on_ns_id_ver_id (batch), none on the delete side 100 / 100 271.99 0.653 ms 384 / 0 1/64 npm_files
datastore.reapNpmTags.ByPackage Delete → Hash Semi Join unique_npm_tags_ns_id_pkg_id_name (batch), none on the delete side 100 / 100 237.48 0.615 ms 176 / 0 1/64 npm_tags
datastore.reapNpmTags.ByVersion Delete → Hash Semi Join index_npm_tags_on_ns_id_version_id (batch), none on the delete side 100 / 100 233.48 0.748 ms 176 / 0 1/64 npm_tags

Every statement prunes to one partition of every table it touches. There is no partition fan-out to report.

Two changed methods carry no new statement and are therefore not in the table:

  • NpmPackageStore.DecrementNpmPackageVersionsCount and NpmPackageStore.decrementNpmPackageColumn: the jet chain is byte-identical to the merge base. The change is the qrm.DB handle the statement runs on, which no plan can show.
  • NpmPackageManagementDeleter.removeCacheRows: the diff deletes a statement (queryNpmMetadataFilesSelectLockCacheRows) and adds none.

Query notes:

  • NpmPackageReaper.Reap: the npm_packages delete fires three referential-integrity checks, and the one against npm_versions has no usable index. Every npm_versions index whose leading columns are (namespace_id, npm_package_id) is partial on soft_deleted_at IS NULL (unique_npm_versions_ns_id_pkg_id_version, index_npm_versions_on_ns_id_pkg_id_created_at_id, index_npm_versions_on_ns_id_pkg_id_last_downloaded_at, index_npm_versions_on_ns_id_pkg_id_size_bytes), and the check carries no such predicate, so PostgreSQL sequentially scans the whole npm_versions partition once per package deleted. The scan reads every heap page of the partition, and a partition holds every namespace whose namespace_id hashes to it. One partition therefore holds roughly 1/64 of all npm_versions rows in the deployment, not one tenant's rows. Measured at 5000 versions in the partition: Seq Scan, Rows Removed by Filter: 5000, 72 buffers, 0.19 ms. That filter count reads as one namespace only because the fixture seeds one namespace. The cost grows with the partition's row count, not with the package's. Reap issues exactly one DELETE FROM npm_packages per call, so the scan is paid once per package reaped. The chunk limit gates whether that delete runs at all, through the budget < 1 early return, and it bounds the rows in the legs above it. The limit does not bound how many parent deletes a sweep performs. The same probe over npm_tags and npm_metadata_files uses unique_npm_tags_ns_id_pkg_id_name and unique_npm_metadata_files_ns_id_pkg_id_kind and reads 2 buffers each, because neither index is partial. Issue #684 tracks this defect class and its remedy, from the same shape measured on maven_files. The npm instance is recorded there. This is a pre-existing index shape rather than a regression; what is new is that this MR adds the first hard delete of an npm_packages row, which is what reaches it.
  • reapNpmAttachmentHolder.VersionFiles, reapNpmTags.ByVersion and reapNpmTags.ByPackage: the delete side of the batch statement is a Seq Scan over the 5000 seeded rows, joined to the batch ids by a hash semi join. This is a small-table cost artifact, not a missing index. A scale probe over 50000 npm_files rows in one partition, same statement and same limit of 100, flips the plan to Nested Loop with Index Scan using npm_files_p00_pkey, and execution falls from 0.913 ms to 0.597 ms. The PackageMetadataFiles leg already takes the index path at 5000 rows, because its batch returns 3 ids rather than 100. The plan therefore follows the ratio of the batch size to the partition size, and it moves the right way as the partition grows.
  • BlobStorageAttachmentStore.DeleteIfUnreferencedByNpm: removing the row fires ten referential-integrity checks, one per table with a foreign key into blob_storage_attachments — the four npm tables plus container_blobs, container_manifests, maven_files, maven_remote_files, container_remote_manifests and container_remote_blobs. The narrowing to four NOT EXISTS arms bounds the guard's own query, and it does not bound these checks, which is what the method's doc already states. Cost per call: 15.4 ms for the first delete in a session (9.6 ms planning plus 12.2 ms of trigger plan setup), then 1.4 ms with 1.2 ms of it in the ten triggers. A chunk that frees a full page of attachments pays that warm figure per attachment.
  • No unbounded SELECT, no sort above an index scan, no plan-versus-actual divergence past 10×, and no buffer read pattern suggesting a degraded index lookup.
datastore.BlobStorageAttachmentStore.DeleteIfUnreferencedByNpm

Summary: The plan matches the method's intent. Each of the four NOT EXISTS arms becomes an anti join driven by that table's (namespace_id, blob_storage_attachment_id) index, and each prunes to one partition of 64 — the correlated ref.namespace_id = bsa.namespace_id reaches the planner's equivalence class, so the bound namespace_id propagates into every arm. The outer delete prunes blob_storage_attachments on sha256. The ten referential-integrity triggers the removal fires are almost the whole of the statement's execution time; see the query notes above.

Seed shape: namespaces=1, repositories=2, npm_repositories=1, npm_remote_repositories=1, blob_storage_blobs=5000, blob_storage_attachments=5000, npm_packages=1668, npm_versions=1668, npm_files=5004, npm_metadata_files=5004, npm_remote_packages=1668, npm_remote_versions=1668, npm_remote_files=5004, npm_remote_metadata_files=5004

The 5000 attachments are filtered with satisfies_hash_partition('blob_storage_attachments', 64, 0, …) so they all land in one hash partition, and the bound sha256 comes from that seeded set. The target attachment is the one no npm row references; the other 4999 are referenced from all four npm tables.

Rendered SQL (hand-written constant deleteUnreferencedNpmAttachmentStmt, not a jet chain):

DELETE FROM blob_storage_attachments bsa
	WHERE bsa.namespace_id = $1 AND bsa.id = $2 AND bsa.sha256 = $3
	AND NOT EXISTS (
		SELECT 1 FROM npm_files ref
		WHERE ref.namespace_id = bsa.namespace_id AND ref.blob_storage_attachment_id = bsa.id
	)
	AND NOT EXISTS (
		SELECT 1 FROM npm_metadata_files ref
		WHERE ref.namespace_id = bsa.namespace_id AND ref.blob_storage_attachment_id = bsa.id
	)
	AND NOT EXISTS (
		SELECT 1 FROM npm_remote_files ref
		WHERE ref.namespace_id = bsa.namespace_id AND ref.blob_storage_attachment_id = bsa.id
	)
	AND NOT EXISTS (
		SELECT 1 FROM npm_remote_metadata_files ref
		WHERE ref.namespace_id = bsa.namespace_id AND ref.blob_storage_attachment_id = bsa.id
	)

Bound args: [eab1b478-a34a-4d0d-b795-1031f3bf3ad6, 25013, \x000000000000000000000000000000000000000000000000000000000000002c]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on blob_storage_attachments bsa  (cost=1.41..41.55 rows=0 width=0) (actual time=0.296..0.297 rows=0 loops=1)
   Delete on blob_storage_attachments_p00 bsa_1
   Buffers: shared hit=57 read=4
   ->  Nested Loop Anti Join  (cost=1.41..41.55 rows=1 width=50) (actual time=0.060..0.062 rows=1 loops=1)
         Buffers: shared hit=11
         ->  Nested Loop Anti Join  (cost=1.13..33.24 rows=1 width=64) (actual time=0.051..0.053 rows=1 loops=1)
               Buffers: shared hit=9
               ->  Nested Loop Anti Join  (cost=0.85..24.93 rows=1 width=54) (actual time=0.042..0.044 rows=1 loops=1)
                     Buffers: shared hit=7
                     ->  Nested Loop Anti Join  (cost=0.56..16.62 rows=1 width=44) (actual time=0.026..0.027 rows=1 loops=1)
                           Buffers: shared hit=5
                           ->  Index Scan using blob_storage_attachments_p00_namespace_id_sha256_idx on blob_storage_attachments_p00 bsa_1  (cost=0.28..8.30 rows=1 width=34) (actual time=0.014..0.015 rows=1 loops=1)
                                 Index Cond: ((namespace_id = 'eab1b478-a34a-4d0d-b795-1031f3bf3ad6'::uuid) AND (sha256 = '\x000000000000000000000000000000000000000000000000000000000000002c'::bytea))
                                 Filter: (id = '25013'::bigint)
                                 Buffers: shared hit=3
                           ->  Index Scan using npm_files_p49_namespace_id_blob_storage_attachment_id_idx on npm_files_p49 ref  (cost=0.28..8.30 rows=1 width=34) (actual time=0.011..0.011 rows=0 loops=1)
                                 Index Cond: ((namespace_id = 'eab1b478-a34a-4d0d-b795-1031f3bf3ad6'::uuid) AND (blob_storage_attachment_id = '25013'::bigint))
                                 Buffers: shared hit=2
                     ->  Index Scan using npm_metadata_files_p49_namespace_id_blob_storage_attachment_idx on npm_metadata_files_p49 ref_1  (cost=0.28..8.30 rows=1 width=34) (actual time=0.016..0.016 rows=0 loops=1)
                           Index Cond: ((namespace_id = 'eab1b478-a34a-4d0d-b795-1031f3bf3ad6'::uuid) AND (blob_storage_attachment_id = '25013'::bigint))
                           Buffers: shared hit=2
               ->  Index Scan using npm_remote_files_p49_namespace_id_blob_storage_attachment_i_idx on npm_remote_files_p49 ref_2  (cost=0.28..8.30 rows=1 width=34) (actual time=0.009..0.009 rows=0 loops=1)
                     Index Cond: ((namespace_id = 'eab1b478-a34a-4d0d-b795-1031f3bf3ad6'::uuid) AND (blob_storage_attachment_id = '25013'::bigint))
                     Buffers: shared hit=2
         ->  Index Scan using npm_remote_metadata_files_p49_namespace_id_blob_storage_att_idx on npm_remote_metadata_files_p49 ref_3  (cost=0.28..8.30 rows=1 width=34) (actual time=0.008..0.008 rows=0 loops=1)
               Index Cond: ((namespace_id = 'eab1b478-a34a-4d0d-b795-1031f3bf3ad6'::uuid) AND (blob_storage_attachment_id = '25013'::bigint))
               Buffers: shared hit=2
 Planning:
   Buffers: shared hit=28
 Planning Time: 0.494 ms
 Trigger for constraint container_blobs_blob_storage_attachment_id_namespace_id_bl_fkey on blob_storage_attachments_p00: time=2.242 calls=1
 Trigger for constraint container_manifests_blob_storage_attachment_id_namespace_i_fkey on blob_storage_attachments_p00: time=2.745 calls=1
 Trigger for constraint npm_files_blob_storage_attachment_id_namespace_id_blob_sha_fkey on blob_storage_attachments_p00: time=0.200 calls=1
 Trigger for constraint npm_metadata_files_blob_storage_attachment_id_namespace_id_fkey on blob_storage_attachments_p00: time=0.173 calls=1
 Trigger for constraint maven_files_blob_storage_attachment_id_namespace_id_blob_s_fkey on blob_storage_attachments_p00: time=2.160 calls=1
 Trigger for constraint npm_remote_metadata_files_blob_storage_attachment_id_names_fkey on blob_storage_attachments_p00: time=0.239 calls=1
 Trigger for constraint npm_remote_files_blob_storage_attachment_id_namespace_id_b_fkey on blob_storage_attachments_p00: time=0.199 calls=1
 Trigger for constraint maven_remote_files_blob_storage_attachment_id_namespace_id_fkey on blob_storage_attachments_p00: time=3.839 calls=1
 Trigger for constraint container_remote_manifests_blob_storage_attachment_id_name_fkey on blob_storage_attachments_p00: time=3.532 calls=1
 Trigger for constraint container_remote_blobs_blob_storage_attachment_id_namespac_fkey on blob_storage_attachments_p00: time=2.591 calls=1
 Execution Time: 18.520 ms

Timings: planning 0.494 ms, execution 18.520 ms, total 19.014 ms. Of the execution, 17.9 ms is the ten referential-integrity triggers on their first call in the session. A second and third delete in the same session cost 1.536 ms and 1.384 ms, with 1.2 ms of each in the same ten triggers.

datastore.deleteNpmVersionRow

Summary: The plan matches the method's intent: a single-row Index Scan on the primary key, pruned to one partition of 64, with the RETURNING projection adding no extra scan. The delete then pays two referential-integrity checks, against npm_files and npm_tags, and each has a non-partial index on (namespace_id, npm_version_id) to serve it. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=50, npm_versions=5000

Rendered SQL:

DELETE FROM public.npm_versions
WHERE (npm_versions.namespace_id = $1::uuid) AND (npm_versions.id = $2::uuid)
RETURNING npm_versions.npm_package_id AS "npm_versions.npm_package_id";

Bound args: [df7eda03-6c62-4736-939c-379a47bea9ac, 001ea0e0-80b7-47af-8306-1f8215d0f0c8]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_versions  (cost=0.28..8.30 rows=1 width=10) (actual time=0.121..0.122 rows=1 loops=1)
   Delete on npm_versions_p50 npm_versions_1
   Buffers: shared hit=18
   ->  Index Scan using npm_versions_p50_pkey on npm_versions_p50 npm_versions_1  (cost=0.28..8.30 rows=1 width=10) (actual time=0.024..0.025 rows=1 loops=1)
         Index Cond: ((id = '001ea0e0-80b7-47af-8306-1f8215d0f0c8'::uuid) AND (namespace_id = 'df7eda03-6c62-4736-939c-379a47bea9ac'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=1
 Planning Time: 0.181 ms
 Trigger for constraint npm_files_npm_version_id_namespace_id_fkey50 on npm_versions_p50: time=3.899 calls=1
 Trigger for constraint npm_tags_npm_version_id_namespace_id_fkey50 on npm_versions_p50: time=2.961 calls=1
 Execution Time: 7.333 ms

Timings: planning 0.181 ms, execution 7.333 ms, total 7.514 ms. The two trigger figures are first-call plan setup in this session, the same effect measured for the attachment delete above.

datastore.NpmMetadataFileStore.DeleteNpmMetadataFiles

Summary: The plan matches the method's intent: one Index Scan on unique_npm_metadata_files_ns_id_pkg_id_kind, pruned to one partition of 64, reaching the package's three cache rows. The RETURNING this change adds costs no extra node — the attachment coordinates come off rows the delete already visits, where the shape it replaces read them with a separate locking SELECT. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=1668, blob_storage_blobs=5004, blob_storage_attachments=5004, npm_metadata_files=5004 (3 rows for the target package, which is the kind domain's maximum)

Rendered SQL:

DELETE FROM public.npm_metadata_files
WHERE (npm_metadata_files.namespace_id = $1::uuid) AND (npm_metadata_files.npm_package_id = $2::uuid)
RETURNING npm_metadata_files.blob_storage_attachment_id AS "npm_metadata_files.blob_storage_attachment_id",
          npm_metadata_files.blob_sha256 AS "npm_metadata_files.blob_sha256";

Bound args: [b050368e-576e-4a55-aff0-cbd21630a69d, 32566e5e-291a-4d77-bbcd-e80fadc314e8]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_metadata_files  (cost=0.28..11.84 rows=3 width=10) (actual time=0.031..0.036 rows=3 loops=1)
   Delete on npm_metadata_files_p54 npm_metadata_files_1
   Buffers: shared hit=11
   ->  Index Scan using npm_metadata_files_p54_namespace_id_npm_package_id_kind_idx on npm_metadata_files_p54 npm_metadata_files_1  (cost=0.28..11.84 rows=3 width=10) (actual time=0.022..0.025 rows=3 loops=1)
         Index Cond: ((namespace_id = 'b050368e-576e-4a55-aff0-cbd21630a69d'::uuid) AND (npm_package_id = '32566e5e-291a-4d77-bbcd-e80fadc314e8'::uuid))
         Buffers: shared hit=5
 Planning:
   Buffers: shared hit=129
 Planning Time: 0.844 ms
 Execution Time: 0.247 ms

Timings: planning 0.844 ms, execution 0.247 ms, total 1.091 ms.

datastore.NpmPackageReaper.Reap

Summary: The row-selection half is what the method intends: a single-row Index Scan on the primary key, pruned to one partition of 64. What the plan also shows is the cost of the delete itself — three referential-integrity checks, of which the npm_versions one has no usable index and scans the whole partition. The query notes above give the measurement and the tracker.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=5000

Rendered SQL:

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

Bound args: [ae5cf374-26eb-4f0f-af5b-8d811f32cf20, 0017ab2f-6663-4455-8674-413408393193]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_packages  (cost=0.28..8.30 rows=0 width=0) (actual time=0.230..0.230 rows=0 loops=1)
   Delete on npm_packages_p50 npm_packages_1
   Buffers: shared hit=23
   ->  Index Scan using npm_packages_p50_pkey on npm_packages_p50 npm_packages_1  (cost=0.28..8.30 rows=1 width=10) (actual time=0.029..0.030 rows=1 loops=1)
         Index Cond: ((id = '0017ab2f-6663-4455-8674-413408393193'::uuid) AND (namespace_id = 'ae5cf374-26eb-4f0f-af5b-8d811f32cf20'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=1
 Planning Time: 0.257 ms
 Trigger for constraint npm_versions_npm_package_id_namespace_id_fkey50 on npm_packages_p50: time=2.932 calls=1
 Trigger for constraint npm_tags_npm_package_id_namespace_id_fkey50 on npm_packages_p50: time=1.554 calls=1
 Trigger for constraint npm_metadata_files_npm_package_id_namespace_id_fkey50 on npm_packages_p50: time=1.995 calls=1
 Execution Time: 7.260 ms

Timings: planning 0.257 ms, execution 7.260 ms, total 7.517 ms. The rows=0 on the Delete node is the absence of a RETURNING clause, not an unmatched row: the Index Scan under it reports 1 row.

Supplementary probe — the three referential-integrity checks, each run as the shape the trigger runs, against a package with no children while the namespace's partition holds 5000 npm_versions, 5000 npm_tags and 5004 npm_metadata_files rows:

-- SELECT 1 FROM npm_versions x WHERE $1 = x.npm_package_id AND $2 = x.namespace_id FOR KEY SHARE OF x
 LockRows  (cost=0.00..147.01 rows=1 width=14) (actual time=0.186..0.186 rows=0 loops=1)
   Buffers: shared hit=72
   ->  Seq Scan on npm_versions_p16 x  (cost=0.00..147.00 rows=1 width=14) (actual time=0.185..0.186 rows=0 loops=1)
         Filter: (('c9396c6f-601b-452b-ac49-1a4336e6ba9a'::uuid = npm_package_id) AND ('36c81d92-3fc6-453d-9c1d-9fa347dfc3ff'::uuid = namespace_id))
         Rows Removed by Filter: 5000
         Buffers: shared hit=72

-- SELECT 1 FROM npm_tags x WHERE $1 = x.npm_package_id AND $2 = x.namespace_id FOR KEY SHARE OF x
 LockRows  (cost=0.28..8.31 rows=1 width=14) (actual time=0.017..0.017 rows=0 loops=1)
   Buffers: shared hit=2
   ->  Index Scan using npm_tags_p16_namespace_id_npm_package_id_name_idx on npm_tags_p16 x  (cost=0.28..8.30 rows=1 width=14) (actual time=0.017..0.017 rows=0 loops=1)
         Index Cond: ((namespace_id = '36c81d92-3fc6-453d-9c1d-9fa347dfc3ff'::uuid) AND (npm_package_id = 'c9396c6f-601b-452b-ac49-1a4336e6ba9a'::uuid))
         Buffers: shared hit=2

-- SELECT 1 FROM npm_metadata_files x WHERE $1 = x.npm_package_id AND $2 = x.namespace_id FOR KEY SHARE OF x
 LockRows  (cost=0.28..11.87 rows=3 width=14) (actual time=0.011..0.011 rows=0 loops=1)
   Buffers: shared hit=2
   ->  Index Scan using npm_metadata_files_p16_namespace_id_npm_package_id_kind_idx on npm_metadata_files_p16 x  (cost=0.28..11.84 rows=3 width=14) (actual time=0.011..0.011 rows=0 loops=1)
         Index Cond: ((namespace_id = '36c81d92-3fc6-453d-9c1d-9fa347dfc3ff'::uuid) AND (npm_package_id = 'c9396c6f-601b-452b-ac49-1a4336e6ba9a'::uuid))
         Buffers: shared hit=2
datastore.reapNpmAttachmentHolder.PackageMetadataFiles

Summary: The plan matches the leg's intent. The batch subquery takes unique_npm_metadata_files_ns_id_pkg_id_kind and returns the package's 3 rows, and the delete reaches each one through the primary key in a nested loop. Both scans prune to the same single partition of 64, which is what the redundant namespace_id on the outer delete buys. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=1668, blob_storage_blobs=5004, blob_storage_attachments=5004, npm_metadata_files=5004

Rendered SQL:

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

Bound args: [de43290d-cd44-4e1b-8b2c-51224b97acc7, de43290d-cd44-4e1b-8b2c-51224b97acc7, df7f650a-7152-418a-a231-b337ea263ccc, 100]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_metadata_files  (cost=12.16..36.85 rows=3 width=50) (actual time=0.042..0.050 rows=3 loops=1)
   Delete on npm_metadata_files_p25 npm_metadata_files_1
   Buffers: shared hit=20
   ->  Nested Loop  (cost=12.16..36.85 rows=3 width=50) (actual time=0.033..0.039 rows=3 loops=1)
         Buffers: shared hit=14
         ->  HashAggregate  (cost=11.88..11.91 rows=3 width=56) (actual time=0.020..0.021 rows=3 loops=1)
               Group Key: "ANY_subquery"."npm_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.016..0.018 rows=3 loops=1)
                     Buffers: shared hit=5
                     ->  Limit  (cost=0.28..11.84 rows=3 width=16) (actual time=0.010..0.012 rows=3 loops=1)
                           Buffers: shared hit=5
                           ->  Index Scan using npm_metadata_files_p25_namespace_id_npm_package_id_kind_idx on npm_metadata_files_p25 npm_metadata_files_2  (cost=0.28..11.84 rows=3 width=16) (actual time=0.010..0.011 rows=3 loops=1)
                                 Index Cond: ((namespace_id = 'de43290d-cd44-4e1b-8b2c-51224b97acc7'::uuid) AND (npm_package_id = 'df7f650a-7152-418a-a231-b337ea263ccc'::uuid))
                                 Buffers: shared hit=5
         ->  Index Scan using npm_metadata_files_p25_pkey on npm_metadata_files_p25 npm_metadata_files_1  (cost=0.28..8.30 rows=1 width=26) (actual time=0.005..0.005 rows=1 loops=3)
               Index Cond: ((id = "ANY_subquery"."npm_metadata_files.id") AND (namespace_id = 'de43290d-cd44-4e1b-8b2c-51224b97acc7'::uuid))
               Buffers: shared hit=9
 Planning:
   Buffers: shared hit=47
 Planning Time: 0.813 ms
 Execution Time: 0.516 ms

Timings: planning 0.813 ms, execution 0.516 ms, total 1.329 ms.

datastore.reapNpmAttachmentHolder.VersionFiles

Summary: The batch subquery is what the leg intends: index_npm_files_on_ns_id_ver_id under the LIMIT, one partition of 64 on both scans. The delete side joins the 100 batch ids to a Seq Scan of the partition rather than probing the primary key, which is the cheaper plan for 100 ids out of 5000 rows and reverses as the partition grows — the 50000-row probe in the query notes takes the Nested Loop and primary-key path and runs faster in absolute terms.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=1, npm_versions=50, blob_storage_blobs=5000, blob_storage_attachments=5000, npm_files=5000 (100 files per version, so the bound version holds exactly one full page)

Rendered SQL:

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

Bound args: [94cd6315-4cfb-4b67-a133-1814ac18f7ab, 94cd6315-4cfb-4b67-a133-1814ac18f7ab, dbe623bb-4ecc-4745-ab5e-f65994c06e94, 100]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_files  (cost=104.25..271.99 rows=100 width=50) (actual time=0.126..0.653 rows=100 loops=1)
   Delete on npm_files_p31 npm_files_1
   Buffers: shared hit=384
   ->  Hash Semi Join  (cost=104.25..271.99 rows=100 width=50) (actual time=0.120..0.609 rows=100 loops=1)
         Hash Cond: (npm_files_1.id = "ANY_subquery"."npm_files.id")
         Buffers: shared hit=184
         ->  Seq Scan on npm_files_p31 npm_files_1  (cost=0.00..153.50 rows=5000 width=26) (actual time=0.005..0.301 rows=5000 loops=1)
               Filter: (namespace_id = '94cd6315-4cfb-4b67-a133-1814ac18f7ab'::uuid)
               Buffers: shared hit=91
         ->  Hash  (cost=103.00..103.00 rows=100 width=56) (actual time=0.111..0.112 rows=100 loops=1)
               Buckets: 1024  Batches: 1  Memory Usage: 17kB
               Buffers: shared hit=93
               ->  Subquery Scan on "ANY_subquery"  (cost=5.31..103.00 rows=100 width=56) (actual time=0.025..0.096 rows=100 loops=1)
                     Buffers: shared hit=93
                     ->  Limit  (cost=5.31..102.00 rows=100 width=16) (actual time=0.021..0.082 rows=100 loops=1)
                           Buffers: shared hit=93
                           ->  Bitmap Heap Scan on npm_files_p31 npm_files_2  (cost=5.31..102.00 rows=100 width=16) (actual time=0.020..0.078 rows=100 loops=1)
                                 Recheck Cond: ((namespace_id = '94cd6315-4cfb-4b67-a133-1814ac18f7ab'::uuid) AND (npm_version_id = 'dbe623bb-4ecc-4745-ab5e-f65994c06e94'::uuid))
                                 Heap Blocks: exact=91
                                 Buffers: shared hit=93
                                 ->  Bitmap Index Scan on npm_files_p31_namespace_id_npm_version_id_idx  (cost=0.00..5.28 rows=100 width=0) (actual time=0.013..0.013 rows=100 loops=1)
                                       Index Cond: ((namespace_id = '94cd6315-4cfb-4b67-a133-1814ac18f7ab'::uuid) AND (npm_version_id = 'dbe623bb-4ecc-4745-ab5e-f65994c06e94'::uuid))
                                       Buffers: shared hit=2
 Planning:
   Buffers: shared hit=47
 Planning Time: 0.384 ms
 Execution Time: 0.913 ms

Timings: planning 0.384 ms, execution 0.913 ms, total 1.297 ms.

Supplementary probe — the same statement at 50000 rows in the partition (500 versions of 100 files, 1000 shared attachments, limit still 100):

 Delete on npm_files  (cost=177.02..958.10 rows=100 width=50) (actual time=0.125..0.349 rows=100 loops=1)
   Delete on npm_files_p00 npm_files_1
   Buffers: shared hit=677
   ->  Nested Loop  (cost=177.02..958.10 rows=100 width=50) (actual time=0.118..0.305 rows=100 loops=1)
         Buffers: shared hit=477
         ->  HashAggregate  (cost=176.60..177.60 rows=100 width=56) (actual time=0.105..0.112 rows=100 loops=1)
               Group Key: "ANY_subquery"."npm_files.id"
               Batches: 1  Memory Usage: 32kB
               Buffers: shared hit=77
               ->  Subquery Scan on "ANY_subquery"  (cost=0.29..176.35 rows=100 width=56) (actual time=0.017..0.091 rows=100 loops=1)
                     Buffers: shared hit=77
                     ->  Limit  (cost=0.29..175.35 rows=100 width=16) (actual time=0.012..0.077 rows=100 loops=1)
                           Buffers: shared hit=77
                           ->  Index Scan using npm_files_p00_namespace_id_npm_version_id_idx on npm_files_p00 npm_files_2  (cost=0.29..175.35 rows=100 width=16) (actual time=0.012..0.073 rows=100 loops=1)
                                 Index Cond: ((namespace_id = 'a6695826-48b2-4b7d-8a3f-f92b7ecfe6e3'::uuid) AND (npm_version_id = '056d28ff-1de9-4c75-a2d6-6fcb1d9eeb7c'::uuid))
                                 Buffers: shared hit=77
         ->  Index Scan using npm_files_p00_pkey on npm_files_p00 npm_files_1  (cost=0.41..7.79 rows=1 width=26) (actual time=0.002..0.002 rows=1 loops=100)
               Index Cond: ((id = "ANY_subquery"."npm_files.id") AND (namespace_id = 'a6695826-48b2-4b7d-8a3f-f92b7ecfe6e3'::uuid))
               Buffers: shared hit=400
 Planning:
   Buffers: shared hit=50
 Planning Time: 0.417 ms
 Execution Time: 0.597 ms
datastore.reapNpmTags.ByPackage

Summary: The batch subquery takes the unique_npm_tags_ns_id_pkg_id_name prefix, which is the index the npm_package_id predicate needs, and prunes to one partition of 64. The delete side is the same hash semi join the VersionFiles leg takes, for the same reason and with the same behaviour as cardinality grows. No anomalies beyond that shared note.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=50, npm_versions=50, npm_tags=5000 (100 tags per package, so the bound package holds one full page)

Rendered SQL:

DELETE FROM public.npm_tags
WHERE (npm_tags.namespace_id = $1::uuid) AND (npm_tags.id IN ((
           SELECT npm_tags.id AS "npm_tags.id"
           FROM public.npm_tags
           WHERE (npm_tags.namespace_id = $2::uuid) AND (npm_tags.npm_package_id = $3::uuid)
           LIMIT $4
      )));

Bound args: [5e6222e4-62c0-4c6b-a659-7ad5a094badf, 5e6222e4-62c0-4c6b-a659-7ad5a094badf, 00a29242-6788-4ffb-8110-7a774210a60b, 100]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_tags  (cost=88.74..237.48 rows=0 width=0) (actual time=0.614..0.615 rows=0 loops=1)
   Delete on npm_tags_p35 npm_tags_1
   Buffers: shared hit=176
   ->  Hash Semi Join  (cost=88.74..237.48 rows=100 width=50) (actual time=0.549..0.587 rows=100 loops=1)
         Hash Cond: (npm_tags_1.id = "ANY_subquery"."npm_tags.id")
         Buffers: shared hit=76
         ->  Seq Scan on npm_tags_p35 npm_tags_1  (cost=0.00..134.50 rows=5000 width=26) (actual time=0.006..0.324 rows=5000 loops=1)
               Filter: (namespace_id = '5e6222e4-62c0-4c6b-a659-7ad5a094badf'::uuid)
               Buffers: shared hit=72
         ->  Hash  (cost=87.49..87.49 rows=100 width=56) (actual time=0.056..0.057 rows=100 loops=1)
               Buckets: 1024  Batches: 1  Memory Usage: 17kB
               Buffers: shared hit=4
               ->  Subquery Scan on "ANY_subquery"  (cost=9.31..87.49 rows=100 width=56) (actual time=0.024..0.041 rows=100 loops=1)
                     Buffers: shared hit=4
                     ->  Limit  (cost=9.31..86.49 rows=100 width=16) (actual time=0.021..0.030 rows=100 loops=1)
                           Buffers: shared hit=4
                           ->  Bitmap Heap Scan on npm_tags_p35 npm_tags_2  (cost=9.31..86.49 rows=100 width=16) (actual time=0.020..0.026 rows=100 loops=1)
                                 Recheck Cond: ((namespace_id = '5e6222e4-62c0-4c6b-a659-7ad5a094badf'::uuid) AND (npm_package_id = '00a29242-6788-4ffb-8110-7a774210a60b'::uuid))
                                 Heap Blocks: exact=2
                                 Buffers: shared hit=4
                                 ->  Bitmap Index Scan on npm_tags_p35_namespace_id_npm_package_id_name_idx  (cost=0.00..9.28 rows=100 width=0) (actual time=0.015..0.015 rows=100 loops=1)
                                       Index Cond: ((namespace_id = '5e6222e4-62c0-4c6b-a659-7ad5a094badf'::uuid) AND (npm_package_id = '00a29242-6788-4ffb-8110-7a774210a60b'::uuid))
                                       Buffers: shared hit=2
 Planning:
   Buffers: shared hit=120
 Planning Time: 0.734 ms
 Execution Time: 0.880 ms

Timings: planning 0.734 ms, execution 0.880 ms, total 1.614 ms. The rows=0 on the Delete node is the absence of a RETURNING clause; the semi join under it reports the 100 rows removed.

datastore.reapNpmTags.ByVersion

Summary: The batch subquery takes index_npm_tags_on_ns_id_version_id, the index for the npm_version_id predicate, and prunes to one partition of 64. The delete side is the shared hash semi join described above. No anomalies beyond that shared note.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=50, npm_versions=50, npm_tags=5000

Rendered SQL:

DELETE FROM public.npm_tags
WHERE (npm_tags.namespace_id = $1::uuid) AND (npm_tags.id IN ((
           SELECT npm_tags.id AS "npm_tags.id"
           FROM public.npm_tags
           WHERE (npm_tags.namespace_id = $2::uuid) AND (npm_tags.npm_version_id = $3::uuid)
           LIMIT $4
      )));

Bound args: [4f7a2200-f621-4344-97ff-786c6f48ab66, 4f7a2200-f621-4344-97ff-786c6f48ab66, 1b6d6647-1743-48b3-a653-7b1f9da64d42, 100]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on npm_tags  (cost=84.74..233.48 rows=0 width=0) (actual time=0.746..0.748 rows=0 loops=1)
   Delete on npm_tags_p00 npm_tags_1
   Buffers: shared hit=176
   ->  Hash Semi Join  (cost=84.74..233.48 rows=100 width=50) (actual time=0.432..0.700 rows=100 loops=1)
         Hash Cond: (npm_tags_1.id = "ANY_subquery"."npm_tags.id")
         Buffers: shared hit=76
         ->  Seq Scan on npm_tags_p00 npm_tags_1  (cost=0.00..134.50 rows=5000 width=26) (actual time=0.006..0.385 rows=5000 loops=1)
               Filter: (namespace_id = '4f7a2200-f621-4344-97ff-786c6f48ab66'::uuid)
               Buffers: shared hit=72
         ->  Hash  (cost=83.49..83.49 rows=100 width=56) (actual time=0.053..0.054 rows=100 loops=1)
               Buckets: 1024  Batches: 1  Memory Usage: 17kB
               Buffers: shared hit=4
               ->  Subquery Scan on "ANY_subquery"  (cost=5.31..83.49 rows=100 width=56) (actual time=0.021..0.038 rows=100 loops=1)
                     Buffers: shared hit=4
                     ->  Limit  (cost=5.31..82.49 rows=100 width=16) (actual time=0.018..0.027 rows=100 loops=1)
                           Buffers: shared hit=4
                           ->  Bitmap Heap Scan on npm_tags_p00 npm_tags_2  (cost=5.31..82.49 rows=100 width=16) (actual time=0.018..0.024 rows=100 loops=1)
                                 Recheck Cond: ((namespace_id = '4f7a2200-f621-4344-97ff-786c6f48ab66'::uuid) AND (npm_version_id = '1b6d6647-1743-48b3-a653-7b1f9da64d42'::uuid))
                                 Heap Blocks: exact=2
                                 Buffers: shared hit=4
                                 ->  Bitmap Index Scan on npm_tags_p00_namespace_id_npm_version_id_idx  (cost=0.00..5.28 rows=100 width=0) (actual time=0.013..0.013 rows=100 loops=1)
                                       Index Cond: ((namespace_id = '4f7a2200-f621-4344-97ff-786c6f48ab66'::uuid) AND (npm_version_id = '1b6d6647-1743-48b3-a653-7b1f9da64d42'::uuid))
                                       Buffers: shared hit=2
 Planning:
   Buffers: shared hit=21
 Planning Time: 0.355 ms
 Execution Time: 0.988 ms

Timings: planning 0.355 ms, execution 0.988 ms, total 1.343 ms.

Related to #507

This is a bot message 🤖 — /smurfit

Edited by Pawel Rozlach

Merge request reports