feat(managementapi): emit s22 accounting deltas on management deletes (S17 Phase 4 plan: 38/38)
Why
S22 criterion 37: every management-API delete emits its storage-accounting deltas once its own transaction commits, covering both halves of the endpoint, the targets it tombstones and the targets it hard-deletes. Until this lands, the delete handlers and bulk workers from this plan's earlier steps move rows without moving artifacts_count or size_bytes. Reconciliation does correct those columns, because its recompute filters tombstoned rows out and overwrites whatever it finds, so what the emit buys is timeliness between passes rather than a correction that would otherwise never arrive. The emit API (internal/accounting) landed through S22's own plan. This step is the one emit site joining the two, across the eight format arms of the five delete handlers and the three bulk workers.
What (non-obvious)
- The handler arms dispatch the emit off the request path; the bulk workers emit inline. An emit is up to two
accounting.Emitter.recordcalls, one per counter family, and each opens its owndetachedEmitTimeoutwindow on a context it detaches withcontext.WithoutCancel, so no caller can shorten it and a blocked-but-answering Redis spends both. Against the deployed 10sserver.timeouts.writethat is the whole budget, andwriteAcceptedsets the status without flushing, so inline the 202 of an already-committed delete was at risk. Workers keep the inline call because they have no response to protect, and because one dispatch per applied batch entry would burst a whole batch against the shedding cap and drop its own tail along with every concurrent handler's emit.deleteCounterEmitsmeters the split, and a zero-delta emit is suppressed before it spawns. - The container-manifest arm's freed-size probe runs on
deleteByDigest's envelope rather than insidecascade, which became a sequence shared withContainerReaper's image walk in chore(datastore): container hosted reap (S20-A ... (!1711 - merged) • Pawel Rozlach • 19.4 and whose signaturereapManifestCascaderpins. The walk discards any report, so widening the sequence would make every reaped manifest pay the probe's image-row lookup and its twoEXISTSarms for nothing.cascadeReportingPayloadSHA256carries the payload address up to the envelope, which already holds the transaction. - Every delta comes out of the delete's own transaction (the widened composer returns), never a post-commit recount. Closed beta's retention window is zero, so a recount can find the purger already through and lose the decrement for good. The reap-race walks pin this with fixtures every post-commit read diverges on. The literal commit-then-reap interleaving rides the emit-side gofail seam (
mgmtDeleteCountersEmitHeld) and lands with the S20-A purger. - Tag and dist-tag deletes move no counter, pinned as tests rather than left as absence.
- The npm version arm keeps its landed ungated event while gating the counter on
Marked: the event counts committed operations, the counter moves once per row transition. A test pins the asymmetry. - The Maven version arm now gates its event as well as its counter, which changes an existing usage-data stream. Its seam widens from
SoftDeleteMavenVersion(...) errortoMarkMavenVersion(...) (applied bool, err error), so a delete that loses the mark race answers202and emits nothing where it previously emitted oneartifact_registry_artifact_deleted.TestMavenVersionDelete_LostRaceEmitsNothingpins it. The arm now matches the Maven package, container image, and npm package arms, and the npm version arm is the only route left on the other side. A consumer comparing delete-event volume across this merge sees a step change on that route. - Three remote bulk arms arrived from
mainafter this MR's emit plumbing (the Maven one with S17 Phase 6 step 31, the npm one with step 32, and the container one with step 33 throughbulk_container_worker_remote.go), so the rebase had to say what each does with theCountersseam. All three emit nothing. The Maven and npm eviction composers report neither the rows a mark hid nor the bytes a file delete freed, sorepositories.artifacts_countandrepositories.size_bytesread too high until the next reconciliation pass, whilenamespace_statistics.components_countis correct and right not to move, on the ruledeleteCounterTargetTombstonerecords for the hosted arms.remoteOpsrecords the gap and points at Remote eviction leaves repository counters stal... (#775) • Pawel Rozlach (scoped to Maven before the npm arm landed),bulkNpmPassForKindrefers to it for the npm twin, and the container worker's kind dispatch points atremoteContainerImageEvict, which records the container split: onlyartifacts_countreads stale until reconciliation, because its recompute filters marked rows while the size and components recomputes count them. Each Maven remote wrapper names its own target so the suppression rests on the zero amount rather than the enum's order,TestMavenBulkWorker_RemoteArm_EmitsNothingpins the Maven arm, and the container remote rig's drain walk pins the container one. The single-target container delete handlers gained their remote evict arms frommainin an earlier rebase and sit on the same split. MavenPackageStore.SoftDeleteMavenPackageandMavenVersionStore.SoftDeleteMavenVersionlose their last production callers to the marker composers. Both are kept rather than removed: they stay under test, and six doc comments plus the S20-A and S22 documents name them.- This branch no longer builds its own emitter:
main's S22 upload steps addedwireAccounting, which builds the samewiring.counterEmitterahead of both of this MR's consumers in tier order, so the rebase resolution adopted that build, dropped the branch's field and wireCache-adjacent construction, and one emitter now serves the upload and delete paths. - A dedicated commit conforms every comment block this diff touches to the ci: gate Go comment caps on changed blocks (!1828 - merged) • Hayley Swimelar • 19.4 caps: 191 flagged blocks, a net 1,525 comment lines removed, with the review-settled facts and the #775/#761 (closed) pointers kept in capped form. A follow-on pass re-conforms six blocks to the checker's current head, whose blank-line join postdates that commit. The conformance deliberately leads the unmerged gate: the checker reads only blocks a diff touches, so landing unconformed would leave these files to a post-merge sweep that re-touches every reviewed block, and this diff is the widest comment surface the gate would meet.
requireDepsregainsmaintidx, because the merged tree re-crosses the threshold after the eviction rebase.
Test plan
- Handler-level emit assertions for all eight arms, per-committed-entry assertions for the three workers, and nothing emitted on lost races, failed writes, and misses.
- Criterion-37 integration walks over the production composers, including the npm reap-race divergence fixture and a concurrent-delete race asserting exactly one emit.
- Local, against PG 16 and Redis 7:
go build ./...,go veton both tag sets, unit and integration suites forinternal/managementapi,internal/datastore,cmd/..., andinternal/format/oci, all green, with the reaper suites under-raceand each new freed-size subtest checked against its mutant. - golangci-lint 2.12.2 (
--build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false): 0 new-line findings ininternal/managementapi. The datastore test files add 74contextcheckfindings on new lines, matching the package's pre-existing pattern under the integration tag (about 3.9k findings, invisible to CI lint, and the previous datastore MR added 64 of the same shape).
Diff size
+3970/−1442 over 88 files: tests +2841/−464, datastore composers +498/−403, managementapi handlers, workers, and the emit helper with its dispatcher +542/−400, cmd wiring +74/−148, and docs and OCI glue +15/−27. The name cardinality budget bump left the diff when main adopted the same 450. The deletion side grew by the comment-caps conformance, which is comment-only. The deleted cmd code is glue whose only job was adapting the old store signatures for the handlers, and the widened composers fit the handlers directly, so the glue and its tests go. The composer widenings, the emit helper, its fourteen call sites, and the suites pinning them reference one another, so a split would ship a widened return with no reader or an emit with no test.
e2e catalogs
Adds a storage-counters-after-management-deletes scenario to each format catalog (docker, oci, npm, and maven) and the matching usage-data row to each. All four scenario rows are automatable. They were blocked on the buffered counter pipeline's drain until main wired the S22 periodic drain trigger, and each row now names that trigger as what bounds its retry. The Maven lifecycle intro keeps the drain as the counters row's dependency, now satisfied, beside the metadata reconciler that still blocks the other rows. The npm row splits its size_bytes expectation per clock, because the package delete hard-deletes the packument cache rows inside the request and only a reconciliation pass carries their bytes (npm package delete does not carry its packument... (#761 - closed) • Pawel Rozlach).
Related to #313 (closed)