Skip to content

Commit 8e887e3

Browse files
committed
perf(webapp): keep the snapshot store inert until a Redis host is configured
With no snapshot-store Redis host set, the feature must add zero standing cost. Two paths did not honour that: - The org census started its periodic organization.findMany poll at import, gated only on NODE_ENV, so a production process with no host still polled the replica every reload interval. Gate the poll on a host check that is independent of NODE_ENV. - The run store wired the redis-only Postgres-suppression predicate into every PostgresRunStore unconditionally, so every snapshot write ran a mode resolution (and a background per-org replica read on a cold cache). Thread the predicate through buildRunStore and wire it only when configured; unset leaves the store a plain passthrough that writes every snapshot row. Both now route through a single isSnapshotStoreConfigured gate.
1 parent 30a6be9 commit 8e887e3

6 files changed

Lines changed: 90 additions & 9 deletions

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
} from "~/db.server";
2121
import { env } from "~/env.server";
2222
import { singleton } from "~/utils/singleton";
23+
import { isSnapshotStoreConfigured } from "./snapshotStoreConfigured.server";
2324
import { decorateWithSnapshotStore } from "./snapshotStoreInstance.server";
2425
import { snapshotStoreModeResolver } from "./snapshotStoreMode.server";
2526
import {
@@ -55,15 +56,20 @@ type BuildRunStoreDeps = {
5556
singleResilience?: TransactionResilienceConfig;
5657
newResilience?: TransactionResilienceConfig;
5758
legacyResilience?: TransactionResilienceConfig;
59+
/** The redis-only PG-suppression predicate, or undefined for a plain passthrough. Passed in (never
60+
* read from env here) so this builder stays pure; the caller wires it only when the store is
61+
* configured, so an unconfigured deploy writes every snapshot row with no per-write resolution. */
62+
snapshotWrites?: (organizationId?: string) => boolean;
5863
};
5964

6065
/**
6166
* Pure run-store builder (no env / no boot side effects — webapp testability rule).
6267
*
6368
* Split OFF (default / self-host): returns the exact passthrough PostgresRunStore we
6469
* have always returned, built from the single control-plane handles. No second store
65-
* is constructed and no marker predicate is consulted, so behavior is byte-identical
66-
* to single-DB today.
70+
* is constructed, and the redis-only marker predicate is consulted only when the caller
71+
* wired one (i.e. the snapshot store is configured); with it unset behavior is
72+
* byte-identical to single-DB today.
6773
*
6874
* Split ON: returns a RoutingRunStore that selects between a NEW store (where new runs
6975
* are born) and a LEGACY store (draining) by run-id residency (id shape). There is no cuid
@@ -74,14 +80,18 @@ type BuildRunStoreDeps = {
7480
const suppressPgAtRedisOnly = (organizationId?: string) =>
7581
snapshotStoreModeResolver.resolve(organizationId) !== "redis-only";
7682

83+
// Wired into the store only when the snapshot store is configured. Unconfigured, this stays undefined
84+
// and PostgresRunStore writes every snapshot row with no per-write mode resolution (the inert state).
85+
const snapshotWritesPredicate = isSnapshotStoreConfigured() ? suppressPgAtRedisOnly : undefined;
86+
7787
export function buildRunStore(deps: BuildRunStoreDeps): RunStore {
7888
if (!deps.splitEnabled) {
7989
return new PostgresRunStore({
8090
prisma: deps.singleWriter,
8191
readOnlyPrisma: deps.singleReplica,
8292
maxWait: deps.singleResilience?.maxWait,
8393
transactionStartRetry: deps.singleResilience?.startRetry,
84-
snapshotWrites: suppressPgAtRedisOnly,
94+
snapshotWrites: deps.snapshotWrites,
8595
});
8696
}
8797

@@ -97,14 +107,14 @@ export function buildRunStore(deps: BuildRunStoreDeps): RunStore {
97107
schemaVariant: "dedicated",
98108
maxWait: deps.newResilience?.maxWait,
99109
transactionStartRetry: deps.newResilience?.startRetry,
100-
snapshotWrites: suppressPgAtRedisOnly,
110+
snapshotWrites: deps.snapshotWrites,
101111
});
102112
const legacyStore = new PostgresRunStore({
103113
prisma: deps.legacyWriter,
104114
readOnlyPrisma: deps.legacyReplica,
105115
maxWait: deps.legacyResilience?.maxWait,
106116
transactionStartRetry: deps.legacyResilience?.startRetry,
107-
snapshotWrites: suppressPgAtRedisOnly,
117+
snapshotWrites: deps.snapshotWrites,
108118
});
109119

110120
// Gen-2 shards: one dedicated store per descriptor, handed to the N-way router. An aliased shard
@@ -118,7 +128,7 @@ export function buildRunStore(deps: BuildRunStoreDeps): RunStore {
118128
schemaVariant: "dedicated" as const,
119129
maxWait: shard.resilience?.maxWait,
120130
transactionStartRetry: shard.resilience?.startRetry,
121-
snapshotWrites: suppressPgAtRedisOnly,
131+
snapshotWrites: deps.snapshotWrites,
122132
}),
123133
aliasOf: shard.aliasOf,
124134
}));
@@ -198,6 +208,7 @@ export const runStoreWithoutSnapshotDecorator: RunStore = singleton("RunStore.un
198208
singleWriter: prisma,
199209
singleReplica: $replica,
200210
singleResilience: resilienceForClient(prisma),
211+
snapshotWrites: snapshotWritesPredicate,
201212
});
202213
}
203214
const { shardHandles, ...storeHandles } = handles;
@@ -216,6 +227,7 @@ export const runStoreWithoutSnapshotDecorator: RunStore = singleton("RunStore.un
216227
singleResilience: resilienceForClient(prisma),
217228
newResilience: resilienceForClient(handles.newWriter),
218229
legacyResilience: resilienceForClient(handles.legacyWriter),
230+
snapshotWrites: snapshotWritesPredicate,
219231
});
220232
});
221233

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { env } from "~/env.server";
2+
3+
/**
4+
* The single bootstrap switch for the snapshot store. With no Redis host configured the feature is
5+
* inert: the org census does not poll, the run store is a plain Postgres passthrough with no per-write
6+
* mode resolution, and no decorator is attached. Every optional piece gates on this one predicate so
7+
* that merging the feature with the host unset adds zero standing cost.
8+
*/
9+
export function isSnapshotStoreConfigured(
10+
host: string | undefined = env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST ?? undefined
11+
): boolean {
12+
return !!host;
13+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { env } from "~/env.server";
1313
import { logger } from "~/services/logger.server";
1414
import { singleton } from "~/utils/singleton";
1515
import { getSnapshotRepairEnqueuer } from "./snapshotStoreBindings.server";
16+
import { isSnapshotStoreConfigured } from "./snapshotStoreConfigured.server";
1617
import { snapshotStoreHalted, snapshotStoreModeResolver } from "./snapshotStoreMode.server";
1718
import { createSnapshotStoreMetrics } from "./snapshotStoreMetrics.server";
1819
import { snapshotStoreOrgCensus } from "./snapshotStoreOrgCensus.server";
@@ -21,7 +22,7 @@ import { meter } from "./tracer.server";
2122
const KEY_PREFIX = "engine:";
2223

2324
function isConfigured(): boolean {
24-
return !!env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST;
25+
return isSnapshotStoreConfigured();
2526
}
2627

2728
function redisOptions(): RedisOptions {

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { env } from "~/env.server";
44
import { createReloadingRegistry } from "~/utils/reloadingRegistry.server";
55
import { singleton } from "~/utils/singleton";
66
import { FEATURE_FLAG } from "~/v3/featureFlags";
7+
import { isSnapshotStoreConfigured } from "~/v3/snapshotStoreConfigured.server";
78
import { cachedOrgModeFor, NO_OVERRIDE } from "~/v3/snapshotStoreMode.server";
89

910
/** The narrow slice of Prisma the census reads, so a test injects a fake without a mocking library. */
@@ -58,6 +59,18 @@ function classify(rows: Array<{ id: string; featureFlags: unknown }>): OrgCensus
5859
return { readEnabled, redisOnly, cohort, everEnabled };
5960
}
6061

62+
/**
63+
* The census poll runs only when the store is configured AND we are outside test. The host gate is
64+
* independent of NODE_ENV so a production process with no Redis host never starts the poll: the
65+
* merged-but-off deploy pays no standing organization.findMany.
66+
*/
67+
export function defaultCensusAutoStart(
68+
host: string | undefined = env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST ?? undefined,
69+
nodeEnv: string | undefined = process.env.NODE_ENV
70+
): boolean {
71+
return isSnapshotStoreConfigured(host) && nodeEnv !== "test";
72+
}
73+
6174
export function createSnapshotStoreOrgCensus(
6275
clients?: { replica: SnapshotStoreOrgCensusClient },
6376
opts?: { intervalMs?: number; autoStart?: boolean }
@@ -66,7 +79,7 @@ export function createSnapshotStoreOrgCensus(
6679
const registry = createReloadingRegistry<OrgCensusSnapshot>({
6780
name: "snapshot-store-org-census",
6881
intervalMs: opts?.intervalMs ?? env.GLOBAL_FLAGS_RELOAD_INTERVAL_MS,
69-
autoStart: opts?.autoStart ?? process.env.NODE_ENV !== "test",
82+
autoStart: opts?.autoStart ?? defaultCensusAutoStart(),
7083
load: async () =>
7184
// WHERE returns orgs with EITHER key, so an ever-enabled org that is now off (or holds only
7285
// the latch after a clear) still returns. Classification stays in code, identical to the resolver.
@@ -116,7 +129,10 @@ export function createSnapshotStoreOrgCensus(
116129
};
117130
}
118131

119-
/** Built at import, like globalFlagsRegistry: reads the DB-backed census on GLOBAL_FLAGS_RELOAD_INTERVAL_MS. */
132+
/**
133+
* Built at import, like globalFlagsRegistry, but the poll starts only when a Redis host is configured
134+
* (see defaultCensusAutoStart). Unconfigured, the object is inert: constructed, never polling.
135+
*/
120136
export const snapshotStoreOrgCensus = singleton("snapshotStoreOrgCensus", () =>
121137
createSnapshotStoreOrgCensus()
122138
);
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { describe, expect, it } from "vitest";
2+
import { isSnapshotStoreConfigured } from "~/v3/snapshotStoreConfigured.server";
3+
4+
describe("isSnapshotStoreConfigured", () => {
5+
it("is false when no Redis host is set (feature merged but inert)", () => {
6+
expect(isSnapshotStoreConfigured(undefined)).toBe(false);
7+
expect(isSnapshotStoreConfigured("")).toBe(false);
8+
});
9+
10+
it("is true once a Redis host is set", () => {
11+
expect(isSnapshotStoreConfigured("snap-redis.internal")).toBe(true);
12+
});
13+
});

apps/webapp/test/snapshotStoreOrgCensus.server.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it } from "vitest";
22
import {
33
createSnapshotStoreOrgCensus,
4+
defaultCensusAutoStart,
45
type SnapshotStoreOrgCensusClient,
56
} from "~/v3/snapshotStoreOrgCensus.server";
67

@@ -126,6 +127,31 @@ describe("snapshot store org census", () => {
126127
});
127128
});
128129

130+
describe("snapshot store org census — autoStart host gate", () => {
131+
it("never polls when no Redis host is configured, even in production", () => {
132+
expect(defaultCensusAutoStart(undefined, "production")).toBe(false);
133+
expect(defaultCensusAutoStart("", "production")).toBe(false);
134+
});
135+
136+
it("polls only when configured and outside test", () => {
137+
expect(defaultCensusAutoStart("snap-redis", "production")).toBe(true);
138+
expect(defaultCensusAutoStart("snap-redis", "test")).toBe(false);
139+
});
140+
141+
it("does not issue a query when built with a disabled autoStart", async () => {
142+
const calls: FindManyArgs[] = [];
143+
const census = createSnapshotStoreOrgCensus(
144+
{ replica: fakeClient([{ id: "org_a", featureFlags: null }], calls) },
145+
{ autoStart: defaultCensusAutoStart(undefined, "production"), intervalMs: 5 }
146+
);
147+
148+
await new Promise((resolve) => setTimeout(resolve, 30));
149+
150+
expect(calls).toHaveLength(0);
151+
census.stop();
152+
});
153+
});
154+
129155
describe("snapshot store org census — definite ever-enabled set", () => {
130156
// A: latched but dialled back to off. B: at redis-read and latched. C: has the mode key but off,
131157
// never latched. D: not returned by the query at all.

0 commit comments

Comments
 (0)