## Why this work item exists Merge request [!2262](https://gitlab.com/gitlab-org/ops/artifact-registry/-/merge_requests/2262) makes the Container remote cache fill emit its four storage-accounting deltas. Its description ends with `Closes #834`, so [work item 834](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/834) closes on that merge. Three pieces of work stay open at that point. Work item 834 is the only item that names them today, so this work item takes that role and the three survive the close. Nothing here changes service behavior. The emit itself belongs to !2262, and this work item takes no position on it. | Part | Carrier | Ordering | | --- | --- | --- | | 1. Ten records in `docs/dev/storage-accounting.md`, and one row in `docs/dev/observability.md` | a `docs:` merge request, proposed title `docs(accounting): the container remote fill's ten stale records` | opens **only after** !2262 merges | | 2. One probe test case in `internal/datastore/container_remote_fill_accounting_integration_test.go` | a `test:` merge request, proposed title `test(datastore): a tombstoned container_remote_blobs row reads held` | needs !2262's commits under its branch | | 3. A note on two nil-guard idioms in `internal/format/oci` | no merge request | none | The ordering on part 1 is the point of the whole item. While !2262 is open, every record listed in part 1 reads correctly on `main`. An edit to any of them is !2262's own change made a second time. Two copies of one change conflict, and the second one loses. Once !2262 merges, the same ten records describe a service that no longer behaves that way, and the `docs:` merge request corrects them. Part 2 extends a file !2262 creates, so its branch needs !2262's commits under it. It can be a stacked branch, or it can wait for the merge. ## Part 1 — the documentation records `docs/dev/storage-accounting.md` carries fourteen records about the container remote cache fill. !2262's diff corrects four of them: the emit-record table row, the `## Known gaps` row, the `### ReapTotals: what one chunk reports` sentence, and the `### Hosted npm reapers` sentence. Ten stay untouched. Five of the ten state a fact that the merge makes false. The other five are constructions built on that same premise, and each one keeps a condition the merge settles. The record ids below are this work item's own labels. Anchor on the quoted text rather than on the line number, because this file moved several times a day while the two arms were in flight. Long URLs inside the quotes below are shortened to `…`, and nothing else in a quote is changed. | Record | Section | Kind after the merge | | --- | --- | --- | | R2 | `#### Container, remote`, the grid | false | | R3 | `#### Container, remote`, the prose under the grid | false | | R6 | `### Hosted Maven reapers` | false | | R8 | `### Container remote cache reapers` | false | | R9 | `### Container remote cache reapers` | false | | R10 | `### Container remote cache reapers` | stale | | R11 | `### ReapTotals: what one chunk reports` | stale | | R12 | `### Hosted npm reapers` | stale | | R13 | `### Container remote cache reapers` | stale | | R14 | `### Container remote cache reapers` | stale | | O1 | `docs/dev/observability.md`, the `gitlab_artifact_registry_oci_buffered_counter_updates_total` row | see its own entry | ### R2 — the cache-fill row of the `#### Container, remote` grid Anchor, at `docs/dev/storage-accounting.md:679`: ```markdown | cache fill | `GAP` | `GAP` | `GAP` | `GAP` | ``` Today the four `GAP` cells say the fill moves no counter of either scope. After the merge all four read `emits`. This anchor is unique in the file today. The `#### npm, remote` grid's cache-fill row at `:611` and the `#### Maven, remote` one at `:652` both read `emits` already. ### R3 — the prose under that grid Anchor, at `:696`: > While the cache fill emits nothing, which is the `GAP` the row above carries, a container remote reap takes bytes off a column that fill never raised; [Container remote cache reapers](#container-remote-cache-reapers) states that negative-read window and the two conditions [work item 834](…) closes. Today it says the fill emits nothing, and it points the reader at a negative-read window. After the merge the fill raises the column, so the reap takes its bytes off bytes a fill added. The trailing clause names two conditions that work item 834 closes. The merge closes both, so that clause goes with the rest of the sentence. ### R6 — the direction comparison in `### Hosted Maven reapers` Anchor, at `:2322`: > The direction is the reverse of the same form in [Hosted npm reapers](…) and [Container remote cache reapers](…): each of those reports bytes against a column no write raised, so its window is a column reading negative, where this one reports none against a column an upload did raise. Today the sentence puts two arms in one class: each reports freed bytes against a column no write raises. After the merge the container remote arm leaves that class, because its fill raises the column. The sentence keeps the npm half and drops the container half. A second sentence can then say that the container remote arm belongs to neither form, since both of its sides move. ### R8 — the missing increment Anchor, at `:2816`: > That figure has no matching increment, and no precedent one format over settles whether it should be reported anyway. Today the reap's byte figure has no increment to match. After the merge it has one: `RemoteCacheStore.emitCommittedFillCounters` in `internal/format/oci/remote_fill_emit.go`. The two sides key on one predicate, which is what makes them a pair. The fill charges the bytes when no `container_remote_manifests` row and no `container_remote_blobs` row of the repository carries the digest yet. The reap frees the same bytes when no such row still carries it. ### R9 — the reason for reporting the freed bytes Anchor, at `:2817`: > the absent increment is the gap rather than a reason to stay silent Today this clause rests the reporting decision on the absent increment. After the merge no increment is absent. ADR 007 still carries the decision on its own, because it puts the remote cache inside the repository's footprint. The clause about the gap goes, and the ADR 007 reason stays. ### R10 — the reading off the Maven remote arm Anchor, at `:2818`: > Reading the reason off the Maven remote arm gets it backwards in both directions: that arm reports its bytes too, and a Maven remote cache fill does raise the column, so the sentence borrowed from it would be false at both ends. Each half of this sentence about the Maven arm stays true. What fails is the contrast it draws, because the container remote arm then holds the same pairing the Maven one holds. The sentence has no work left to do, and the paragraph reads better with the pairing stated once. ### R11 — the negative-read conditional in `### ReapTotals` Anchor, at `:2013`: > Once a caller turns the container remote figure into a counter delta, that repository's `size_bytes` can read negative until a reconciliation pass recomputes the column from source rows rather than adjusting it. The conditional holds today because nothing raises the column for this arm. After the merge the antecedent no longer carries the consequent. This is the clearest case of the ten: !2262 rewrites two lines of the same paragraph and leaves this one standing. The paragraph then says the fill raises the column and then says the column can read negative. One residual loss case remains, and it is worth keeping in some form. A fill whose emit dispatch the in-flight cap sheds loses its increment until the next reconciliation pass. ### R12 — the cross-reference in `### Hosted npm reapers` Anchor, at `:2500`: > [Container remote cache reapers](#container-remote-cache-reapers) carries the same two-condition form over a work item of its own; this instance is [work item 939](…), which names `npm_metadata_files` and the packument write. Today the container remote section carries a two-condition form, which R13 and R14 state. After the merge that section carries none, so the cross-reference points at a form that is no longer there. `### Hosted Maven reapers` carries a live two-condition form at `:2320`, over work item 1012, and it is the target the reference can move to. ### R13 — the two conditions Anchor, at `:2819`: > [Work item 834](…) records that gap, and two conditions open a window on it — some caller turning this figure into a counter delta, and that work item still being open. Today the second condition holds, because work item 834 is open. After the merge it can never hold again, because `Closes #834` closes the work item this sentence names. The sentence goes. ### R14 — what holds while both conditions hold Anchor, at `:2820`: > While both hold, a container remote repository's `size_bytes` can read negative between a cached-manifest purge and the next reconciliation pass, which recomputes the column from source rather than adjusting it. This sentence is R13's consequent, and it goes with R13. The shed-dispatch loss named under R11 is the case that survives, and this is the paragraph that can carry it. ### O1 — the OCI buffered-counter row in `docs/dev/observability.md` Anchor, at `docs/dev/observability.md:53`, inside the `gitlab_artifact_registry_oci_buffered_counter_updates_total` row: > `column="size_bytes"` is the storage-accounting emit path, and it reports only the outcomes that lose a delta **!2262 falsifies nothing in this row.** The fill records under the same `column="size_bytes"` series through the same dispatcher, so every claim the row makes about that series stays true. Two edits still belong in the same `docs:` merge request, and both are improvements rather than corrections of !2262. First, the row names one producer under `column="size_bytes"`. After the merge the container remote cache fill is a second one. It can produce two of the four drop causes the row lists: `dropped` from a shed dispatch, and `panic` from the recovered-panic arm. The other two `dropped` causes stay the hosted sites' own, because the fill reads its membership inside the datastore transaction that writes it. The `emit_site` attribute on the panic log line is what names the producer, as `remote_cache_fill`. No sentence in the row is wrong without this edit. What the reader who arrives from the fill lacks is any text to match their producer against. Second, and independent of !2262, this row carries a condition that has already flipped: > Which column moves depends on a merge: while a container-remote read that finds no fresh row is answered as not implemented, no retention write is ever dispatched and every sample comes from the emit path; once [!1764](…) makes that path serve from the cache, the series carries both The four `newDownloadSignals(...).record(...)` call sites exist on `main` today, at `internal/format/oci/remote_serve.go:647`, `internal/format/oci/remote_blob_serve.go:200`, and `internal/format/oci/remote_blob.go:271` and `:433`. The retention write is therefore dispatched, and both columns carry samples. This defect is older than either arm of the 834 work, and the `docs:` merge request is a good place to correct it. ## Part 2 — the `container_remote_blobs` tombstone case Acceptance item 8 of work item 834 asks that the blob-membership probe carry no soft-delete predicate on any of three levels: `container_remote_blobs`, `container_remote_manifests` and `container_remote_images`. The probe SQL !2262 adds is `containerRemoteRepoStillReferencesBlobStmt` in `internal/datastore/repo_blob_references.go`, and it has two `EXISTS` legs: one over `container_remote_manifests`, one over `container_remote_blobs`. `TestContainerRemoteCacheStore_RemoteRepositoryHoldsBlob` in `internal/datastore/container_remote_fill_accounting_integration_test.go` covers two of the three levels as tombstone cases: - "a tombstoned manifest holding the digest still reads true" - "a manifest under a tombstoned image still reads true" It has a positive case for the blob leg, "a cached layer holding the digest reads true", and that case seeds a live row. So an `AND crb.soft_deleted_at IS NULL` added to the second `EXISTS` leg leaves the whole suite green. The follow-up adds one subtest to that same test. It seeds a layer fill and tombstones the `container_remote_blobs` row that carries the digest. It then fills the same digest under a fresh coordinate, and asserts that the probe still reports the blob as held by the repository. The assertion is the same shape the two existing tombstone subtests use. One option worth taking in the same merge request: pair each tombstone subtest with a `RepositoryReconcileStore.RecomputeSizeBytes` assertion over the marked rows. Acceptance item 8 asks that the probe reach exactly the rows the size recompute reaches. A predicate added to both walks passes a probe-only assertion. The suite the discarded branch built did pair them, in `TestContainerRemoteCacheStore_UpsertCacheFill_ProbeKeepsMarkedRows`. ## Part 3 — two nil-guard idioms in `internal/format/oci` This part records a review observation so it is not lost. It is not a defect in !2262, and it asks for no change to !2262. `internal/format/oci/emit.go` carries `isNilCollaborator`, a reflect-based test that catches a nil interface and an interface carrying a nil pointer, map, channel, function or slice. `NewCounterSink` uses it and panics on either shape, so a `*CounterSink` a caller holds always carries a real emitter. The package also uses the plain `x == nil` interface test in several places, and a typed nil passes that one. A second implementation of this same unit, on the branch `prozlach/issue-834-container-remote-fill-deltas`, threaded the `CounterEmitter` interface rather than the sink, and guarded its dispatch with `c.emitter == nil`. On that branch the emitter reached the fill through `buildOCIRemoteSlots`. `cmd/artifact-registry/wire_oci.go` calls that builder outside the `if store != nil` branch that builds the sink, so `NewCounterSink`'s panic was not on the path. That branch is discarded and its production diff is not proposed here. !2262 has no such gap, because it threads `*CounterSink` and inherits the constructor's check. What survives is the idiom question. A future collaborator threaded into `internal/format/oci` as an interface, rather than as a constructed sink, gets the weaker guard by default. Two options answer it, and neither is urgent: route every such collaborator through a constructor that calls `isNilCollaborator`, or use `isNilCollaborator` at the guard site itself. Whoever adds the next one should pick one of the two deliberately rather than by copying the nearest neighbour. ## How this was measured Every anchor and every line number above was read at `origin/main` `89fa85ea8`, where `docs/dev/storage-accounting.md` is 3,450 lines. !2262 was read at its head `6e8803f31`, and its documentation diff is 5 hunks over `docs/dev/storage-accounting.md`, `+6/-5`. Both readings were taken on 2026-09-02. The ten records survive the merge without a conflict. `main` gained three commits on `docs/dev/storage-accounting.md` after !2262's merge base, and none of their hunks reaches the ten anchors. ## Not established - Whether the `cmd`-level wiring hop is exercised in a running service. No test drives a container remote fill from `cmd`, on either implementation, and !2262's own follow-up list names why: `stubUpstreamDoer` panics rather than serving. - Whether a typed-nil emitter can reach `wireOCIWithStore` from a production path. Part 3 does not rest on the answer, because the branch that had the weaker guard is discarded. The question is open for the next interface-threaded collaborator, and answering it needs a caller sweep that no pass here ran. _This is a bot message 🤖 — /smurfit_