fix(registry): fault-isolate per-tenant restore so one bad CSDL doesn't fail boot (ARN-190)#348
Open
rita-aga wants to merge 1 commit into
Open
fix(registry): fault-isolate per-tenant restore so one bad CSDL doesn't fail boot (ARN-190)#348rita-aga wants to merge 1 commit into
rita-aga wants to merge 1 commit into
Conversation
The live Postgres/Turso boot path (`populate_registry`) parsed CSDL and registered each tenant with `?` inside the per-tenant loop, so one tenant's corrupt or unparsable CSDL returned Err, propagated through `build_registry`, and failed boot for every tenant — a multi-tenant outage from one bad row. Only the DST-only `restore_registry_from_platform_store` was fault-isolating, and its doc falsely claimed the CLI bootstrap used it, so the DST harness exercised a different path than production shipped. Extract one shared fault-isolating core `restore_grouped_specs` that every restore path funnels through: a bad tenant is logged, quarantined, and skipped; the rest still restore. `populate_registry` becomes a thin wrapper that layers verification-status restoration on top; the platform_store path delegates to the core and reconciles orphaned specs. The live paths keep the offending rows for inspection (no auto-delete of persisted specs); the platform_store path deletes them to preserve P1. Corrected the false doc claim. Red-green: a unit test drives the production Postgres/Turso wrapper with the corrupt tenant ordered first (fails pre-fix: whole restore aborts); a DST test drives the shared core via SimPlatformHarness::restart() and asserts the healthy tenant boots, the corrupt tenant is quarantined and reconciled, and boot invariants hold across seeds. ADR-0155. Fixes ARN-190. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Collaborator
Author
|
@greptile review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
rita-aga has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
rita-aga has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Collaborator
Author
ARN-165 principle audit — request changesExtracting a shared restore core is a good step. The PR still does not prove or implement the same production behavior across adapters. Blocking findings
Required directionRun contract tests through the real Postgres and Turso adapters; restore only committed records; persist a tenant/type/version quarantine record with reason and repair workflow; emit bounded metrics; prove one corrupt tenant cannot block healthy tenants and cannot silently disappear. The branch is behind main. Rebase and rerun after these semantic changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes ARN-190 (High) under epic ARN-165. Draft for review — do not merge.
Bug
populate_registry(live boot path) parsed CSDL + registered each tenant with?inside the per-tenant loop, so one tenant's corrupt CSDL aborted the whole registry restore → boot failed for all tenants. Only the DST-onlyrestore_registry_from_platform_storewas fault-isolating, and its doc falsely claimed the CLI bootstrap used it — so DST tested a different path than prod shipped.Fix
Extracted one shared fault-isolating core
restore_grouped_specs(log + quarantine the bad tenant + continue); all three restore functions funnel through it, so DST exercises the real production isolation. Genuine store I/O errors still propagate. Corrected the false doc. ADR-0160.Verification
Red→green corrupt-tenant boot test; new DST
dst_boot_isolates_corrupt_tenant_csdldrives the production path (5 seeds); dst_platform_boot 10/10; registry_bootstrap 6/6; fmt/clippy clean; code review PASS + DST-READY (genuine).🤖 Generated with Claude Code