Refuse the connection pool on the three Maven ReVerify*Alive methods

What

Three merged Maven methods take a SELECT ... FOR SHARE on a parent row and accept the connection pool:

  • MavenRepositoryStore.ReVerifyRepositoryAlive (internal/datastore/maven_repositories.go:248)
  • MavenPackageStore.ReVerifyPackageAlive (internal/datastore/maven_packages.go:197)
  • MavenVersionStore.ReVerifyVersionAlive (internal/datastore/maven_versions.go:858)

That lock holds only to the end of the caller's transaction. On the pool the statement runs in its own implicit transaction, so the lock ends with the statement and the caller writes unfenced. Each method guards a nil context, a nil db and a zero UUID. None guards the handle.

Refuse the pool at run time in all three, with the validateDrainWriteHandle shape (internal/datastore/counter_drain.go:568).

Why it matters

No caller passes the pool today. The five Maven production call sites all take their handle from datastore.RunInTx, whose closure receives a qrm.DB with the dynamic type *sql.Tx: internal/format/maven/upload.go:690, :695 and :703, and internal/format/maven/reconciler.go:782 and :812. The guard is defense in depth. docs/specs/S20-a-lifecycle-closed-beta.md states the doctrine in ### Closing the tombstone-visibility gap: "Adding the predicate now costs one clause; adding it after such a caller exists costs an audit of every caller."

The npm and container ReVerifyRepositoryAlive methods gained the guard on merge request !2268 (merged). While this issue is open, the five ReVerify*Alive methods carry two different handle contracts, and the three Maven argument-guard tables carry no connection-pool row where the npm and container tables carry one.

The format seam does not block it

A run-time type assertion crosses mavenStoreAdapter untouched. cmd/artifact-registry/wire_maven.go:294 says so: "ReVerifyRepositoryAlive forwards the caller's db handle untouched". The parameter stays qrm.DB, so the two format-layer interfaces that declare these methods do not move: internal/format/maven/upload.go:50-52 and internal/format/maven/reconciler.go:47-48. Their test doubles do not move either.

Typing the parameter *sql.Tx is the alternative that does take those seams with it, and it also breaks composition inside RunInTx, which hands its closure a qrm.DB. This issue asks for the run-time assertion.

Scope

  1. One sentinel per store, named and prefixed as docs/dev/database-query-patterns.md prescribes in ### Argument-guard sentinels.
  2. The assertion after the nil-db arm and before the zero-UUID arms, so the pool is refused ahead of any statement.
  3. A connection-pool row in each of the three argument-guard tables, one per store.
  4. The integration subtests that pass client.DB() to these three methods move to a transaction handle. Measured at 32d14b366: 11 call sites, 4 in internal/datastore/maven_repositories_integration_test.go, 3 in internal/datastore/maven_packages_integration_test.go, and 4 in internal/datastore/maven_versions_integration_test.go.

Related

  • !2268 (merged) landed the same guard on NpmRepositoryStore.ReVerifyRepositoryAlive and ContainerRepositoryStore.ReVerifyRepositoryAlive. Its review thread on note 3783024445 carries the argument for the guard.
  • #1122 tracks the six write paths that still commit after a repository tombstone, and it names those two methods as their mechanism. It does not track the handle contract, which is why this is a separate issue.