fix(maven): execute the last_updated_at write after the upload commits (repository-column-writers plan: 2/5)
What
Maven primary-file uploads now advance repositories.last_updated_at. The write existed as MavenRepositoryStore.MarkRepoLastUpdated, a documented no-op stub that built a jet UPDATE and discarded it, so the column has read null for every Maven repository since S10 shipped. This step deletes that stub and routes the Maven adapter at the format-neutral RepositoryStore.MarkRepositoryLastUpdated that npm publish already uses.
Step 2 of the repository-column-writers plan.
How
The stamp is dispatched from writeCommitOutcome's txErr == nil arm, beside recomputeVersionSizeAfterCommit, and runs off the request goroutine through the package's bufferedUpdate helper under a new sixth label, last_updated_at.
It must not run inside the commit transaction, and that is measured rather than assumed. The statement takes FOR NO KEY UPDATE on the same repositories row the upload's transaction holds at FOR SHARE, and a stamp issued from inside that transaction waits on a second pooled connection while the holder waits on the client. Postgres sees no lock cycle there and cannot break it. With the in-transaction call live, -run TestUpload_RepoLastUpdated took 627s and timed out; with it removed, 20s. internal/format/maven/upload_commit.md records the reason where the next reader will find it.
The arms that stamp are unchanged from the two deleted in-transaction call sites: a mutable upload (snapshot or package-level metadata) whether fresh or an overwrite, and a release upload only on insert. The byte-identical and conflict arms abort the transaction and so reach no dispatch.
Per-label quota re-split
A sixth label forces a re-split, because the five quotas already summed to bufferedUpdateMaxInFlight (64) exactly, which is what keeps bufferedUpdateSem's shed arm unreachable. They are now 16/6/16/6/16/4, still summing to 64.
The sizing is by what a shed write costs before any repair, not by dispatch rate, and repair does not sort with the numbers. The three 16s are the dearest sheds: an additive downloads_count increment is gone for good, a remote fill's byte delta stands until a file delete recomputes it, and a stale remote retention signal is what a premature eviction acts on. The next remote read does re-issue last_downloaded_at, so its 16 buys that window rather than covering a permanent loss. One of the cheaper sheds does not heal at all: version_size_bytes has no repairing upload when it is shed on the last primary file of a release deploy, the version being immutable and the reconciler owning no maven_versions column. The stamp takes the smallest quota: it fires only on a committed primary-file upload, its statement declines the write for all but the first upload per repository per hour, and the next row-changing upload re-issues it.
Merge order
- !2362 (merged) (
repository-column-writers plan: 3/5, this step's container equivalent) conflicts with this branch ondocs/dev/observability.md: both add alast_updated_atvalue to adjacent rows of the buffered-counter table. Whichever lands second resolves it, and the resolution keeps both rows' additions. It also editsinternal/metrics/column_budget_test.go, in a different owner block, which merges clean. !2362 (merged) also conflicts withmainitself, indocs/dev/observability.md,docs/dev/remote-read-budgets.mdanddocs/dev/storage-accounting.md, so it needs a rebase whatever this branch does. Itsobservability.mdconflict againstmainis not this branch's merge order to own, and the overlap above wants re-checking once !2362 (merged) rebases. - !2358 (merged) (
repository-column-writers plan: 1/5) editsdocs/specs/S17-rest-management-api.md:93, a different line of the one spec file this MR also touches, and merges clean. Whichever lands second rebases. !2358 (merged) also rewrites the S18 dependency row's stale "no-op stub" clause, which the plan had assigned to both steps; that clause is aboutlast_downloaded_atanddownloads_count, not this MR's column, so it is left to !2358 (merged) rather than duplicated here. - !2331 (merged) (
fence the Maven version insert on a live package row) editsinternal/format/maven/upload.goand the same S20-A section, and merges clean today. - !2351 (merged) (
upload-inactivity-deadline plan: 3/4) conflicts with this branch and withmainalike, and only indocs/dev/configuration-reference.md, which this diff does not touch. Its one overlapping file,internal/format/maven/upload.go, auto-merges. Not this branch's to fix. - !1598 conflicts with this branch and with
mainalike, indocs/dev/observability.md. Not this branch's to fix. api/openapi/v1.yamljoins this diff in the second review round, for thelast_updated_atlag wording. !2343 (merged), !2332 and the draft !1011 (closed) also touch that file. !2343 (merged) and !2332 merge clean against both this branch andmain; !1011 (closed)'s conflicts are its own againstmainand do not include this file.
Conflicts above measured with git merge-tree --write-tree at head 5cf0174cb, base 07f4e8521, main at ec806b44c.
!2268 (merged) was a required predecessor and merged on 2026-09-04. It added the one-hour freshness window to RepositoryStore.MarkRepositoryLastUpdated, which is why the acceptance below is "the dispatch happens" rather than "the column moves every time".
Spec coverage
Specs: S10, S17. Counted at 07f4e8521: 37 acceptance criteria, 15 error-case rows, 14 security considerations. This step reaches 5 of the acceptance criteria and 2 of the error cases; the remaining 32 and 13 describe read, sidecar, parse, cap, concurrency and reconciler paths this diff does not touch, and their coverage is unchanged.
| Criterion | Tests |
|---|---|
S10 ### Interim implementation of buffered columns: the write runs off the request path after the transaction commits, and a failed emission does not fail the request |
TestMarkRepoUpdatedAfterCommit_RunsOffTheRequestGoroutine, ..._FailedStampIsMeteredNotSurfaced, ..._ShedStampMetersDropped |
| S10 Upload Step 5: the stamp advances for a fresh row and for a snapshot or package-level overwrite, and not for a byte-identical release re-upload | TestWriteCommitOutcome_StampsOnlyTheCommittedArm (7 arms), TestUpload_RepoLastUpdated_FreshReleaseStamps, ..._SnapshotOverwriteStamps, ..._PackageMetadataOverwriteStamps |
| AC 10: release re-PUT with different content is 409, no stamp | TestUpload_RepoLastUpdated_ReleaseConflictDispatchesNothing |
| AC 11: byte-identical release re-PUT is 200, modifies no row, no stamp | TestUpload_RepoLastUpdated_ByteIdenticalReleaseDispatchesNothing |
| The shared statement's hourly throttle, from the Maven caller | TestUpload_RepoLastUpdated_SecondPublishInsideTheWindowStillDispatches |
The stamp keys on repositories.id, not the maven_repositories binding surrogate |
TestMarkRepoUpdatedAfterCommit_StampsTheParentRepositoryRow |
Both mavenStoreAdapter construction sites key every store |
TestMavenStoreAdapterLiterals_EveryConstructionSiteKeysEveryStore |
| The six quotas sum to the global cap | TestBufferedUpdate_EveryLabelHasBoundedSemaphore, TestBufferedUpdate_PerLabelQuotaFitsGlobalCap |
S10's implementation requirement that every interim goroutine site carry a swap-out TODO is met by the TODO(buffered) on markRepoUpdatedAfterCommit. It is not test-asserted, as it is not for the six production sites already dispatching through bufferedUpdate.
Size
1326 reviewable LOC, past the 500 in development-model.md. Split by file group, counted as added plus deleted at head 5cf0174cb against base 07f4e8521:
| Group | LOC |
|---|---|
| Tests | 826 |
| Production Go | 351 |
New sidecar .md |
128 |
docs/ specs and dev |
19 |
api/openapi |
2 |
Splitting would not help. 826 is the test-first suite, which has to land with the code it constrains. Of the 351 production-Go lines, 192 are comment lines deleted by moving three doc comments to upload_commit.md and upload_emit.md; the behavioural surface is about 30 lines across upload.go, wire_maven.go and buffered.go.
That relocation is not optional cleanup. Three doc comments described the stamp as running inside the commit transaction, which this change makes false, and all three sit on unexported functions that comment caps allow one line. Correcting one clause forces the whole block (29, 11 and 144 lines) to its cap, so the prose moves to sidecars the way internal/format/npm and internal/format/oci already carry theirs.
What this does not reach
- The metadata reconciler.
MetadataReconcilerrewritesmaven-metadata.xmlthrough the same adapter and dispatches no stamp, so a reconciler-driven content change leaveslast_updated_atwhere it was. Out of scope for this step. - Deletes. No Maven delete path advances the timestamp, so a repository whose artifacts were just emptied does not move in the Phase 1 list's sort. S17 records this as accepted.
- A shed or failed stamp is not retried. It is metered under
column="last_updated_at"withresult="dropped"orresult="error", andresult="dropped"carries no log line naming the repository.last_updated_atis the only one of the six Maven buffered columns that is a customer-visible list sort key, so a shed stamp misorders a repository in the management API listing for up to an hour. Accepted: the loss is bounded by the next upload to that repository. repositories.artifacts_countandsize_bytesare unaffected here. They reach the row as deltas on S22's pipeline, exactly as npm's do.
e2e scenarios
No scenario is added. docs/testing/e2e/ carries no last_updated_at scenario for any format, and the acceptance this step could reach end to end (a just-published Maven repository sorting first by last_updated_at) crosses into the management-API list, whose own tests own that sort. The e2e.maven.setup.repository-statistics row already scopes which repository counters settle asynchronously, and is where a last_updated_at sentence would go when a scenario is written for it.
Review fixes applied
A first branch review is addressed by five items: the quota re-split above, the cardinality audit gaining the new column, an unfalsifiable pool.calls assertion dropped, the three sidecar moves, and corrections to S10:1199 (which still promised an unthrottled NOW(), where S11:595 already had the right wording for npm), docs/dev/observability.md:56 (a closed enumeration missing the sixth column), and S20-a:579.
A second branch review is addressed by five further items: the emit sidecar's false "nothing schedules a pass" claims, the quota rationale re-sorted by cost before repair rather than by repair, the commit sidecar's own justification put in a past frame, the last_updated_at OpenAPI description gaining the "may lag" wording its three siblings carry, and S20-A's two surviving BumpRepoCounters sentences.
The plan's Status table records this MR in !2363 (merged).
Related to #1046