## What this covers Three entries in [S16's Follow-ups](https://gitlab.com/gitlab-org/ops/artifact-registry/-/blob/main/docs/specs/S16-container-remote.md#follow-ups) are one argument with three surfaces, and none of them carried a tracking issue. They are the only S16-owned entries in that section without one. The argument: `last_downloaded_at` is a key column of a btree index on the tables that carry it, so an `UPDATE` of that column can never be a heap-only tuple. It writes a new heap tuple plus an entry in every index the live row belongs to. On a read path that is the most frequent operation the service serves, an unconditional bump puts that amplification on the hottest path there is. S16 acts on the argument for one of its two routes — the manifest bump is throttled on the manifest row's own freshness — and leaves three things open. ## 1. The container blob route bumps unthrottled `ContainerRemoteCacheStore.bumpBlobImageDownloadedAt` in `internal/datastore/container_remote_download.go` issues an unconditional `UPDATE container_remote_images SET last_downloaded_at = GREATEST(last_downloaded_at, NOW())` per blob read. Every layer of one image resolves to the same row, `NOW()` always advances so the write is never a no-op, and `last_downloaded_at` is a key column of `index_container_remote_images_on_ns_id_repo_id_last_dl_at`, so each write is non-HOT and touches all three of the table's indexes. Parallel layer pulls serialize on that row lock. This is the higher-volume of the two routes, because a multi-layer image pull is mostly blob reads. A ten-layer image pulled 100 times a minute is roughly 17 row versions per second on one row plus 51 index entries. The fix S16 names: key the skip on the image row's own `last_downloaded_at`, since a blob read has no manifest row whose freshness it could check. The column is there — the throttle is deferred, not impossible, and the code comment now says so, which is why it needs a number to point at. ## 2. The manifest throttle's effect is unmeasured A manifest read whose bump the staleness window suppresses produces no signal. It would show up only in Postgres's own `n_tup_upd`, which S16 does not own. `gitlab_artifact_registry_oci_buffered_counter_updates_total{column,result}` does not close this. It meters what happened to each dispatched write — delivered, failed, panicked, or shed before it ran — and a throttled read is none of those: the statement executes, matches no row, and returns nil, so it meters `result=ok` exactly as an applied bump does. What is missing is the ratio between applied and skipped, which is the only number that says whether the staleness window is doing anything. Without it, `last_downloaded_at_staleness_window` cannot be tuned on evidence. S16 notes that `auth_discovery_writes_total` exists for the identical reason: a write that never applies is otherwise invisible. ## 3. The same argument applies to shipped npm and Maven bumps — with a correction S16 records that the argument "applies unchanged to unconditional bumps already shipped elsewhere", and names five tables. **Two of the five do not hold**, and the difference changes the work, so the spec text needs correcting alongside this. Checked against `internal/datastore/migrations/structure.sql` and each store method: | Table | Index on the column | Writer | Amplified today? | |---|---|---|---| | `npm_versions` | `index_npm_versions_on_ns_id_pkg_id_last_downloaded_at` | `NpmVersionStore.BumpLastDownloadedAt`, executes via `instrumentExec` | **Yes** | | `npm_packages` | `index_npm_packages_on_ns_id_repo_id_last_downloaded_at` | `NpmPackageStore.BumpLastDownloadedAt`, executes | **Yes** | | `maven_remote_versions` | `index_maven_remote_versions_on_ns_id_pkg_id_last_downloaded_at` | `MavenRemoteCacheStore.bumpLastDownloadedAt`, executes | **Yes** | | `container_remote_images` | `index_container_remote_images_on_ns_id_repo_id_last_dl_at` | this spec's blob route, unthrottled | **Yes** — surface 1 above | | `maven_versions` | `index_maven_versions_on_ns_id_pkg_id_last_downloaded_at` | `MavenPackageStore.BumpAccessTimestamps` discards both `UPDATE`s into `_` and returns nil | **No — no write happens** | | `maven_packages` | `index_maven_packages_on_ns_id_repo_id_last_downloaded_at` | same non-executing stub | **No — no write happens** | So the claim holds on three of the five tables S16 names, not five. `maven_versions` and `maven_packages` carry the index but their writer is a `TODO(s18-buffered-counters)` stub that executes nothing, so they take no amplification today. That is the more useful framing for those two: the throttle should be built into the writer when S18 lands it, rather than retrofitted afterwards. Two further findings from the same sweep, neither in the spec's list: - `npm_remote_packages`, `npm_remote_versions`, and `maven_remote_packages` all carry executing unconditional bumps and **no** index keyed on `last_downloaded_at`, so those writes can be heap-only. The argument does not reach them, and they should not be swept up in a fix. - `container_images` and `container_manifests` (hosted) carry the index with no bump method yet. Whatever adds the hosted download path inherits this, so it is worth knowing before that lands rather than after. ## Why one issue rather than three The three surfaces are one argument, and splitting them would put the reasoning in one place and the work in another — which is how it ended up living only in the spec with nothing pointing at the code it argues about. Surface 3 also cannot be scoped without the corrected table above, and that table is what tells surfaces 1 and 2 how much company they have. Ownership stays as the spec has it: surfaces 1 and 2 are S16-owned; surface 3 is S16-owned to record and npm- and Maven-owned to act on. ## Not covered by existing issues - [#668](https://gitlab.com/gitlab-org/ops/artifact-registry/-/issues/668) measures the container remote **tag freshness** bump's index usage — `bumpContainerRemoteTagCheckedAtStmt` writing `upstream_checked_at` on a `304`. Different statement, different column, and its question is which plan the planner picks, not write volume. - [#632](https://gitlab.com/gitlab-org/ops/artifact-registry/-/issues/632) covers buffered counter columns having no reconciliation, so a **shed** write drifts permanently. That is the inverse: writes that are lost. This is writes that all land and should not. - [#559](https://gitlab.com/gitlab-org/ops/artifact-registry/-/issues/559) derives the detached-write in-flight caps from the pool's capacity. Connection-pool starvation, not write volume on one row. ## Suggested order 1. Correct S16's five-table claim to the three that hold, and fold in the two findings above. Cheapest, and it scopes everything else. 2. Add the applied-versus-skipped signal (surface 2). It is what makes the blob-route change measurable rather than asserted, so it is worth having first. 3. Throttle the blob route (surface 1), keyed on the image row's own `last_downloaded_at`. 4. Route the npm and Maven surfaces to their own slices with the corrected table (surface 3), and note the `maven_versions` / `maven_packages` case as work for whoever lands the S18 writer. Surfaced while reviewing the S16 Step 11a branch, which adds the blob route in surface 1 and whose code comment now tells the next reader the throttle is deferred rather than impossible.