chore(lifecycle): bound the purger's counter emit behind a shedding cap

What this delivers

A per-attempt bound on the lifecycle purger's storage-accounting counter emit, and a counter for what the bound loses.

The cause. emitChunkCounters called the accounting emit seam inline, on the chunk driver's own goroutine. The call site is internal/lifecycle/emit.go, reached from internal/lifecycle/chunk.go. That seam detaches its two Redis calls from the caller's cancellation, and it spawns nothing. internal/accounting/emit.go states it: record "spawns nothing and sheds nothing, so this is a bound on whichever goroutine calls it". So a degraded Redis charged the purge attempt accounting.detachedEmitTimeout, five seconds, once per arm that fires. A chunk that owes both a repository delta and a namespace delta paid it twice. Nothing bounded an attempt's total across its chunks. The defect is at the call site and not in the emitter, so internal/accounting is unchanged.

The fix. dispatchCounterEmit, in the new internal/lifecycle/emit_dispatch.go, runs one committed chunk's emit on a worker of its own.

  • The worker runs on context.WithoutCancel of the caller's context, and it recovers its own panic.
  • The slot comes from a non-blocking send before the spawn. So the cap bounds the goroutine population rather than the number of goroutines that do work. A dispatch that finds no slot is shed rather than queued.
  • counterEmitMaxInFlight is 64, the value every sibling dispatcher in the tree uses.
  • Both of a chunk's deltas travel on one dispatch. A chunk that owes neither delta takes no slot, for the same reason a nil emitter takes none.

The new family. gitlab_artifact_registry_lifecycle_purge_counter_emit_drops_total is a counter vector with one label. purge_emit_drop_reason takes shed and panic, a closed set of two with no ok value. The vector joins lifecycleCollectors, and internal/metrics/cardinality.go gains the label's budget of two and its pinned value set. The label name is scoped rather than bare, because metrics.AuditCardinality keys budgets and closed sets by label name across the whole service.

The prose moves with the code. docs/dev/observability.md gains the catalog row. docs/dev/storage-accounting.md replaces its inline-emit section with one that carries the three losses, and it corrects the sentences the bound falsifies. docs/specs/S20-a-lifecycle-closed-beta.md gains an observability row, and its error-case row now names which series records which loss. That spec also drops its count of the purger's unleased signals. It does not raise the count, because the S20-A plan put three signals on that seam and the fourth is this change's.

What this shape loses

This shape is not lossless, and no file in the diff says that it preserves every delta. Three losses stand.

Loss Counted What repairs it
A shed at the in-flight cap yes, as purge_emit_drop_reason="shed" the storage-accounting reconciliation pass, which recomputes the namespace from its committed rows. The worst-case latency is up to roughly twice storage_accounting.reconciliation_interval, not one, and it holds only while reconciliation keeps pace
A full Redis stall, where every slot is held until it times out and every dispatch behind them sheds yes, as sheds the same pass. The dispatch moved the wait off the chunk driver. It did not remove the wait
Process exit, for the workers the process outruns no, recorded nowhere the same pass. A dispatched goroutine is not a River job, so neither River's graceful drain nor its force-cancel grace waits for one. The drain finds workers in three shapes and only this one goes unrecorded: a healthy worker finishes inside the drain and its delta lands, and a parked worker's loss reaches counter_emits_total{result="dropped"} and a Warn line. That increment is unscrapeable, because /-/metrics closes first in the LIFO drain, so the Warn line is the observable record

Every delta a purge chunk owes is negative. So the counters read high until reconciliation recomputes the namespace.

A delta the emit pipeline itself refuses is a different seam. That one lands on gitlab_artifact_registry_counter_emits_total{result="dropped"} and never on the new family. Neither series alone covers the purge site.

Coverage

Source: the issue card's own acceptance list for #871 part 1, derived by the enricher at c0eec53b — the issue states no acceptance criteria. docs/specs/S20-a-lifecycle-closed-beta.md governs behaviour where the two meet; its "Emit API unavailable at a purge or tombstone site" error case is the row items 4, 8 and 13 bear on.

Acceptance criteria

# Criterion Tests
1 emitChunkCounters returns without waiting on the emitter TestEmitChunkCounters_ReturnsWithoutWaitingOnTheEmitter
2 A dispatch past the cap is shed, never queued TestEmitChunkCounters_ShedsPastTheCap
3 Goroutines in flight never exceed the cap; slot before the spawn TestEmitChunkCounters_ShedsPastTheCap, TestEmitChunkCounters_ReturnsWithoutWaitingOnTheEmitter, TestCounterEmitSem_IsSizedFromTheSharedCap
4 A shed increments a series on the composition root's registerer TestEmitChunkCounters_ShedsPastTheCap, TestPurgeWorker_Work_ShedsEveryEmitAndStillReaps, TestWiring_LifecyclePurgerFamiliesRegistered
5 A dispatched emit runs on a detached context TestEmitChunkCounters_RunsOnAContextTheCallersCancellationDoesNotReach
6 A panicking seam is recovered and metered as a panic TestEmitChunkCounters_ContainsAPanickingEmitterSeam
7 A nil emitter dispatches nothing and takes no slot TestEmitChunkCounters_ANilEmitterDispatchesNothing, TestEmitChunkCounters_TolerantOfANilEmitter (pre-existing)
8 An attempt whose every emit sheds still reaps and reports success TestPurgeWorker_Work_ShedsEveryEmitAndStillReaps
9 The delta rule is unchanged TestEmitChunkCounters_IssuesTheDeltasTheChunkOwes (joined), and the nine cases of emit_integration_test.go, each joined
10 The family is registered, catalogued, and passes AuditCardinality TestWiring_LifecyclePurgerFamiliesRegistered, TestWireLifecycle_RegistersThePurgerMetrics, TestWiring_CardinalityAuditPasses (pre-existing)
11 No file describes the emit as lossless; the prose names three losses Prose, not a test. metrics.go's purgeEmitDropReason* doc and the family Help name all three; the docs/ half is the documentation pass's
12 docs/dev/storage-accounting.md no longer says the emit is inline Documentation, out of this commit's scope
13 The S20-A error-case row gains the new lifecycle family Documentation, out of this commit's scope
14 The shed arm does not copy Maven's false reconciliation claim Prose, not a test. Nothing here claims no reconciliation pass runs; the arm's own comment lands with the dispatch

Rows 12 and 13 read "out of this commit's scope". That was true of the test(...) commit whose body carries this table. Both obligations are met on this branch, at the documentation paths named above, and so is row 11's docs/ half.

Evidence

The shed arm was driven live rather than reasoned about.

  • 110 tombstoned maven_versions rows sat under one namespace, each with no maven_files rows beneath it, so each drains in one chunk and dispatches once. One discovery page is 100 rows wide, so one attempt takes 100 of them.
  • The Redis container was paused, not stopped. A paused container leaves the connection open and never answers, so each dispatched emit parks for the whole five seconds and holds its slot. A stopped container is the wrong fault, because a refused connection returns the slot at once and never fills the cap.
  • Result on river_job id 77: the attempt finalized completed in 354 ms, with Redis stalled for its whole length. It reaped 100 rows and left ..._purge_counter_emit_drops_total{purge_emit_drop_reason="shed"} at 36. 36 is 100 dispatches against 64 slots. Under the inline shape that same attempt waits 100 emits at five seconds each.
  • grep -i shed over the 65 log lines the drive window wrote returns nothing. A shed writes no log line, and the series is the whole of its record. That is why the family exists.

The red phase is on record. Seven tests failed against the unfixed tree, each on a behavior assertion rather than a build error. The hand-back records the message each one printed. Three more passed before the fix as well as after. Each of those three is named an invariant guard rather than a reproduction. Two of them pin the nil-emitter early return and the cap's value. The third pins the delta rule, whose whole claim is that the rule does not move.

One case closes a mutation. With the zero-delta early return in internal/lifecycle/emit.go deleted, every suite stayed green, because every case built a chunk that owed both deltas. TestEmitChunkCounters_AChunkOwingNeitherDeltaDispatchesNothing closes that gap. It drives against a full cap, because a free cap cannot separate the guard from a dispatch. The worker frees its slot before the join returns.

Gates. bash scripts/ci/check-comment-caps.sh --base origin/main returns OK at this head. git merge-tree --write-tree --name-only origin/main HEAD reports no conflicted path at main 8f1a31643. The one conflict that had stood, in docs/dev/storage-accounting.md, is cleared: the branch is rebased onto main 11234c958. Both conflicting hunks in that file were adjacent-line layout, each side having edited a different line, so both sides are kept. git range-diff --creation-factor=100 across the rebase reports the four later commits byte-identical and four changed lines on the first, all four of them context carrying main's new text. The runtime pass re-ran driver.sh smoke after its recipe edits and got 73 of 73, at pre-squash commit f38afa465043f912c45a2e1663a58a5bfa8245ce, which no ref reaches.

The e2e scenario catalogs

Nothing is owed. The type is chore and the label is type::maintenance, so the guardrail's feat-or-fix trigger does not fire. The runtime pass read all five scenarios in docs/testing/e2e/ that read a counter after a purge or a delete. Each one states a poll for the buffered drain, which absorbs the extra hop.

Diff size, and why it is one reviewable unit

Guardrail 18 asks for a split or a justification past 500 reviewable lines. Measured at 08d0dd03e with git diff --numstat origin/main...HEAD:

Group Files Added Deleted
Go production — emit.go, emit_dispatch.go, metrics.go, chunk.go, internal/metrics/cardinality.go 5 168 13
Go tests — five internal/lifecycle files and two cmd/artifact-registry files 7 579 14
Documentation and the run recipe 5 180 52
Total 17 927 79

1006 reviewable lines, of which the production Go is 168 added and 13 deleted across five files. The largest single file is the new emit_dispatch_internal_test.go.

A split does not help, for two reasons. The project rule is that every merge request includes tests for the code it introduces. So the 579 added test lines travel with the 167 production lines. The documentation corrections are obligations this branch's own code creates. A split lands the false prose first. The corrected section and one stale section sit in the same file, about three hundred lines apart. The reviewer of a code-only merge request then sees no sign of the second.

The bounded blocking acquire, and why it is declined

Raised in review on internal/lifecycle/emit_dispatch.go:17, note 3786951921. The arithmetic behind it holds: the 64 raises this site's concurrent-emit ceiling from jobsriver's defaultMaxWorkers of 25 to 64, a rise of 2.56 times, and the cap starts shedding at 26 ms to 256 ms of emit latency rather than at the five-second stall the design reasons about. That range rests on a chunk cadence nothing in the tree measures, and each surface carrying it now says so. Work item 871's accepted decision, note 3767932126, sized the cap against one attempt and two states and never multiplied by River's 25 shared workers, so the intermediate latency state is a hole in that reasoning rather than a settled point.

What is declined is the remedy, not the arithmetic. The acquire stays non-blocking, and no per-namespace sub-quota is added. internal/format/npm/packument_cache.md:254 names the shed-do-not-queue rule the dispatch caps follow, and the purger fails its first exemption reason outright: dispatchCounterEmit takes its slot before the spawn, so it is the rebuildSem shape where refusing is the point, not the shape that serializes goroutines which already exist. The second reason there is about holding a database connection, which the purger does not hold at that point, so what tests the real cost is docs/dev/storage-accounting.md:1136: a handler that blocks on such a semaphore keeps occupying a shared worker while it waits. That is written for the asynq pool, and River's single QueueDefault at 25 workers is the same shape.

Three operator-facing sentences this merge request introduces were wrong about where the shed starts, and 0adeb8c7f, f41edd094 and 82dcdc297 correct them in docs/dev/observability.md, docs/dev/storage-accounting.md and .claude/skills/run-artifact-registry/SKILL.md. Those three were f120ef544, af8eb9a1a and 7a30fb7fb before two rebases onto main, and the first two are the pair the reply in that thread cites. A later review pass found that the corrected sentence still gave one boundary for a case that has two, and 6daff26c8 and 25a30af1e split it by the scopes a chunk owes: a chunk owing one scope stops filling the cap above 78 ms, and one owing both stops above 156 ms. The latency state itself is carried by #1175, together with the open questions on the value of 64, the bounded blocking budget, the per-namespace sub-quota, and an alert threshold.

Corrections this review round landed

Four more claims were wrong and are corrected on the branch.

  1. The single-attempt boundary gave one number, 78 ms, for a case that has two. Filling 64 slots takes 64 chunk durations, and a slot is held for one five-second accounting.detachedEmitTimeout per scope the chunk emits to, so a chunk owing one scope stops filling above 78 ms and one owing both stops above 156 ms. 6daff26c8 and 25a30af1e split the sentence, label the fast-end figure by scope too, and attribute the hold to the slot rather than to the emit.
  2. The drop family's Help string said a shed loses both of the chunk's deltas. A repository purge owes only the namespace delta, and an artifact chunk whose freed digests all survive elsewhere owes the same one, so 123b1b76e drops the count and keeps the mechanism.
  3. The S20-A error-case row named shed and not panic, and it claimed the two series do not overlap. A panic loses only the arms the worker had not run, so one chunk can reach both, and 504e5e9ad states the per-delta rule instead.
  4. The full-Redis-stall paragraph said every delta is lost and not that there are now more of them to lose. 08d0dd03e states the rate: inline, the chunk driver paid an emit bound per scope, so a stall throttled the purge itself; detached, the loss follows the reap rather than the emit.

Files the issue card does not list

docs/dev/remote-read-budgets.md is the one file the card gives a reader nothing to place. It is absent from the card's derived files list, and absent from that list's "Not required, checked" line as well. The chain is one hop per link, and every link is inside this diff.

  1. internal/lifecycle/emit_dispatch.go adds counterEmitMaxInFlight = 64. That is a fifth cap in the accounting-emit partition of the in-flight cap census.
  2. !2269 merged on 2026-09-03 and added that census to docs/dev/storage-accounting.md. The merged census counts eleven caps, four of them on Redis. This branch makes those counts twelve and five, so it corrects the census, the accounting-emit heading, that heading's table, and the heading's anchor.
  3. docs/dev/remote-read-budgets.md links to that same section and states the accounting-emit count in two sentences. This branch falsifies both sentences, so it corrects both.

The census names its own command. git grep -n "MaxInFlight = 64" -- internal returns 11 at origin/main and 12 at this head.

Five more files sit outside the card's Modify and Create list, for smaller reasons. internal/lifecycle/chunk.go is on the card's "Not required, checked" line and takes one comment change, because the call site said the deltas were issued there. internal/lifecycle/export_test.go is a same-package helper the integration suite needs, because that suite is package lifecycle_test and cannot reach the unexported hooks. internal/lifecycle/metrics_test.go and cmd/artifact-registry/wire_lifecycle_test.go take a test rename and two doc comments whose count claims the fourth family makes false. The run recipe at .claude/skills/run-artifact-registry/SKILL.md is the fifth. The runtime pass drove this change from it and found three stale claims. It corrected them, and it added the dispatch and how to tell a shed from a late delta. A later pass named the chunk cadence the shed onset rests on as unmeasured, on this file and on the two documents, and restamped the shed drive so the stamp gives a branch commit a reader can check out. An earlier pass corrected a fourth claim, inside the section it had added: 82dcdc297 replaces "a Redis that stalls rather than refusing is what fills the cap" with the same shed-onset rule the other two documents now carry, and 25a30af1e splits that rule's boundary by scope alongside the two documents.

Merge order

!2269 merged at 2026-09-03T13:18Z, so this is history rather than a pending order. This branch is rebased past it. What it required is the census correction above: without that correction, !2269's merged prose is false the moment this branch lands.

!2270 has since merged too, as 244b911df, and it edited the same table. It closed the six single-delete remote eviction gap and deleted that Known-gaps row, which sat directly above the row this branch rewrites. This branch is rebased past it: the rebase took upstream's deletion of that row and kept this branch's own census rewrite below it.

The file header in internal/lifecycle/metrics.go

Disclosed here because no in-place repair is available.

scripts/ci/check-comment-caps.sh counts that file header at 46 lines against a cap of 2. The header is pre-existing prose and it already exceeds the cap on origin/main. The fourth family forced a correctness edit to it, which pulled it under the ratchet.

The repair taken is a relocation. This branch restores both legacy blocks to their origin/main text exactly. It puts its own new prose in a capped segment below, behind the script's documented // --- divider.

What a reviewer needs to know: the header's opening sentence names "the three per-pod signals one purge attempt records", and the file now carries four. A reader who lands on line 1 reads that sentence as a claim about the whole file. The segment below the divider introduces the fourth family. A correction to that opening sentence pulls the whole 46-line block back under the cap. The block has no other in-place repair.

Specs and ADRs

docs/specs/S03-b-metrics.md is not touched, and it is not triggered. Its own cardinality-governance section routes an owner-spec metric's labels to the owner spec, and its expected-distinct-values table carries the foundation labels only. The label's home is S20-A, which now carries it.

ADR 007 and ADR 023 were checked, and both conform. ADR 023: the new files land in the existing internal/lifecycle package, with no sub-package and no format import. ADR 007: its rule that every increment to a buffered counter is preserved covers decrements as well. docs/dev/storage-accounting.md already reads it that way, and says a dropped delta leaves the counter over-counted for a decrement until a reconciliation pass reaches its scope. The same passage carries the conclusion this change rests on: a site that cannot afford the wait owes a dispatch rather than a drop. That is what this change does, and the reconciliation pass is the repair for what a shed still loses.

One limit, stated rather than glossed. ADR adherence is not fully established here. One internal ADR exists, in the internal handbook, and no run from this repository can reach it. ADR 007 is 2,489 lines long. The validation pass read three of its sections in full and swept the rest by keyword. That sweep returned nothing that governs goroutine dispatch, in-flight caps or metric families.

The link form is Related to rather than the closing keyword, and both validation passes reached that independently. All 14 acceptance items were derived by the enricher. The issue states no acceptance criteria for part 1, and eight of the fourteen have no line in the issue body behind them. A close on that derived list removes the issue author's chance to disagree with it. Parts 2 and 3 of the issue merged under !2226 on 2026-09-02, and that does not change the form.

Related to #871

What remains: the issue author reads the derived acceptance list and decides whether the issue is answered.

This is a bot message 🤖 — /smurfit

Edited by Pawel Rozlach

Merge request reports

Loading
Loading