feat(oci): stamp repositories.last_updated_at on the container writes (repository-column-writers plan: 3/5)

What

Gives repositories.last_updated_at a production writer on the Container/OCI write path. Six emitting sites dispatch the stamp, covering the seven endpoints S12's event table names: the blob upload PUT and single-shot POST (one site, CompleteUpload), the cross-repository mount, the blob DELETE, the manifest PUT, the by-digest manifest DELETE, and the by-tag untag, which is a new emitSite value and the one site that moves no counter.

The stamp is attached outside each of the three zero-delta counter guards. That is the one place riding the counter sites costs something: an implementation that gets it wrong ships green, because every other committed write moves a counter too, so only an idempotent re-push, a re-mount, and a finalize of an already-referenced blob can show the difference. All three are pinned.

Deviation from the plan, declared

The plan's Step 3 chose to run the stamp behind the existing counterEmitMaxInFlight, and says so where it describes the widened pool draw. This MR does not. The stamp gets its own cap, repositoryStampMaxInFlight.

The reason is a rule docs/dev/storage-accounting.md states in each section that partitions the caps by the backend they draw on, "Seven bound a worker population that holds a pooled Postgres connection" and "Four bound the accounting emit, which holds a Redis connection": one cap across both backends lets a stall in either shed the other's work, because a counter emit holds a Redis connection and this write holds a pooled Postgres one. Both sibling formats keep the pair apart, npm through bufferedUpdateSem against its own counterEmitSem, and this package's own downloadSignalSem is a third instance. Sharing the cap would also have made the column label separate two failure classes that compete for a slot, which is the opposite of what the label is for.

Consequence worth knowing: the stamp is the higher-volume dispatch of the pair, since it is issued once per committed write whatever the deltas, so a blob finalize dispatches one per layer where the counter emit dispatches only for a layer whose bytes are new. It carries no per-repository sub-quota, the same exposure counterEmitMaxInFlight already accepts and downloadSignalMaxPerRepository mitigates for its own cap.

Merge order

Nothing here blocks on another MR, but two open MRs conflict with this branch, so whichever lands second resolves. No pipeline reports this. Re-measured at cb76300f7 against origin/main 33c0b46e0; !2324 (merged) has merged since this table was first written, and !2392 (merged) no longer merges cleanly into main either.

MR Overlap with this branch Verdict
!2262 (merged) wire_oci.go, emit.go, emit_dispatch.go, emit_dispatch.md, emit_test.go, storage-accounting.md conflicts with this branch, clean against main
!2392 (merged) remote-read-budgets.md, storage-accounting.md, emit_dispatch.md conflicts with this branch and with main

!2392 (merged) deletes the cap counts and file:line coordinates from the in-flight cap census that this branch renumbers, and its own description asks to merge before !2362 (merged) (and before !2324 (merged), which has since merged). Taking that order makes this branch's census edit redundant rather than wrong; taking the other order leaves !2392 (merged) rebasing onto the corrected counts. Either resolves, but the census wording is the thing to read on whichever lands second.

!2262 (merged) adds a seventh emitting site, emitSiteRemoteCacheFill. Both branches replace main's "Five sites emit" with a byte-identical Six sites emit: prefix and a different member list, so a resolution that takes one side ships a document claiming six of seven. The correct resolution is seven. Whether the fill stamps repositories.last_updated_at is settled on #1046: the column moves on any committed write that changed what the repository contains or offers, so a fill that stored new content owes a stamp. !2262 (merged) writes none at 6f267b27c, which makes the stamp a follow-up on !2262 (merged) rather than something this conflict's resolution decides. The plan records the question as open at :1682-1683; that passage is what #1046 now answers. !2262 (merged) also adds emitRepoByID because the fill has no namespace.Resolution; stampRepository takes one, so wiring the fill later needs a by-id variant.

Sibling step of the same plan, independent of this one and also targeting main: !2358 (merged) (plan: 1/5). It shares no changed file with this branch.

Diff size

2230 reviewable LOC across 28 files, past the 500-LOC ceiling development-model.md sets, so here is the split:

Group LOC
Production 261
Tests 1556
Docs and sidecars 413

Derived at 44f4a2284 with git diff $(git merge-base HEAD origin/main)...HEAD --numstat, grouping *_test.go as Tests and *.md plus docs/ and .claude/ as docs.

Production is 261 LOC. Splitting it from its suite is what I would not do: the tests are the only thing that falsifies the placement of the stamp outside each delta guard, which is the one property the plan says would otherwise ship green. The docs total is inflated by the comment-caps gate rather than by new prose: editing one line of a long comment block forces the whole block to its cap (two lines in a test file, one for an unexported declaration), so the AwaitEmits, emit_dispatch_test.go header and emitMeter blocks moved into emit_dispatch.md and the emit_test.go header into a new emit_test.md.

Observability

column="last_updated_at" on gitlab_artifact_registry_oci_buffered_counter_updates_total, a value npm already emits, so no label budget moves. It reports dropped for a shed dispatch, panic for a recovered seam panic, and error for a statement the backend refused. No ok, so read it as a rate rather than a share.

The error arm matters: without it a backend refusing every stamp would move no series at all, leaving one unsampled Warn per container write as the only signal, since internal/logging's sampler touches only access records. That is the arm remote_download.go already books for the retention write, and this now matches it.

Do not read result across formats on this column, though. npm carries column="last_updated_at" for the same statement and books a refused one as result="ok": bufferedUpdate in internal/format/npm/buffered.go sets the outcome before the call and only a panic changes it. So {column="last_updated_at",result="error"} is an OCI-only signal, and an alert written across both vectors reads npm's identical outage as healthy. emit_dispatch.md and docs/dev/observability.md both say so now.

A lost stamp is not repaired the way a lost byte delta is: no reconciliation pass recomputes this column. The next write into the repository re-stamps it, and needs no window to elapse because the loss left the row past its freshness window; if no write arrives the value stays as stale as it was.

Tests

Seven positive endpoint cases and twelve negatives, driven through handlers rather than per site, because two endpoints share CompleteUpload and a per-site suite would leave the single-shot POST unasserted while reading as complete.

Every new guard and assertion was checked by breaking the thing it covers and confirming the suite reddens, not by reading it:

Mutation Test that reddens
drop the result="error" meter TestStampRepository_RefusedStatementMetersItsOwnColumn
drop the nil-writer guard, at both rows: the nil cap sheds, the free slot panics TestStampRepository_UnwiredSinkMetersNothing
drop column from the panic record TestRepositoryLastUpdated_PanicRecordNamesItsColumn
point stampSem at counterEmitSem TestNewCounterSink_SharesTheProcessWideCap
stampRepository acquires s.sem TestStampRepository_DrawsOnItsOwnCap
move the stamp inside the finalize delta guard TestRepositoryLastUpdated_StampsTheZeroDeltaSuccesses

Two of those existed only after this MR's review round. findEmitSiteRecord was filtering column out of the map it returns, so the attribute separating a panicked stamp from a panicked counter emit was unreachable from every assertion, and the two share one emit_site at five of the six sites. And nothing pinned which cap the stamp acquires, which only shows up if you saturate one cap at a time.

AwaitEmits and saturate now cover both caps. Waiting on one would return with the other's worker still running, which is the race every positive stamp assertion reads across.

internal/format/oci/emit_integration_test.go reads the column off a real row after a push, which is what makes the identifiers the site passes falsifiable: a stamp scoped to the container repository id instead of the repositories one matches no row and the statement reports no error for it.

internal/metrics/column_budget_test.go gains this package's two missing column values. last_updated_at is already pinned under npm's name, so the union is unchanged and both assertions stay green either way. That file's own header writes the case down and says to extend the owning package's list anyway; size_bytes was missing before this branch and goes in the same edit.

Guardrails

  • e2e (12): docs/testing/e2e/oci.md's repository-storage-counters row gains the column, which behaves opposite to its two siblings on both counts the row would otherwise lead a reader to assume: it is a direct write needing no drain tick, and it is throttled to one write per repository per hour.
  • Conformance (11): not engaged. No status, header or body changes. Four sites dispatch after their arm's WriteHeader; the blob finalize and the cross-repository mount dispatch inside CompleteUpload and MountBlob, which return before the caller writes the 201. Neither placement can reach the response: the acquire is a non-blocking select on the request goroutine and the worker recovers its own panic, so a dispatch can neither delay the write nor put a 500 in its place.
  • ADRs (22): conforms to ADR-007, which annotates repositories.last_updated_at "nullable" with no "buffered" marker, unlike every sibling timestamp in that file, and scopes the column to content changes rather than downloads. The six stamped sites match and no read path stamps. The clause this does not reach is the same one the plan leaves open: ADR-007 writes content changes as "artifact publish/modify/delete, cache events", and a remote cache fill is a cache event that dispatches no stamp.
  • Configuration reference (14): not engaged, no config surface changes.
  • Integration lint (7): golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false over ./internal/format/oci/... and ./internal/metrics/... reports one finding, a pre-existing ireturn in upload_finalize_integration_test.go, which this branch does not touch.

Known residuals, with owners

  • docs/specs/S12-container-oci-hosted.md:1716 and :1718 are false once this merges, and :1731 says the blob mount 202 fallback writes nothing when mountFallback reaches CreateSession, which commits a container_images row. Spec text belongs in a spec MR; plan Step 1 is open as !2358 (merged) and its new prose re-asserts :1716, so the cheapest fix is there. Not yet raised anywhere durable: this description is the only record, and it does not survive the merge, so it needs a note on !2358 (merged) or an issue of its own.
  • A bare POST .../blobs/uploads/ and that mount fallback each commit a client-visible container_images row and dispatch no stamp. That matches S12's event table, which books the auto-create's accounting at the finalize row, so it is deliberate rather than missed, and the observability wording says "the six container write sites S12's event table names" rather than "every container write endpoint".
  • The plan's Step 3 states the opposite of what this MR does: "it is the clause rather than the cap that has to change". Quoted here because the deviation section above overturns it, and step MRs do not edit the plan file (guardrail 4), so correcting the passage needs the batch or standing table MR.
  • The plan's Status table and its two TestNewCounterSink_RejectsAHalfBuiltPair references keep the old name; the test is now ...RejectsAHalfBuiltSink. Step MRs do not edit the plan file (guardrail 4), so this needs the batch or standing table MR.
  • Two commit bodies on this branch cite guardrail 19 for the comment-agreement rule. Recounted at HEAD it is 17; 19 is forward-referencing. The plan's Step 3 entry carries the same slip, already on main.

Related to #1046

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading