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'srepository-storage-countersrow 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 insideCompleteUploadandMountBlob, which return before the caller writes the 201. Neither placement can reach the response: the acquire is a non-blockingselecton 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=falseover./internal/format/oci/...and./internal/metrics/...reports one finding, a pre-existingireturninupload_finalize_integration_test.go, which this branch does not touch.
Known residuals, with owners
docs/specs/S12-container-oci-hosted.md:1716and:1718are false once this merges, and:1731says the blob mount 202 fallback writes nothing whenmountFallbackreachesCreateSession, which commits acontainer_imagesrow. 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-visiblecontainer_imagesrow 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_RejectsAHalfBuiltPairreferences 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