docs(plans): add the repository column writers plan
What this is
The plan MR for issue #1046. It adds one file,
docs/plans/2026-09-03-repository-column-writers.md, and changes no code.
Merging it is the approval signal for the five step MRs it describes; no step
MR opens before it lands.
What the review round changed
The review left 21 blocking: and 2 escalate: findings. Both
rulings are in and every finding is answered in its own thread. The two that
change what a reviewer should look at first:
- Coalescing is taken. The plan had ruled S22's buffered pipeline out
because
counterbuf.Family.Fields()returns[2]string, so routingdownloads_countthrough it "means widening that array and every path that reads it". That is the cost of a third field onRepoScoped, not the cost of the pipeline.Familyis an open string type, so a third family keyed on the repository leaves the array and its readers untouched. Steps 4 and 5 now book buffered increments, and Postgres sees one write-back per drain tick per dirty repository whatever the read rate. - One open question, and it blocks Step 4. The third family carries one
counter while
Fields()is two slots wide, andBuffer.IncrementandBuffer.Mergeboth iterate the whole array, so a family holdingdownloads_countalone would send an empty field name to Redis withdeltas[1]. It needs either a second repository-level counter to pair with or a skip-empty leg in both scripts, and the second edits the shipped script path S22 owns.## Open Questionscarries it.
Every governing ADR was checked, the one the local mirror does not carry
included. ./scripts/adr-freshness.sh exits 0, so the mirror is current, and
the one internal ADR that script reports but never mirrors locally was read at
its source before merge. It governs a policy area no step in this plan
touches, so there is no deviation and no handbook amendment is owed.
The defect it plans against
repositories.last_updated_at and repositories.downloads_count each have a
production writer on one format.
last_updated_atmoves on an npm publish and on nothing else. Maven builds theUPDATEand discards it (MavenRepositoryStore.MarkRepoLastUpdatedassigns its statement to_), and no container path has a writer at all.downloads_countmoves on a Maven archive-class download and on nothing else. npm and container repositories read zero forever.
Both columns ship and both are read: the management API maps each as a sort key and binds each into the keyset cursor. So a Maven repository published a minute ago sorts by its creation date, and a download sort over a mixed namespace orders only its Maven rows.
The three decisions a reviewer should check first
All three are recorded in the plan with their reasoning rather than left implicit.
1. The Maven timestamp write goes post-commit, not inside the upload
transaction. The in-transaction site the tree invites is a row-lock upgrade.
upsertFileRow already holds FOR SHARE on the repositories row, through
ReVerifyRepositoryAlive, whose statement joins maven_repositories to
repositories with no OF clause and so locks both rows. An UPDATE repositories later in that same transaction asks for FOR NO KEY UPDATE on
a row the transaction already holds at share strength, and two concurrent
uploads into one repository then wait on each other: a deadlock, aborted by
Postgres, on an upload whose bytes are already in storage. A parallel
multi-module publish is the ordinary shape that produces it. Step 2 therefore
dispatches from writeCommitOutcome's txErr == nil arm, where the emission
decision S10's numbered commit step describes is already made (201 fresh, 200
overwrite, byte-identical carried out by its own sentinel), and pays a
sixth bufferedUpdate label plus the re-split of five per-label
quotas that forces. ## Approach also names the alternative of strengthening
the re-verify's lock and why that is worse.
2. Step 1 reverses the hosted half of the downloads_count assignment, and
keeps the remote half where it is. S10 and S12 assign the column to S18;
Step 1 moves the hosted writers to this work and answers the reasoning in
those rulings, which is about the column's mechanism. S16's two sentences
stay: no step here writes the container remote read path, and npm remote is
served by a different handler than the one Step 4 touches, so removing S16's
gate would leave its criteria asserting downloads that nothing books. Both
remote writers stay with #292, named in ## Dependencies, and so does the
mechanism decision for those two paths.
npm hosted is the one half with no ruling to reverse: no npm spec defines a
downloads_count event at all, so Step 1 defines one.
3. The plan routes the read-path counter through the buffered pipeline, and
names the shed it does not remove. This is the decision the review
reversed, so it is the one worth reading closely. Steps 4 and 5 stand up a
third repository-keyed counter family, its drain worker and its write-back,
and the read paths book increments into it. That removes two bounds the
earlier revision carried: the per-row Postgres ceiling, where an N-layer pull
issued N+1 UPDATEs contending on one row, and the timeout ordering where a
writer's 5s worker deadline was shorter than a container manifest push's 10s
lock hold. What it does not remove is the dispatch shed: the emit still runs
under counterEmitMaxInFlight, so a saturated cap or a process exit can lose
an increment before it is staged in Redis, and downloads_count is not among
the columns reconciliation recomputes. ADR-007 asks that every increment be
preserved; ## Dependencies states what is preserved once staged, what the
residual is, and that #632 (closed) owns it.
Measurement basis
Pinned at d5ee57ffdf602e852c14c7cce93fb202c82449ac, which is also this
branch's merge base with main, and every file:line and every derived count
in the document is a claim at that commit unless a later basis is named at the
claim itself. That sentence used to cover file:NN citations alone, which is
what let two stale facts through review, so it now covers derived counts,
quoted comment bodies and file-existence claims as well.
The facts the review found stale are re-derived at e590735c4 and say so
where they appear, rather than by re-pinning the document: a pin whose
citations were not all re-derived against it asserts more than was checked.
The paragraph carrying the pin, under ## Spec Reference, lists which facts
those are and where each one says so.
Worth knowing while reviewing: the issue measured at 7f766481cd1d and the
tree moved a long way since, so several of the issue's own citations are stale
at this basis. The plan lists them under ## Corrections this plan implements rather than silently substituting corrected numbers, and it also
flags the claims the issue makes that the tree contradicts.
Two false claims already in the tree get closed in passing, both named in the
steps that touch them: docs/specs/S17-rest-management-api.md:93 still calls
MavenRepositoryStore.BumpRepoDownloadCount a no-op stub (it executes) and
still says nothing maintains downloads_count, and the sentence at :1645
calls MarkRepoLastUpdated a stub, which Step 2 makes false. That second
sentence also names BumpRepoCounters, which exists in no Go file, so Step 2
rewrites it whole rather than amending the half it falsifies. The first is
what #975 tracks.
!2268 (merged) is a merge dependency for Steps 2, 3 and 4, for two different reasons.
For Steps 2 and 3 it adds a one-hour freshness window to
RepositoryStore.MarkRepositoryLastUpdated, the statement every
last_updated_at writer in this plan lands on, so a repeat write inside the
window books nothing, and their acceptance criteria and testing strategy are
written against that. For Step 4 it is a shared seam rather than a behaviour
change: !2268 (merged) reworks the same internal/datastore/repositories.go and
internal/datastore/query_names.go that step adds a write-back to.
Diff size
1486 insertions in one file, past the 500 reviewable-LOC line in
docs/dev/development-model.md. Splitting would not help: it is a single plan
document whose sections cross-reference each other, and the guardrails ask for
one plan file per initiative committed through one plan MR. The document is
the unit of review.
Testing
No code changes, so no tests. The plan's own ## Testing Strategy states the
e2e scenario impact for each of the five steps: Steps 2 through 5 are
behavior-changing and each amends the scenario in its format's catalog under
docs/testing/e2e/, and Step 1 is documentation and affects none. All
pre-commit hooks pass on the file, markdownlint, lychee and Vale included.
Related to #1046