Skip to content

Commit 08f630b

Browse files
committed
feat(run-store,webapp): make the lowest dial position genuinely inert
The lowest position stopped new runs joining the mirror but did not take the store off the run path. A transition has to ask whether its own run is resident, the keyspace is the only record of that, so every transition of every run still made one request. Two per cent with a healthy endpoint, four times the run duration with a slow one, for every run, and it did not decay as resident runs drained. Before a deployment has ever enabled the store nothing can be resident: only a birth creates a keyspace and every birth was refused. So the question has one possible answer and asking it is pure cost. A one-way latch records whether the store has ever been enabled, and while it is unset a transition skips the store entirely. Once set it is never cleared automatically, and the lowest position goes back to meaning drain: a run resident from an earlier ramp keeps mirroring, because freezing its head while Postgres moves on makes turning the dial down worse than leaving it alone. The save guard refuses to enable the deployment dial or any per-organisation override until the latch is true. Without that a run born between the two writes would be resident with its transitions skipped, and its head would freeze. Refusing makes the ordering impossible to get wrong rather than merely documented. The latch reads false only when the flag is explicitly false, so a cold or unreadable flag registry reports enabled and can never suppress a transition for a run that is resident. This moves the safe deployment posture from something an operator has to remember to the default. The store can ship with its endpoint configured, off the run path, and every step after that is a flag that converges in seconds rather than a deploy that takes hours.
1 parent d9ad46e commit 08f630b

9 files changed

Lines changed: 259 additions & 11 deletions

apps/webapp/app/routes/admin.api.v1.feature-flags.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ import type { ActionFunctionArgs } from "@remix-run/server-runtime";
22
import { json } from "@remix-run/server-runtime";
33
import { prisma } from "~/db.server";
44
import { env } from "~/env.server";
5+
import { globalFlagsRegistry } from "~/v3/globalFlagsRegistry.server";
56
import { requireAdminApiRequest } from "~/services/personalAccessToken.server";
67
import {
78
applyGlobalGracedFlips,
89
makeSetMultipleFlags,
910
touchesGracedGroup,
1011
withoutDerivedKeys,
1112
} from "~/v3/featureFlags.server";
12-
import { validatePartialFeatureFlags } from "~/v3/featureFlags";
13+
import { validatePartialFeatureFlags, FEATURE_FLAG } from "~/v3/featureFlags";
1314
import {
1415
globalOnlySnapshotStoreFlagError,
1516
snapshotStoreFlagSaveError,
@@ -41,6 +42,10 @@ export async function action({ request }: ActionFunctionArgs) {
4142

4243
const snapshotStoreError = snapshotStoreFlagSaveError(body as Record<string, unknown>, {
4344
redisHostConfigured: !!env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST,
45+
// Read from the live registry, not from the payload: the latch must ALREADY be true before
46+
// anything can be enabled, or a run born in the gap would be resident with its transitions
47+
// skipped.
48+
everEnabled: globalFlagsRegistry.current()?.[FEATURE_FLAG.snapshotStoreEverEnabled] === true,
4449
});
4550
if (snapshotStoreError) {
4651
return json({ error: snapshotStoreError }, { status: 400 });

apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.feature-flags.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,15 @@ import { env } from "~/env.server";
66
import { prisma } from "~/db.server";
77
import { requireAdminApiRequest } from "~/services/personalAccessToken.server";
88
import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server";
9+
import { globalFlagsRegistry } from "~/v3/globalFlagsRegistry.server";
910
import { snapshotStoreFlagSaveError } from "~/v3/snapshotStoreFlagGuard.server";
1011
import { invalidateSnapshotStoreOrgMode } from "~/v3/snapshotStoreMode.server";
1112
import { selectMintBaselineSource, stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace";
12-
import { validatePartialFeatureFlags, withoutOrgForbiddenSnapshotKeys } from "~/v3/featureFlags";
13+
import {
14+
FEATURE_FLAG,
15+
validatePartialFeatureFlags,
16+
withoutOrgForbiddenSnapshotKeys,
17+
} from "~/v3/featureFlags";
1318
import { flags as getGlobalFlags } from "~/v3/featureFlags.server";
1419

1520
const ParamsSchema = z.object({
@@ -80,6 +85,10 @@ export async function action({ request, params }: ActionFunctionArgs) {
8085

8186
const snapshotStoreError = snapshotStoreFlagSaveError(requestedFlags, {
8287
redisHostConfigured: !!env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST,
88+
// Read from the live registry, not from the payload: the latch must ALREADY be true before
89+
// anything can be enabled, or a run born in the gap would be resident with its transitions
90+
// skipped.
91+
everEnabled: globalFlagsRegistry.current()?.[FEATURE_FLAG.snapshotStoreEverEnabled] === true,
8392
});
8493
if (snapshotStoreError) {
8594
return json({ error: snapshotStoreError }, { status: 400 });

apps/webapp/app/routes/admin.feature-flags.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useFetcher } from "@remix-run/react";
22
import { useEffect, useState } from "react";
33
import stableStringify from "json-stable-stringify";
44
import { json } from "@remix-run/server-runtime";
5+
import { globalFlagsRegistry } from "~/v3/globalFlagsRegistry.server";
56
import { typedjson, useTypedLoaderData } from "remix-typedjson";
67
import { z } from "zod";
78
import { LockClosedIcon } from "@heroicons/react/20/solid";
@@ -140,6 +141,10 @@ export const action = dashboardAction(
140141

141142
const snapshotStoreError = snapshotStoreFlagSaveError(parsed.data.flags, {
142143
redisHostConfigured: !!env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST,
144+
// Read from the live registry, not from the payload: the latch must ALREADY be true before
145+
// anything can be enabled, or a run born in the gap would be resident with its transitions
146+
// skipped.
147+
everEnabled: globalFlagsRegistry.current()?.[FEATURE_FLAG.snapshotStoreEverEnabled] === true,
143148
});
144149
if (snapshotStoreError) {
145150
return json({ error: snapshotStoreError }, { status: 400 });

apps/webapp/app/v3/featureFlags.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ export const FEATURE_FLAG = {
6262
// snapshot reads are global, so an org at a read position would read state its own writes never
6363
// created. Stripped from org payloads by withoutOrgForbiddenSnapshotKeys.
6464
snapshotStoreOrgMode: "snapshotStoreOrgMode",
65+
// One-way residency latch. See the catalog entry below.
66+
snapshotStoreEverEnabled: "snapshotStoreEverEnabled",
6567
} as const;
6668

6769
export const FeatureFlagCatalog = {
@@ -177,6 +179,20 @@ export const FeatureFlagCatalog = {
177179
[FEATURE_FLAG.additionalApiKeyLookupEnabled]: z.boolean(),
178180
[FEATURE_FLAG.snapshotStoreMode]: z.enum(["off", "dual-write", "redis-read", "redis-only"]),
179181
[FEATURE_FLAG.snapshotStoreOrgMode]: z.enum(["off", "dual-write"]),
182+
/**
183+
* Whether this deployment has EVER had the store enabled. One way: set when the first dial or
184+
* per-organisation override moves past `off`, and never cleared automatically.
185+
*
186+
* It exists so that `off` means genuinely inert before a ramp. A transition has to ask whether its
187+
* run is resident, and the keyspace is the only record of that, so at `off` after a ramp every
188+
* transition must still ask or a resident run's head freezes. Before any ramp nothing CAN be
189+
* resident, so the question has one possible answer and asking it is pure cost: measured at 2 per
190+
* cent with a healthy endpoint and four times the run duration with a slow one.
191+
*
192+
* Strict boolean, like the other kill switches: a stringified "false" read as true would put the
193+
* whole fleet back on the run path.
194+
*/
195+
[FEATURE_FLAG.snapshotStoreEverEnabled]: z.boolean(),
180196
// Strict, like the other kill switches: a stringified "false" read as true would freeze every
181197
// resident run's Redis head.
182198
[FEATURE_FLAG.snapshotStoreHalt]: z.boolean(),
@@ -221,6 +237,7 @@ export const ORG_LOCKED_FLAGS: FeatureFlagKey[] = [
221237
// The dial and the hard stop are deployment-wide; only snapshotStoreOrgMode is per-org.
222238
FEATURE_FLAG.snapshotStoreMode,
223239
FEATURE_FLAG.snapshotStoreHalt,
240+
FEATURE_FLAG.snapshotStoreEverEnabled,
224241
];
225242

226243
/**

apps/webapp/app/v3/snapshotStoreFlagGuard.server.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,28 @@ import { FEATURE_FLAG } from "~/v3/featureFlags";
77
*/
88
export function snapshotStoreFlagSaveError(
99
requested: Record<string, unknown>,
10-
opts: { redisHostConfigured: boolean }
10+
opts: { redisHostConfigured: boolean; everEnabled?: boolean }
1111
): string | undefined {
12-
if (opts.redisHostConfigured) {
12+
// Both keys, because either one past `off` is equally silent without a connection, and either one
13+
// equally makes a run resident once there is one.
14+
const enabling = ([FEATURE_FLAG.snapshotStoreMode, FEATURE_FLAG.snapshotStoreOrgMode] as const)
15+
.map((key) => ({ key, value: requested[key] }))
16+
.filter(({ value }) => typeof value === "string" && value !== "off");
17+
18+
if (!opts.redisHostConfigured) {
19+
for (const { key, value } of enabling) {
20+
return `Cannot set ${key} to "${String(value)}": RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST is not configured in this deployment, so the snapshot store is never constructed and the flag would have no effect.`;
21+
}
1322
return undefined;
1423
}
1524

16-
// Both keys, because either one past `off` is equally silent without a connection.
17-
for (const key of [FEATURE_FLAG.snapshotStoreMode, FEATURE_FLAG.snapshotStoreOrgMode] as const) {
18-
const value = requested[key];
19-
if (typeof value === "string" && value !== "off") {
20-
return `Cannot set ${key} to "${value}": RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST is not configured in this deployment, so the snapshot store is never constructed and the flag would have no effect.`;
25+
// The latch must already be set before anything can become resident. Transitions skip Redis
26+
// entirely while it is unset, so a run born after the dial moved but before the latch landed would
27+
// be resident with its transitions skipped, and its head would freeze while Postgres moved on.
28+
// Refusing here makes that ordering impossible to get wrong rather than merely documented.
29+
if (opts.everEnabled === false) {
30+
for (const { key, value } of enabling) {
31+
return `Cannot set ${key} to "${String(value)}" before ${FEATURE_FLAG.snapshotStoreEverEnabled} is true. Set that flag first: until it is, transitions skip the store entirely, so a run born now would be resident with its transitions skipped and its head would freeze.`;
2132
}
2233
}
2334

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,15 @@ type ResolverOrgSource = Pick<OrgModeSource, "get" | "refresh"> &
5555

5656
export function buildSnapshotStoreModeResolver(deps: {
5757
globalMode: () => DialMode | undefined;
58+
/** The one-way residency latch. Absent is treated as latched, so behaviour is unchanged. */
59+
everEnabled?: () => boolean | undefined;
5860
orgMode: ResolverOrgSource;
5961
envFloor: DialMode;
6062
}): SnapshotStoreModeResolver {
6163
return {
64+
// False ONLY when the flag is explicitly false. An unreadable or cold registry must never
65+
// report "never enabled", because that would suppress transitions for runs that are resident.
66+
everEnabled: (): boolean => deps.everEnabled?.() !== false,
6267
// Awaited at birth sites only. Absent org id means nothing to look up, so it is a no-op.
6368
warm: async (organizationId: string): Promise<void> => {
6469
await deps.orgMode.warm?.(organizationId);
@@ -263,6 +268,7 @@ function orgModeSource(): OrgModeSource {
263268

264269
export const snapshotStoreModeResolver: SnapshotStoreModeResolver = buildSnapshotStoreModeResolver({
265270
globalMode: () => globalFlagsRegistry.current()?.[FEATURE_FLAG.snapshotStoreMode],
271+
everEnabled: () => globalFlagsRegistry.current()?.[FEATURE_FLAG.snapshotStoreEverEnabled],
266272
orgMode: {
267273
get: (organizationId) => orgModeSource().get(organizationId),
268274
refresh: (organizationId) => orgModeSource().refresh(organizationId),

apps/webapp/test/snapshotStoreFlagGuard.test.ts

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import {
33
globalOnlySnapshotStoreFlagError,
44
snapshotStoreFlagSaveError,
55
} from "~/v3/snapshotStoreFlagGuard.server";
6-
import { FEATURE_FLAG, GLOBAL_LOCKED_FLAGS } from "~/v3/featureFlags";
6+
import {
7+
FeatureFlagCatalog,
8+
FEATURE_FLAG,
9+
GLOBAL_LOCKED_FLAGS,
10+
ORG_LOCKED_FLAGS,
11+
} from "~/v3/featureFlags";
712

813
describe("snapshotStoreFlagSaveError", () => {
914
it("refuses a flip past off when no host is configured", () => {
@@ -97,3 +102,60 @@ describe("the global page and the save guard agree", () => {
97102
expect(GLOBAL_LOCKED_FLAGS).not.toContain(FEATURE_FLAG.snapshotStoreMode);
98103
});
99104
});
105+
106+
describe("the residency latch", () => {
107+
it("refuses to enable the deployment dial before the latch is set", () => {
108+
// Ordering matters and must be impossible to get wrong. Transitions skip Redis entirely while
109+
// the latch is unset, so a run born after the dial moved but before the latch landed would be
110+
// resident with its transitions skipped, and its head would freeze. Latch first, always.
111+
expect(
112+
snapshotStoreFlagSaveError(
113+
{ snapshotStoreMode: "dual-write" },
114+
{ redisHostConfigured: true, everEnabled: false }
115+
)
116+
).toMatch(/snapshotStoreEverEnabled/);
117+
});
118+
119+
it("refuses to enable a per-organisation override before the latch is set", () => {
120+
expect(
121+
snapshotStoreFlagSaveError(
122+
{ snapshotStoreOrgMode: "dual-write" },
123+
{ redisHostConfigured: true, everEnabled: false }
124+
)
125+
).toMatch(/snapshotStoreEverEnabled/);
126+
});
127+
128+
it("allows enabling once the latch is set", () => {
129+
expect(
130+
snapshotStoreFlagSaveError(
131+
{ snapshotStoreMode: "dual-write" },
132+
{ redisHostConfigured: true, everEnabled: true }
133+
)
134+
).toBeUndefined();
135+
});
136+
137+
it("never blocks a move back to off, whatever the latch says", () => {
138+
// Turning it down must never be gated. That is the rollback path.
139+
expect(
140+
snapshotStoreFlagSaveError(
141+
{ snapshotStoreMode: "off" },
142+
{ redisHostConfigured: true, everEnabled: false }
143+
)
144+
).toBeUndefined();
145+
});
146+
147+
it("does not block setting the latch itself", () => {
148+
expect(
149+
snapshotStoreFlagSaveError(
150+
{ snapshotStoreEverEnabled: true },
151+
{ redisHostConfigured: true, everEnabled: false }
152+
)
153+
).toBeUndefined();
154+
});
155+
156+
it("is a deployment-wide flag, and takes only a real boolean", () => {
157+
expect(FeatureFlagCatalog.snapshotStoreEverEnabled.safeParse(true).success).toBe(true);
158+
expect(FeatureFlagCatalog.snapshotStoreEverEnabled.safeParse("true").success).toBe(false);
159+
expect(ORG_LOCKED_FLAGS).toContain("snapshotStoreEverEnabled");
160+
});
161+
});
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// `off` was not inert. A transition must ask whether its run is resident, and the keyspace is the
2+
// only record of that, so at `off` AFTER a ramp every transition still has to ask or a resident
3+
// run's head freezes while Postgres moves on.
4+
//
5+
// Before any ramp, nothing CAN be resident: only a birth creates a keyspace and every birth was
6+
// refused. So the question has one possible answer and asking it is pure cost. Measured: 2 per cent
7+
// with a healthy endpoint, four times the run duration with a slow one, for every run, and it did
8+
// not decay.
9+
//
10+
// The latch is one way. Unset means this deployment has never enabled the store, so transitions skip
11+
// it entirely. Once set it is never cleared, and `off` returns to meaning "drain".
12+
import { describe, expect, it } from "vitest";
13+
import { TaskRunExecutionSnapshotStore } from "./taskRunExecutionSnapshotStore.js";
14+
import type { SnapshotStoreModeResolver } from "./taskRunExecutionSnapshotStore.js";
15+
import type { RedisSnapshotStore } from "./redisSnapshotStore.js";
16+
import type { RunStore } from "./types.js";
17+
18+
const SCOPE = {
19+
environmentId: "env_1",
20+
environmentType: "PRODUCTION",
21+
projectId: "proj_1",
22+
organizationId: "org_1",
23+
} as const;
24+
25+
function harness(opts: { mode: "off" | "dual-write"; everEnabled?: boolean }) {
26+
const touched: string[] = [];
27+
const redis = new Proxy({} as RedisSnapshotStore, {
28+
get: (_t, prop) => {
29+
return (...__: unknown[]) => {
30+
touched.push(String(prop));
31+
return Promise.resolve({ outcome: "written", seq: 1 });
32+
};
33+
},
34+
});
35+
const delegate = new Proxy({} as Record<string, unknown>, {
36+
get: () => () => Promise.resolve({}),
37+
}) as unknown as RunStore;
38+
39+
const modeResolver: SnapshotStoreModeResolver = {
40+
resolve: () => opts.mode,
41+
...(opts.everEnabled !== undefined && { everEnabled: () => opts.everEnabled! }),
42+
};
43+
44+
const decorated = new TaskRunExecutionSnapshotStore(delegate, {
45+
store: redis,
46+
mode: opts.mode,
47+
modeResolver,
48+
});
49+
return { decorated, touched };
50+
}
51+
52+
const completion = {
53+
completedAt: new Date(),
54+
outputType: "application/json",
55+
usageDurationMs: 1,
56+
costInCents: 0,
57+
snapshot: {
58+
executionStatus: "FINISHED" as const,
59+
description: "done",
60+
runStatus: "COMPLETED_SUCCESSFULLY" as const,
61+
attemptNumber: 1,
62+
...SCOPE,
63+
},
64+
};
65+
66+
describe("the residency latch", () => {
67+
it("a transition touches the store NOT AT ALL while the latch is unset", async () => {
68+
const h = harness({ mode: "off", everEnabled: false });
69+
70+
await h.decorated.completeAttemptSuccess("run_1", completion, { select: { id: true } });
71+
72+
// The assertion the whole change exists for: no probe, so Redis is off the run path entirely.
73+
expect(h.touched).toEqual([]);
74+
});
75+
76+
it("a transition still asks once the latch is set, even at off", async () => {
77+
// Non-negotiable. A run resident from an earlier ramp must keep mirroring, or its head freezes
78+
// and the remedy becomes worse than the fault.
79+
const h = harness({ mode: "off", everEnabled: true });
80+
81+
await h.decorated.completeAttemptSuccess("run_1", completion, { select: { id: true } });
82+
83+
expect(h.touched).toContain("append");
84+
});
85+
86+
it("asks when the resolver offers no latch, so an unlatched deployment is unchanged", async () => {
87+
const h = harness({ mode: "off" });
88+
89+
await h.decorated.completeAttemptSuccess("run_1", completion, { select: { id: true } });
90+
91+
expect(h.touched).toContain("append");
92+
});
93+
94+
it("never suppresses a transition once the dial itself is past off", async () => {
95+
// Belt and braces: the guard makes this state unreachable, but if it ever occurred, suppressing
96+
// transitions while births are mirroring is the one combination that strands a head.
97+
const h = harness({ mode: "dual-write", everEnabled: false });
98+
99+
await h.decorated.completeAttemptSuccess("run_1", completion, { select: { id: true } });
100+
101+
expect(h.touched).toContain("append");
102+
});
103+
});

internal-packages/run-store/src/taskRunExecutionSnapshotStore.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,18 @@ export type SnapshotStoreModeResolver = {
9797
* behaviour, never an error.
9898
*/
9999
warm?(organizationId: string): Promise<void>;
100+
/**
101+
* Optional one-way latch: has this deployment EVER had the store enabled.
102+
*
103+
* Absent, or true, means behave as before. False means nothing can be resident yet, because only a
104+
* birth creates a keyspace and every birth so far was refused, so a transition's question has one
105+
* possible answer and asking it is pure cost. It is what makes `off` genuinely inert rather than
106+
* merely quiet: measured at 2 per cent with a healthy endpoint and four times the run duration
107+
* with a slow one, for every run, with no decay.
108+
*
109+
* MUST be synchronous and MUST NOT query, for the same reason `resolve` must not.
110+
*/
111+
everEnabled?(): boolean;
100112
};
101113

102114
/**
@@ -278,7 +290,25 @@ export class TaskRunExecutionSnapshotStore extends DelegatingRunStore {
278290
* outright is the halt switch, and it is a resync control rather than a rollback.
279291
*/
280292
protected writesRedisForTransition(): boolean {
281-
return !this.halted();
293+
if (this.halted()) {
294+
return false;
295+
}
296+
297+
// The latch, and the ONLY case where a transition may be skipped without a keyspace check.
298+
// Sound because residency is created by births alone: with the latch unset no birth has ever
299+
// mirrored, so no keyspace exists and the append script would refuse every one of these anyway.
300+
//
301+
// Deliberately ignores the dial otherwise. Once the latch is set, `off` still mirrors a resident
302+
// run's transitions, because the alternative freezes its head while Postgres moves on, and that
303+
// makes turning the dial down worse than leaving it alone.
304+
//
305+
// The guard on the flag save refuses to enable anything until the latch is true, so "latch unset
306+
// while runs are resident" is unreachable rather than merely unlikely.
307+
if (this.modeResolver?.everEnabled?.() === false && this.mode === "off") {
308+
return false;
309+
}
310+
311+
return true;
282312
}
283313

284314
/** Test seams for the two predicates. Not for production callers. */

0 commit comments

Comments
 (0)