fix(datastore): fence the Maven version insert on a live package row (!2331) · Merge requests · GitLab.org / ops / Artifact Registry

What this delivers

The Maven hosted publish can no longer commit a maven_versions row under a package tombstone.

The cause sat in the statement rather than at the call site. MavenVersionStore.FindOrCreateVersion runs on the pool before the request body streams, and its INSERT bound maven_package_id as a literal. The statement read no maven_packages row, so nothing ordered the version insert against a package mark. A mark that committed inside that window left a live version row under a package tombstone.

The INSERT now draws maven_package_id from a source SELECT over maven_packages. That SELECT filters soft_deleted_at IS NULL and locks the matching row FOR SHARE, in the same statement as the INSERT. FOR SHARE conflicts with the FOR NO KEY UPDATE that a package mark's UPDATE holds to commit. The insert therefore commits before the mark takes its lock, or it parks until the mark's transaction ends and then meets the state it left: no live parent if the mark committed, and a live one if it rolled back. FOR SHARE does not conflict with a sibling publish's own shared hold, so concurrent publishes under one package still run together.

The conflict read-back carries the same predicate as an uncorrelated EXISTS. An empty source SELECT and a conflict both give qrm.ErrNoRows, so without that check the two arms are indistinguishable. Both arms share liveMavenPackagePredicate and cannot drift apart. Both report datastore.ErrNotFound, which the upload answers with the 404 not_found it already gives a missing parent. No new status and no new problem code.

Both arms are scoped to a mark that has committed. The read-back takes no row lock, so a mark still in flight is invisible to it and the call returns the live version row it found. That publish then streams its body and is refused by MavenPackageStore.ReVerifyPackageAlive, whose SELECT ... FOR UPDATE upsertFileRow runs inside the commit transaction and holds to commit. FindOrCreateVersion's doc comment and the two matching sentences in docs/specs/S10-maven-hosted.md carry that scope in the same terms.

FindOrCreateVersion also rejects a transaction handle in its db position, which is a behavior change and a new error return. A caller inside a transaction holds the source SELECT's FOR SHARE to commit. That lock then spans the caller's whole upload and parks every competing write to the package row. The check is a *sql.Tx type assertion, placed where checkMavenReapArgs puts its own handle check, and it returns the new sentinel errMavenVersionTransactionHandle. The assertion sees a bare *sql.Tx only, so a transaction behind a wrapper is admitted, and so is a *sql.Conn wrapper, which autocommits. The doc sentence states the precondition the check can carry, "db must not be a transaction handle", rather than an enforcement claim the assertion cannot deliver. The one production call site is mavenStoreAdapter.FindOrCreateVersion in cmd/artifact-registry/wire_maven.go, and it passes the pool that mavenDBProvider resolves, so no merged caller changes behavior.

The fence reaches the package tier alone. The repository tier stays unfenced for the version insert, because the predicate reads maven_packages.soft_deleted_at and a repository tombstone stamps repositories.soft_deleted_at. The client is still refused there by ReVerifyRepositoryAlive, which upsertFileRow calls inside the commit transaction in internal/format/maven/upload.go. What that route can leave behind is one orphan maven_versions row, and the repository purge removes it. The package tier alone is in scope, because the issue asks for the package tombstone and !2268 (merged) owns the repository tier for npm and container.

The documentation pairing accounts for the prose in the diff.

docs/specs/S10-maven-hosted.md states the write-side liveness invariant and adds the refusal to the Responses table under ### PUT /{slug}/maven/{repository_name}/{path...}, and to the #### Failure modes summary row for Step 4c. Its ## Error Cases table is untouched: the generic row there, "Slug, repository, package, version, file, or digest sidecar parent does not exist -> 404 not_found", already covers the case.
docs/specs/S20-a-lifecycle-closed-beta.md rewrites the two follow-up entries that described the reconciler comment. It also rewrites the package-tier ## Error Cases row that !2302 (merged) added, and adds a third follow-up entry, already marked closed, which records that rewrite.
docs/dev/storage-accounting.md separates the package route from the repository route for the version insert.
docs/dev/database-query-patterns.md gains ### Maven metadata rewrite transaction, which is now the home of persistRewrite's transaction shape. persistRewrite's own doc comment points there, which is where 69 of the removed lines went.

The diff is 559 added and 134 removed lines across 14 files at head c3eee6925, so 693 reviewable lines. That is past the 500-line review threshold, so the diff owes a justification. The figures come from git diff --numstat origin/main...HEAD at that head, grouped by file.

documentation: 127 added, 14 removed
production Go: 56 added, 98 removed
tests: 376 added, 22 removed

Two of those groups are not new reading. 69 of the 98 removed production-Go lines are one hunk, the deletion of the relocated persistRewrite doc comment in internal/format/maven/reconciler.go. What is left of the production change is one statement in one function and the handle guard in front of it. The rest is one INFO log line at the refusal and one comment line in internal/datastore/maven_bulk_markers.go. 202 of the 376 added test lines are the two integration files that pin that statement, internal/datastore/maven_version_tombstone_race_integration_test.go and internal/datastore/maven_versions_integration_test.go. A split would put the fence in one merge request and the tests and pages that state it in another.

Coverage

The rows come from the issue card's derived acceptance items A1 to A7, because the issue states no acceptance section of its own. The governing spec is docs/specs/S10-maven-hosted.md, and the S10 sections the work touches carry their own table below.

Card acceptance criteria

#	Criterion	Tests
A1	Soft-deleted parent: no row, `ErrNotFound`	T1, T2
A2	The refusal is the existing 404, no new code	T3, T4
A3	Insert parks on the mark, then is refused	T5
A4	hiddenVersions exact against a live publish	T5
A5	No row lands, so none pins the package reap	T1, T2
A6	The four staged-race arms keep their status	T6-T9
A7	Sibling publishes do not serialize	T10

S10 sections this work touches

Section	Claim	Tests
`### PUT /{slug}/maven/{repository_name}/{path...}` Responses	A parent marked before the version insert commits is 404 `not_found`	T3
`#### Failure modes summary`	Step 4c can answer 404, leaving the package row and no version row	T1, T2, T3
`### Upload flow`	No application-level publish lease	T10
Soft-delete visibility	Stated for reads only	T1, T2

Test keys

T1 TestMavenVersionStore_FindOrCreateVersion, subtest "refuses to create a version under a soft-deleted package"
T2 the same test, subtest "refuses the conflict read-back when the package is soft-deleted"
T3 TestServePrimaryUpload_VersionFindOrCreateNotFound_Yields404
T4 TestWriteUploadStoreError_ErrNotFound_Yields404, existing
T5 TestMavenVersionStore_FindOrCreateVersion_LosesToACommittingPackageMark
T6 TestUpload_PackageMarkedMidCommit_FailsTheCommit, existing
T7 TestUpload_VersionMarkedMidCommit_FailsTheCommit, existing
T8 TestUpload_RepositoryMarkedMidCommit_FailsTheCommit, existing
T9 TestUpload_RepeatUploadAgainstMarkedPackage_DoesNotRewriteTheHiddenRow, existing
T10 TestMavenVersionStore_FindOrCreateVersion_AdmitsAConcurrentSiblingPublish

Coverage gaps

A5's reap-side half, ErrReapParentPinned, is now asserted. The subtest "a version committed under the tombstone refuses the package delete, and the next chunk drains" in internal/datastore/lifecycle_reap_maven_integration_test.go runs require.ErrorIs(tt, reapErr, ErrReapParentPinned). That file seeds its rows with raw SQL and calls no FindOrCreateVersion, so the fence is invisible to it. What the subtest pins is the reap's own refusal. The datastore side of A5 is covered separately: no row lands, so none pins.
A7's body-stream half is not asserted by any test. It follows from the production wiring — a pool handle with no surrounding transaction — rather than from anything a datastore test can observe. The lock-strength half is covered by T10.

The evidence is not uniform

The seven items do not carry the same weight of evidence. A7 has two halves: the lock-strength half is CI-guarded by siblingPublishBudget in the new race test, and the hold-span half rests on a runtime measurement alone. Nothing in CI catches a regression of that hold-span half. A5's ErrReapParentPinned half has a test and no measurement. That test seeds its version row with raw SQL, so it pins the reap's refusal and not the fence. The link between the two is still a code-symbol chain a reader can check rather than a test.

e2e catalog

No row in docs/testing/e2e/maven.md changes, and this branch touches no file under docs/testing/. No row there states an outcome the fence changes. e2e.maven.lifecycle.deploy-after-delete redeploys a coordinate after a package delete. FindOrCreatePackage inserts a fresh live maven_packages row beside the tombstone, so the version insert draws a live parent and the fence never fires. The behavior the fence adds is a race, and the catalog carries no row for it today. Merge request !2297 (merged) is still open against docs/testing/e2e/maven.md.

Runtime evidence

These figures come from a running service rather than from a reading of the code.

A refused publish parks 3.54 s, then answers 404, and it writes no version row.
The refusal body is field-identical to a pre-flight-miss 404, so the refusal is no existence oracle.
Three concurrent 16 MiB PUTs under one package took 0.871 s, against 0.580 s for one alone. The same three PUTs in series take about 1.74 s.
A PUT under a different package answered in 0.104 s during a FOR UPDATE hold on the first package.
Maven conformance ran 31 of 31.

Accepted costs

Correction: an earlier revision of this description claimed no production precedent for INSERT … SELECT. That revision made two claims, and both were wrong. It said that no such statement exists in this repository's production Go code. It said that the Go tree holds two of them, and that both are raw-SQL integration-test fixtures. The counts that follow replace them, and every one is re-taken at head b1068ad24 rather than carried over from the revision they replace.

go-jet builds this statement one way only, through InsertStatement.QUERY, so a search for QUERY( over the Go tree finds every builder-built case. It finds two, re-confirmed at head c3eee6925. One is this branch's fence in internal/datastore/maven_versions.go. The other is NamespaceEncryptionKeyStore.insertKeyRow in internal/datastore/namespace_encryption_keys.go, which is merged production code this branch does not touch. The builder-built form therefore has a production precedent, and the fence is the second use rather than the first.

The Go tree also holds 88 raw-SQL INSERT … SELECT statements across 47 files, counted by the source clause that follows the column list. 87 of them sit in files that carry //go:build integration, and the last one is in scripts/conformance/maven-provision/main.go, a conformance provisioning tool rather than the service. Migration SQL under internal/datastore/migrations/sql/ holds four more. A previous version of this count read 86 over the same 47 files, taken at a pre-rebase head. The difference of two is a re-measurement, not a change this branch makes.

What is left of the cost is the row lock, not the statement shape. At head b1068ad24 no INSERT … SELECT in the repository takes a row lock. The 88 raw statements, the four migration statements and insertKeyRow all leave the source SELECT unlocked. The fence puts FOR SHARE on that source SELECT, so the combination is new even though neither half is. The deadlock account in this section is what that combination costs.

The wrong claim still stands in a commit body on this branch. Commit 9b0061113 reads "INSERT ... SELECT has no precedent in this repository's production Go code", and its two-statement count is the same wrong count. That sha is post-rebase. The branch has been rebased onto origin/main several times since the body was written, most recently at ec806b44c, and every replay carried the body across unchanged. Earlier versions of this paragraph named 96db7dd74, 51a9e12c6 and 928cb3782. Each was that same commit before a rebase, and none is an ancestor of the head. Correcting a commit body needs a further history rewrite, so this description is the correction of record and the two disagree.

The pre-mark residue survives by design. A version row committed before the mark is still live under the package tombstone when the drain begins, and no fence removes it. That residue is the shape ADR-007 already specifies. The reap's version page is state-blind, so the chunk that drains the tombstone selects the pre-mark row in that same chunk.

Every conflicting holder of the live maven_packages row now delays a publish's version insert. Five transactions hold that row in a conflicting mode. Two are UPDATEs: the soft-delete mark, and the last_downloaded_at bump. Two are SELECT ... FOR UPDATE: the metadata reconciler's ReVerifyPackageAlive, and upsertFileRow's ReVerifyPackageAlive. The fifth is a DELETE, deleteReapedMavenPackageStmt in internal/datastore/lifecycle_reap_maven.go, and its row lock conflicts with FOR SHARE too. upsertFileRow already holds row-lock waits of the same kind on the repository row and on the package row. An upload that binds a version waits on the version row as well. The five holds are read off the lock modes, and none of them is measured.

Retraction: an earlier revision of this description denied the new hazard. That revision said the new wait was "a second instance of a wait class the same request path already carries, rather than a new hazard". The claim was wrong, and this section replaces it. The fence gives the Maven repository purge a deadlock partner, so the new wait is not only another instance of an existing wait class.

The deadlock. The fenced INSERT ... SELECT holds FOR SHARE on the live maven_packages row across the ON CONFLICT arbiter probe. MavenPackageReaper.Reap runs the opposite order in one chunk transaction, deleteReapedMavenVersionPageStmt before deleteReapedMavenPackageStmt, and it takes no maven_packages lock of its own. If the chunk's own version page already took the raced-in maven_versions row and did not commit it, the probe waits on that tuple. The package delete then waits on the share lock, and PostgreSQL breaks the cycle with SQLSTATE 40P01.

The branch does not invert a lock order. MavenPackageReaper's child-first order is older than this branch. What the fence adds is the counterparty that makes that pre-existing order reachable.

One route reaches the cycle: the repository purge walk. mavenPackagesReapPageStmt in internal/datastore/lifecycle_reap_repository.go reads no soft_deleted_at, so it hands the reaper live package rows and the publish takes the lock. The package-scope purge never reaches it. Against a marked package the fence's soft_deleted_at IS NULL predicate matches nothing, so the publish takes no lock at all.

The cycle needs three things at once. The first is a Maven repository purge in flight. The second is a maven_versions row that the chunk's own version page takes. The third is a concurrent PUT of that same coordinate.

The first two are narrow. RepositoryReaper.Reap returns as soon as a level page comes back non-empty, and mavenHostedReapWalk pages maven_versions before maven_packages. The chunk therefore reaches the package reaper only when the repository held no maven_versions row at the version page's own statement. The racing row must commit inside that chunk, in a window of about three statements at READ COMMITTED. The third leg is ordinary rather than exotic. A mvn deploy sends several primary PUTs per version, and each one calls FindOrCreateVersion for the same coordinate.

Neither side loses data, which is why this merge request accepts the cycle. The purge chunk rolls back whole, and the at-least-once re-queue redoes it. The publish answers 500 on a request that ReVerifyRepositoryAlive in internal/format/maven/upload.go refuses later in the same upload.

The repository already accepts this class of reap-versus-request-path inversion in writing, for the npm remote family. The ### Remote npm cache reapers section of docs/dev/storage-accounting.md records the same split cost. The losing chunk rolls back whole and the re-queue redoes it, and the losing request surfaces the error as its own. The change that ends the cycle is a reaper change, in a file this branch does not touch. The npm family shipped its equivalent as its own merge request.

What the cycle costs operationally. 40P01 is not a foreign-key violation, so it falls past mapReapParentDeleteError in internal/datastore/lifecycle_scan.go. It then books purge_outcome="error" through the catch-all in purgeOutcomeLabelValue, in internal/lifecycle/metrics.go. The 23503 ending books under the named parent_pinned arm of the same metric. The label is purge_outcome, which internal/lifecycle/metrics.go defines as metricLabelPurgeOutcome, and not outcome.

This merge request answers that with documentation rather than with a SQLSTATE mapping. Four other reap families already land 40P01 in the same error arm, so a Maven-only mapping is incoherent with them. A repository-wide mapping is its own merge request.

Where the cycle is recorded. Commit 75d627f80 puts both endings of the repository route into docs/dev/storage-accounting.md, with a pointer to the work item this paragraph names. Work item #1168 (closed) carries whether the reap should take the parent row first.

What this merge request deliberately does not do. It does not give MavenPackageReaper.Reap the parent-first lock, and internal/datastore/lifecycle_reap_maven.go is untouched. It adds no SQLSTATE mapping. It adds no test that pins the deadlock, because a test that asserts a deadlock as expected behavior states a position nobody took.

The conflict arm now takes a package row lock where it took none. The foreign key's after-row referential check does not fire when ON CONFLICT DO NOTHING skips the row, so before this change the conflict arm touched maven_packages not at all. The source SELECT runs on both arms, so package-row tuple-lock traffic moves from one per new version to one per primary file: a mvn deploy sends a pom, a jar and often sources and javadoc under one version, and only the first of them inserts. Concurrent share holders on one row are recorded through multixacts. Per call the added work is point lookups into one of 64 hash partitions: one index probe on pk_maven_packages for the source scan, and one more for the EXISTS the read-back gained. Nothing is owed on cost grounds; the line is here so the traffic is a known consequence rather than a surprise if same-package publish latency is ever profiled.

The lock wait is not bounded

The fence adds no context.WithTimeout and no context.WithDeadline, so its wait on the package row is unbounded. Two written reasons hold whatever the merge order turns out to be.

A merged ## Error Cases row in docs/specs/S20-a-lifecycle-closed-beta.md already accepts an unbounded request-path row-lock wait. It rules on deleter.Delete at internal/managementapi/delete.go:84, which takes the request context and no timeout. The scope of that row is exact: the management API repository DELETE is an operator route, and this reason claims no parity with a client route.

The stronger reason is !2268 (merged)'s own, which declines the same bound for the Maven wait on three grounds. All three read the same before that merge request lands and after it.

A bound puts a new 500 class on a merged client path.
Its value depends on a budget nobody has measured.
Work item 978 owns the shape of that answer across several sites rather than one.

The Maven hosted primary upload request path already holds unbounded row-lock waits of the same kind. A bound on the new wait alone is therefore incoherent as a rule and invisible as a fix.

The asymmetry across formats is deliberate. !2268 (merged) bounds the npm and container waits at ten seconds, and this Maven arm carries no bound, from the same reasoning and the same author. That merge request's description carries the paragraph which answers the question, and it pre-empts it better than this branch can.

The new wait site is recorded on #978, which owns the row-lock budget question for the whole repository.

Deliberate omissions

docs/dev/go-style.md asks for a declined comment edit to be named in the merge request description. Two are declined here, and two further points are recorded for a reviewer.

The UploadStore seam comment in internal/format/maven/upload.go stays silent on parent liveness. It never spoke about parent liveness, so this change falsifies nothing in it. The comment already runs 5 counted lines against an interface-method cap of 2. TestServePrimaryUpload_VersionFindOrCreateNotFound_Yields404 pins the refusal at the call site instead.

writeRewrittenMetadata gets no pointer to the relocated page. Its doc comment runs 10 counted lines against an unexported cap of 1, so scripts/ci/check-comment-caps.sh refuses an edit that leaves the block over the cap. persistRewrite carries the pointer, and the page names writeRewrittenMetadata as the function that runs the numbered steps.

One claim inside the reconciler's restore paragraph was dropped rather than relocated. "Symmetric to SoftDeleteVersion" names no symbol in the Go tree, and SoftDeleteMavenVersion writes no other row and takes no maven_packages lock. No restore path exists in the tree at all: nothing clears maven_versions.soft_deleted_at, and S20-A defers the restore surface to GA. The relocated page states both halves, and this claim is independent of this branch.

The #### Failure modes summary row for Step 5 is left as it stands. It lists 500 alone, and this branch's new prose leans on that step answering 404 when upsertFileRow's liveness re-check aborts the commit transaction. That half predates this branch, and it is wider than the one value: the same row also omits the 409 a release conflict answers. The Step 4c row is this branch's own, because this branch is what puts a 404 on that step; the Step 5 row is not, and closing it takes more than the value this change would have added.

A divergence in docs/specs/S10-maven-hosted.md is left as it stands. The bullet that opens the version find-or-create says it uses the same partial-index pattern as the package find-or-create, which is a DO UPDATE SET id = … RETURNING upsert. The sentence this branch adds to that bullet names a conflict read-back. Both stores run a two-trip DO NOTHING plus read-back, so the amendment made the divergence visible and did not cause it.

Lint on the touched Go files

This branch does not claim clean lint on the Go files it touches. The command was golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false, run over ./internal/datastore/... and ./internal/format/maven/... with golangci-lint 2.13.2, which .tool-versions's golangci-lint 2.13 line resolves to. The figures below come from that command at head c3eee6925. 79 findings fall on the ten Go files this branch touches. 73 of them sit in internal/datastore/maven_versions_integration_test.go, and the other 6 are dupl in internal/datastore/lifecycle_reap_maven_integration_test.go, which the subtest repair made a touched file. The six sit at lines 330, 446, 673, 738, 789 and 1204. The repair's own lines are 1282 to 1283, 1327 to 1336, and 1363, so none of the six is in it. Nine of the 73 land on lines this branch added, and all nine are contextcheck. Each of the nine repeats an idiom that the same file already uses about sixty times, on lines this branch never touched. Neither lll nor thelper fires on any of the ten files.

The seven //nolint tokens this branch adds were measured rather than reasoned about, by deleting the directives and re-running that command over ./internal/datastore/. All seven fire: ireturn on liveMavenPackagePredicate, wrapcheck on each of pinnedPoolConn's four pass-through methods, and paralleltest on both tests in the new race file. So every token stays and no trailing comment is trimmed. The funlen token on FindOrCreateVersion is not in that set: it predates this branch, with identical text at line 88 of the merge-base file.

Merge order

!2302 (merged) merged first, and that order is now history rather than a plan. It merged on 2026-09-04 as 71cdc5b1c, which is an ancestor of this branch's base 8ee672382. !2302 (merged) recorded the publish-under-tombstone window as accepted for closed beta and named this work item as the owner of the fence that ends that acceptance. This branch is that fence, and it was rebased onto a main carrying !2302 (merged).

The two changes contended on documentation alone, and the rebase resolved that contention. The conflicts came back in the two files git merge-tree predicted, docs/dev/storage-accounting.md and docs/specs/S20-a-lifecycle-closed-beta.md, and no Go file conflicted. Each resolution kept the text main carried and folded this branch's text into it, so nothing the target branch wrote was dropped.

The package-tier ## Error Cases row in docs/specs/S20-a-lifecycle-closed-beta.md is rewritten, in commit b1068ad24. The row !2302 (merged) added said that FindOrCreateVersion reads no parent soft_deleted_at, which the fence makes false. The rewrite states what the statement does, the two readings it does not close, and which of the window's states stay reachable through which tier. The ## Follow-ups entry that asked for the rewrite is restated in the same commit and marked closed. The file then reads correctly to someone who arrives at it after this merges.

!2302 (merged)'s own new subtest did not survive the fence, and the repair is on this branch. !2302 (merged) added a subtest of TestMavenPackageReaper_Reap in internal/datastore/lifecycle_reap_maven_integration_test.go, "a version committed under the tombstone refuses the package delete, and the next chunk drains". That subtest marked the package with SoftDeleteMavenPackage and then called FindOrCreateVersion against the marked row, asserting require.NoError on the error and require.True on created. Every later assertion in it rested on the row that call inserts. The fence refuses that call twice over: the source SELECT finds no live parent, and FindOrCreateVersion rejects the *sql.Tx the subtest passes before the statement runs. Commit 4a6369f29 seeds the same row with a raw INSERT on the publishing transaction instead. The subtest's framing comment and one assertion message were reworded as well, because they attributed the row to the publish path the fence closes. No assertion changed, so ErrReapParentPinned, the empty ReapTotals and the next chunk's drain still run.

!2328 (merged) changes FindOrCreateVersion's signature and conflicts in production Go. It was open and not a draft at head 274c68ce5, read on 2026-09-04, and it adds gitlabUserID *string as a trailing parameter of MavenVersionStore.FindOrCreateVersion and of the UploadStore seam, bound as a fifth element of the VALUES list this branch replaces with QUERY(livePackage). git merge-tree --write-tree between that head and this branch's reports content conflicts in docs/specs/S10-maven-hosted.md, internal/datastore/maven_versions.go, internal/datastore/maven_versions_integration_test.go and internal/format/maven/upload_internal_test.go. Whichever of the two merges second has to move gitlabUserID into a fifth projection of the source SELECT and re-check the projection order against the INSERT column list; neither absorbs the other mechanically. The silent half is internal/datastore/maven_version_tombstone_race_integration_test.go, which this branch adds: it calls FindOrCreateVersion at lines 103 and 156, merge-tree reports no conflict in that file, and the arity change would land there as a compile error with no marker. No merged-results pipeline is built when main moves under an open merge request, so that break surfaces on the next pipeline after the second of the two lands.

Database Review Evidence

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.

Migration mode did not run. The three-dot diff against origin/main names no file under internal/datastore/migrations/sql/ at head 63a697831, so this merge request adds and modifies no migration.

The plans below ran on a statement one projection narrower than the one head builds. Every figure in this table and in the two <details> blocks below was measured at 41e21da9f, and Control E at 03077852a. Both shas are gone, and so are the four that an earlier version of this paragraph offered to reach them. git cat-file --batch-check in a clone of this branch reports 41e21da9f, 03077852a, 986f35a5b, 75d627f80, 42f6c2721 and 5ef48dcfe all missing. The labels are left as the tool wrote them rather than moved onto a head where nothing was measured. The plan figures are not re-measured at head and are not claimed there.

The divergence came from the base, not from a later change on this branch. !2328 (merged) merged on 2026-09-04 and added gitlab_user_id to the maven_versions insert, as the fifth element of its VALUES list. The rebase folded that element into this branch's oldest commit 9b0061113, which replaces VALUES with QUERY(livePackage). So head's source SELECT carries five projections, where the measured statement carried four.

The Insert arm's Rendered SQL and Bound args below are re-taken at head 63a697831, through a .Sql() harness that dispatches nothing to a database. The plan under them is still the plan that ran. The fifth projection is a scalar into the inserted table, so it adds one bound argument and shifts every later placeholder number. It adds no table access, so the index, the LockRows node and the one-partition pruning in the plan are the ones head produces. ConflictReadBack projects no publisher, so its rendering needs no re-take and matches head as printed.

Method	Plan node	Index	Rows (plan / actual)	Cost	Time	Buffers (hit / read)	Partitions
`datastore.MavenVersionStore.FindOrCreateVersion.Insert`	Insert	`maven_packages_p61_pkey`; arbiter `unique_maven_versions_ns_id_pkg_id_version`	1 / 1	8.32	0.991ms	176 / 0	1/64 `maven_packages`, 1/64 `maven_versions`
`datastore.MavenVersionStore.FindOrCreateVersion.ConflictReadBack`	Limit	`maven_versions_p34_namespace_id_maven_package_id_version_idx`; `maven_packages_p34_pkey` in InitPlan 1	1 / 1	16.61	0.048ms	6 / 0	1/64 `maven_versions`, 1/64 `maven_packages`

Query notes:

Both statements prune to one partition of each table they touch, and neither reads a buffer from disk. No anomaly in the skill's catalog fired.
The seed gives maven_packages 5000 rows rather than the one row the recipe gives an ancestor table. The fence makes that table the driving scan of both arms, and a one-row parent cannot show whether the planner picks an index.
Five control plans answer the questions this change raises. Each control ran under the same seed, inside the same transaction as the statement it is compared against.

datastore.MavenVersionStore.FindOrCreateVersion.Insert

Measured at head: 41e21da9f.

Summary: The plan matches the fence's intent. The source SELECT reaches the live package through maven_packages_p61_pkey with both key columns bound, and FOR SHARE adds one LockRows node above that scan without changing the scan method or the index. LIMIT(1) adds a Limit node above LockRows and changes no cost, no row estimate, no index and no buffer count. soft_deleted_at IS NULL lands as a Filter rather than an Index Cond, because the primary key is not partial, and at one matched row this costs nothing. No anomalies.

Seed shape: namespaces=1, repositories=1, maven_repositories=1, maven_packages=5000, maven_versions=5000

Rendered SQL:

INSERT INTO public.maven_versions (id, namespace_id, maven_package_id, version, gitlab_user_id) (
     SELECT $1::uuid,
          $2::uuid,
          maven_packages.id AS "maven_packages.id",
          $3::text,
          $4::text
     FROM public.maven_packages
     WHERE ((maven_packages.namespace_id = $5::uuid) AND (maven_packages.id = $6::uuid)) AND (maven_packages.soft_deleted_at IS NULL)
     LIMIT $7
     FOR SHARE
)
ON CONFLICT (namespace_id, maven_package_id, version) WHERE soft_deleted_at IS NULL DO NOTHING
RETURNING maven_versions.namespace_id AS "maven_versions.namespace_id",
          maven_versions.id AS "maven_versions.id",
          maven_versions.maven_package_id AS "maven_versions.maven_package_id",
          maven_versions.version AS "maven_versions.version";

Bound args: ['01a07be2-5d50-72d6-ac6f-1d5b6439bf7b', '5fdf8b3d-875c-798e-97cf-d5c8d489888d', 'review-prep-version-999999', '42', '5fdf8b3d-875c-798e-97cf-d5c8d489888d', 'ace28156-2c60-77f1-bf1d-7593526d1e73', 1]

The publisher stub is "42", and the first element is a fresh newID(). This chain has two renderings, because nullableStringExpr branches on the publisher. A nil publisher renders NULL::text in place of $4::text, and that form emits no placeholder. The same chain then binds six arguments and ends LIMIT $6. The column list is five wide in both.

Plan (EXPLAIN (ANALYZE, BUFFERS) output, head 41e21da9f):

 Insert on maven_versions  (cost=0.28..8.32 rows=1 width=208) (actual time=0.990..0.991 rows=1 loops=1)
   Conflict Resolution: NOTHING
   Conflict Arbiter Indexes: unique_maven_versions_ns_id_pkg_id_version
   Tuples Inserted: 1
   Conflicting Tuples: 0
   Buffers: shared hit=176
   ->  Subquery Scan on "*SELECT*"  (cost=0.28..8.32 rows=1 width=208) (actual time=0.016..0.017 rows=1 loops=1)
         Buffers: shared hit=4
         ->  Limit  (cost=0.28..8.31 rows=1 width=90) (actual time=0.015..0.015 rows=1 loops=1)
               Buffers: shared hit=4
               ->  LockRows  (cost=0.28..8.31 rows=1 width=90) (actual time=0.014..0.015 rows=1 loops=1)
                     Buffers: shared hit=4
                     ->  Index Scan using maven_packages_p61_pkey on maven_packages_p61 maven_packages  (cost=0.28..8.30 rows=1 width=90) (actual time=0.011..0.012 rows=1 loops=1)
                           Index Cond: ((id = 'ace28156-2c60-77f1-bf1d-7593526d1e73'::uuid) AND (namespace_id = '5fdf8b3d-875c-798e-97cf-d5c8d489888d'::uuid))
                           Filter: (soft_deleted_at IS NULL)
                           Buffers: shared hit=3
 Planning:
   Buffers: shared hit=427
 Planning Time: 1.563 ms
 Trigger for constraint fk_maven_versions_maven_package_id_maven_packages on maven_versions_p61: time=0.162 calls=1
 Trigger for constraint fk_maven_versions_namespace_id_namespaces on maven_versions_p61: time=0.365 calls=1
 Execution Time: 2.148 ms

Timings: planning 1.563ms, execution 2.148ms, total 3.711ms. The planning figure is a first-plan figure: it reads 427 planning buffers, which is the catalog cost of pruning a 64-partition table for the first time in the session. A second EXECUTE of the same prepared statement, under the same seed, planned in 0.116ms and executed in 0.427ms.

Control: what LIMIT(1) costs. Nothing. Control A and control E differ by the clause alone: E is the prior head's LockRows over the same Index Scan, and A wraps it in a Limit node. Both estimate cost=0.28..8.31 rows=1 width=90, both read 4 buffers, and both execute in 0.021 to 0.032 ms. The Limit is parameterized, so the planner cannot fold the value into its estimate. It does not need to, because the underlying scan already estimates one row from a two-column primary-key bind. The placeholder is LIMIT $6 in the four-projection form this control measured, and LIMIT $7 at head with a publisher bound.

Control: does FOR SHARE change the plan? It does not. The same source SELECT without FOR SHARE picks the same index, with the same Index Cond and the same 3 scan buffers. FOR SHARE adds a LockRows node, one buffer, and 0.01 of estimated cost.

=== CONTROL A: source SELECT WITH LIMIT AND FOR SHARE (head 41e21da9f) ===
 Limit  (cost=0.28..8.31 rows=1 width=90) (actual time=0.017..0.017 rows=1 loops=1)
   Buffers: shared hit=4
   ->  LockRows  (cost=0.28..8.31 rows=1 width=90) (actual time=0.016..0.017 rows=1 loops=1)
         Buffers: shared hit=4
         ->  Index Scan using maven_packages_p52_pkey on maven_packages_p52 maven_packages  (cost=0.28..8.30 rows=1 width=90) (actual time=0.013..0.014 rows=1 loops=1)
               Index Cond: ((id = 'd9e5dd40-aa0d-713e-96c6-e470df3d5adb'::uuid) AND (namespace_id = 'b345420f-ad2c-7b82-85ac-c5e89d704b08'::uuid))
               Filter: (soft_deleted_at IS NULL)
               Buffers: shared hit=3
 Planning:
   Buffers: shared hit=64
 Planning Time: 0.424 ms
 Execution Time: 0.032 ms

=== CONTROL B: same source SELECT WITH LIMIT, WITHOUT FOR SHARE (head 41e21da9f) ===
 Limit  (cost=0.28..8.30 rows=1 width=80) (actual time=0.012..0.012 rows=1 loops=1)
   Buffers: shared hit=3
   ->  Index Scan using maven_packages_p52_pkey on maven_packages_p52 maven_packages  (cost=0.28..8.30 rows=1 width=80) (actual time=0.012..0.012 rows=1 loops=1)
         Index Cond: ((id = 'd9e5dd40-aa0d-713e-96c6-e470df3d5adb'::uuid) AND (namespace_id = 'b345420f-ad2c-7b82-85ac-c5e89d704b08'::uuid))
         Filter: (soft_deleted_at IS NULL)
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=1
 Planning Time: 0.088 ms
 Execution Time: 0.021 ms

=== CONTROL E: prior-head source SELECT, FOR SHARE, no LIMIT (head 03077852a) ===
 LockRows  (cost=0.28..8.31 rows=1 width=90) (actual time=0.011..0.012 rows=1 loops=1)
   Buffers: shared hit=4
   ->  Index Scan using maven_packages_p52_pkey on maven_packages_p52 maven_packages  (cost=0.28..8.30 rows=1 width=90) (actual time=0.010..0.011 rows=1 loops=1)
         Index Cond: ((id = 'd9e5dd40-aa0d-713e-96c6-e470df3d5adb'::uuid) AND (namespace_id = 'b345420f-ad2c-7b82-85ac-c5e89d704b08'::uuid))
         Filter: (soft_deleted_at IS NULL)
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=1
 Planning Time: 0.066 ms
 Execution Time: 0.021 ms

Control: what the merge-base INSERT planned. The literal VALUES form plans a bare Result node with no scan, so the source SELECT is the whole marginal read. The FK trigger fk_maven_versions_maven_package_id_maven_packages fires on both forms, so the row this statement now locks is a row the statement already reached before the change. The merge base moved with !2328 (merged) after this control ran. At ec806b44 the merge base builds the same VALUES form, with nullableStringExpr(gitlabUserID) as a fifth element. That element adds no scan node, so a bare Result is still what the current merge base plans.

=== CONTROL D: merge-base INSERT, literal VALUES ===
 Insert on maven_versions  (cost=0.00..0.01 rows=1 width=208) (actual time=0.388..0.388 rows=1 loops=1)
   Conflict Resolution: NOTHING
   Conflict Arbiter Indexes: unique_maven_versions_ns_id_pkg_id_version
   Tuples Inserted: 1
   Conflicting Tuples: 0
   Buffers: shared hit=87
   ->  Result  (cost=0.00..0.01 rows=1 width=208) (actual time=0.001..0.001 rows=1 loops=1)
 Planning Time: 0.034 ms
 Trigger for constraint fk_maven_versions_maven_package_id_maven_packages on maven_versions_p52: time=1.720 calls=1
 Trigger for constraint fk_maven_versions_namespace_id_namespaces on maven_versions_p52: time=0.032 calls=1
 Execution Time: 2.162 ms

datastore.MavenVersionStore.FindOrCreateVersion.ConflictReadBack

Measured at head: 41e21da9f. The LIMIT(1) that head added sits on the source SELECT of the Insert arm and does not reach this statement, so this block is the earlier run's result re-measured rather than a changed one.

Summary: The planner folds the EXISTS into InitPlan 1 and gates the whole scan with a One-Time Filter, so maven_packages is probed once for the statement rather than once per candidate row. The subquery binds parameters only and reads no column of maven_versions, so it is uncorrelated and the planner can hoist it. The maven_versions scan is unchanged: the partial unique index unique_maven_versions_ns_id_pkg_id_version serves all four predicates, and soft_deleted_at IS NULL is covered by the index predicate rather than by a Filter. No anomalies.

Seed shape: namespaces=1, repositories=1, maven_repositories=1, maven_packages=5000, maven_versions=5000

Rendered SQL:

SELECT maven_versions.namespace_id AS "maven_versions.namespace_id",
     maven_versions.id AS "maven_versions.id",
     maven_versions.maven_package_id AS "maven_versions.maven_package_id",
     maven_versions.version AS "maven_versions.version"
FROM public.maven_versions
WHERE ((((maven_versions.namespace_id = $1::uuid) AND (maven_versions.maven_package_id = $2::uuid)) AND (maven_versions.version = $3::text)) AND (maven_versions.soft_deleted_at IS NULL)) AND (EXISTS (
           SELECT $4
           FROM public.maven_packages
           WHERE ((maven_packages.namespace_id = $5::uuid) AND (maven_packages.id = $6::uuid)) AND (maven_packages.soft_deleted_at IS NULL)
      ))
LIMIT $7;

Bound args: ['3707f4ce-7169-7d27-a493-a477d3c9d921', '957dc358-2a52-7cb0-a0a3-d9d3a8f22894', 'review-prep-version-002500', 1, '3707f4ce-7169-7d27-a493-a477d3c9d921', '957dc358-2a52-7cb0-a0a3-d9d3a8f22894', 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output, head 41e21da9f):

 Limit  (cost=8.59..16.61 rows=1 width=75) (actual time=0.047..0.048 rows=1 loops=1)
   Buffers: shared hit=6
   InitPlan 1
     ->  Index Scan using maven_packages_p34_pkey on maven_packages_p34 maven_packages  (cost=0.28..8.30 rows=1 width=0) (actual time=0.024..0.025 rows=1 loops=1)
           Index Cond: ((id = '957dc358-2a52-7cb0-a0a3-d9d3a8f22894'::uuid) AND (namespace_id = '3707f4ce-7169-7d27-a493-a477d3c9d921'::uuid))
           Filter: (soft_deleted_at IS NULL)
           Buffers: shared hit=3
   ->  Result  (cost=0.28..8.30 rows=1 width=75) (actual time=0.046..0.046 rows=1 loops=1)
         One-Time Filter: (InitPlan 1).col1
         Buffers: shared hit=6
         ->  Index Scan using maven_versions_p34_namespace_id_maven_package_id_version_idx on maven_versions_p34 maven_versions  (cost=0.28..8.30 rows=1 width=75) (actual time=0.020..0.020 rows=1 loops=1)
               Index Cond: ((namespace_id = '3707f4ce-7169-7d27-a493-a477d3c9d921'::uuid) AND (maven_package_id = '957dc358-2a52-7cb0-a0a3-d9d3a8f22894'::uuid) AND (version = 'review-prep-version-002500'::text))
               Buffers: shared hit=3
 Planning:
   Buffers: shared hit=332
 Planning Time: 3.838 ms
 Execution Time: 0.080 ms

Timings: planning 3.838ms, execution 0.080ms, total 3.918ms. The planning figure is a first-plan figure, as in the Insert block above. A second EXECUTE under the same seed planned in 0.277ms and executed in 0.044ms.

Control: what the EXISTS costs against the merge-base read-back. The merge-base statement reads 3 buffers and executes in 0.025 ms. The fenced statement reads 6 and executes in 0.080 ms. The whole difference is InitPlan 1: one index probe on one partition of maven_packages, run once. The maven_versions scan is the same node with the same index and the same buffer count across the two.

=== CONTROL C: merge-base read-back, no EXISTS ===
 Limit  (cost=0.28..8.30 rows=1 width=75) (actual time=0.015..0.015 rows=1 loops=1)
   Buffers: shared hit=3
   ->  Index Scan using maven_versions_p52_namespace_id_maven_package_id_version_idx on maven_versions_p52 maven_versions  (cost=0.28..8.30 rows=1 width=75) (actual time=0.014..0.014 rows=1 loops=1)
         Index Cond: ((namespace_id = 'b345420f-ad2c-7b82-85ac-c5e89d704b08'::uuid) AND (maven_package_id = 'd9e5dd40-aa0d-713e-96c6-e470df3d5adb'::uuid) AND (version = 'review-prep-version-002500'::text))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=267
 Planning Time: 1.124 ms
 Execution Time: 0.025 ms

Related to #1135

Related to #535

What remains: every acceptance item graded for this unit comes from the enrichment step rather than from the issue author. An automatic close therefore removes the author's chance to disagree with that reading. The repository tier of the version insert stays unfenced. Of work item 535's three scope bullets, this work satisfies the first two. The third stays outstanding: an interleave test that asserts the served metadata includes a version uploaded inside a reconcile's window.

This is a bot message 🤖 — /smurfit

Edited Sep 07, 2026 by Pawel Rozlach

fix(datastore): fence the Maven version insert on a live package row