## The window A hosted repository accepts a client write, and that write resolves the repository while it is live. A destructive repository `DELETE` then stamps `soft_deleted_at` on the repository's row and enqueues the purge job, both in one transaction (`lifecycle.Tombstoner.Tombstone`). When the write commits after that stamp, three things happen together. The write answers its own success status. The purge then removes the rows the write created. No error reaches the writer. A write path reaches this window when all three of these conditions hold: - It writes rows the purge removes. - It answers the caller a success status. - No statement re-reads the repository's own row between that path's resolve and its commit. The third condition is the one a fix changes. The first two describe a write shape, and they stay true of a path after it is fixed. ## What a client sees The client gets `200`, `201`, `202`, or `204`, and the status depends on the operation. The client is then told that the artifact, tag, or blob exists. The artifact never resolves. A retry does not recover it, because the repository itself is gone. This is the difference from the accepted `delete_all` case in `docs/specs/S17-rest-management-api.md`. There the repository survives, so the caller can read the collection again and publish again. Here the success status names something that can never exist. ## What is already closed, and by what `MavenRepositoryStore.ReVerifyRepositoryAlive` closes the Maven upload path. `internal/format/maven/upload.go` calls it inside the upload's write transaction, immediately before the upsert it guards. The statement joins `repositories` on `soft_deleted_at IS NULL` and takes `FOR SHARE`, so the transaction holds that verdict to its own commit. A miss returns `datastore.ErrNotFound`, and the upload's `writeCommitOutcome` answers it as `404`. Merge request !2268 extends the same mechanism to two more paths. While !2268 is open, all eight write paths named in this issue reach the window. Once it merges, an npm publish and a container manifest push each answer `404`, and the six paths under `## What remains` are what is left. The two paths !2268 closes work like this: - `NpmRepositoryStore.ReVerifyRepositoryAlive` runs as the first statement of the transaction in `NpmPublishCommitter.CommitPublish`. - `ContainerRepositoryStore.ReVerifyRepositoryAlive` runs as the first statement of the transaction in `ContainerManifestPersister.PersistManifest`. Both wrap a miss as `datastore.ErrRepositoryConcurrentlyDeleted`. That sentinel is deliberately distinct from `ErrNotFound`, so a caller cannot read a lost race as a name the namespace never held. `npm.PublishHandler.writeCommitError` and `oci.ManifestHandler.writePersistError` each map that sentinel to the repository-tier `404`. Each of the two also bounds the new wait. The handler arms a `context.WithTimeout` at `lifecycle.TombstoneTimeout` (10 seconds), because the service sets no `lock_timeout` and no `statement_timeout`. Without that bound the opening `FOR SHARE` has no cap at all. Those two paths need no further work for this window. ## Why the cut was narrow The two-path cut is an accepted scope decision, not an oversight. The operator accepted !2268 on the condition that the remainder is recorded rather than lost. This issue is that record, and filing it was the condition. ## What remains While merge request !2332 is open, six write paths satisfy all three conditions of `## The window`, five of them client-protocol operations and one a management-API operation. Once it merges, the container blob finalize and the cross-repository blob mount's link arm each answer `404`, and four are left: the npm dist-tag `PUT`, the npm deprecate, the blob upload initiate, and `upsertContainerTag`. The mount's `202` fallback arm is not a fifth: `mountFallback` opens its upload session through `CreateSession`, which is the blob upload initiate's own write, so that arm reaches the window on the initiate's terms until the initiate is re-verified too. The table below enumerates the write shapes that reach the window rather than the paths still open, so all six rows stand. `docs/specs/S20-a-lifecycle-closed-beta.md`'s `## Error Cases` row on this window enumerates the same set, and the `deleteRepository` description in `api/openapi/v1.yaml` states it for callers. | Write path | Success status | Commit transaction | Rows the purge removes | | --- | --- | --- | --- | | npm dist-tag `PUT` | `204` | `datastore.NpmDistTagTxWriter.SetTag` | the `npm_tags` row | | npm deprecate | `200 {"ok": true}` | `datastore.NpmVersionDeprecateWriter.DeprecateVersions` | the `npm_versions` rows the update rewrote | | container blob finalize | `201` | `datastore.ContainerBlobLinker.LinkBlob` | the `container_blobs` row with its `blob_storage_attachments` row, and the `container_images` row | | cross-repository blob mount, link arm | `201` | `datastore.ContainerBlobLinker.LinkBlob` | the same rows as the finalize | | cross-repository blob mount, `202` fallback arm | `202` | none — `CreateSession`, the blob upload initiate's own write | the `container_images` row | | blob upload initiate | `202` | none — one statement on the pool | the `container_images` row | | `upsertContainerTag` | `201` or `204` | `datastore.ContainerTagUpserter.UpsertTag` | the `container_tags` row | A remote repository answers `405` to every client write, so no client write into one reaches this window. Its cache rows are filled by the read path. Losing one to the purge costs a re-fetch, not an artifact the caller was told it had. A virtual repository is out one step earlier. `RepositoryStore.Delete` applies `repositoryDeleteKindGate`, and the gate refuses the virtual kind, so no `destructive=true` request stamps one. A window that opens with a tombstone then has no start. ### npm dist-tag `PUT` `PUT /{slug}/npm/{repository_name}/-/package/{package_name}/dist-tags/{tag}` - **Resolve.** `npm.Middleware.ServeHTTP` calls `RepositoryResolver.Resolve` on the pool and puts the `Resolution` in the request context. The `Resolution` carries `NpmRepositoryID`. - **Between.** `npm.DistTagsHandler.servePut` runs `validatePutName`, `readDistTagVersion`, `resolveWritePackage` (`NpmPackageByName`), `distTagCapCheck`, and `resolveWriteVersion` (`NpmVersionByPackageAndVersion`). Each runs on the pool. None of them reads `repositories`. - **Commit.** `datastore.NpmDistTagTxWriter.SetTag` opens one transaction through `RunInTx`. Inside it, `setTagTx` runs `RotatePackumentRebuildTokenTx`, `ActiveNpmVersionExistsTx`, `upsertTagWithinCap`, and `ForceExpireNpmMetadataTx`. No statement in that transaction reads `repositories`. - **Still open.** Yes, read at the head of !2268's branch. The in-transaction re-check this path already has is version liveness, not repository liveness. `ActiveNpmVersionExistsTx` catches a concurrent unpublish of the target version and returns `ErrParentNpmVersionMissing`. It cannot see a repository tombstone. ### npm deprecate `PUT /{slug}/npm/{repository_name}/{package_name}`, with a body that carries no `_attachments` This route is shared with an npm publish. `newPublishOrDeprecateHandler` splits the two by peeking for the top-level `_attachments` key. The publish arm and the deprecate arm are separate handlers behind one `mux.Handle` pattern. The re-verification !2268 adds sits in the publish arm's committer, so it does not reach the deprecate arm. - **Resolve.** The same `npm.Middleware` resolve, then `NpmPackageByName(ctx, res.NamespaceID, res.NpmRepositoryID, packageName)`. - **Between.** `npm.DeprecateHandler.ServeHTTP` decodes the body and runs `collectUpdates`, which resolves every targeted version and patches its stored `package_json`. All of it runs on the pool. - **Commit.** `datastore.NpmVersionDeprecateWriter.DeprecateVersions` opens one transaction with `BeginTx` at `READ COMMITTED`. Inside it: `RotatePackumentRebuildTokenTx`, `BatchUpdateNpmVersionPackageJSON`, `ForceExpireNpmMetadataTx`, then `Commit`. No statement reads `repositories`. - **Still open.** Yes. What is lost here is an `UPDATE`, not an `INSERT`. The purge removes the `npm_versions` rows the deprecate rewrote, so the deprecation and the versions go together. The client still gets `200 {"ok": true}`. ### container blob finalize `PUT /v2/{slug}/container/{repository_name}/{image_name}/blobs/uploads/{id}?digest=<digest>`, and the single-request `POST /v2/{slug}/container/{repository_name}/{image_name}/blobs/uploads/?digest=<digest>` - **Resolve.** `oci.UploadHandler.ServeHTTP` calls `h.resolver.ResolveRepository` directly. The handler is self-contained, so it resolves even when it is mounted without the dispatcher's `resolveAndGate`. - **Between.** `handlePut` resumes the session, validates the digest and the `Content-Range`, and streams the final chunk. `handleSinglePost` calls `CreateSession` and streams the whole body. Both then call `finalizeAndRespond`, which calls `oci.CompleteUpload`. - **Commit.** `CompleteUpload` has three write points, and only the last is a transaction: 1. `session.Commit` creates the `blob_storage_blobs` row and removes the session. 2. `images.UpsertContainerImage` runs one statement on the pool, through the `containerImageUpserter` adapter in `cmd/artifact-registry/wire_oci.go`. 3. `linker.LinkBlob` opens one transaction over the `blob_storage_attachments` insert and the `container_blobs` insert. - **Still open.** While !2332 is open, yes: `datastore.ContainerBlobLinker.LinkBlob` reads no `repositories` row, and its seam signature carries `namespaceID`, `containerImageID`, and the digest — no container repository id. Once it merges, `LinkBlob` takes a `containerRepositoryID` and opens its transaction with `ContainerRepositoryStore.ReVerifyRepositoryAlive`, a `SELECT ... FOR SHARE OF repositories` that runs before either insert and holds the row to the commit. A refusal is `datastore.ErrRepositoryConcurrentlyDeleted`, which `finalizeAndRespond` answers as the repository-tier `404 NAME_UNKNOWN` inside `blobLinkTimeout`, which is `lifecycle.TombstoneTimeout` and ten seconds. Past that bound the deadline error carries no sentinel, so the answer is `500`, or `499` with no envelope once the client has already left. ### cross-repository blob mount `POST /v2/{slug}/container/{repository_name}/{image_name}/blobs/uploads/?mount=<digest>&from=<source>` - **Resolve.** The destination is resolved by the same `oci.UploadHandler.ServeHTTP` resolve. `resolveMountSource` then resolves the source repository and runs `mountAuthorizer.CanPullSource`. - **Between.** `handleMount` parses the mount digest and the `from=` value, then calls `oci.MountBlob`, which runs `finder.FindBlobInRepository` against the source. - **Commit.** `MountBlob` then repeats the finalize's last two write points: `images.UpsertContainerImage` on the pool, then `linker.LinkBlob` in one transaction. - **Still open.** While !2332 is open, yes, and it shares `LinkBlob` with the finalize, so one fenced linker closes both `201` arms. Once it merges, a mount that links the blob answers `404 NAME_UNKNOWN` from `handleMount`, in the same repository-tier envelope the finalize uses. The `202 source_missing` fallback stays open: `mountFallback` opens its session through `CreateSession`, which is the blob upload initiate's own write, so that arm reaches the window on the initiate's terms until the initiate is re-verified too. The read of the source repository's row does not fence the destination's write. That holds even for a mount whose `from=` names the destination, where the re-read is the destination's own row. A tombstone there only selects the `source_missing` fallback, which opens a session and answers `202`. ### blob upload initiate `POST /v2/{slug}/container/{repository_name}/{image_name}/blobs/uploads/`, with no `?mount=` and no `?digest=` - **Resolve.** The same `oci.UploadHandler.ServeHTTP` resolve. - **Between.** `handlePost` reads the query, finds neither parameter, and calls `handleInitiate`. - **Commit.** `oci.CreateSession` calls `images.UpsertContainerImage`, which is one `INSERT ... ON CONFLICT DO NOTHING RETURNING` on the pool handle, in its own implicit transaction. It then calls `store.NewSession`. - **Still open.** Yes, and this one is open differently from the other five. This path has no transaction to place a re-verification inside. `ReVerifyRepositoryAlive` on a pool handle releases its `FOR SHARE` lock when the statement ends, so it fences nothing. The `oci.ImageUpserter` seam in `internal/format/oci/store.go` carries no `qrm.DB` parameter, so no caller can pass it a transaction today. The datastore method behind it, `ContainerImageStore.UpsertContainerImage`, does take a `qrm.DB`. This is also the member whose lost row is the least visible to a client. The row the purge removes is an empty `container_images` row, and the `202` names an upload session rather than an artifact. Whether that difference changes the scope for this member is a decision for whoever closes this issue. ### `upsertContainerTag` `PUT /api/v1/{slug}/repositories/{repository_name}/{format}/images/{image_id}/tags/{tag_name}` `registerArtifactWriteRoutes` in `internal/managementapi/handler.go` registers it. It is the only artifact write that function registers that reaches this window. Every other route there is a delete or a bulk delete. A delete removes rows at its own caller's request, so what the purge later finds of theirs is nothing the caller was told it had. - **Resolve.** `resolveContainerArtifactRepository` reads the `repositories` row. `hostedContainerTagUpsert` then calls `resolveContainerHostedRepositoryID`, which runs `FindContainerRepositoryID` against `container_repositories` alone. It then calls `findContainerImageForRequest`. - **Between.** The handler validates the tag name against `containerTagNamePattern`, decodes the body, and parses the digest with `parseCanonicalBodyDigest`. - **Commit.** `datastore.ContainerTagUpserter.UpsertTag` opens one transaction through `runInSQLTx`. Inside it: `resolveAndLockManifest` (a `FOR UPDATE` lock on the manifest row), `tags.FindByName`, `enforceTagCap`, and `writePointer`. No statement reads `repositories`. - **Still open.** Yes. This is the member closest in shape to the two !2268 closes. `hostedContainerTagUpsert` already holds `containerRepositoryID` in scope, so the id a re-verification needs does not have to be found. ## The fix shape, and where the six differ Three of the six hold all their repository-scoped rows behind one committing transaction, so the merged mechanism transfers to them directly. Two more hold part of their rows behind one transaction and write the rest on the pool ahead of it. One has no transaction at all. ### The three with one transaction npm dist-tag `PUT`, npm deprecate, and `upsertContainerTag` each hold their rows behind one transaction. For each, the shape is: 1. Thread the format's repository id through the write seam, because none of the three carries it today. `npm.NpmDistTagWriter.SetTag` takes `namespaceID`, `npmPackageID`, `npmVersionID`, the tag, and the cap. `npm.NpmVersionDeprecator.DeprecateVersions` takes `namespaceID`, `npmPackageID`, and the updates. `managementapi.ContainerTagUpserter.UpsertTag` takes `namespaceID`, `containerImageID`, the tag name, and the digest. 2. Call the format's `ReVerifyRepositoryAlive` inside the transaction that writes the operation's own rows. 3. Wrap a miss as `datastore.ErrRepositoryConcurrentlyDeleted`, not as `ErrNotFound`. 4. Add a repository-tier arm to the handler's error mapping, because none of the three has one today. `npm.DistTagsHandler.writeSetTagError` maps `ErrNotFound` to `404 tag_not_found`. `npm.DeprecateHandler` routes every commit failure through `writeCollectUpdatesError`. `managementapi.writeContainerTagUpsertError` has arms for `ErrManifestNotFoundInImage` and `ErrTagLimitExceeded`, and a `500` for everything else. Without a new arm the sentinel falls to that `500`. 5. Bound the wait on the `repositories` row lock with a `context.WithTimeout`, as the two closed paths do. Where the call sits inside the transaction is an open choice with a measurable cost, and the two merged precedents differ on it. The Maven upload places the re-verification as the last statement before the upsert it guards, which holds the lock for fewer statements. The two paths in !2268 place it first in the transaction, which gives a parent-before-child lock order against a repository-first delete cascade. Both are in-tree, so either can be cited. ### The two blob paths A container blob finalize and a cross-repository blob mount share `LinkBlob`, and both run `UpsertContainerImage` on the pool before it. A blob upload initiate has only that pool statement. Two options carry different costs, and this issue does not choose between them. **Option A — fence `LinkBlob` alone.** Thread the container repository id through the `oci.blobLinker` seam and re-verify inside `LinkBlob`'s transaction. This closes the finalize and the mount together, and it is the smaller change. The client then gets `404` instead of `201`, which is the outcome that matters most. The `201` is what tells the client the blob exists. The cost is residue. The `blob_storage_blobs` row from `session.Commit`, and any `container_images` row from the earlier upsert, are already committed. Both stay behind as orphans for reclamation. It leaves a blob upload initiate open, because that path never reaches `LinkBlob`. **Option B — fence the image write too.** Give `oci.ImageUpserter` a `qrm.DB` parameter, or add a datastore method that upserts the image under a re-verified repository. Then fold the initiate's write into a transaction of its own. This closes all three container blob paths on one mechanism. The cost is a wider seam change across `internal/format/oci/store.go`, `internal/format/oci/upload.go`, and `cmd/artifact-registry/wire_oci.go`, and it adds a transaction to the initiate, which has none today. ## What the locking form costs `internal/datastore/repository_parent_gate.go` records the cost of holding `FOR SHARE` on a `repositories` row, and every option above pays it. `FOR SHARE` is the weakest lock that conflicts with the tombstone. `RepositoryStore.SoftDelete` is a conditional `UPDATE` of that row, and it takes `FOR NO KEY UPDATE`. `FOR SHARE` therefore waits for the tombstone's verdict instead of reading past it from its own snapshot. `FOR KEY SHARE` does not conflict with `FOR NO KEY UPDATE` at all, so a gate that takes it never waits for a pending tombstone and the window stays open. Holding `FOR SHARE` blocks every write to that row: the repository's counter writes, `RepositoryStore.Update`, the hard delete in `RepositoryStore.Delete`, and the tombstone. The wait is short today by circumstance rather than by design. Every current writer of a `repositories` row runs one statement on the pool, in its own implicit transaction. The buffered counter pipeline in `docs/specs/S22-storage-accounting.md` ends that circumstance. It commits a whole chunk of scopes in one transaction, and takes `FOR NO KEY UPDATE` on every `repositories` row in the chunk. It holds those locks across Redis round trips. A write waiting on `FOR SHARE` then waits for the chunk. The `lifecycle.TombstoneTimeout` bound the two closed paths arm is what keeps that wait finite, and any new path needs the same bound. One shape has to be avoided. A caller that takes `FOR SHARE` and later `UPDATE`s the same `repositories` row in the same transaction has to upgrade past every other holder. Two such callers on one parent then deadlock, where without the shared lock they queue. None of the six paths writes its repository's row inside its commit transaction, so none of them hits that shape as written. ## A cheaper partial, and what it does not do An unlocked re-read of the repository with `soft_deleted_at IS NULL`, as the last statement before the first write, sees a committed tombstone under `READ COMMITTED`. It collapses the exposure from the whole request duration to the few statements between the read and the commit. It costs one partition-pruned point `SELECT` per write, with no new lock and no new conflict edge. It does not close the window. A tombstone that commits inside the residual still yields a success status. Its other cost is that it leaves the project with more than one per-format answer to one race, and nothing recording why they differ. ## Related - Issue #901 carries the window's first statement and the asymmetry with the accepted `delete_all` case. It also names a position in merge request !1942 that whoever closes this issue reads first. - Issue #1044 is a different defect: a Maven publish committing a `maven_versions` row under a **package** tombstone. That one leaves a residue row and the client gets a `404`. This one is a **repository** tombstone and the client gets a success status. - Issue #944 covers the observability side of the same row lock, not the defect. _This is a bot message 🤖 — /smurfit_