feat(oci): emit counter deltas at the five write and delete sites (S22 plan: 18/21)

What this step delivers

This step wires OCI's counter emission onto the storage-accounting emit API. It adds three increments and two decrements, at five sites:

  • the blob finalize (CompleteUpload)
  • the blob mount (MountBlob)
  • the manifest PUT
  • the manifest DELETE
  • the blob DELETE

The five sites read the repository-membership predicate on two different sides of their own rows. The blob finalize, the blob mount and the manifest PUT read before their write, because the write is what puts the digest in the repository. The manifest DELETE and the blob DELETE read after their rows are committed and gone. Before that commit, the row being removed still answers for the repository. All five emit after their rows commit. The counts come from the caller's own committed result, so a delete that removed no row emits nothing.

The namespace byte delta takes no membership read. The blob finalize decides it from the commit's own row outcome, and the manifest PUT from its payload write's outcome. No other site moves that counter.

The two decrements sit at the handler, never inside datastore.ContainerManifestDeleter or datastore.ContainerBlobUnlinker. The client delete, the artifact purge and the repository purge all reach those deleters, and each of the three owes a different delta set. An emit inside a deleter is therefore wrong for two of its three callers.

Δsize follows the repository's last detach from a digest. The handler decides that predicate from the repository's own container_blobs and container_manifests rows, which is the pair the size_bytes recompute walks. A payload blob can leave at the manifest DELETE, and a layer blob leaves only at the blob DELETE.

Two new files carry the machinery. internal/format/oci/emit.go holds the three seams the five sites share: CounterEmitter, RepositoryFootprint and the CounterSink that pairs them. internal/format/oci/emit_dispatch.go holds the dispatcher that runs the counter movements off the request goroutine.

The emits run off the request goroutine

No site can bound an inline emit from its own side. accounting.Emitter's record discards the caller's deadline and applies a bound of its own to each of its two Redis commands. A stalled pipeline therefore adds that bound once per scope the site emits to, on an operation whose rows are already committed. The blob finalize and the cross-repository mount emit before their 201 is written, so there the wait is the client's. The other three write their status first. net/http buffers that status until the handler returns, so the response still leaves only once the emit is done.

CounterSink.dispatch runs the movements on a goroutine instead. It takes a non-blocking slot from a process-wide semaphore, then spawns. The worker runs on a context detached from the request with context.WithoutCancel, under a 5-second bound of its own, and it recovers its own panic. context.WithoutCancel keeps the carried values, so the worker's records still land on the request that produced them. The cap is 64 dispatches in flight, and a dispatch past the cap is shed rather than queued. NewCounterSink hands the one semaphore to every sink it builds, so the cap covers every handler rather than each handler separately.

The two decrement sites dispatch the whole method, so their repository-membership read travels inside the detached context. That read runs after the response. On the request context, an abandoned client can cancel the read before it reaches Postgres, and CounterSink.readBlobFootprint answers a failed read as "still referenced". That answer retires the byte decrement for rows that are already gone, and it leaves repositories.size_bytes reading high. The three increment sites keep their read on the request goroutine. That read runs before their own write, on a context the write needs live anyway.

A shed drops the deltas, and nothing repairs them today. The operation's rows are already committed and the site has no retry. A reconciliation pass repairs a dropped delta, because it recomputes the scope from those same rows rather than from anything the emit recorded. No such pass runs. accounting.RegisterAsynqHandlers has an empty body, so nothing is registered to fire one. Until something is, a drop stands for as long as the counter row does.

Process exit drops the deltas on the same terms. Nothing drains these goroutines, where an inline emit was inside http.Server.Shutdown's drain.

This is the eighth in-tree copy of the detach-bound-spawn-shed shape. The seven it copies each hold a process-wide semaphore of their own:

  • npm's buffered counter
  • npm's packument rebuild
  • npm's inline build
  • Maven's buffered counter
  • the management API's buffered counter
  • the remote package's standalone download bump
  • this package's own download signals

Every one of them admits its own 64 workers against the connection pool the request path draws on. The total therefore grows with each copy, and no cap bounds it. Unifying them behind one pool-aware helper is #559.

The drop paths are metered

Three arms of the emit path retire a byte delta and move no counter:

  • a dispatch shed at a saturated cap
  • a repository-membership read that failed
  • a manifest payload digest that does not decode

Each arm writes a Warn line and answers 2xx. Before this change, the question "is the emit path losing deltas right now" had an answer in the logs and none on /-/metrics. reconciliation_drift_bytes shows the resulting drift one interval later, and it carries level and direction only. It therefore separates neither a dropped delta from a wrong one, nor one arm from another.

The three arms are metered on the existing bufferedCounterUpdates vector rather than on a new one. result=dropped and result=panic already exist, and the column label gains one value, size_bytes, inside the count budget internal/metrics/cardinality.go carries for that label. A shed dispatch also loses the artifact and component counts the same operation owed, and no label separates that. size_bytes names the byte counter every drop arm has in common. The vector's doc comment says so, rather than leaving the reader to infer an inventory. The vector stays S16's and this path reuses it, so no spec amendment follows.

Reuse costs two edits. The Help text scoped the vector to the container remote read path, and it now covers both producers. The catalog row in docs/dev/observability.md is rewritten to match.

Merge order for the metric. While a container-remote read that finds no fresh row is answered as not implemented, no retention write is ever dispatched. Every sample on this vector then comes from the emit path. Once !1764 (merged) makes that path serve from the cache, the same series carries both producers. A query that does not filter on column then sums two unrelated failure classes. Both the doc comment and the catalog row state it in that form rather than as a status.

The membership read keys on digest

dbRepositoryFootprint.BlobFootprint is the SQL stand-in for the production membership read, and it lives in internal/format/oci/emit_integration_test.go. It filtered both EXISTS arms on blob_sha256, and no index on container_blobs or container_manifests carries that column. The planner had no index to probe for the digest. It enumerated the repository's images and tested the column on the heap, at a cost that grows with the repository's own row count. That cost is paid in full on a miss, which is the answer that moves a counter.

On digest, the blobs arm probes index_container_blobs_on_namespace_id_and_digest directly, and the manifests arm becomes one equality probe per image on the unique (namespace_id, container_image_id, digest) index. This merge request adds no index and writes no migration. container_manifests gains no (namespace_id, digest) index of its own. A parent-level CREATE INDEX on these tables recurses to all 64 partitions and blocks writes on every one of them. CONCURRENTLY is unavailable on a partitioned parent, so a later drop is on the same terms.

The two columns hold one value by construction rather than by constraint. The only non-test writers of either table are internal/datastore/container_blob_linker.go and internal/datastore/container_manifest_persister.go, and each sets digest and blob_sha256 from one variable. No CHECK constraint ties the pair, so this equality rests on those writers rather than on the database. The production statement this stand-in represents belongs in internal/datastore, behind the same RepositoryFootprint seam. Work item #758 (closed) carries it as the first of its four deliverables.

The fixtures did not hold that equality, and one of them was wrong. seedContainerManifestRow wrote a random digest beside the caller's blob_sha256, which is a row no production writer can produce. Once the two columns agree, unique_container_manifests_ns_id_ci_id_digest forbids the shape the shared-payload fixtures staged, which was a sibling manifest in the same image. Both fixtures now stage the sibling in a second image of the repository. Both columns hold the payload's sum there, which is the shape a push writes.

The emit sites are inert until a composition root wires the sink

This merge request adds five OCI emit sites. Each site reaches the accounting pipeline through a *oci.CounterSink field on its handler. That field is nil until a composition root passes WithUploadCounters, WithManifestCounters or WithBlobCounters. No call site under cmd/artifact-registry/ passes one, and no call site calls oci.NewCounterSink. No OCI counter moves in the running service after this merge request lands.

The dispatch is inert on the same terms, and this matters more now that the shed, the cap and the metric hang off it. CounterSink.dispatch returns before it takes a slot when the sink carries no emitter. An unwired handler therefore spawns no goroutine, sheds nothing, and moves no sample on the buffered-write vector.

A zero-value oci.CounterSink{} is inert on the same terms. Both fields are unexported, so a composite literal in any package reaches no constructor check. Every sink method therefore guards the collaborator it is about to reach, and TestCounterSink_ZeroValueIsInertAtAnEmittingSite drives a finalize through such a literal.

The five sites are the blob finalize (CompleteUpload), the blob mount (MountBlob), the manifest PUT, the manifest DELETE and the blob DELETE. The delta arithmetic, the emit order and the repository-membership predicate are written and tested at all five, including against real Postgres rows. The tests drive the handlers through the exported options, so no test changes when a composition root wires the sink later.

This is not a regression. OCI emitted no counter before this change. Container repositories report artifacts_count 0 and size_bytes 0 both before and after this merge request.

A tracking work item carries the wiring: #758 (closed). It carries four deliverables. The first is the production RepositoryFootprint store. The second is the sink construction in cmd/artifact-registry/wire_oci.go, together with the boot check on the counters option. The third is the docs/dev/storage-accounting.md section on how a composition root wires the sink. The fourth is a guard for a nil pointer boxed into CounterEmitter, a shape the per-site guards above do not cover.

Measured against a running service

This is a measurement, not a reading of the code. A run against a booted service drove all five sites. The exercise did two blob finalizes, a manifest PUT, a cross-repository mount, a manifest DELETE and a blob DELETE. Each request returned its normal status. repositories.artifacts_count, repositories.size_bytes, namespace_statistics.components_count and namespace_statistics.deduplicated_size_bytes read 0 after every one of those steps.

The accounting pipeline was up for that run, so the zeros mean that no emit ran, not that no target existed. The service acquired the metrics lease ar:lease:accounting:reconciliation_backlog, and it registered both periodic job kinds, counter_drain_chunk_repo and counter_drain_chunk_namespace. One emit at any of the five sites creates a counter hash and adds its scope to a dirty set. After the exercise, redis-cli --scan 'ar:*' returned the lease key alone, and both dirty sets were absent. A control write of test values into the four columns came back through the same reading query, so that query does reach the columns.

The run predates the dispatch change described in The emits run off the request goroutine. Its result still holds, because no composition root passes a sink on either revision, so no emit ran on either.

Spec amendment: acceptance criterion 19

This merge request amends docs/specs/S22-storage-accounting.md in two hunks, and both sit inside acceptance criterion 19. Hunk one replaces line 1027. Hunk two turns line 1032 into lines 1032 through 1035. No other line of the spec changes.

Hunk one: the membership rows, and the recompute they must match. The line said the handler decides the repo-scoped last detach from the repository's own live container_manifests rows alone. Repository membership for a container digest spans two tables, and the criterion named one of them. The line now names container_blobs and container_manifests, and it says they are the pair the size_bytes recompute walks.

The same line also drops the word live, because the recompute it now cites carries no soft-delete predicate. Line 584 of the same document records that repositories.size_bytes includes soft-deleted rows, and criterion 9 says the same. The comment on recomputeContainerBlobsSizeStmt in internal/datastore/reconcile_repository.go states that no level of the walk carries the predicate. Neither container_blobs nor container_manifests has a soft_deleted_at column to filter on. An implementer who kept the word decrements where the recompute does not, and every reconciliation pass then restores the bytes.

The table pair introduces no new position. Line 266 of the same document, in Two trigger sites, two event shapes, already says repository membership is reachable only through the format's own rows, and it names both container tables. Line 610, the Recompute/delete consistency paragraph, already says the size_bytes recompute joins through the same pair. ADR-007 says the same in its Repository-level storage accounting reconciliation section. The amendment removes a disagreement inside one document.

Hunk two: the two race interleavings. Criterion 19 closed with one sentence for the two interleavings, client-then-purge and purge-then-client. That sentence said that components_count falls by exactly one on each interleaving, and that a following reconciliation does not move it. The purge-then-client arm needs a decrement from S20-A's purger. That purger exists on no branch, so this merge request cannot verify that arm in full.

The closing sentence is now three sentences. Client-then-purge keeps the full claim. Purge-then-client claims S22's own side, and it attributes the purge's own decrement to S20-A's purger. Reconciliation supplies the movement until that emit exists.

This merge request asks the spec author or the DRI to review both hunks.

One question for that review

The membership read's failure path. The Error Cases table has no row for a failed repository-membership read. CounterSink.readBlobFootprint in internal/format/oci/emit.go answers "referenced" when that read fails. The byte delta is then retired and the client operation succeeds. A Warn line and one dropped sample on the buffered-write vector are the whole record of the drop. The counts the same operation owes still move, because they come from the caller's own affected-row result. This matches the table's standing posture: drop the delta, and let reconciliation correct the value. The enumeration is incomplete rather than the behavior wrong, so this is a request for one more row.

The plan correction, routed to the single writer

Three lines of docs/plans/2026-08-04-s22-storage-accounting.md disagree with the spec after this merge request. Lines 229 and 1291 name container_manifests alone as the repository-membership rows. Line 1300 carries the closing verification clause that criterion 19 carried before this change.

This merge request corrects none of the three. CLAUDE.md:275 says that step merge requests do not edit the plan file. That file has a single writer, and one docs(plans) change carries all three lines together.

Files beyond the plan's list

The plan's step 18 Files: entry names four files to modify: internal/format/oci/store.go, internal/format/oci/manifest.go, internal/format/oci/manifest_delete.go and internal/format/oci/blob.go. This merge request touches eight more production files, and it also edits docs/dev/observability.md. The spec amendment has its own section, Spec amendment: acceptance criterion 19. The plan file is not corrected here, because a step merge request does not edit it. A single writer fills the plan file in a docs(plans) change.

File Why it is in this diff
internal/format/oci/emit.go (Create) Holds the CounterEmitter and RepositoryFootprint seams, the CounterSink type and the sink's helpers. The plan named no file for them.
internal/format/oci/emit_dispatch.go (Create) Holds CounterSink.dispatch, the in-flight cap, the bound and the drop meter. The plan named no file for them either, because it planned the emits inline.
internal/format/oci/manifest_push.go (Modify) The plan named manifest.go for the PUT emit. The PUT arm is ManifestHandler.persist, which lives in manifest_push.go. manifest.go holds the handler type and its options, and it took the WithManifestCounters option and the PayloadDeduplicated field.
internal/format/oci/upload.go (Modify) Carries the sink to CompleteUpload and to MountBlob, and adds WithUploadCounters.
internal/format/oci/metrics.go (Modify) Widens bufferedCounterUpdates to the emit path's drop arms. The Help text and the doc comment name both producers and say which column value belongs to each.
internal/metrics/cardinality.go (Modify) Comment only. The column budget is unchanged at 10, and the comment now records size_bytes as the value the emit path adds.
docs/dev/observability.md (Modify) The catalog row for gitlab_artifact_registry_oci_buffered_counter_updates_total, rewritten to match the new Help text and to carry the merge-order condition for !1764.
cmd/artifact-registry/wire_oci.go (Modify) Comments only. assertManifestOptionsWired fails boot on a ManifestOption that degrades silently. Its doc now names the two options it covers, and says why the counters option is not one of them while no composition root passes a sink.
internal/format/oci/remote_serve.go (Modify) Comments only. The emit path's two drop records reuse the namespace_id and repository_id log field names this file pins, rather than respelling them. The doc comment on those constants now says so.

Test files beyond the plan's list

The plan's step 18 Tests: entry names internal/format/oci/emit_test.go and an assertion in the manifest-delete suite. This merge request adds four more test files and widens five existing ones. The plan file is not corrected here, because a step merge request does not edit it. A single writer fills the plan file in a docs(plans) change.

internal/format/oci/emit_integration_test.go (Create) carries the cases the step's Acceptance asks for and the fakes in emit_test.go cannot produce:

  • a reconciliation pass that runs after the deltas settle and does not move them
  • a push-then-hard-delete round trip that nets the four counters back
  • a cross-repository mount that moves the destination bytes, leaves the namespace bytes and the source repository alone, and agrees with a recompute on all three
  • a concurrent-detach bound, and the reconciliation pass that repairs it
  • a client delete racing a purge, on both interleavings
  • a probe that reads each site's rows from outside the writing transaction

The Tests: entry under-specifies its own Acceptance.

internal/format/oci/emit_dispatch_test.go (Create) covers the properties the dispatch owns and no site can show:

  • the caller's cancellation does not reach the work
  • a saturated cap sheds rather than queues
  • a panicking seam is contained rather than fatal
  • every path returns the slot it took

It sits in package oci, because the cap and the bound are unexported. A deterministic saturation also needs a sink built over a semaphore of its own.

Three of its cases come from a diff against remote_download_test.go, which covers the same shape and already had them:

  • the process-wide cap's channel identity
  • a nil context that leaks no slot
  • a slog.Handler that panics inside the worker's own recover

internal/format/oci/export_test.go (Create) exports two test-only helpers. NewTestCounterSink builds a sink over a private cap, because the process-wide one is shared with every other suite in the package. AwaitEmits blocks until every dispatch a sink launched has finished, and a suite calls it between driving a site and reading what the site emitted. It waits by taking every slot of the cap and giving the slots back, which orders the reads after the workers. Reading the channel's length is not a synchronizing operation and orders nothing.

internal/format/oci/emit_internal_test.go (Create) covers one guard in ManifestHandler.readPayloadFootprint. The guard answers a manifest digest that does not decode. Both manifest routes hand that method a canonical digest, so no exported entry point reaches the guard. Only a call from inside the package can drive it.

Five existing test files are widened:

File What changed
internal/format/oci/manifest_test.go Adds the manifestRepositoryID fixture and the fakePersister.payloadDeduplicated field. The manifest PUT emit reads both.
internal/format/oci/manifest_delete_test.go newDeleteManifestHandler takes opts ...oci.ManifestOption, so a case can wire the sink.
internal/format/oci/blob_delete_test.go newDeleteHandler takes opts ...oci.BlobOption, for the same reason.
internal/format/oci/metrics_test.go The cardinality audit now observes both column values. column is count-bounded rather than pinned, so only a value the gather phase sees counts against the budget, and size_bytes was invisible to the check.
internal/format/oci/tags_integration_test.go seedContainerManifestRow takes both content-address columns from its caller, rather than generating a random digest of its own. A caller that needs a distinct row and reads neither column back still passes a random digest.

Spec coverage

Spec: docs/specs/S22-storage-accounting.md

Acceptance criteria

# Criterion Tests
AC-1 A repo-scoped increment reaches repositories.artifacts_count/size_bytes after the next drain tick TestEmitIntegration_PushMovesAllFourCountersAndAgreesWithTheRecompute, TestEmitIntegration_SecondPushOfHeldBlobsMovesOnlyTheCounts (this step composes OCI's sites with the pipeline; the pipeline half is Step 8's)
AC-2 A namespace-scoped increment reaches namespace_statistics.components_count/deduplicated_size_bytes after the next drain tick TestEmitIntegration_PushMovesAllFourCountersAndAgreesWithTheRecompute, TestEmitIntegration_SecondPushOfHeldBlobsMovesOnlyTheCounts
AC-3 Concurrent increments to one scope sum exactly Step 8's chunk-worker suite. Not this step.
AC-4 A scope re-marked mid-claim is captured with no lost delta Step 8. Not this step.
AC-5 A chunk past drain_chunk_stale_timeout bails and re-adds Step 8. Not this step.
AC-6 A chunk failing every attempt re-adds before its terminal error Step 8. Not this step.
AC-7 Redis unavailable at increment time does not fail the operation Step 6's emit_faults_integration_test.go. Not this step.
AC-8 Reconciliation clears the buffer before its scan Step 14. Not this step.
AC-9 Reconciliation recomputes with the correct soft-delete visibility Steps 11-13. Not this step.
AC-10 Every version-type table is a positive hit for the recomputes Steps 11-13. Not this step.
AC-11 A discrepancy is recorded on the unit-matched drift histogram Step 14. Not this step.
AC-12 A crash between HINCRBY and SADD still converges Step 6. Not this step.
AC-13 A hash's TTL is refreshed on every write Step 5. Not this step.
AC-14 The three migrations apply cleanly Steps 1 and 2. Not this step.
AC-15 The blob_storage_blobs shadow triggers stay exactly consistent Step 2. Not this step.
AC-16 Every namespace has a zero-valued namespace_statistics row Step 1. Not this step.
AC-17 npm's publish increment is swapped onto the pipeline Step 17. Not this step.
AC-18 OCI emits increments at CompleteUpload, MountBlob and manifest PUT; a push new to repository and namespace moves all four counters, a push over present blobs moves only the counts TestCompleteUpload_EmitsFirstAttachDeltas, TestMountBlob_EmitsRepositoryBytesOnFirstAttachOnly, TestManifestPush_EmitsArtifactComponentAndPayloadDeltas, TestEmitIntegration_PushMovesAllFourCountersAndAgreesWithTheRecompute, TestEmitIntegration_SecondPushOfHeldBlobsMovesOnlyTheCounts, TestEmitIntegration_MountMovesTheDestinationBytesAndAgreesWithTheRecompute, TestEmitIntegration_IdempotentRePushMovesNothing
AC-19 OCI emits the decrements at the delete handler, never inside the deleters; Δsize on the repository's last detach from either client operation TestManifestDelete_EmitsCountsAlwaysAndBytesOnRepositoryLastDetach, TestManifestDelete_ByTagEmitsNothing, TestBlobDelete_EmitsBytesOnRepositoryLastDetach, TestEmitIntegration_ByteDeltaFollowsRepositoryMembershipOnDelete (the three sequential cases), TestEmitIntegration_LayerBytesLeaveOnlyAtTheBlobDelete (the fourth), TestEmitIntegration_ConcurrentDetachIsBoundedAndRepaired (the fifth), TestEmitIntegration_PushThenHardDeleteNetsBackAndSurvivesReconciliation, TestEmitIntegration_ManifestDeleterIssuesNoDeltaOfItsOwn, TestManifestDeleteCascadeWritesNoCounterColumn, TestEmitIntegration_ClientDeleteRacingAPurgeCountsTheManifestOnce, TestManifestDelete_UncommittedDeleteEmitsNothing, TestBlobDelete_UncommittedDeleteEmitsNothing. Partial: the round-trip case asserts deduplicated_size_bytes holds rather than returns, because its bytes leave at the GC pass, which does not exist yet (S28).
AC-20 Maven's upload emits from a post-commit site Step 19. Not this step.
AC-21 Repository cascade hard-delete emits at the purger Gated on #464 (closed) and S20-a. Not this step.
AC-22 Reconciliation fans out one task per namespace Step 15. Not this step.
AC-23 A namespace with no statistics row gets one on its first pass Step 14. Not this step.
AC-24 In-flight reconciliation tasks never exceed the cap Step 14. Not this step.
AC-25 Source-first ordering, asserted per site (OCI's four rows) TestCompleteUpload_EmitsAfterTheLinkAndReadsBeforeIt, TestMountBlob_EmitsAfterTheLinkAndReadsBeforeIt, TestManifestPush_EmitsAfterThePersistAndReadsBeforeIt, TestManifestDelete_ReadsMembershipAfterTheCascadeCommits, TestBlobDelete_ReadsMembershipAfterTheUnlinkCommits, TestEmitIntegration_RowsAreCommittedBeforeTheEmit (one subtest per site, all five, probed from outside the writing transaction); zero-emission arms in TestCompleteUpload_FailedFinalizeEmitsNothing, TestMountBlob_UnmountedOrFailedEmitsNothing, TestManifestPush_RejectedPushEmitsNothing, TestManifestDelete_UncommittedDeleteEmitsNothing, TestBlobDelete_UncommittedDeleteEmitsNothing. The npm and Maven rows are Steps 17 and 19's.
AC-26 last_reconciled_at is stamped only after every repository is written back Step 14. Not this step.
AC-27 A trigger fire selects only stale namespaces Step 15. Not this step.
AC-28 A namespace with an outstanding task is enqueued at most once Step 15. Not this step.
AC-29 reconciliation_backlog is a single-writer collector Step 16. Not this step.
AC-30 A chunk whose scope was reconciled mid-flight skips it Steps 8 and 14. Not this step.
AC-31 counter_dirty_set_size is sampled once per tick before SPOP Step 10. Not this step.
AC-32 Config load rejects each invalid configuration Step 3. Not this step.
AC-33 The six metrics are registered with their exact descriptors Steps 6, 8, 10, 14 and 16. Not this step. The drop meter this step adds is not one of the six: it reuses S16's OCI buffered-write vector, whose descriptor TestRegisterMetrics_PassesCardinalityAudit covers.
AC-34 The reconciliation saturation policy re-enqueues rather than sheds Step 14. Not this step.
AC-35 A namespace-scoped chunk with no statistics row drops its delta and deletes :flushed Step 8. Not this step.
AC-36 A failing recovery SADD does not lose the delta Step 8. Not this step.
AC-37 Every management-API delete emits once its transaction commits Deferred with #313 (closed). Not this step.
AC-38 A persistently failing reconciliation task stays re-enqueueable Step 14. Not this step.

Error cases

# Condition Tests
E-1 Redis unavailable at increment time Step 6's fault-injection suite. Not this step. An OCI site cannot observe the drop: both emit methods return nothing, and the dispatch that calls them returns nothing either.
E-2 Redis unavailable at drain-trigger time Step 10. Not this step.
E-3 Chunk job's Postgres UPDATE fails Step 8. Not this step.
E-4 Chunk job's :flushed DEL fails after the UPDATE Step 8. Not this step.
E-5 Chunk job exhausts all retry attempts Step 8. Not this step.
E-6 Recovery SADD itself fails Step 8. Not this step.
E-7 Chunk dequeued past drain_chunk_stale_timeout Step 8. Not this step.
E-8 Worker dies mid-chunk after merging into :flushed Step 8. Not this step.
E-9 Two chunks run one scope concurrently Step 8. Not this step.
E-10 Trigger's EnqueueTx fails while the process is alive Step 10. Not this step.
E-11 Crash between a trigger's SPOP and its commit Step 10. Not this step.
E-12 Assigned row hard-deleted before its chunk drains Step 8. Not this step.
E-13 Namespace-scoped chunk finds no namespace_statistics row Step 8. Not this step.
E-14 Crash between HINCRBY and SADD Step 6. Not this step.
E-15 Crash between reconciliation's clear and its SET Step 14. Not this step.
E-16 Reconciliation scan races a concurrent increment Step 14. Not this step.
E-17 A chunk and a reconciliation process one scope concurrently Steps 8 and 14. Not this step.
E-18 Reconciliation finds a discrepancy Step 14. Not this step. The OCI side asserts the complement — that a pass finds none: every integration case reconciles after its drain and asserts the counter does not move.
E-19 Namespace has no namespace_statistics row when its task runs Step 14. Not this step.
E-20 Reconciliation task fails before its final UPSERT Step 14. Not this step.
E-21 A namespace can never be reconciled Step 14. Not this step.

The three arms that retire a byte delta have no row of their own in the table. One question for that review asks the spec author for the membership-read row. Each of the three is covered by a test: TestCounterSinkDispatch_ShedsPastTheCap, TestCounterSink_FailedMembershipReadRetiresTheByteDelta and TestReadPayloadFootprint_UndecodableDigestRetiresTheByteDelta. TestCounterSinkDispatch_ContainsAPanickingSeam and TestCounterSinkDispatch_ContainsAPanickingLogHandler cover the fourth outcome the vector carries.

Security considerations

# Concern Tests
S-1 Redis keys carry only internal UUIDs, so no key injection or cross-slot relocation Step 6 owns the key-grammar assertions. This step's contribution is that every OCI site passes uuid.UUID identifiers straight from its resolved rows and no request text reaches a key: assertRepoScope (emit_test.go) pins the identifiers each site emits against, at all five sites.
S-2 Counter values feed billing, so a wrong value has financial impact Every integration case runs a reconciliation pass after its drain and asserts the counter does not move, which is what says the emitted delta and the source rows agree: TestEmitIntegration_PushMovesAllFourCountersAndAgreesWithTheRecompute, TestEmitIntegration_SecondPushOfHeldBlobsMovesOnlyTheCounts, TestEmitIntegration_MountMovesTheDestinationBytesAndAgreesWithTheRecompute, TestEmitIntegration_ByteDeltaFollowsRepositoryMembershipOnDelete, TestEmitIntegration_PushThenHardDeleteNetsBackAndSurvivesReconciliation, TestEmitIntegration_ConcurrentDetachIsBoundedAndRepaired, TestEmitIntegration_ClientDeleteRacingAPurgeCountsTheManifestOnce.
S-3 No new credential surface Owned by the composition root. Not tested here: this step's sites reach Redis and Postgres only through collaborators handed to them.

The metric label values are compile-time constants and carry no user input. The new column value therefore adds no cardinality risk beyond the one budget entry.

e2e scenarios

This merge request adds no scenario to docs/testing/, and it changes no scenario there. No client-observable OCI behavior changes. The manifest PUT, the blob upload, the blob mount and the two DELETE routes return what they returned before. Every counter emit sits behind a *oci.CounterSink that is nil until a composition root passes one. No composition root passes one, so the emits stay inert in the deployed service. docs/testing/e2e/oci.md therefore has no counter behavior to describe, and it needs no new scenario.

Conformance

The OCI conformance suite ran against this branch: 80 specs, 75 ran, 75 passed, 0 failed and 5 skipped. The harness is scripts/conformance/run.sh, at its own pinned revision of opencontainers/distribution-spec. CI runs the same suite on two backends, and conformance:oci:s3-garage and conformance:oci:gcs-key-creds both passed on the gating pipeline named below.

Verification

Pipeline 2778129154 is green. The second parent of its merge commit is 070b51fd4, which is this branch's head, so the pipeline tested this head. test:race, go_unittests and golangci_lint passed there. test:integration passed on PostgreSQL 16, 17 and 18. That job's package list includes ./internal/format/oci/... under -tags=integration, so the whole OCI integration package ran and passed in CI.

Locally, go test -race on internal/format/oci passed, and the targeted integration runs passed. The whole internal/format/oci integration package did not run locally. Six attempts ended in SQLSTATE 53200, which is a condition of the host rather than a result from the suite. The CI runs above are the evidence for that package as a whole.

Reviewable size

The diff is 25 files, 4746 insertions and 58 deletions, measured against the merge base with main (3f0d92e6). docs/dev/development-model.md asks for a split or a justification past 500 reviewable lines.

Group Files Added Deleted
Production Go 12 923 35
Unit tests 8 2202 6
Integration tests 3 1615 14
Spec and docs 2 6 3
Total 25 4746 58

The production change is 923 added lines. internal/format/oci/emit.go is 381 of them, and internal/format/oci/emit_dispatch.go is 206. The five call sites and their options take 286 more, across blob.go (90), store.go (57), manifest_push.go (48), manifest_delete.go (41), manifest.go (29) and upload.go (21). internal/format/oci/metrics.go takes 28 for the reused vector. The last 22 added lines are comment corrections in cmd/artifact-registry/wire_oci.go (12), internal/format/oci/remote_serve.go (6) and internal/metrics/cardinality.go (4). Test code is 3817 of the 4746 added lines, which is 80 percent of the diff.

A split does not help here. The five sites share one emit surface, oci.CounterSink, and one integration harness. Acceptance criterion 25 asks for the committed-before probe and the zero-emission arm at every site, not at a representative site. The plan defines the five sites as one step. A split by site opens five merge requests over one contract, and each one carries a copy of the same harness.

The dispatch and the drop meter are not separable from the sites either. The dispatch changes where every one of the five emits runs, and the meter records what the dispatch drops. A separate merge request for either one lands a shed with no counter, or a counter with nothing to count.

Merge order with !1754 (merged)

!1754 (merged) (feat(managementapi): emit s22 accounting deltas on management deletes (S17 Phase 4 plan: 38/38), source branch 313/step-38-s22-accounting-emit) also changes internal/format/oci/manifest_delete.go. It widens the manifestDeleteTxRunner seam with a freedSizeBytes int64 return, and it discards that value in ManifestDeleter.DeleteManifestByDigest. It adds two comments that give the reason for the discard: the OCI protocol handler moves no storage-accounting counter.

No merge order is forced. The two merge requests edit different parts of the file, so git merges them without a conflict. This branch declares no implementation of the widened seam. It calls only the exported adapter, whose signature !1754 (merged) leaves unchanged, so the widening cannot break this branch either.

Whichever merge request merges second rewrites both comments. This step makes the OCI protocol handler move exactly those counters, through ManifestHandler.emitDeleteDeltas in the same file. Both comments then state the opposite of what the file does. The discard loses its stated reason at the same moment, because that reason is the claim this step falsifies.

The two changes compute one predicate twice. !1754 (merged)'s freedSizeBytes is the payload blob's size when the cascade removed the repository's last reference to it. The cascade evaluates that predicate inside its own transaction and then discards the answer. This step's ManifestHandler.readPayloadFootprint reaches the same predicate through an EXISTS over the same two tables, scoped to the same repository, read after the commit. After both merge requests land, the transaction computes the answer and the handler reads it again on a different consistency window. The two mechanisms can converge later, with the handler consuming freedSizeBytes instead of reading the predicate again. That is a design question for the two authors once both changes are on main.

This branch is not pre-edited to match !1754 (merged). The two comments do not exist on main, and !1754 (merged) can still change or be rejected.

Merge order for the sink's own wiring

The wiring in #758 (closed) carries one merge-order constraint, and this merge request is not the deadline. For OCI, an unwired sink leaves a counter at 0, which is the value the counter carries today. Plan step 17 moves npm publish off its live direct UPDATE and onto the same emit API. The same gap then costs npm a counter that works today. The wiring must therefore merge before plan step 17.

A documented check this branch falsifies

docs/dev/storage-accounting.md states a check: grep -rn 'accounting\.NewEmitter' --include='*.go' . matches only files under internal/accounting/. This branch adds two matches in internal/format/oci/emit_integration_test.go, so the stated result of that check is now wrong. The claim the check supports stays true, because no production caller constructs an emitter.

This merge request does not correct the line. The correction belongs to the merge request for plan step 17, which owns that page. Plan step 17 has no merge request in any state. If this merge request merges first, main carries a documented check that fails for the next reader who runs it.

Related to #515

This is a bot message 🤖 — /smurfit

Edited by Pawel Rozlach

Merge request reports

Loading
Loading