Skip to content

Commit ea6de93

Browse files
committed
feat(webapp,run-engine): take up the store and sweeper connection contract
The store now receives a pre-built client instead of building its own, so cluster mode reaches the hot path and not only the sweep. The webapp owns both sockets and closes both, with the hot-path client last so an append in flight can still land. The sweep receives the dedicated client, the confirm window that was parsed and unused, and the deadline and abort signal it now accepts. So the pass budget is enforced inside a pass rather than only bounding the lock, and a truncated pass reports itself as partial. The engine also arms an abort a minute past the budget. The deadline is the sweep's own stopping rule, checked at batch boundaries; this is the backstop for a pass that has stopped reaching one, so the signal is not decorative.
1 parent 6cc24de commit ea6de93

3 files changed

Lines changed: 16 additions & 15 deletions

File tree

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ function buildClient(name: string): RedisClient {
6464

6565
type Instance = {
6666
sweepClient: RedisClient;
67+
hotPathClient: RedisClient;
6768
redisSnapshotStore: RedisSnapshotStore;
6869
decorate: (store: RunStore) => RunStore;
6970
};
@@ -79,16 +80,17 @@ const instance = singleton<Instance | undefined>("snapshotStoreInstance", () =>
7980
// transition append. It also backs the sweep's exclusion lock.
8081
const sweepClient = buildClient("sweep");
8182

83+
const hotPathClient = buildClient("store");
84+
8285
const redisSnapshotStore = new RedisSnapshotStore({
83-
// The store still builds its own single-node client, so cluster mode reaches the sweep but not
84-
// the hot path. Hand it a pre-built client once its options accept one.
85-
redisOptions: redisOptions(),
86+
client: hotPathClient,
8687
completedTtlMs: env.RUN_ENGINE_SNAPSHOT_STORE_COMPLETED_TTL_MS,
8788
metrics: metrics.store,
8889
});
8990

9091
return {
9192
sweepClient,
93+
hotPathClient,
9294
redisSnapshotStore,
9395
decorate: (store: RunStore) =>
9496
new TaskRunExecutionSnapshotStore(store, {
@@ -145,6 +147,8 @@ export async function quitSnapshotStoreClients(): Promise<void> {
145147
await quit().catch(() => undefined);
146148
}
147149
await instance.sweepClient.quit().catch(() => undefined);
148-
// Last: an append in flight must still land.
150+
// Last: an append in flight must still land. The store's own quit() returns early on a
151+
// caller-supplied client, so closing the socket is ours.
149152
await instance.redisSnapshotStore.quit().catch(() => undefined);
153+
await instance.hotPathClient.quit().catch(() => undefined);
150154
}

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

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,27 +23,20 @@ export function registerSnapshotStoreWiring(): boolean {
2323
});
2424

2525
const sweeper = new SnapshotOrphanSweeper({
26-
redisOptions: {
27-
keyPrefix: "engine:",
28-
host: env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_HOST ?? undefined,
29-
port: env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_PORT ?? undefined,
30-
username: env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_USERNAME ?? undefined,
31-
password: env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_PASSWORD ?? undefined,
32-
...(env.RUN_ENGINE_SNAPSHOT_STORE_REDIS_TLS_DISABLED === "true" ? {} : { tls: {} }),
33-
},
26+
// Its own connection, so a scan of every master can never stall a transition append.
27+
client: sweepClient,
3428
// The undecorated router: rule 2 asks Postgres whether a run row exists, and must never be
3529
// able to ask Redis whether Redis is an orphan.
3630
runStore: runStoreWithoutSnapshotDecorator,
3731
completedTtlMs: env.RUN_ENGINE_SNAPSHOT_STORE_COMPLETED_TTL_MS,
3832
orphanAgeMs: env.RUN_ENGINE_SNAPSHOT_STORE_ORPHAN_AGE_MS,
33+
confirmOrphanAfterMs: env.RUN_ENGINE_SNAPSHOT_STORE_CONFIRM_ORPHAN_AFTER_MS,
3934
});
4035

4136
setSnapshotSweepRunner(
4237
buildSnapshotSweepRunner({
4338
client: sweepClient,
44-
// The sweep does not yet accept a deadline, so the budget is not enforced inside a pass. The
45-
// fenced lock is what keeps two passes apart; its TTL covers the expected pass duration.
46-
sweep: async () => ({ ...(await sweeper.sweep()) }),
39+
sweep: async ({ deadline, signal }) => ({ ...(await sweeper.sweep({ deadline, signal })) }),
4740
lockTtlMs: env.RUN_ENGINE_SNAPSHOT_STORE_GC_SWEEP_BUDGET_MS + 3_600_000,
4841
})
4942
);

internal-packages/run-engine/src/engine/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2961,6 +2961,9 @@ export class RunEngine {
29612961

29622962
const budgetMs = this.options.snapshotStore?.sweepBudgetMs ?? 10_800_000;
29632963
const controller = new AbortController();
2964+
// The deadline is the sweep's own stopping rule; this is the backstop for a pass that has
2965+
// stopped reaching a batch boundary, so the signal is not merely decorative.
2966+
const abortAt = globalThis.setTimeout(() => controller.abort(), budgetMs + 60_000);
29642967
let outcome = "failed";
29652968
let counts: Record<string, number | boolean> | undefined;
29662969

@@ -2974,6 +2977,7 @@ export class RunEngine {
29742977
} catch (error) {
29752978
this.logger.error("sweepSnapshotOrphans threw", { error });
29762979
} finally {
2980+
globalThis.clearTimeout(abortAt);
29772981
this.snapshotSweepPassCounter?.add(1, { outcome });
29782982
for (const [field, value] of Object.entries(counts ?? {})) {
29792983
if (typeof value === "number") {

0 commit comments

Comments
 (0)