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.record calls, one per counter family, and each opens its own detachedEmitTimeout window on a context it detaches with context.WithoutCancel, so no caller can shorten it and a blocked-but-answering Redis spends both. Against the deployed 10s server.timeouts.write that is the whole budget, and writeAccepted sets 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. deleteCounterEmits meters 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 inside cascade, which became a sequence shared with ContainerReaper's image walk in chore(datastore): container hosted reap (S20-A ... (!1711 - merged) • Pawel Rozlach • 19.4 and whose signature reapManifestCascader pins. The walk discards any report, so widening the sequence would make every reaped manifest pay the probe's image-row lookup and its two EXISTS arms for nothing. cascadeReportingPayloadSHA256 carries 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(...) error to MarkMavenVersion(...) (applied bool, err error), so a delete that loses the mark race answers 202 and emits nothing where it previously emitted one artifact_registry_artifact_deleted. TestMavenVersionDelete_LostRaceEmitsNothing pins 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 main after 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 through bulk_container_worker_remote.go), so the rebase had to say what each does with the Counters seam. All three emit nothing. The Maven and npm eviction composers report neither the rows a mark hid nor the bytes a file delete freed, so repositories.artifacts_count and repositories.size_bytes read too high until the next reconciliation pass, while namespace_statistics.components_count is correct and right not to move, on the rule deleteCounterTargetTombstone records for the hosted arms. remoteOps records the gap and points at Remote eviction leaves repository counters stal... (#775) • Pawel Rozlach (scoped to Maven before the npm arm landed), bulkNpmPassForKind refers to it for the npm twin, and the container worker's kind dispatch points at remoteContainerImageEvict, which records the container split: only artifacts_count reads 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_EmitsNothing pins 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 from main in an earlier rebase and sit on the same split.
  • MavenPackageStore.SoftDeleteMavenPackage and MavenVersionStore.SoftDeleteMavenVersion lose 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 added wireAccounting, which builds the same wiring.counterEmitter ahead 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. requireDeps regains maintidx, 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 vet on both tag sets, unit and integration suites for internal/managementapi, internal/datastore, cmd/..., and internal/format/oci, all green, with the reaper suites under -race and 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 in internal/managementapi. The datastore test files add 74 contextcheck findings 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)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading