perf(cli): strategy-driven parallel provisioning for pg-delta next plan shadows - #6215
perf(cli): strategy-driven parallel provisioning for pg-delta next plan shadows#6215avallete wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04da465f86
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… at acquire The export dedupe skipped whenever a tar existed at the final path, but the warm-fallback cold path deliberately RETAINS an unusable tar (an extraction failure does not implicate its contents) precisely so the fallback's own export atomically replaces it. Skipping there left a genuinely corrupt tar in place forever, failing every later warm restore into another cold provision. The skip now applies only on the cold path whose tar was absent when the acquisition began, where a tar found at export time can only be a same-key sibling's fresh publish. The existing warm-fallback test asserted only the tar COUNT after the republish, which is why the skip slipped through — it now corrupts the tar's bytes up front and asserts the exported bytes replaced them. Review: Codex on #6215. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NYVv1jjXdrTzqDTVrU4Had
avallete
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9a2f2ba1d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tive shadow pg-delta's planSchemaFiles refuses to load declarative SQL when the shadow and target report the same PostgreSQL identity unless allowSameDatabaseIdentity is set. The baseline handoff restores the declarative shadow from the tar the migrations shadow just exported — an exact physical clone, same system identifier and database OIDs — but the flag was computed as "both handles were warm restores", which the cold migrations side fails, so the first cold plan would be rejected by the guard it exists to bypass. The flag now encodes actual snapshot lineage: true exactly when the declarative shadow was RESTORED from the same snapshot key that also produced the migrations cluster — same key means same tar, and the migrations side is that tar's lineage whether it warm-restored from it or cold-exported it this run. A freshly initdb'd declarative shadow has its own identity and different keys mean different originating clusters, so those stay false and keep the guard armed. Key equality is taken from the peeks (deterministic over inputs), so disk-state races cannot make it lie. A true alongside identities that happen to differ is harmless: pg-delta's bypass only takes effect on an exact identity match, never on a same-lineage sibling. Review: Codex on #6215 (P1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NYVv1jjXdrTzqDTVrU4Had
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b817476b5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The declarative shadow's acquire is delayed under the handoff (waits for the baseline seam) and sequential (waits for the whole migrations provision, including a possibly long replay) strategies, and the cache key hashes mid-run-mutable inputs — supabase/roles.sql and the remote JWKS — that a cold setup re-reads at its own time. Reusing the up-front peek's resolved inputs there could publish a baseline under a key that no longer describes it, letting a later run warm-restore the wrong roles. Peeked inputs are now passed through only where the acquire follows the peek immediately: the migrations acquire always, the declarative one only under the parallel strategy. A delayed acquire re-resolves at acquire time, which also self-corrects a handoff whose key genuinely changed mid-run — the recomputed key misses the just-exported tar and the declarative side cold-provisions with the current inputs. Review: Codex on #6215 (P2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NYVv1jjXdrTzqDTVrU4Had
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0511f1211
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The stale-peek refresh comment listed the remote JWKS among the inputs a delayed acquire re-resolves, but that resolver is deliberately memoized per shadow input (shadow-database.ts, review: Codex on #6184) so the cache key and the baked baseline always carry the same value and cannot diverge — a delayed acquire keeps the command-start JWKS, well inside the staleness the snapshot cache accepts by design (a warm hit serves a tar up to 14 days old under its matching key). Scope the comment to roles.sql, the one input the refresh actually re-reads, and state the JWKS exemption explicitly. Review: Codex on #6215. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NYVv1jjXdrTzqDTVrU4Had
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5390632fcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66911c3164
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@1477e0c1f9d20f643a72b365c97f0521943c08b1Preview package for commit |
66911c3 to
1477e0c
Compare
2 participants
Summary
Rebased onto
developnow that #6102, #6184, and #6203 are merged. The PR is only the plan-shadow overlap — not the lower-stack cache/stop work.A declarative sync provisions two shadow databases (migrations + declarative). Those used to run strictly sequentially, so the declarative shadow's whole provision was added to the runtime instead of overlapping it.
Provisioning now peeks each shadow's baseline-cache state (
legacyPeekShadowBaseline) and picks one of three strategies (legacy-pgdelta-next-shadow.plan.ts):parallel— both snapshots already published: both warm-restore concurrently. A warm restore skips the platform baseline, so only the migrations fiber prints (Applying migration ...), live and in order.baseline-handoff— first run, both cold under one cache key: the platform baseline is built once. The migrations shadow cold-provisions; its snapshot export at the baseline seam signals the declarative fiber, which warm-restores from the just-published tar concurrently with migration replay. A handle that will never snapshot signals immediately, and the runnerEffect.ensurings the signal onto the whole provision as a liveness backstop.sequential— different keys, mixed states,--no-cache, cache env off, PG≤14/OrioleDB: no baseline can be shared, so this keeps the pre-parallel flow and transcript.Output ordering is a hard guarantee. In the concurrent strategies the declarative fiber's
Outputwrites go throughlegacyBufferedShadowOutputand flush after the join. Post-flush writes pass through live so late teardown warnings are never lost.Same-database identity still uses the acquired handles'
snapshotKeys (not the peeks), so a delayed declarative re-resolve cannot lie about snapshot lineage.Supporting cache changes:
legacyPeekShadowBaselineanswers "what would the acquire do right now" without provisioning. The acquire always re-checks disk state.precomputedKeyInputsso the JWKS discovery request is not resolved twice. Delayed declarative acquires re-resolve so a mid-runroles.sqledit cannot publish under a stale key.legacyExportPgDataTar's temp name is pid-scoped). The absent-at-acquire dedupe skips a re-export only when the tar appeared after the cold acquisition began; a tar retained through a failed warm restore is still atomically replaced.Dogfooded this PR against its merge base (
develop@38f31b4) onsupabase db schema declarative sync --no-apply --experimental(pg-delta next), full 2×2 matrix of {base, PR} × {SUPABASE_SHADOW_CACHEoff/on}, 5 timed runs per cell plus extra cold-cache samples. All docker images pre-pulled and the local stack already running, so no run pays pull or stack-start cost. Times are full CLI wall-clock on a converged project ("No schema changes found" — both shadows still provisioned every run).SUPABASE_SHADOW_CACHE=0SUPABASE_SHADOW_CACHE=1, cold cacheSUPABASE_SHADOW_CACHE=1, warm cachePer-run warm values — base:
6.03, 6.80, 6.79, 7.21· PR:5.38, 5.14, 5.12, 5.35. The PR is also noticeably less noisy (σ 0.14 vs 0.49).