Skip to content

docs: specify durable data backfills - #7326

Open
TheSentinel454 wants to merge 12 commits into
mainfrom
tornquist/data-backfill-spec
Open

TheSentinel454 wants to merge 12 commits into
mainfrom
tornquist/data-backfill-spec

Conversation

@TheSentinel454

@TheSentinel454 TheSentinel454 commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Why

Data backfills need a durable contract outside schema migrations. The contract must survive crashes and support automatic or manual operation without adding version-selection machinery.

What

  • Define one PostgreSQL-backed lifecycle for finite data repair
  • Treat the stable backfill ID as the only durable execution and readiness identity
  • Use eventually consistent, idempotent registration; compatible workers compete for one exclusive claim
  • Require bounded batches, atomic mutation and checkpoint commits, generation fencing, and validation before completion
  • Define all four migration and automatic-backfill configurations
  • Define the authorized admin API and honest client projection

How

A backfill captures one immutable upper bound, processes finite batches, and commits each target mutation with its checkpoint. PostgreSQL owns lifecycle state, claims, progress, and completion.

Registration order and build metadata do not choose a definition. Compatible processes are interchangeable. A material behavior change requires a new stable ID. The orchestrator does not resolve different behavior registered under one active ID. Diagnostic validation on completed rows records only its bounded outcome and audit; it does not create an execution claim.

The spec now starts with a six-step mental model and follows the operator flow. It removes repeated requirements and cuts the document from 614 lines and 4,835 words to 474 lines and 3,656 words.

Risk

Low. This PR changes documentation only. It commits Buzz to a design contract, but it does not add runtime, schema, configuration, or protocol behavior.

Testing

No manual runtime testing. A fresh full-spec review checked the lifecycle, safety rules, four configuration cells, admin contract, and production-seam conformance requirements. git diff --check and the repository file-size policy pass at 13681664674d4667b181a6211f012090fa40a2cf.

Bigger picture

The design keeps the relay as orchestrator, a focused backfill crate as domain owner, and buzz-db as narrow transaction infrastructure. It removes deployment generations and definition precedence from the implementation plan.

Generated with Codex

Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 20131488528e35e6c50f4ccdb0490a9135c28edf...72a07511fbeaff2ad0e6192b5aae59b468137c1e.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 72a07511fbeaff2ad0e6192b5aae59b468137c1e to authorize a new review.
Any previous review applies only to its recorded range.

TheSentinel454 and others added 3 commits September 4, 2026 14:37
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
@TheSentinel454
TheSentinel454 marked this pull request as ready for review September 4, 2026 14:56
@TheSentinel454
TheSentinel454 requested a review from a team as a code owner September 4, 2026 14:56
TheSentinel454 and others added 4 commits September 16, 2026 06:08
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 Combined review from Paul + Thufir — both reviewed independently, findings reconciled and deduped, per the beekeepers-channel request.

OSS-genericity: passes. Plain PostgreSQL, generic deployment-operator authorization consistent with the multi-tenant relay model, no Block-specific endpoints, identity systems, deployment tooling, or vendor dependency. Component mapping targets only OSS crates.

1. Make the write-path prerequisite explicit (Correctness — both reviewers flagged this independently; Luke clarified intent in-channel). As written (docs/data-backfills.md lines 185–204), the captured upper bound plus a never-rewinding checkpoint do not by themselves freeze the admitted source set: a transaction in flight at start can commit a row at a key ≤ bound after the checkpoint has passed it, and validation can reach immutable completed before that late commit lands — leaving a permanently unrepaired row behind a validated completion. Luke's clarification — new write paths roll out before any backfill starts, so no concurrent write produces a backfill-actionable row — resolves the design question, but the spec never states that prerequisite. Suggested fix: (a) add a normative requirement that each definition document its admission/visibility guarantee (e.g., write paths deployed before start enforce the repaired postcondition for every insert/update at any key, including transactions already in flight at bound capture); (b) add a controlled late-commit conformance case proving a conforming deployment either repairs such a row or can never produce one. Line 203 currently covers only rows beyond the bound.

2. retry has no destination state (MINOR). The lifecycle table lists validating among next states for both blocked and failed, but the retry action contract (line 145) never names the state it enters. resume spells this out ("running, or validating when no admitted work remains"); retry should use the same formula — otherwise blocked/failed → validating is an edge with no defined trigger.

3. Admin surface reachability while the readiness gate is closed (MINOR). Automatic mode holds serving unready on paused/blocked/failed (lines 284–298), and every path out of those states runs through the admin API. State explicitly that the authorized admin boundary stays reachable while ordinary serving is gated, and exercise pause/remediate/retry through the admin boundary in that condition — otherwise a conforming implementation can lock operators out of their only recovery controls.

Solid throughout: PostgreSQL sole authority, atomic mutation+checkpoint commits, commit-time generation fencing, immutable completion, honest-projection client rules, and a falsifiable conformance suite.

Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
Comment thread docs/data-backfills.md Outdated
The rolling-deployment section said a violated rollout ordering means
"rerunning or replacing the repair." Rerunning is not reachable for the
same stable ID: `retry` preserves the bound and checkpoint, workers select
strictly after the checkpoint, and `completed` is terminal and immutable.

State instead that the existing checkpoint may have advanced past writes
that were never repaired, that no rerun under the same stable ID can
revisit them, and that operators must run a new repair under a new stable
ID -- possibly repeatedly -- until the normal write path is safe. This
matches the existing rule that incompatible work uses a new stable ID.

Wording only; the rollout guidance stays procedural and no runtime
enforcement, conformance test, gate, fence, lease, or state is added.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: tornquist <tornquist@squareup.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no blocking defects found in the documentation contract. Reviewed head 1f033eb3d6ff0d3ec81f01583c9bb5ae2618f17b against base 01bacb8df3d2f5718e0a468828e07ae874a38eae.

The latest text closes the earlier rollout-recovery, retry-destination, and admin-reachability findings. The one inline suggestion clarifies worker fencing versus operator transitions; it is non-blocking and does not request new lifecycle machinery.

Source-only review of lifecycle, persistence/fencing, readiness, operator authority, and client projections. No code or tests executed. Existing CI was inspected, not rerun; it does not establish runtime conformance for this design. Current deployment routing/auth implementation work remains a future implementation obligation, not a prerequisite for this documentation PR.

Comment thread docs/data-backfills.md
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no blocking defects found in the documentation contract. The previous optional clarification is addressed: worker execution commits retain current-owner/live-claim checks, while claimless operator transitions and execution-advancing validation/completion retain their lifecycle/generation fences.

Reviewed HEAD bb2b0a7d75840c1041758d09a4c6065f2d8f520f against BASE 01bacb8df3d2f5718e0a468828e07ae874a38eae, focusing re-review on the single fencing-language hunk since previously clear 1f033eb3d6ff0d3ec81f01583c9bb5ae2618f17b and its consequences across the full lifecycle contract. No new rollout machinery or runtime implementation prerequisites are requested.

Source-only review on the pinned Blox; no PR code or tests executed. Observed existing exact-head CI checks passed, but do not prove future runtime conformance to this design. This is a non-approving review comment.

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 Re-review at bb2b0a7d75840c1041758d09a4c6065f2d8f520f (base 01bacb8df). Thanks for working through the earlier round. Review clear: no blocking findings.

The three things from my earlier review at 136816646 are closed:

  1. Write-path prerequisite. Lines 124–132 now require the normal write path to be fully rolled out, with old in-flight behavior drained, before start. They make that an operator/developer responsibility rather than something the framework detects, and they say plainly that a violation needs a new stable ID. Carl's P1 about the "rerun" wording is folded in too. I'm fine without the late-commit conformance case now that the requirement is explicit.
  2. retry destination. Line 155 now uses the same running/validating formula as resume, which matches the lifecycle table.
  3. Admin reachability while gated. Lines 307–309 state it, and conformance case 13 (lines 482–487) exercises recovery from paused/blocked/failed through the production admin boundary while serving stays gated.

The fencing split at lines 255–272 also reads correctly. Worker execution commits carry the owner/live-claim checks. Operator transitions and execution-advancing validation keep the lifecycle and generation fences without needing a claim.

Non-blocking:

  1. MINOR / Correctness: automatic mode can't honor the rollout prerequisite if the writer and the backfill ship together. In the on/on and off/on cells, start happens at relay startup. Under the chart's RollingUpdate with maxUnavailable: 0, the first new pod would capture the bound and begin repairing while old pods are still serving and writing with the old behavior. That's exactly the case lines 124–132 say leaves unrepaired rows behind the checkpoint. The spec already implies the answer, but I think one sentence would save an implementer from the trap: a required automatic backfill must ship in a later release than the write path it depends on, or else be started manually after the writer rollout completes.

Two implementation notes, not spec defects: on current main, admin routes sit on the same readiness-filtered Service and router as ordinary traffic, and startup runs before the health listener binds. So the admin reachability and "remain unready" requirements will need a path that doesn't depend on ready endpoints, plus a non-blocking gate rather than a pre-listener wait. The spec states the outcomes without claiming current code provides them, which seems right for a design contract.

Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 Re-review at 72a07511fbeaff2ad0e6192b5aae59b468137c1e (base 01bacb8df). Review clear: no blocking findings.

The new paragraph at L301-L305 closes the MINOR from my last review. A required automatic backfill now has to ship in a release after its writer is rolled out and drained, so startup capturing the bound in the off/on and on/on cells can't race old writers the way L124-L132 warns about. A definition that ships alongside its writer is manual-only for that release. I also checked the deadlock case: a new pod gated on a pending repair means the rollout can't finish, and manual start isn't allowed until the rollout finishes. L302-L303 already excludes that, because by the L69 definition an ID that's part of startup readiness in that release is a required automatic backfill, and it can't ship with its writer. The L307-L311 serving gate is untouched.

One optional wording nit: "MUST NOT auto-start" on its own could be read as "stays in the required set but just isn't started". If you touch it again, I think saying it must not be a required automatic backfill in that release (neither started nor gated on) would make the exclusion explicit, instead of leaving it to be inferred from the sentence before. Not blocking.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no blocking documentation defects found. The release-ordering paragraph addresses the remaining non-blocking rollout note: a required automatic repair ships after its safe write path is fully deployed and old writers are drained; a same-release definition waits for manual initiation after rollout. The manual configuration cells provide that path without adding a readiness gate or framework rollout-detection machinery.

Reviewed HEAD 72a07511fbeaff2ad0e6192b5aae59b468137c1e against BASE 01bacb8df3d2f5718e0a468828e07ae874a38eae, focusing on the six-line delta since the previous clear review and its consequences for configuration, startup readiness, and the settled lifecycle contract. No further changes requested for this documentation PR; deployment routing and persistence details remain future implementation obligations.

Source-only review on the pinned Blox; no PR code, builds, or tests executed. Existing CI associated with this head was still pending at the inspected snapshot. CI does not establish runtime conformance to a documentation-only design. This is a non-approving review comment.

This branch has not been deployed

No deployments
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.

3 participants