-
Notifications
You must be signed in to change notification settings - Fork 511
perf(cli): strategy-driven parallel provisioning for pg-delta next plan shadows #6215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
04da465
ac55002
d9a2f2b
b817476
c0511f1
5390632
66911c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,9 +22,16 @@ import { | |
| import { legacyWaitForShadowReady } from "../../../shared/db-bootstrap/health-check.ts"; | ||
| import { | ||
| legacyAcquireShadowDatabase, | ||
| type LegacyShadowAcquiredHandle, | ||
| legacyPeekShadowBaseline, | ||
| type LegacyShadowBaselinePeek, | ||
| type LegacyShadowCacheOpts, | ||
| type LegacyShadowAcquiredHandle, | ||
| } from "../../../shared/db-bootstrap/shadow-cache.ts"; | ||
| import { | ||
| legacyBufferedShadowOutput, | ||
| legacyResolvePlanShadowStrategy, | ||
| legacyRunPlanShadowProvisions, | ||
| } from "./legacy-pgdelta-next-shadow.plan.ts"; | ||
| import { | ||
| legacyConnectShadowDatabase, | ||
| legacyMigrateNextShadowDatabase, | ||
|
|
@@ -82,10 +89,6 @@ interface NativeShadowBase { | |
| readonly image: string; | ||
| } | ||
|
|
||
| interface ProvisionedMigrationsShadow extends LegacyPgDeltaNextMigrationsShadow { | ||
| readonly snapshotKey: string | undefined; | ||
| } | ||
|
|
||
| interface ProvisionedDeclarativeShadow { | ||
| readonly declarativeUrl: string; | ||
| readonly restoredFromPgDataSnapshot: boolean; | ||
|
|
@@ -100,11 +103,11 @@ interface ProvisionedDeclarativeShadow { | |
| * tar, and the migrations side is that tar's lineage whether it warm-restored FROM the tar or | ||
| * cold-exported it this very run — the baseline handoff, where requiring the migrations handle | ||
| * itself to be a warm restore would leave the guard armed against its own clone and fail the | ||
| * first cold plan (review: Codex on #6184, P1). A freshly initdb'd declarative shadow always | ||
| * first cold plan (review: Codex on #6215, P1). A freshly initdb'd declarative shadow always | ||
| * carries its own new identity, and different keys mean tars exported from different clusters, | ||
| * so both of those stay `false` and keep the guard armed. A `true` alongside identities that | ||
| * happen to differ is harmless by design: pg-delta's bypass only takes effect on an exact | ||
| * identity match, never on a same-lineage sibling. | ||
| * identity match (`schema-plan.ts`'s `trustedCloneBypass`), never on a same-lineage sibling. | ||
| */ | ||
| export function legacyAllowSameDatabaseIdentityForPlanShadows(opts: { | ||
| readonly declarativeRestoredFromPgDataSnapshot: boolean; | ||
|
|
@@ -170,19 +173,23 @@ export const legacyPgDeltaNextShadowLayer = Layer.effect( | |
| const dbConnection = yield* LegacyDbConnection; | ||
| const httpClient = yield* HttpClient.HttpClient; | ||
|
|
||
| const runtime = Layer.mergeAll( | ||
| Layer.succeed(FileSystem.FileSystem, fs), | ||
| Layer.succeed(Path.Path, path), | ||
| Layer.succeed(LegacyDebugFlag, debugFlag), | ||
| Layer.succeed(LegacyExperimentalFlag, experimentalFlag), | ||
| Layer.succeed(LegacyNetworkIdFlag, networkIdFlag), | ||
| Layer.succeed(CliArgs, cliArgs), | ||
| Layer.succeed(Output, output), | ||
| Layer.succeed(RuntimeInfo, runtimeInfo), | ||
| Layer.succeed(LegacyDockerRun, docker), | ||
| Layer.succeed(LegacyDbConnection, dbConnection), | ||
| Layer.succeed(HttpClient.HttpClient, httpClient), | ||
| ); | ||
| // Parameterized on the Output service so `provisionPlan` can hand a concurrently running | ||
| // provision a buffering decorator (`legacyBufferedShadowOutput`) instead of the live one. | ||
| const runtimeWith = (outputService: typeof Output.Service) => | ||
| Layer.mergeAll( | ||
| Layer.succeed(FileSystem.FileSystem, fs), | ||
| Layer.succeed(Path.Path, path), | ||
| Layer.succeed(LegacyDebugFlag, debugFlag), | ||
| Layer.succeed(LegacyExperimentalFlag, experimentalFlag), | ||
| Layer.succeed(LegacyNetworkIdFlag, networkIdFlag), | ||
| Layer.succeed(CliArgs, cliArgs), | ||
| Layer.succeed(Output, outputService), | ||
| Layer.succeed(RuntimeInfo, runtimeInfo), | ||
| Layer.succeed(LegacyDockerRun, docker), | ||
| Layer.succeed(LegacyDbConnection, dbConnection), | ||
| Layer.succeed(HttpClient.HttpClient, httpClient), | ||
| ); | ||
| const runtime = runtimeWith(output); | ||
|
|
||
| const nextPort = (excluded?: number) => | ||
| Effect.gen(function* () { | ||
|
|
@@ -263,19 +270,39 @@ export const legacyPgDeltaNextShadowLayer = Layer.effect( | |
| }, | ||
| ); | ||
|
|
||
| const provisionMigrations = (input: NativeShadowInput, opts: LegacyShadowCacheOpts) => | ||
| const provisionMigrations = ( | ||
| input: NativeShadowInput, | ||
| opts: LegacyShadowCacheOpts, | ||
| onBaselineSeam: Effect.Effect<void> = Effect.void, | ||
| ) => | ||
| Effect.gen(function* () { | ||
| const handle = yield* acquireShadow(input, opts); | ||
| yield* awaitShadowReady(input, handle); | ||
| const setup = setupRunInput(input, handle); | ||
| yield* legacyMigrateNextShadowDatabase(input.spawner, setup, handle); | ||
| // The baseline-handoff strategy (`legacy-pgdelta-next-shadow.plan.ts`) waits on | ||
| // `onBaselineSeam` before warm-restoring the declarative shadow. A snapshot-cold handle | ||
| // reaches that seam when its export publishes the tar; any other handle (warm because | ||
| // another process published between peek and acquire, or uncached) never runs a | ||
| // snapshot, so signal immediately — the waiter then just re-peeks current disk state. | ||
| const seamWillRun = handle.snapshotRequired && !handle.baselinePresent; | ||
| const seamHandle: LegacyShadowAcquiredHandle = seamWillRun | ||
| ? { | ||
| ...handle, | ||
| snapshotBaseline: handle.snapshotBaseline.pipe(Effect.ensuring(onBaselineSeam)), | ||
| } | ||
| : handle; | ||
| if (!seamWillRun) yield* onBaselineSeam; | ||
| yield* awaitShadowReady(input, seamHandle); | ||
| const setup = setupRunInput(input, seamHandle); | ||
| yield* legacyMigrateNextShadowDatabase(input.spawner, setup, seamHandle); | ||
| return { | ||
| migrationsUrl: legacyToPostgresURL(setup.connConfig), | ||
| snapshotKey: handle.snapshotKey, | ||
| } satisfies ProvisionedMigrationsShadow; | ||
| } satisfies LegacyPgDeltaNextMigrationsShadow; | ||
| }).pipe(Effect.provide(runtime), Effect.mapError(nextShadowError)); | ||
|
|
||
| const provisionDeclarative = (input: NativeShadowInput, opts: LegacyShadowCacheOpts) => | ||
| const provisionDeclarative = ( | ||
| input: NativeShadowInput, | ||
| opts: LegacyShadowCacheOpts, | ||
| outputService: typeof Output.Service = output, | ||
| ) => | ||
| Effect.gen(function* () { | ||
| const handle = yield* acquireShadow(input, opts); | ||
| yield* awaitShadowReady(input, handle); | ||
|
|
@@ -295,7 +322,7 @@ export const legacyPgDeltaNextShadowLayer = Layer.effect( | |
| restoredFromPgDataSnapshot: handle.baselinePresent, | ||
| snapshotKey: handle.snapshotKey, | ||
| } satisfies ProvisionedDeclarativeShadow; | ||
| }).pipe(Effect.provide(runtime), Effect.mapError(nextShadowError)); | ||
| }).pipe(Effect.provide(runtimeWith(outputService)), Effect.mapError(nextShadowError)); | ||
|
|
||
| const cacheOpts = ( | ||
| opts: LegacyPgDeltaNextShadowInput, | ||
|
|
@@ -320,22 +347,78 @@ export const legacyPgDeltaNextShadowLayer = Layer.effect( | |
| const built = yield* buildNativeBase(opts); | ||
| const migrationsInput = buildNativeInput(opts, built, migrationsPort); | ||
| const declarativeInput = buildNativeInput(opts, built, declarativePort); | ||
| const migrations = yield* provisionMigrations(migrationsInput, cacheOpts(opts, "config")); | ||
| const declarative = yield* provisionDeclarative( | ||
| declarativeInput, | ||
| cacheOpts(opts, "disabled"), | ||
| ); | ||
| // The two shadows are independent — anonymous containers on the distinct host ports | ||
| // allocated above, per-invocation scoped temp dirs, and a race-tolerant network | ||
| // ensure — so warm provisions run fully concurrently. How much can safely overlap | ||
| // when a baseline still has to be BUILT is the strategy question: peek both cache | ||
| // states up front and dispatch (see `legacy-pgdelta-next-shadow.plan.ts` for the | ||
| // three strategies and their transcript guarantees). The peeks also resolve the | ||
| // cache-key inputs once; passing them back through `precomputedKeyInputs` keeps the | ||
| // acquire from repeating a live JWKS discovery request. | ||
| const [migrationsPeek, declarativePeek] = yield* Effect.all([ | ||
| legacyPeekShadowBaseline(migrationsInput.base, cacheOpts(opts, "config")), | ||
| legacyPeekShadowBaseline(declarativeInput.base, cacheOpts(opts, "disabled")), | ||
| ]); | ||
| const withPeek = ( | ||
| cache: LegacyShadowCacheOpts, | ||
| peek: LegacyShadowBaselinePeek, | ||
| ): LegacyShadowCacheOpts => | ||
| peek.state === "uncachable" | ||
| ? cache | ||
| : { ...cache, precomputedKeyInputs: peek.keyInputs }; | ||
| const strategy = legacyResolvePlanShadowStrategy(migrationsPeek, declarativePeek); | ||
| // Peeked inputs are only reused where the acquire follows the peek IMMEDIATELY: the | ||
| // migrations acquire always does, the declarative one only under `parallel`. In the | ||
| // handoff (waits for the seam) and sequential (waits for the whole migrations | ||
| // provision) strategies the declarative acquire is DELAYED, and the key hashes | ||
| // `supabase/roles.sql` while the cold setup re-reads that file at its own time — | ||
| // reusing a stale peek there could publish a baseline under a key that no longer | ||
| // describes it (review: Codex on #6215). Re-resolving at acquire time also | ||
| // self-corrects a handoff whose key genuinely changed mid-run: the recomputed key | ||
| // misses the just-exported tar and the declarative side correctly cold-provisions | ||
| // with the current inputs. The key's OTHER live input, the JWKS resolver, is | ||
| // deliberately exempt from this refresh: it is memoized per shadow input | ||
| // (`legacyShadowRunInputFromLocalContainerInputs`), so the 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). | ||
| const migrationsOpts = withPeek(cacheOpts(opts, "config"), migrationsPeek); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When realtime is enabled and the declarative peek's JWKS resolution is slow, the migrations peek may have already read Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No change here — this window is not materially reducible and is already dominated by one this layer cannot close. The gap flagged (migrations peek → acquire) is bounded by the declarative peek's duration: essentially one JWKS discovery request, and only when realtime on PG15+ makes that a live fetch — otherwise it's filesystem reads measured in milliseconds. The window that follows the acquire on every cold provision — Generated by Claude Code |
||
| const declarativeOpts = | ||
| strategy === "parallel" | ||
| ? withPeek(cacheOpts(opts, "disabled"), declarativePeek) | ||
| : cacheOpts(opts, "disabled"); | ||
|
Comment on lines
+386
to
+389
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When realtime is enabled on PG15+ and the remote issuer rotates its JWKS during a long migrations provision, omitting Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The memoization is deliberate and keeping it is the correct behavior, so no code change — only my comment overclaiming "remote JWKS" as refreshed was wrong, corrected in 5390632. What made the roles.sql half of this review a real bug was key/content divergence. JWKS cannot diverge: the cache-key resolution and Generated by Claude Code |
||
|
|
||
| // In the concurrent strategies the declarative fiber's writes are buffered and | ||
| // flushed after the join, so nothing can land between two of the migrations fiber's | ||
| // live lines; sequential needs no buffer (one fiber at a time). `Effect.ensuring` on | ||
| // the JOIN (not the declarative fiber, which can finish first) so anomaly warnings | ||
| // survive failures without ever interleaving. | ||
| const buffered = | ||
| strategy === "sequential" ? undefined : legacyBufferedShadowOutput(output); | ||
| const provisions = legacyRunPlanShadowProvisions({ | ||
| strategy, | ||
| provisionMigrations: (onBaselineSeam) => | ||
| provisionMigrations(migrationsInput, migrationsOpts, onBaselineSeam), | ||
| provisionDeclarative: provisionDeclarative( | ||
| declarativeInput, | ||
| declarativeOpts, | ||
| buffered === undefined ? output : buffered.output, | ||
| ), | ||
| }); | ||
| const [migrations, declarative] = yield* buffered === undefined | ||
| ? provisions | ||
| : provisions.pipe(Effect.ensuring(buffered.flush)); | ||
| return { | ||
| migrationsUrl: migrations.migrationsUrl, | ||
| declarativeUrl: declarative.declarativeUrl, | ||
| // Key equality is what encodes lineage: the declarative shadow restored the very tar | ||
| // the migrations side either restored or exported this run, so the two clusters are | ||
| // physical clones. An absent key (uncached/bypassed/uncachable) is never lineage. | ||
| // Key equality comes from the peeks (deterministic over inputs, not disk state), | ||
| // so a between-fibers eviction or publish cannot make it lie about lineage. | ||
| allowSameDatabaseIdentity: legacyAllowSameDatabaseIdentityForPlanShadows({ | ||
| declarativeRestoredFromPgDataSnapshot: declarative.restoredFromPgDataSnapshot, | ||
| sameSnapshotKey: | ||
| migrations.snapshotKey !== undefined && | ||
| migrations.snapshotKey === declarative.snapshotKey, | ||
| migrationsPeek.state !== "uncachable" && | ||
| declarativePeek.state !== "uncachable" && | ||
| migrationsPeek.key === declarativePeek.key, | ||
|
Comment on lines
418
to
+421
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the delayed declarative acquire resolves a different key from its peek, these peek keys no longer describe the provisioned shadows. For example, if Useful? React with 👍 / 👎. |
||
| }), | ||
| } satisfies LegacyPgDeltaNextPlanShadows; | ||
| }).pipe(Effect.mapError(nextShadowError)), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| /** | ||
| * Orchestration for pg-delta next's two plan shadows (migrations + declarative) — the strategy | ||
| * choice, the concurrency runner, and the output buffering that keeps the user-visible | ||
| * transcript free of cross-fiber interleaving. Extracted from | ||
| * `legacy-pgdelta-next-shadow.layer.ts` so the branch logic, the baseline-handoff signal, and | ||
| * the flush ordering are unit-testable with plain fakes instead of a full Docker/runtime layer | ||
| * graph. | ||
| * | ||
| * The three strategies, chosen from a {@link legacyPeekShadowBaseline} of each shadow: | ||
| * | ||
| * - `parallel` — both snapshots are published: both provisions warm-restore concurrently. A warm | ||
| * provision skips the platform baseline entirely (`legacySetupShadowDatabase`'s | ||
| * `baselinePresent` branch), so the declarative fiber prints nothing and the migrations fiber's | ||
| * `Applying migration ...` lines stream live and in order. | ||
| * - `baseline-handoff` — both are cold with the SAME cache key (webhooks agree): the baseline is | ||
| * paid exactly once. The migrations shadow cold-provisions; its snapshot export runs at the | ||
| * baseline seam (after platform setup, before migration replay) and signals the declarative | ||
| * fiber, which then warm-restores from the just-published tar CONCURRENTLY with the migration | ||
| * replay. All normal-mode output still comes from the single migrations fiber. | ||
| * - `sequential` — everything else (different keys, mixed warm/cold, `--no-cache`, cache env off, | ||
| * PG<=14/OrioleDB): no baseline can be shared, so run migrations then declarative exactly as | ||
| * the pre-parallel code did, preserving that transcript byte for byte. | ||
| */ | ||
|
|
||
| import { Deferred, Effect } from "effect"; | ||
|
|
||
| import { Output } from "../../../../shared/output/output.service.ts"; | ||
| import type { LegacyShadowBaselinePeek } from "../../../shared/db-bootstrap/shadow-cache.ts"; | ||
|
|
||
| export type LegacyPlanShadowStrategy = "parallel" | "baseline-handoff" | "sequential"; | ||
|
|
||
| /** | ||
| * Pure strategy choice from the two peeks. Equal-key implies equal warm/cold state (one key = | ||
| * one tar), so `cold`+`cold`+equal-keys is the only shareable-baseline shape; a mixed warm/cold | ||
| * pair always means different keys, where nothing can be shared and sequential keeps the cold | ||
| * side's baseline prints off the migration replay's live stream. | ||
| */ | ||
| export function legacyResolvePlanShadowStrategy( | ||
| migrations: LegacyShadowBaselinePeek, | ||
| declarative: LegacyShadowBaselinePeek, | ||
| ): LegacyPlanShadowStrategy { | ||
| if (migrations.state === "warm" && declarative.state === "warm") return "parallel"; | ||
| if ( | ||
| migrations.state === "cold" && | ||
| declarative.state === "cold" && | ||
| migrations.key === declarative.key | ||
| ) { | ||
| return "baseline-handoff"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the cache is empty and effective webhooks are disabled (the default), this strategy exports the migrations shadow's PGDATA and restores that exact snapshot into the declarative shadow, so both servers have the same PostgreSQL system identity. However, the migrations handle still reports Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed and fixed in b817476. The flag now encodes snapshot lineage instead of "both handles were warm restores": Generated by Claude Code |
||
| } | ||
| return "sequential"; | ||
| } | ||
|
|
||
| /** | ||
| * Runs the two provisions under the chosen strategy. | ||
| * | ||
| * `provisionMigrations` receives an `onBaselineSeam` effect it MUST arrange to run once its | ||
| * baseline seam passes (the snapshot-export point, before migration replay) — the layer wires it | ||
| * into the acquired handle's `snapshotBaseline` via `Effect.ensuring`, and fires it immediately | ||
| * when the acquired handle will never run a snapshot (a warm or uncached acquire, e.g. when | ||
| * another process published the tar between peek and acquire). The runner additionally | ||
| * `Effect.ensuring`s the signal onto the WHOLE migrations provision as a liveness backstop, so | ||
| * the declarative waiter can never deadlock: seam reached → early signal; provision ends without | ||
| * a seam (success, failure, or interruption) → backstop signal, and on failure `Effect.all`'s | ||
| * fail-fast interrupts the waiter anyway. | ||
| */ | ||
| export const legacyRunPlanShadowProvisions = <M, D, EM, ED, RM, RD>(opts: { | ||
| readonly strategy: LegacyPlanShadowStrategy; | ||
| readonly provisionMigrations: (onBaselineSeam: Effect.Effect<void>) => Effect.Effect<M, EM, RM>; | ||
| readonly provisionDeclarative: Effect.Effect<D, ED, RD>; | ||
| }): Effect.Effect<readonly [M, D], EM | ED, RM | RD> => { | ||
| switch (opts.strategy) { | ||
| case "parallel": | ||
| return Effect.all([opts.provisionMigrations(Effect.void), opts.provisionDeclarative], { | ||
| concurrency: 2, | ||
| }); | ||
| case "baseline-handoff": | ||
| return Effect.gen(function* () { | ||
| const seam = yield* Deferred.make<void>(); | ||
| const signal = Deferred.succeed(seam, undefined).pipe(Effect.asVoid); | ||
| return yield* Effect.all( | ||
| [ | ||
| opts.provisionMigrations(signal).pipe(Effect.ensuring(signal)), | ||
| Deferred.await(seam).pipe(Effect.andThen(opts.provisionDeclarative)), | ||
| ], | ||
| { concurrency: 2 }, | ||
| ); | ||
| }); | ||
| case "sequential": | ||
| return Effect.gen(function* () { | ||
| const migrations = yield* opts.provisionMigrations(Effect.void); | ||
| const declarative = yield* opts.provisionDeclarative; | ||
| return [migrations, declarative] as const; | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| export interface LegacyBufferedShadowOutput { | ||
| /** The wrapped service to provide to the fiber whose writes must not interleave. */ | ||
| readonly output: typeof Output.Service; | ||
| /** | ||
| * Replays every buffered write to the real output, in order. Run it AFTER the live fiber has | ||
| * finished (e.g. `Effect.ensuring` on the join, not on the buffered fiber — the buffered fiber | ||
| * can finish first, and flushing then would interleave after all). Idempotent; writes arriving | ||
| * after a flush pass straight through live so late teardown warnings are never lost. | ||
| */ | ||
| readonly flush: Effect.Effect<void>; | ||
| } | ||
|
|
||
| /** | ||
| * An {@link Output} decorator that buffers `raw`/`rawBytes` (the only channels the shadow | ||
| * provisioning paths write to) and delegates everything else live. This is the hard guarantee | ||
| * that a concurrently provisioned shadow can never land a line BETWEEN two of the live fiber's | ||
| * lines — in normal mode the buffer stays empty (a warm restore prints nothing), so this exists | ||
| * for the anomaly paths: cache warnings and cold-fallback baseline prints. | ||
| * | ||
| * Deliberately NOT covering writes that bypass `Output` entirely (`SUPABASE_SHADOW_DEBUG` timing | ||
| * lines and failure-path container-log dumps write straight to `process.stderr`) — those are | ||
| * opt-in diagnostics where immediacy beats ordering. | ||
| */ | ||
| export function legacyBufferedShadowOutput( | ||
| real: typeof Output.Service, | ||
| ): LegacyBufferedShadowOutput { | ||
| type BufferedWrite = | ||
| | { readonly kind: "raw"; readonly text: string; readonly stream: "stdout" | "stderr" } | ||
| | { | ||
| readonly kind: "rawBytes"; | ||
| readonly bytes: Uint8Array; | ||
| readonly stream: "stdout" | "stderr"; | ||
| }; | ||
| const buffer: Array<BufferedWrite> = []; | ||
| let flushed = false; | ||
| const output = Output.of({ | ||
| ...real, | ||
| raw: (text, stream = "stdout") => | ||
| Effect.suspend(() => { | ||
| if (flushed) return real.raw(text, stream); | ||
| buffer.push({ kind: "raw", text, stream }); | ||
| return Effect.void; | ||
| }), | ||
| rawBytes: (bytes, stream = "stdout") => | ||
| Effect.suspend(() => { | ||
| if (flushed) return real.rawBytes(bytes, stream); | ||
| buffer.push({ kind: "rawBytes", bytes, stream }); | ||
| return Effect.void; | ||
| }), | ||
| }); | ||
| const flush = Effect.suspend(() => { | ||
| flushed = true; | ||
| const pending = buffer.splice(0); | ||
| return Effect.forEach( | ||
| pending, | ||
| (write) => | ||
| write.kind === "raw" | ||
| ? real.raw(write.text, write.stream) | ||
| : real.rawBytes(write.bytes, write.stream), | ||
| { discard: true }, | ||
| ); | ||
| }); | ||
| return { output, flush }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the strategy is
sequential, both peeks happen before the migrations shadow runs, but the declarative shadow may not acquire until a long migration replay finishes. Passing itsprecomputedKeyInputshere means a concurrent edit or removal ofsupabase/roles.sqlis not reflected in the cache key, whilelegacySetupDatabaselater reads and applies the current file (db-setup.ts:1084-1103); the resulting baseline is therefore published under the stale key and subsequent commands can silently restore the wrong roles. Recompute mutable key inputs when the delayed acquire begins, or carry the resolved roles content through to setup.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed and fixed in c0511f1. Peeked key inputs are now reused only where the acquire follows the peek immediately — the migrations acquire always, the declarative one only under the
parallelstrategy. The delayed declarative acquires (handoff and sequential) re-resolve 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 (andallowSameDatabaseIdentitystays consistent, since a cold declarative reportsrestoredFromPgDataSnapshot: false).One residual note for completeness: even with acquire-time resolution,
roles.sqlis read once for the key and again bylegacySetupDatabasea few seconds later during the same cold provision — that acquire→setup window predates this PR (it's inherent to #6184's key design) and is unchanged here.Generated by Claude Code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The memoization is deliberate and keeping it is the correct behavior here, so no code change — only the comment overclaiming "remote JWKS" as refreshed was wrong, corrected in 5390632.
The property that made the roles.sql half of this thread a real bug was key/content divergence. JWKS cannot diverge: the cache-key resolution and
legacyResolveDbSetupPreludeconsume the same memoized effect on the same shadow input, so a published tar always carries exactly the value its key was computed from — that consistency guarantee is what the memo was added for on #6184 ("guarantees the published snapshot carries the exact value its key was computed from, even if the issuer rotates mid-run"). Resetting the resolver at delayed acquisition would trade that away for freshness the cache doesn't promise anywhere: a warm hit already serves a baseline whose JWKS was resolved up to 14 days earlier under its matching key, so command-start vs acquire-time (seconds to minutes apart) is immaterial — and it would add a second live discovery request per cold sync.Generated by Claude Code