## Context Every write to a virtual repository's upstream list waits on row locks with no bound. Five management API operations write it: associate, reposition, and dissociate, and the virtual repository create and update carrying `upstream_repository_ids` ([S17 Virtual upstream associations](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/specs/S17-rest-management-api.md#virtual-upstream-associations-phase-6)). They run nine lock statements across the npm, Maven, and container stores, plus the update's write to the virtual repository's own row. The service sets no `lock_timeout` or `statement_timeout`, and no handler on these operations sets a deadline. A write queued behind a conflicting holder keeps its pooled connection until the holder releases. The design pages name the buffered counter drain the likeliest holder. The client sees nothing until the handler answers, then reads an EOF if that comes after the 10-second write deadline. The GA epic https://gitlab.com/groups/gitlab-org/-/epics/22707 takes these operations as delivered Closed Beta foundations, so the issue carries `AR-Blocks::GA`. The two container sites and the npm arm's matching pair came from the security review of gitlab-org/ops/artifact-registry!2778+s. ## What exists on main Line numbers hold at `d7fcf10e6` (2026-10-01). - `internal/datastore/npm_virtual_upstreams.go`: `npmVirtualUpstreamWrite` (`:310`) runs `Associate`, `Reposition`, and `Dissociate` in one `READ COMMITTED` transaction. It locks the virtual child row `FOR UPDATE` first (`:897`), so every write to one list serializes. `Associate` then takes `FOR SHARE` on the upstream's `repositories` row (`:913`), the weakest lock a pending tombstone conflicts with. The comment at `:324` calls every wait unbounded. It landed uncalled, and the npm association routes reached it through gitlab-org/ops/artifact-registry!2735+s. - `internal/datastore/maven_virtual_upstreams.go` and `internal/datastore/container_virtual_upstreams.go`: the same two statements (`:849` and `:865`, `:823` and `:839`) under the same comment (`:273`, `:235`). The Maven store came from gitlab-org/ops/artifact-registry!2780+s. The container store came from the fifteen-part stack opening with gitlab-org/ops/artifact-registry!2778+s. - Each store's reconcile walker judges newly associated entries on an unlocked read, then takes `FOR SHARE` on their `repositories` rows in `(namespace_id, id)` order (`npm_virtual_upstreams.go:935`, `maven_virtual_upstreams.go:887`, `container_virtual_upstreams.go:861`, query names `repositories_select_lock_<format>_virtual_upstream_candidates`). The judging reads, `repositories_select_<format>_virtual_upstream_candidates`, take no lock. The npm walker came from gitlab-org/ops/artifact-registry!2978+s, the Maven walker from gitlab-org/ops/artifact-registry!2977+s, and the container walker from gitlab-org/ops/artifact-registry!2981+s. - `git grep -n -E 'FOR\(pg\.(UPDATE|SHARE)\(\)\)' d7fcf10e6 -- 'internal/datastore/*_virtual_upstreams.go'` returns these nine statements, three per store, and no others. - `internal/datastore/repositories.go`: `RepositoryStore.UpdateVirtual` (`:968`) takes the format's child-row lock through `lockForMutation` (`:1012`), runs the walker (`:1018`), and writes the patch to the virtual repository's own `repositories` row last (`:1025`), all in one transaction from gitlab-org/ops/artifact-registry!2985+s. `RepositoryStore.Create` runs the walker for a create carrying a list (`:2235`). Each format's dispatch record wires that store's own lock and walker into both (`npm_virtual_repositories.go:38-39`, `maven_virtual_repositories.go:39-40`, `container_virtual_repositories.go:39-40`). - `internal/managementapi/handler.go:754-757` routes the npm, Maven, `docker`, and `oci` association arms to the three stores. The five handlers pass `r.Context()` to the store: associate, reposition, and dissociate (`upstream_repositories.go:218`, `:281`, `:308`), create (`create.go:132`), and update (`update.go:375`). The create arm landed in gitlab-org/ops/artifact-registry!2987+s, and the update arm in gitlab-org/ops/artifact-registry!3011+s. - `git grep -n -E 'context\.WithTimeout|context\.WithDeadline|WithTimeoutCause|http\.TimeoutHandler' d7fcf10e6 -- 'internal/managementapi/*.go' 'internal/server/*.go' ':!*_test.go'` returns four hits, none on these paths: a counter write, two audit writes, and a list-authorization drain. `internal/server/server.go:129-131` sets only `ReadTimeout`, `WriteTimeout`, and `IdleTimeout`, which `config.example.yaml` ships at `10s`, `10s`, and `120s`. None of the three cancels a request context. - `internal/managementapi/resolve.go:218`, `logAndWriteInternalError`: returns with no log line and no body for an error matching `context.Canceled` or `context.DeadlineExceeded`, and logs any other error at `ERROR` behind the `500` envelope. The default arms of `writeUpstreamMutationError`, `writeCreateStoreError`, and `writeVirtualUpdateError` end there. `writeVirtualUpdateError` (`update.go:786`) first logs a `WARN` for either context error and writes no status. - `internal/managementapi/upstream_repositories.md`, `## No handler-side deadline on the mutations`: the routes arm no deadline because a fired one has no settled answer, and `server.timeouts.write` cancels nothing. - `internal/lifecycle/tombstone.go:57`, `TombstoneTimeout`: 10 seconds on the tombstone transaction's row-lock wait. `publishCommitTimeout` (npm publish) and `manifestPersistTimeout` (OCI manifest persist) reuse it and answer `500` past it. `boundedFailure` (`:167`) re-reports that deadline without `context.DeadlineExceeded` in the chain, so the delete logs it. `armTombstoneResponseDeadline` (`internal/managementapi/delete.go:175`) extends the write deadline so the answer reaches the client. - `cmd/artifact-registry/operator.go:184`, `statementBoundStore`: puts `operatorStatementTimeout`, one minute, on every row-scoped operator store call through `context.WithTimeout`. - Four design-page passages record the absence and cite this issue: `## This store's lock waits are unbounded` in `npm_virtual_upstreams.md`, `## No lock wait is bounded` in `container_virtual_upstreams.md` (the original description's `## Neither lock wait is bounded`), the share-lock paragraph under `### Judged on an unlocked read, then share-locked in key order` in `maven_virtual_upstreams.md`, and the `UpdateVirtual` paragraph in `repositories.md` (`:340-355`). `git grep -n 'work_items/1401' d7fcf10e6 -- internal/` returns those four and `internal/managementapi/upstream_repositories.md:262`. - The container and Maven pages record what an ended context does: pgx's close sends `CancelRequest`, and PostgreSQL cancels the waiting statement while the holder keeps its lock. https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/1501 measured the waiting backend gone 8 ms after a 2-second deadline fired. - The integration suites hold these locks from a second transaction, such as the `SET LOCAL lock_timeout` probes in `internal/datastore/npm_virtual_upstreams_integration_test.go` and `lockWaitTimeout` in `internal/datastore/repositories_integration_test.go:2572`. - S17 `### Virtual upstream associations (Phase 6)`: sets which write takes which lock and in what order (`:1247-1253`, `:1284`), and the audit and `WARN` rules for a write whose client left during `COMMIT` (`:1296-1306`). It names no bound, and no answer for a fired one. - S17 `## Error Cases`: `500 internal_server_error` for an unexpected failure on every endpoint (`:1768`), and `503` on these routes only when IAM or GLAZ fails (`:1757`). In `api/openapi/v1.yaml`, `createRepository`, `updateRepository`, `associateUpstreamRepository`, `updateUpstreamRepository`, and `dissociateUpstreamRepository` each declare `500` and a `503`, and none declares `504`. Each `503` describes an authorization failure, and `updateRepository`'s also covers the URL change's job backend. - S17 `## Acceptance Criteria` 80, 81, and 147: concurrent association writes, an associate against the virtual repository's delete, and the update's lock order. Each settles as one writer observing the other or both applying in serial order. ## Change Every wait a virtual upstream list write takes ends within one bound, for every format and on all five operations: the nine store statements and `UpdateVirtual`'s patch write. A fired bound rolls the write back, frees every lock its transaction held, and answers a status line and the S17 error envelope instead of an empty `200`. A client that hangs up keeps today's answer. The bound's mechanism and number come from gitlab-org/ops/artifact-registry#978+s. If that mechanism is a context deadline, the handler sets it, because only the handler knows the request's budget. The S17 DRI rules on the outcomes that criteria 3, 6, and 7 name. The virtual repository's own delete stays with gitlab-org/ops/artifact-registry#902+s. ## Acceptance criteria 1. Each of these waits ends within the bound for npm, Maven, `docker`, and `oci`, on every path that reaches it: the child-row `FOR UPDATE` that `Associate`, `Reposition`, `Dissociate`, and `UpdateVirtual` take, the associate's `FOR SHARE` on the upstream's `repositories` row, the walker's `FOR SHARE` on a create or update carrying `upstream_repository_ids`, and `UpdateVirtual`'s patch write to the virtual repository's own `repositories` row. Every statement the `FOR(pg.UPDATE|SHARE)` grep in "What exists on main" returns at the fix's head falls under this criterion, including any added after `d7fcf10e6`. 1. For each wait in criterion 1, an integration test holds the conflicting lock from a second transaction past the bound. It asserts that the call returns an error within the bound plus a tolerance the test names, that the junction and `repositories` rows are unchanged, and that `pg_locks` shows the waiting session holding no lock once the call returns. That last check fails a handler that stops waiting while the store call runs on. The holders are `FOR UPDATE` on the child row, the counter drain's `FOR NO KEY UPDATE` on the upstream's `repositories` row for both share locks, and `FOR SHARE` on the virtual repository's `repositories` row for the patch write, which a `PATCH` carrying `visibility` or `description` reaches. A create whose bound fired leaves no `repositories` row, and a second create of that name, carrying no list, then answers `201`. One test per path releases the holder inside the bound and asserts the write commits. 1. On the five operations, a fired bound answers a status line and the S17 error envelope, and books no audit event. It logs the failure at `ERROR` with `namespace_id`, and with `repository_id` on the four operations that address an existing repository. A handler test per operation asserts each part. S17 names no status for a fired bound. Decision for the S17 DRI (`hswimelar`, per the S17 row of `docs/roadmap/beta.md`): the `500 internal_server_error` that S17's unexpected-failure row and all five operations already declare, or a retryable status like the `503` that ADR-025 gives its own foreground lock timeout. A `503` or a `504` amends S17's `## Error Cases` and the five operations in `api/openapi/v1.yaml` before the code, because every `503` there describes another failure today. 1. A request whose client hangs up keeps today's answer on the five operations: the handler writes no status and no body and logs no `ERROR` line, and the update keeps the `WARN` that `TestUpdateHandler_UpstreamList_AbandonedStoreCallLeavesAWarn` pins. A handler test per operation runs a canceled request context and a fired bound against the same store fake, and asserts the answers differ. 1. With `server.timeouts.write` at the `10s` that `config.example.yaml` ships, the status line from criterion 3 reaches the client. A test sends one operation through an `http.Server` with that write timeout while a holder keeps the lock past the bound. A handler that answers after the write deadline fails this criterion, unless it extends the deadline the way `armTombstoneResponseDeadline` does for the delete. 1. S17 acceptance criteria 80, 81, and 147 settle concurrent writers in two ways: one observes the other, or both apply in serial order. A bound adds a third outcome, a waiter that gives up with nothing applied. S17 says nothing about it. Decision for the S17 DRI: whether those criteria admit it. Either way, the lock order stays as S17 states it, and `TestRepositoryStore_UpdateVirtual_TakesTheDeletesLockOrder` and the stores' concurrency tests pass. 1. Under a context deadline, a bound that fires while `COMMIT` applies can leave the change committed while the store reports the deadline, because pgx runs `COMMIT` under the transaction's context. S17 states that outcome only for a client that hangs up. Decision for the S17 DRI: what criterion 3's answer and log line say in that case. A `SET LOCAL lock_timeout` leaves no such case, because the server ends the wait and reports the failure itself. 1. A fired bound books one sample under the waiting statement's query name, which `recordQueryOutcome` (`internal/metrics/database.go:147`) files by the error: `gitlab_artifact_registry_database_query_cancellations_total{cancellation_reason="deadline"}` for a context deadline, and `gitlab_artifact_registry_database_query_errors_total` for SQLSTATE `55P03`. A test reads the counter for one wait. 1. Each text that records the absence states the bound the change leaves instead: the comments at `npm_virtual_upstreams.go:324`, `maven_virtual_upstreams.go:273`, and `container_virtual_upstreams.go:235`, the four design-page passages, and `## No handler-side deadline on the mutations`. At `d7fcf10e6`, the design pages cite this issue and the three store comments do not. At the fix's head, `git grep -n 'work_items/1401' -- internal/` returns nothing. The comment at `repositories.go:2114` describes the repository delete's waits and stays with gitlab-org/ops/artifact-registry#902+s. ## Specs and ADRs - [S17 Virtual upstream associations (Phase 6)](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/specs/S17-rest-management-api.md#virtual-upstream-associations-phase-6): which write takes which lock and in what order, and the audit rule for a write abandoned during `COMMIT`. - [S17 Non-hosted repository CRUD (Phase 6)](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/specs/S17-rest-management-api.md#non-hosted-repository-crud-phase-6): the create and update bodies that carry `upstream_repository_ids`. - [S17 Error Cases](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/specs/S17-rest-management-api.md#error-cases): the `500` for an unexpected failure and the authorization `503`, the two answers criterion 3 chooses between. - [S17 Acceptance Criteria](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/specs/S17-rest-management-api.md#acceptance-criteria): 80, 81, and 147, the serialization outcomes criterion 6 asks about. - [ADR-009 Repository-level APIs](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/adr/009_api_design.md#repository-level-apis): the association routes. - [ADR-004 Entity Count Limits](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/adr/004_data_and_application_limits.md#entity-count-limits): 20 upstreams per virtual repository, which caps the share locks one walker holds while it waits. - [ADR-025 The blob-row lock serializes every race GC can lose](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/adr/025_garbage_collection.md#the-blob-row-lock-serializes-every-race-gc-can-lose): the one ADR that bounds a foreground lock wait, at five seconds with a retryable `503`, for the blob-row lock alone. It does not govern these rows. - [Database guide, PgBouncer compatibility](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/dev/database.md#pgbouncer-compatibility): transaction pooling stays a supported topology, and under it a session-level `lock_timeout` does not follow the client from one transaction to the next. ## Dependencies - https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/978 comes before the first `fix` MR. It decides which of three mechanisms the service uses for a row-lock wait budget: a per-statement budget inside `internal/datastore`, a context deadline on each store call as `statementBoundStore` sets on the operator's, or a transaction-scoped `SET LOCAL lock_timeout`. It also decides where the number lives and which statements it covers. This issue is the per-site item for the association stores, as the delete and parent-gate items below are for theirs. It cannot close before that record lands. If the record declines a budget, the owner decides whether this issue closes on it, with criterion 9's texts pointing at the record. - https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/252 comes before the first `fix` MR when the mechanism is a context deadline. A fired deadline reaches `logAndWriteInternalError` as `context.DeadlineExceeded`, which it answers with no status and no body, so arming one first turns a hang into an empty `200`. Under `SET LOCAL lock_timeout` the order lapses: the wait fails with SQLSTATE `55P03`, which already reaches the logged `500`. The fix can instead re-report its own deadline as `boundedFailure` does for the tombstone, at the cost of one more classifier for the classification change to update. - https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/1457 has no required order. It proposes one request-context deadline below `server.timeouts.write` on every management API route, and says it would also bound these waits. Under a context deadline, the two are one mechanism at two scopes. Decision for this issue's owner: a per-call bound inside that request deadline, or the request deadline alone. - https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/1473 has no required order. It adds a `WARN` for a create or route mutation abandoned during `COMMIT`, on the error arms criterion 4 changes. Whichever lands second keeps the hang-up arm apart from the fired-bound arm. - https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/1494 has no required order. Its third item asks this issue to name the update handler, the waits of `UpdateVirtual`, and the create's share locks, which criterion 1 does. - https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/902 has no required order. It bounds the repository `DELETE`'s hard-delete phase, the `deleteRepositoryTx` transaction in which a virtual repository's delete also waits on its child row. Closing it leaves every wait in criterion 1 unbounded. - https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/648 has no required order. It bounds the parent gate's `FOR SHARE` for the first request path that calls a remote create. ## First step The decision record for https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/978 comes first, as a `docs/dev/` page naming criterion 1's waits among the statements it covers, because the error each test asserts and criterion 7's `COMMIT` case follow from the mechanism. The S17 DRI's rulings on criteria 3, 6, and 7 can land beside it, as one S17 amendment or as a recorded ruling that S17 needs none. The first `fix` MR then applies the bound in the five handlers, or in the store transactions if the record picks `SET LOCAL lock_timeout`, with criterion 2's tests first. ## Original description ## The gap Each virtual-upstream association store runs one `READ COMMITTED` transaction per write and takes two row locks inside it. Neither wait has a bound. - The virtual child row, `SELECT ... FOR UPDATE`, taken first so every write against one upstream list serializes. - The candidate upstream's `repositories` row, `SELECT ... FOR SHARE`, the weakest lock a pending tombstone conflicts with. The service sets no `lock_timeout` and no `statement_timeout`, so each wait lasts as long as its conflicting holder does, and holds a pooled connection while it lasts. Nothing above the store supplies a deadline either. `internal/server/server.go` sets `ReadTimeout`, `WriteTimeout`, and `IdleTimeout`, and none of the three cancels a request context. The management API registers no `http.TimeoutHandler` and no per-request deadline middleware, and no association handler wraps its store call in a `context.WithTimeout`. ## The three stores Each store records the absence in its own text, and none of them points at a tracking item for these two statements. - npm, `internal/datastore/npm_virtual_upstreams.go`, in the comment above the two reads. The store is on `main` at c7a28c0d6 with no production caller, and gitlab-org/ops/artifact-registry!2735+s wires it to five association routes. - Container, `internal/datastore/container_virtual_upstreams.go`, plus `internal/datastore/container_virtual_upstreams.md` under `## Neither lock wait is bounded`. Both arrive in gitlab-org/ops/artifact-registry!2778+s. - Maven, the same two statements in the store S17 Phase 6 Step 41 adds. The parent gate in gitlab-org/ops/artifact-registry#648+s is a guard to put in place because no request path reaches it. These six statements are the other case: each pair goes live on request-reachable management API routes the moment its MR merges. ## Why this is neither #902 nor #978 gitlab-org/ops/artifact-registry#902+s bounds the repository `DELETE`'s hard-delete phase, a different statement on a different table in a different transaction. Closing it leaves all six statements here unbounded. gitlab-org/ops/artifact-registry#978+s decides the repo-wide policy, which of three shapes the service uses for a row-lock wait budget. This issue is the per-site item for the association stores, the way #902 is for the delete and #648 is for the parent gate. It names three sites that decision has to cover, and it cannot close before the decision lands. ## What the caller owes, once the shape is decided - A deadline set before the call reaches the store, at the handler, because only the handler knows the request's budget. `statementBoundStore` in `cmd/artifact-registry/operator.go` is the existing example. - A server-side classification when that deadline fires. `logAndWriteInternalError` in `internal/managementapi/resolve.go` returns without logging and without a body for `context.DeadlineExceeded` today, which is gitlab-org/ops/artifact-registry#252+s. ## Done when All three stores' association writes carry the bound #978 settles on, the three texts above cite this issue rather than restating the gap, and a fired deadline reaches the client as a server-side timeout instead of a silent success. ## Where this came from The automated AppSec review of gitlab-org/ops/artifact-registry!2778+s raised the two container sites and observed that the npm arm carries the same pair. Neither is a new class, and no item tracked either one.