Skip to content

Commit 03f243d

Browse files
authored
Multi-instance job control: route reconnects to the owner, cancel across machines (#4715)
1 parent 008931c commit 03f243d

26 files changed

Lines changed: 1569 additions & 17 deletions

docs/websocket-api.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,54 @@ same format. Every message is a map/object with at least a `type` field.
3838
reference client retries up to 10 times starting at 1 s).
3939
4. **Close** — call `socket.close()` or let the server close the connection.
4040

41+
## Multi-Instance Deployments
42+
43+
A run's replay buffer and its cancel/stream hooks live in the one server
44+
process executing it, so on a deployment with more than one instance both
45+
`reconnect_job` and `cancel_job` have to reach that process. Two environment
46+
variables drive this, and with neither set the whole mechanism is inert:
47+
48+
- `NODETOOL_INSTANCE_ID` — this instance's identity.
49+
- `FLY_MACHINE_ID` — the fallback, set by Fly on every machine. It is also the
50+
value `fly-replay: instance=<id>` addresses.
51+
52+
The instance executing a run stamps its id on the job row (`runner_instance`).
53+
Two things follow.
54+
55+
**Resuming lands on the owner.** A reconnecting client appends
56+
`?resume_job=<job_id>` to the handshake URL. If that job is non-terminal and
57+
owned by another instance, the server answers the upgrade with
58+
`fly-replay: instance=<owner>` instead of accepting it, and Fly's proxy
59+
re-issues the whole handshake there. A request the proxy already replayed
60+
(`fly-replay-src` present) is never replayed again, so this cannot ping-pong.
61+
The hint names one job — with runs in flight on several instances the rest
62+
reconnect wherever they land and fall back to `reconnect_job`'s persisted-row
63+
answer: the right status, without the replayed frames.
64+
65+
The client retires the hint after two consecutive failed connects, so the third
66+
attempt goes out bare. Without that, a deploy that retires the owning machine
67+
while its row still reads `running` would have every reconnect replayed at a
68+
machine that no longer exists — and since the browser shares one socket across
69+
chat and every other consumer, all of them would stay dark. A successful
70+
connect resets the count.
71+
72+
**Cancel travels through the row.** `cancel_job` for a run this process does
73+
not hold, whose row names a *different* instance, marks the row cancelled with
74+
a conditional update — only while it is still non-terminal, so it cannot
75+
overwrite the owner's own outcome. Every instance re-reads its own running
76+
runs on a timer (`NODETOOL_JOB_CANCEL_POLL_MS`, default 15000, `0` disables)
77+
and cancels any whose row now reads `cancelled`: one indexed query per tick,
78+
bounded by that instance's concurrency.
79+
80+
The row is the only transport, so a cross-instance cancel takes up to a poll
81+
interval to land — the trade for having exactly one signal, the durable one. A
82+
cancel on the machine that *does* hold the run does not go through any of this;
83+
it reaches the session's hooks directly and is immediate.
84+
85+
A row with no `runner_instance` (an HTTP, trigger, or MCP run — nothing holds a
86+
session for those anywhere) is left alone and still answers "Job not found or
87+
already completed".
88+
4189
## Client → Server Commands
4290

4391
All client messages contain `command` and `data` fields.

packages/models/src/db.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ const TABLE_COLUMNS: Record<string, Record<string, string>> = {
271271
suspension_metadata_json: "text",
272272
execution_strategy: "text",
273273
execution_id: "text",
274+
runner_instance: "text",
274275
metadata_json: "text",
275276
created_at: "text",
276277
updated_at: "text"
@@ -648,6 +649,7 @@ function getCreateSchemaSql(): string {
648649
"suspension_metadata_json" text,
649650
"execution_strategy" text,
650651
"execution_id" text,
652+
"runner_instance" text,
651653
"metadata_json" text,
652654
"created_at" text NOT NULL,
653655
"updated_at" text NOT NULL

packages/models/src/job.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* Port of Python's `nodetool.models.job`.
55
*/
66

7-
import { eq, and, desc, lt } from "drizzle-orm";
7+
import { eq, and, desc, lt, inArray, notInArray } from "drizzle-orm";
88
import { DBModel, createTimeOrderedUuid } from "./base-model.js";
99
import { getDb } from "./db.js";
1010
import { jobs } from "./schema/jobs.js";
@@ -52,6 +52,12 @@ export class Job extends DBModel {
5252
declare suspension_metadata_json: Record<string, unknown> | null;
5353
declare execution_strategy: string | null;
5454
declare execution_id: string | null;
55+
/**
56+
* The server instance executing this run, when the deployment has more than
57+
* one (see `getInstanceId` in the websocket package). Null means
58+
* single-machine: nothing routes by it.
59+
*/
60+
declare runner_instance: string | null;
5561
declare metadata_json: Record<string, unknown> | null;
5662
declare created_at: string;
5763
declare updated_at: string;
@@ -85,6 +91,7 @@ export class Job extends DBModel {
8591
this.suspension_metadata_json ??= null;
8692
this.execution_strategy ??= null;
8793
this.execution_id ??= null;
94+
this.runner_instance ??= null;
8895
this.metadata_json ??= null;
8996
this.name ??= "";
9097
}
@@ -228,6 +235,55 @@ export class Job extends DBModel {
228235

229236
// ── Static queries ───────────────────────────────────────────────
230237

238+
/**
239+
* Cancel a run without reading it first.
240+
*
241+
* A cancel arriving on an instance that does not own the run races the
242+
* owner's own terminal write. Loading the row, mutating it and calling
243+
* `save()` would send a full-row upsert built from a snapshot taken before
244+
* that race — resurrecting a `completed` job as `cancelled` and overwriting
245+
* the cost and timestamps the owner had just written. This touches only the
246+
* two columns a cancel owns, and only while the row is still active.
247+
*
248+
* Returns whether a row actually changed: false means the run was already
249+
* terminal (or is not this user's), which is the caller's cue that there was
250+
* nothing to cancel.
251+
*/
252+
static async markCancelledIfActive(
253+
jobId: string,
254+
userId: string
255+
): Promise<boolean> {
256+
const db = getDb();
257+
const now = new Date().toISOString();
258+
const updated = await db
259+
.update(jobs)
260+
.set({ status: "cancelled", finished_at: now, updated_at: now })
261+
.where(
262+
and(
263+
eq(jobs.id, jobId),
264+
eq(jobs.user_id, userId),
265+
notInArray(jobs.status, ["completed", "failed", "cancelled"])
266+
)
267+
)
268+
.returning({ id: jobs.id });
269+
return updated.length > 0;
270+
}
271+
272+
/**
273+
* Which of these ids are now cancelled. The poller's one query per tick —
274+
* indexed on the primary key, and bounded by how many runs an instance is
275+
* actually executing.
276+
*/
277+
static async cancelledAmong(jobIds: string[]): Promise<string[]> {
278+
if (jobIds.length === 0) return [];
279+
const db = getDb();
280+
const rows = await db
281+
.select({ id: jobs.id })
282+
.from(jobs)
283+
.where(and(inArray(jobs.id, jobIds), eq(jobs.status, "cancelled")));
284+
return rows.map((row: { id: string }) => row.id);
285+
}
286+
231287
/** Find a job by id, scoped to the user. */
232288
static async find(userId: string, jobId: string): Promise<Job | null> {
233289
const job = await Job.get<Job>(jobId);

packages/models/src/migrations/versions.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2497,6 +2497,29 @@ export const migrations: MigrationDef[] = [
24972497
await db.execute("DROP INDEX IF EXISTS idx_tsv_timeline");
24982498
await db.execute("DROP TABLE IF EXISTS timeline_sequence_versions");
24992499
}
2500+
},
2501+
2502+
// ── Stamp the machine executing a run onto its job row ─────────────
2503+
// With more than one server instance behind the Fly proxy, a reconnecting
2504+
// client lands on a random machine while the run lives on exactly one. The
2505+
// row records which, so the upgrade can be replayed to the owner and a
2506+
// cancel can be addressed at it. Null everywhere on a single-machine
2507+
// deployment, where the column is inert.
2508+
{
2509+
version: "20260805_000000",
2510+
name: "add_runner_instance_to_jobs",
2511+
createsTables: [],
2512+
modifiesTables: ["nodetool_jobs"],
2513+
async up(db) {
2514+
if (!(await db.tableExists("nodetool_jobs"))) return;
2515+
if (await db.columnExists("nodetool_jobs", "runner_instance")) return;
2516+
await db.execute(
2517+
"ALTER TABLE nodetool_jobs ADD COLUMN runner_instance TEXT"
2518+
);
2519+
},
2520+
async down() {
2521+
// no-op: dropping columns is unsafe across dialects and versions
2522+
}
25002523
}
25012524
];
25022525

packages/models/src/schema-pg/jobs.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ export const jobs = pgTable(
3535
),
3636
execution_strategy: text("execution_strategy"),
3737
execution_id: text("execution_id"),
38+
// The server instance executing this run (Fly machine id). Null on a
39+
// single-machine deployment; see packages/websocket/src/lib/instance-id.ts.
40+
runner_instance: text("runner_instance"),
3841
metadata_json: jsonText<Record<string, unknown>>()("metadata_json"),
3942
created_at: text("created_at").notNull(),
4043
updated_at: text("updated_at").notNull()

packages/models/src/schema/jobs.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ export const jobs = sqliteTable(
4141
),
4242
execution_strategy: text("execution_strategy"),
4343
execution_id: text("execution_id"),
44+
// The server instance executing this run (Fly machine id). Null on a
45+
// single-machine deployment; see packages/websocket/src/lib/instance-id.ts.
46+
runner_instance: text("runner_instance"),
4447
metadata_json: jsonText<Record<string, unknown>>()("metadata_json"),
4548
created_at: text("created_at").notNull(),
4649
updated_at: text("updated_at").notNull()
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/**
2+
* The two conditional job queries the cross-instance cancel path depends on.
3+
*
4+
* Both exist to avoid a read-modify-write: a cancel arriving on an instance
5+
* that does not own the run races the owner's own terminal write, and a full
6+
* row `save()` built from a pre-race snapshot would overwrite whatever the
7+
* owner had just recorded.
8+
*/
9+
import { describe, it, expect, beforeEach } from "vitest";
10+
11+
import { initTestDb } from "../src/db.js";
12+
import { Job } from "../src/job.js";
13+
14+
const create = (id: string, status: string, userId = "1") =>
15+
Job.create({
16+
id,
17+
workflow_id: "wf",
18+
user_id: userId,
19+
status,
20+
params: {},
21+
graph: { nodes: [], edges: [] }
22+
});
23+
24+
describe("Job.markCancelledIfActive", () => {
25+
beforeEach(() => {
26+
initTestDb();
27+
});
28+
29+
it("cancels an active row and says so", async () => {
30+
await create("active", "running");
31+
32+
expect(await Job.markCancelledIfActive("active", "1")).toBe(true);
33+
34+
const row = await Job.get<Job>("active");
35+
expect(row?.status).toBe("cancelled");
36+
expect(row?.finished_at).toBeTruthy();
37+
});
38+
39+
it("leaves a terminal row untouched and says nothing changed", async () => {
40+
for (const status of ["completed", "failed", "cancelled"]) {
41+
await create(status, status);
42+
expect(await Job.markCancelledIfActive(status, "1")).toBe(false);
43+
expect((await Job.get<Job>(status))?.status).toBe(status);
44+
}
45+
});
46+
47+
it("preserves what the owner wrote when it loses the race", async () => {
48+
await create("raced", "running");
49+
50+
// The owner finishes first, cost and all.
51+
const owner = await Job.get<Job>("raced");
52+
owner!.markCompleted();
53+
owner!.cost = 0.42;
54+
await owner!.save();
55+
56+
expect(await Job.markCancelledIfActive("raced", "1")).toBe(false);
57+
58+
const row = await Job.get<Job>("raced");
59+
expect(row?.status).toBe("completed");
60+
expect(row?.cost).toBe(0.42);
61+
});
62+
63+
it("will not cancel another user's run", async () => {
64+
await create("theirs", "running", "2");
65+
66+
expect(await Job.markCancelledIfActive("theirs", "1")).toBe(false);
67+
expect((await Job.get<Job>("theirs"))?.status).toBe("running");
68+
});
69+
});
70+
71+
describe("Job.cancelledAmong", () => {
72+
beforeEach(() => {
73+
initTestDb();
74+
});
75+
76+
it("returns only the cancelled ids", async () => {
77+
await create("a", "running");
78+
await create("b", "cancelled");
79+
await create("c", "completed");
80+
81+
expect(await Job.cancelledAmong(["a", "b", "c", "missing"])).toEqual(["b"]);
82+
});
83+
84+
it("queries nothing for an empty list", async () => {
85+
expect(await Job.cancelledAmong([])).toEqual([]);
86+
});
87+
});

packages/models/tests/migrations.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ describe("MigrationRunner", () => {
423423
// ── Built-in migrations smoke test ───────────────────────────────────
424424

425425
describe("Built-in migrations", () => {
426-
const EXPECTED_BUILT_IN_MIGRATION_COUNT = 57;
426+
const EXPECTED_BUILT_IN_MIGRATION_COUNT = 58;
427427

428428
it("should have correct count of migrations", () => {
429429
expect(migrations.length).toBe(EXPECTED_BUILT_IN_MIGRATION_COUNT);
@@ -479,6 +479,11 @@ describe("Built-in migrations", () => {
479479
await adapter.columnExists("nodetool_messages", "provider_session")
480480
).toBe(true);
481481

482+
// The owning-instance stamp multi-machine routing reads.
483+
expect(
484+
await adapter.columnExists("nodetool_jobs", "runner_instance")
485+
).toBe(true);
486+
482487
// The trigger safety counters are added by migration onto a table an
483488
// earlier migration created.
484489
for (const column of [

0 commit comments

Comments
 (0)