Skip to content

Commit 1cb15ab

Browse files
committed
fix(webapp): make the snapshot store hard stop a flag and nothing else
The environment half of the hard stop converged over a rolling deploy rather than a flag interval. For the length of that deploy the fleet is mixed: a stopped process writes no transition, then a running one asserts a head that was never written and forks. Every fork enqueues a repair, and the repair restores the head but not the entries behind it. A control whose own convergence manufactures the divergence it exists to stop cannot be the way in. The flag now converges in one flag interval. Boot refuses to start when the retired variable is still set to 1, because a variable that no longer halts anything leaves an operator believing the mirror is stopped while it runs. A value of 0 carries no intent and is ignored. The guaranteed-inert state remains an unconfigured host, which is bootstrap config rather than an operational control.
1 parent a16f0d8 commit 1cb15ab

5 files changed

Lines changed: 50 additions & 15 deletions

File tree

‎apps/webapp/app/env.server.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1333,7 +1333,6 @@ const EnvironmentSchema = z
13331333
// The hard stop, "1" to halt. Separate from MODE because MODE governs births only: turning it
13341334
// down cannot stop a resident run mirroring. Holding this from the environment outranks the
13351335
// snapshotStoreHalt flag, which is the no-deploy control for the same thing.
1336-
RUN_ENGINE_SNAPSHOT_STORE_HALT: z.string().default("0"),
13371336
RUN_ENGINE_SNAPSHOT_STORE_ORG_MODE_CACHE_TTL_MS: z.coerce.number().int().default(30_000),
13381337
RUN_ENGINE_SNAPSHOT_STORE_ORG_MODE_CACHE_MAX: z.coerce.number().int().default(10_000),
13391338
// No fallback to REDIS_*: this is a distinct durable endpoint and must be set explicitly, or

‎apps/webapp/app/v3/snapshotStoreBoot.server.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ export type SnapshotStoreBootDeps = {
2121
ping: () => Promise<boolean>;
2222
evictionPolicy: () => Promise<EvictionPolicyReport>;
2323
repairBound: () => boolean;
24+
/**
25+
* Whether the retired RUN_ENGINE_SNAPSHOT_STORE_HALT still asks for a halt. Only "1" counts: the
26+
* old default was "0", which carries no intent, and refusing on it would stop every deployment
27+
* that pins the default while asking for nothing.
28+
*/
29+
legacyEnvHalt: boolean;
2430
log: (message: string, fields?: Record<string, unknown>) => void;
2531
warn: (message: string, fields?: Record<string, unknown>) => void;
2632
};
@@ -29,6 +35,15 @@ const PING_TIMEOUT_MS = 5_000;
2935
const FLAG_READY_TIMEOUT_MS = 10_000;
3036

3137
export async function assertSnapshotStoreBoot(deps: SnapshotStoreBootDeps): Promise<void> {
38+
// Checked before anything else, and it does not depend on the dial or the host. The variable no
39+
// longer halts anything, so leaving it set means an operator believes the mirror is stopped while
40+
// it is running. That is the one failure mode worse than not starting.
41+
if (deps.legacyEnvHalt) {
42+
throw new Error(
43+
"RUN_ENGINE_SNAPSHOT_STORE_HALT is set but no longer does anything; the hard stop is the snapshotStoreHalt feature flag. Set the flag, then unset this variable."
44+
);
45+
}
46+
3247
const pastOff = deps.mode !== "off";
3348
// Configuration is validated as soon as a host is set, NOT only past off. The per-organisation
3449
// override can put one organisation at dual-write while the deployment dial is still off, which is
@@ -199,6 +214,9 @@ export async function assertSnapshotStoreBootFromEnv(): Promise<void> {
199214
ping: pingSweepClient,
200215
evictionPolicy: readEvictionPolicy,
201216
repairBound: () => !!getSnapshotRepairEnqueuer(),
217+
// Read raw: the variable is out of the parsed env schema, and the point is to notice a value
218+
// nothing consumes any more.
219+
legacyEnvHalt: process.env.RUN_ENGINE_SNAPSHOT_STORE_HALT === "1",
202220
log: (message, fields) =>
203221
logger.info(message, {
204222
...fields,

‎apps/webapp/app/v3/snapshotStoreMode.server.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,19 +81,23 @@ export function buildSnapshotStoreModeResolver(deps: {
8181
}
8282

8383
/**
84-
* The hard stop. Either source halts; only the environment can hold a halt the flag cannot lift,
85-
* which is what a deployment needs while it resyncs.
84+
* The hard stop, and the flag is the whole of it.
85+
*
86+
* An environment half used to sit beside this, so a deployment could hold a halt the flag could not
87+
* lift. It is gone. It converged over a rolling deploy rather than a flag interval, and for the
88+
* length of that deploy the fleet is mixed: a halted process writes no transition, then an unhalted
89+
* one asserts a head that was never written and forks. A control whose own convergence manufactures
90+
* the divergence it exists to stop cannot be the way in. The guaranteed-inert state is an
91+
* unconfigured host, which is bootstrap config and stays in the environment.
8692
*/
8793
export function buildSnapshotStoreHaltCheck(deps: {
8894
flag: () => boolean | undefined;
89-
envHalt: boolean;
9095
}): () => boolean {
91-
return () => deps.envHalt || deps.flag() === true;
96+
return () => deps.flag() === true;
9297
}
9398

9499
export const snapshotStoreHalted = buildSnapshotStoreHaltCheck({
95100
flag: () => globalFlagsRegistry.current()?.[FEATURE_FLAG.snapshotStoreHalt],
96-
envHalt: env.RUN_ENGINE_SNAPSHOT_STORE_HALT === "1",
97101
});
98102

99103
const DEFAULT_CACHE_MAX = 10_000;

‎apps/webapp/test/snapshotStoreBoot.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,4 +179,17 @@ describe("assertSnapshotStoreBoot", () => {
179179
const d = deps({ mode: "off", hostConfigured: false, completedTtlMs: 0, orphanAgeMs: -1 });
180180
await expect(assertSnapshotStoreBoot(d)).resolves.toBeUndefined();
181181
});
182+
it("refuses to start when the retired halt environment variable still asks for a halt", async () => {
183+
// Fail loud. The variable no longer does anything, so leaving it set means an operator believes
184+
// the mirror is stopped when it is running.
185+
const d = deps({ legacyEnvHalt: true });
186+
await expect(assertSnapshotStoreBoot(d)).rejects.toThrow(/snapshotStoreHalt/i);
187+
});
188+
189+
it("ignores the retired variable when it is pinned at its old default", async () => {
190+
// "0" carries no intent, and refusing on it would break every deployment that still pins the
191+
// old default while asking for nothing.
192+
const d = deps({ legacyEnvHalt: false });
193+
await expect(assertSnapshotStoreBoot(d)).resolves.toBeUndefined();
194+
});
182195
});

‎apps/webapp/test/snapshotStoreHalt.test.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,21 @@ import { buildSnapshotStoreHaltCheck } from "~/v3/snapshotStoreMode.server";
88

99
describe("the snapshot store halt check", () => {
1010
it("is not halted by default", () => {
11-
expect(buildSnapshotStoreHaltCheck({ flag: () => undefined, envHalt: false })()).toBe(false);
12-
expect(buildSnapshotStoreHaltCheck({ flag: () => false, envHalt: false })()).toBe(false);
11+
expect(buildSnapshotStoreHaltCheck({ flag: () => undefined })()).toBe(false);
12+
expect(buildSnapshotStoreHaltCheck({ flag: () => false })()).toBe(false);
1313
});
1414

1515
it("halts on the flag, so an incident needs no deploy", () => {
16-
expect(buildSnapshotStoreHaltCheck({ flag: () => true, envHalt: false })()).toBe(true);
16+
expect(buildSnapshotStoreHaltCheck({ flag: () => true })()).toBe(true);
1717
});
1818

19-
it("halts on the environment even when the flag registry is cold", () => {
20-
expect(buildSnapshotStoreHaltCheck({ flag: () => undefined, envHalt: true })()).toBe(true);
21-
});
22-
23-
it("cannot be un-halted by the flag once the environment holds it", () => {
24-
expect(buildSnapshotStoreHaltCheck({ flag: () => false, envHalt: true })()).toBe(true);
19+
it("is the flag and nothing else, so it converges in one flag interval", () => {
20+
// The environment half is gone on purpose. It converged over a rolling deploy instead of a
21+
// flag interval, and during that window a halted process skips a transition while an unhalted
22+
// one asserts a head it cannot see, which forks once per process flip per run. A control whose
23+
// own convergence manufactures the divergence it exists to stop is not a control.
24+
expect(buildSnapshotStoreHaltCheck({ flag: () => false })()).toBe(false);
25+
expect(buildSnapshotStoreHaltCheck({ flag: () => true })()).toBe(true);
2526
});
2627
});
2728

0 commit comments

Comments
 (0)