Skip to content

feat(reporting-storage): fence the orphan collector against a cloned or restored ledger database - #3593

Merged
bokelley merged 5 commits into
mainfrom
bokelley/orphan-collector-clone-fence
Nov 2, 2026
Merged

bokelley merged 5 commits into
mainfrom
bokelley/orphan-collector-clone-fence

Conversation

@bokelley

@bokelley bokelley commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Closes #3292

Problem

collectOrphanedRowObjects deletes objects under {prefix}/{namespace_key}/ that no ledger record names. A clone or point-in-time restore of the ledger database carries the same installation identity, namespace key and configuration (including acknowledgeExclusiveNamespace: true), sees an older set of references, and would delete the live database's newer objects. The age threshold does not help once those objects are old enough.

Design: a create-only epoch chain in the bucket, recorded in the database

  • Fence. {prefix}/{namespace_key}/_collector-fence/{epoch:016d}.json, one create-only object per epoch: { v, namespace_key, epoch, token, previous_token, claimed_at }. The highest epoch is the current claim. The database records its last (epoch, token, previous_token) in adcp_reporting_orphan_collector_fences (new migration 0009), keyed by a hash of provider kind, location, prefix and namespace key, so bindings that reach the same bucket and prefix share one fence.
  • Claim. A real pass takes a session advisory lock, reads the DB row and the bucket, and proceeds only if the bucket's highest epoch is exactly the recorded claim. It commits the intent for epoch+1 with a fresh token, writes that object create-only, verifies the token at the key, and prunes all but the predecessor. It holds the lock through the write so a sibling host of the same database cannot interleave. It does this before the first delete of the pass.
  • Check. Before the first delete of every page, and every 25 deletes within one, the pass re-reads the fence (bucket first, then row). Any mismatch stops the pass.
  • Refusal. A refused binding has status: 'fenced' and fence: { cause, detail, fenceEpoch?, recordedEpoch? }; the result gets a required fenced: string[]; the maintenance task warns. Causes: fence_advanced, claimed_elsewhere, token_mismatch, fence_missing, fence_regressed, fence_unreadable, claim_lost, migration_required. Everything unexpected fails closed (refuse), never open. A provider or database error is failed, not fenced.
  • dryRun reads the fence without claiming it (no object, no row) and reports fenced bindings too.
  • Crash safety. The intent commits before the object is written. A crash between the two leaves the DB one epoch ahead of the bucket with the matching previous token; the next pass finishes the claim under a fresh token (see the security finding below).
  • Takeover. store.takeOverOrphanCollectorFence({ acknowledgePreviousOwnerGone: true, bindings?, includeReadOnly? }) claims max(epoch)+1 and supersedes every older object and version, including a damaged chain. It refuses without the acknowledgement. On a namespace nobody has claimed it is a claim and nothing more, which is how an operator claims from the live database first.
  • Clones and restores. A copy that still holds an older claim sees the live database's newer epoch and refuses. A database restored to an earlier state refuses until a takeover.

Why this and not the alternatives

  • Token in the database plus a marker object (the issue's first suggestion), checked and rotated each cycle. This is the same idea, but a single mutable marker needs compare-and-swap, which the provider contract does not have (only create-only putIfAbsent, exact-version get, list, exact-version delete). A chain of create-only epoch objects gives atomic arbitration on the provider contract we already require, with no new provider method.
  • A database-side generation counter only. A clone copies it. The object store is the only thing both databases can see, so it has to be the arbiter.
  • A new installation identity for clones. The earlier spec's remedy, and it prevents the problem for deployments that mint one, but nothing enforces it: a clone made by copying the database carries the identity, so we cannot rely on it. This fence is the enforcement.
  • Fence the per-object delete. Provider deletes cannot be conditioned on another object, so deletion cannot be made atomic with the fence. We narrowed the window instead (recheck every 25 deletes) and documented it.

Known limits, documented

  1. First claim wins. Nothing in a database distinguishes a copy from the original until one of them claims. If a clone collects before the original has run a pass since the copy, the clone claims first and the original is the one fenced (and gets the warning). The clone still cannot delete an object the original wrote after the copy until it is minAgeSeconds old. Rollout step: claim from the live database first (one real pass, or takeOverOrphanCollectorFence).
  2. A takeover does not stop a pass already running; it notices at the next check, at most 25 deletes later. Stop the previous collector first.
  3. The fence needs write access to the binding (including readOnly bindings collected with includeReadOnly), a lifecycle rule must not expire _collector-fence/, and list must be read-after-write consistent (the bundled providers are; the conformance suite already exercises create-only).

Migration (minor changeset for @adcp/reporting-storage)

  1. Run npx adcp-reporting-storage migrate (0009, additive). Until applied, every pass, dryRun included, reports fenced / migration_required and deletes nothing.
  2. Claim from the live database before any copy runs a collector.
  3. Custom stores implementing collectOrphanedRowObjects must return fenced: []; status gains 'fenced'.

See the changeset, docs/guides/REPORTING-OPERATIONS.md and docs/migration-reporting-storage-package.md.

Tests

  • test/reporting-orphan-collector-fence-pg.test.js (PostgreSQL + filesystem provider; a clone is a point-in-time copy of every table in the live schema, a restore resets the fence row): clone fenced with a control showing the same clone deletes the live object once it has taken over; clone that claims first fences the original; restore fenced until takeover; takeover recovers every cause; dry run does not claim; shared-fence bindings; mid-pass claim stops the next delete; provider failure is failed not fenced; crash recovery with a real failing put; copy taken during an unfinished claim cannot adopt the original's token; sibling waits on an in-flight claim; migration absent; malformed, duplicated and foreign fence objects fail closed.
  • test/reporting-orphan-fence.test.js: the decision table, strict parsing and key helpers without a database.
  • Existing orphan collector, maintenance-warning and migration tests updated.
  • Full @adcp/reporting-storage suite against PostgreSQL 5433: 467 + 206 pass. Also: npm run build, root and package typecheck, lint:boundaries, format:check, ci:doc-links, check:package-size (note: reporting-storage has about 19.8 KB of headroom left under its 490,000 byte ceiling).

Expert review

Reviewed read-only by security-reviewer, code-reviewer-deep, nodejs-testing-expert and Codex (review:codex --all, diff piped on stdin because its sandbox cannot read the repo).

Fixed:

  • security, medium: finishing an unfinished claim reused the recorded token, so a copy taken in that window and the original could both "adopt" the same object. A finished claim now always mints a fresh token, and the claim lock is held through the object write so a sibling host cannot interleave (which the fresh token alone would have broken).
  • code review: read-only checks raced a sibling claim and could report a false fenced (now bucket first, then row); fence scope used the host's registry key rather than the provider kind; README prettier; stale 0001–0008; @internal tags and a named ReportingOrphanFenceTakeoverV1; a fenced binding no longer rests a whole interval in the maintenance cycle (looks again within the hour).
  • security/codex: recheck every 25 deletes (was once per page); takeover now handles a damaged or duplicated chain and removes every version it saw; docs no longer say a fenced binding "deleted nothing" (a mid-pass refusal keeps its deleted count); list read-after-write requirement documented.
  • testing: wrong pool removed in freshClone; non-database tests for the decision table; failed-vs-fenced, crash-recovery, sibling and takeover coverage; dead counters removed; a deterministic lock test replaces the loose concurrency range.

Declined:

  • Do the bucket reads before opening the claim transaction (pool pressure): the read must be consistent with the row under the lock. Documented instead.
  • Guard epoch overflow: epochs are safe integers parsed from key names and incremented once per pass; unreachable.
  • Make fence deletion atomic with object deletes: not possible on the provider contract; narrowed and documented.

🤖 Generated with Claude Code

bokelley and others added 3 commits November 2, 2026 11:09
…or restored ledger database

A clone or point-in-time restore shares the live database's installation
identity, namespace key and configuration (including
acknowledgeExclusiveNamespace), so its collector would delete the live
database's newer row objects. A real collectOrphanedRowObjects pass now claims
the namespace first: a create-only chain of epoch objects under
{prefix}/{namespace_key}/_collector-fence/ plus a row in the new
adcp_reporting_orphan_collector_fences table (migration 0009). It deletes only
while the highest epoch in the bucket is exactly the claim the database
recorded, re-checks before every deleting page, and reports a refused binding
as fenced with a cause. dryRun checks without claiming. takeOver
OrphanCollectorFence is the deliberate takeover.

Closes #3292

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claim under a session lock held through the object write, finish an unfinished
claim under a fresh token, recheck the fence every 25 deletes, read the bucket
before the row in read-only checks, scope a fence by provider kind, and widen
takeover to a damaged chain. Adds non-database tests for the decision table.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e fence limits

A takeover now supersedes every object and version it saw. Documents that a
fence that advances mid-pass stops the pass at the next check (at most 25
deletes), that a takeover does not stop a running collector, and that list must
be read-after-write consistent.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@bokelley
bokelley enabled auto-merge (squash) November 2, 2026 11:29
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Nov 2, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve.

Fences the orphan collector against a cloned/restored ledger DB (#3292) in the experimental @adcp/reporting-storage package. No blocking or Medium findings.

Checked:

  • orphan-fence.ts decideOrphanFence is a pure, fail-closed decision table (fence_regressed catch-all) matching the unit matrix.
  • postgres.ts claimOrphanFence commits the (epoch,token) intent before the create-only object write; a crash is finished next pass via complete, which rewrites under a fresh token so a mid-claim copy cannot adopt the original token. Advisory lock serializes claims; lock_timeout=120s; object I/O bounded by read/write deadlines.
  • row-storage.ts listObjects skips fence keys so the collector never deletes its own chain; listCollectorFence page cap fails closed.
  • 929-line PG test covers clone before/after, restore, takeover, token_mismatch, claim_lost, mid-pass recheck, migration_required, concurrent same-DB hosts, provider-failure→failed.
  • Changeset is minor with migration note — correct for this @experimental package. Migration 0009 is additive/idempotent and wired into MIGRATIONS + optionalTables.

Gate checks: gated_paths false; high_risk false; no no-auto-approve team match. Note review_decision: REVIEW_REQUIRED — but with gated_paths false and no blocking findings, no gate fires. Rows 1–8 do not match; falls through to row 9 → approve.

… the orphan collector fence

The fence adds about 22 KB packed (a new module, the claim and takeover, the row
storage fence I/O and their declarations), over the 15 KB per-PR allowance. The
ceiling goes from 490,000 to 520,000 so two further PRs still fit.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Nov 2, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean delta since prior approve.

The only substantive change since the prior clean approve is a single line in scripts/package-size-budget.json raising the @adcp/reporting-storage packed-size ceiling 490000 → 520000 to accommodate the orphan-fence code this PR adds. The bump is proportionate (~6%), unpackedBytes/files ceilings are untouched, and a correct minor changeset is present for this experimental package. No wire-shape or code-behavior change.

Checked: no critical/high findings; no medium findings; witness-not-translator not implicated (no response reshaping); changeset present and correctly typed; no gated paths; no high-risk files; author is not on a no-auto-approve team. Decision table rows 1–8 do not fire → row 9 approve.

Resolves the migrations list, README table and migration test against the
row_binding_id bound (0010), keeping 0009 ahead of it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve.

Subsequent pass following a prior clean approve. The delta (12 files in packages/reporting-storage) widens row_binding_id/credentialRef from 128→255 via a single-source constant, adds a guarded idempotent migration 0010, and maps 23514→STATE_UNAVAILABLE. Reviewer confirmed: migration no-op/idempotency correct on fresh and current schemas, TS validator and DDL CHECK share one bound, the changeset is a correctly-typed minor for the experimental @adcp/reporting-storage, and PG tests cover fresh/legacy/un-migrated/no-table paths.

No critical, high, or medium findings. high_risk is false, gated_paths is false, and no no-auto-approve team match. Decision table rows 1–8 do not fire; falls through to row 9.

Note: review_decision is REVIEW_REQUIRED, but since gated_paths is false, row 2 does not apply and does not block approval.

@bokelley
bokelley merged commit 8c55809 into main Nov 2, 2026
45 checks passed
@bokelley
bokelley deleted the bokelley/orphan-collector-clone-fence branch November 2, 2026 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fence the orphaned row-object collector against a cloned or restored ledger database

1 participant