feat(accounting): buffer and drain the download-count family part 2/3 (repository-column-writers plan: 4/5)

Part 2 of 3 of the repository download-count family. Adds the Redis half, the emitter method and the chunk worker, on top of part 1's statements.

What this adds

  • counterbuf.DownloadsScoped, its dirty-set key, the downloads field and DownloadsScope. The family carries one counter where the other two carry a pair, so Family.Fields pads the trailing slot and carriedFields reads the leading run, refusing a hole rather than truncating at it: a truncated Merge would lose every delta past the hole silently. Buffer.Increment refuses a non-zero delta opposite a padding slot rather than filtering it.
  • DownloadsScope reuses RepoScope's member so ParseRepoMember reads it back. Only the family differs, and the family is in every key, so the two cannot collide.
  • Emitter.EmitRepoDownloads, taking a named scalar rather than an array so no caller can put a value in the padding slot.
  • DownloadsChunkWorker, which merges each claimed scope and commits the batch through part 1's apply.
  • chunkPlan.drain now reads the matched-row count back and logs at Warn when a committed batch moved fewer rows than it carried. The comment that authorised discarding it rested on reconciliation drift, and neither drift histogram can produce a reading for this family, so that batch previously had no signal at all: the emits book recorded, the dirty set drains, no chunk bails, and a column flat at zero is indistinguishable from a repository nobody downloads from.
  • How far the Warn reaches is per family, read off chunkPlan.family rather than carried as a plan field, so no plan can set the floor the wrong way. Every family reports a batch that moved no row; only downloads_scoped reports one that moved some, because a tombstone landing between DownloadsBaselines and ApplyDownloadsDeltas leaves a skipped scope's delta merged and its :flushed key deleted in the same transaction, and no pass recomputes downloads_count. The rows an apply moved travel beside the batch width under counter_applied_rows. Neither line is sampled: internal/logging's sampler acts only on records carrying the access message, so the wider reach is bounded to the one family that cannot recover the loss.

What this part does not reach, and the merge order that closes it

This part registers nothing. No worker and no schedule is bound to a kind, and no producer calls EmitRepoDownloads, so on this part alone nothing marks a scope dirty and nothing claims one: the family is inert rather than half-wired. Part 3 adds the trigger, the registration in accountingRiverKinds and npm's producer in one change, which is what keeps a producer from ever running ahead of the drain that serves it.

Merging part 3 before this one does not build. Merging this one alone is safe and does nothing.

Stack

Merge in this order. Each part builds and tests on its own, and the family is inert until part 3. Each part is hard-gated behind its parent, so none can merge early.

# MR What it adds Reviewable LOC
1 !2494 (merged) The SQL half: the delta type, the two statements, the single-row writer 1683
2 !2495 (merged) The Redis half, Emitter.EmitRepoDownloads, the chunk worker 1695
3 !2496 (merged) The drain trigger, the registration, npm's producer, every doc surface 1338

Diff size

Guardrail 18 asks for a split or a justification past 500 reviewable LOC. This step was split three ways and each part is still over it, so the justification stands per part rather than being cleared by the split.

By file group at c24ca324: Go production 10 files +289/-518 = 807, Go tests 11 files +759/-129 = 888. Production is net negative on lines: comment blocks in internal/accounting were compressed to the caps scripts/ci/check-comment-caps.sh enforces, which is what editing any line of them costs, and the reasoning that was not already duplicated moved to docs/dev/storage-accounting.md and docs/dev/background-jobs.md in part 3 rather than being dropped.

Testing

internal/accounting and internal/accounting/counterbuf unit and integration suites, including the download family's drain case and its tombstone case. Both mutations were run and confirmed to redden: booking staged[1] instead of staged[0] applies +0, and reading RepoBaselines instead of DownloadsBaselines drops the soft-delete filter.

TestChunkPlan_UnmovedRowsWarning_ReportsAPartialApplyOnlyWhereItIsALoss covers every arm of the per-family floor, and each arm was confirmed to redden when dropped. Its cases name a family either side of the floor, so inverting the family comparison to != and deleting the condition outright each redden it; both were run. One gap it does not close: the end-to-end partial apply is reachable only through the chunkBeforeUpdate gofail seam, so no case drives it. That case is owed to the post-part-3 test(accounting) sweep rather than dropped. TestDownloadsChunkFamily_PopulatesEveryAccessor is what holds the four accessors that fail rather than nil-panic, because no case reaches them.

TestFamilies_AccountsForEveryFamilyConstant and TestSharedFamilyLists_AccountForEveryFamilyConstant hold families(), scopeCases() and chunkFamilies() against a catalog of every counterbuf.Family constant plus an exclusion map carrying each excluded family's reason, so a later family cannot be skipped from a shared list in silence. Three failure modes were run in both packages and each reddened: a family in neither the list nor the exclusions, a family in both, and an exclusion recorded with an empty reason. The catalogs are hand-written, as TestDropLogFamilies_ListsEveryFamilyConstant's is, so a constant added to counterbuf and left out of a catalog is accounted by nothing; that step stays manual.

No format protocol behaviour changes, so guardrail 12 does not apply, and no e2e scenario is added or affected: the family has no producer until part 3.

Related to #1223 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading