Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

- Add experimental configuration options for configuring the Aggregate's batch
worker: `WORKER_IDLE_COOLDOWN_MS`, `WORKER_POLL_INTERVAL_MS`, and
`WORKER_SCHEDULE_PING`.

## 0.3.3

- Bump `convex-helpers` to 0.1.128, which isn’t affected by
Expand Down
1 change: 1 addition & 0 deletions src/component/_generated/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type Env = {
readonly CONVEX_SITE_URL: string;
readonly WORKER_IDLE_COOLDOWN_MS: string | undefined;
readonly WORKER_POLL_INTERVAL_MS: string | undefined;
readonly WORKER_SCHEDULE_PING: "true" | "false" | undefined;
};

/**
Expand Down
3 changes: 3 additions & 0 deletions src/component/convex.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ const component = defineComponent("aggregate", {
env: {
WORKER_IDLE_COOLDOWN_MS: v.optional(v.string()),
WORKER_POLL_INTERVAL_MS: v.optional(v.string()),
WORKER_SCHEDULE_PING: v.optional(
v.union(v.literal("true"), v.literal("false")),
),
},
});
component.use(batchWorker);
Expand Down
102 changes: 102 additions & 0 deletions src/component/worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,108 @@ describe("staying awake between bursts", () => {
});
});

describe("scheduling the ping", () => {
afterEach(() => {
vi.unstubAllEnvs();
});

// Pending only: convex-test keeps completed runs in the table too.
async function pendingPings(t: TestConvex<typeof schema>) {
return await t.run(async (ctx) => {
const scheduled = await ctx.db.system
.query("_scheduled_functions")
.collect();
return scheduled.filter(
(f) => f.name.includes("pingWorker") && f.state.kind === "pending",
);
});
}

test("the ping is inline unless asked for", async () => {
const t = initConvexTest();
await enqueue(t, { type: "insert", key: 1, value: "a" });
expect(await pendingPings(t)).toHaveLength(0);
expect(await workerStatus(t)).toEqual({ kind: "running" });
});

test("a parked worker is woken from its own transaction", async () => {
vi.stubEnv("WORKER_SCHEDULE_PING", "true");
const t = initConvexTest();
await enqueue(t, { type: "insert", key: 1, value: "a" });
// The enqueue left the worker alone; the scheduled ping registers it.
expect(await workerStatus(t)).toBeNull();
expect(await pendingPings(t)).toHaveLength(1);

await drainViaWorker(t);
await t.run(async (ctx) => {
expect(await getHandler(ctx, { key: 1 })).toEqual({
k: 1,
v: "a",
s: 0,
});
expect(await ctx.db.query("pendingOperations").collect()).toEqual([]);
});
});

test("each transaction schedules one, and it no-ops on a running worker", async () => {
vi.stubEnv("WORKER_SCHEDULE_PING", "true");
const t = initConvexTest();
await enqueue(t, { type: "insert", key: 1, value: "a" });
vi.advanceTimersByTime(1);
await t.finishInProgressScheduledFunctions();
expect(await workerStatus(t)).toEqual({ kind: "running" });

await enqueue(t, { type: "insert", key: 2, value: "b" });
expect(await pendingPings(t)).toHaveLength(1);
await drainViaWorker(t);
await t.run(async (ctx) => {
expect(await getHandler(ctx, { key: 2 })).toEqual({
k: 2,
v: "b",
s: 0,
});
});
});

test("a transaction that enqueues repeatedly schedules one ping", async () => {
vi.stubEnv("WORKER_SCHEDULE_PING", "true");
const t = initConvexTest();
await enqueue(
t,
{ type: "insert", key: 1, value: "a" },
{ type: "insert", key: 2, value: "b" },
{ type: "insert", key: 3, value: "c" },
);
expect(await pendingPings(t)).toHaveLength(1);

await drainViaWorker(t);
await t.run(async (ctx) => {
expect(await getHandler(ctx, { key: 3 })).toEqual({ k: 3, v: "c", s: 0 });
expect(await ctx.db.query("pendingOperations").collect()).toEqual([]);
});
});

test("work enqueued after the worker parks still drains", async () => {
vi.stubEnv("WORKER_SCHEDULE_PING", "true");
const t = initConvexTest();
await enqueue(t, { type: "insert", key: 1, value: "a" });
await drainViaWorker(t);
expect(await workerStatus(t)).toEqual({ kind: "idle" });

await enqueue(t, { type: "insert", key: 2, value: "b" });
expect(await pendingPings(t)).toHaveLength(1);
await drainViaWorker(t);
await t.run(async (ctx) => {
expect(await getHandler(ctx, { key: 2 })).toEqual({
k: 2,
v: "b",
s: 0,
});
expect(await ctx.db.query("pendingOperations").collect()).toEqual([]);
});
});
});

describe("pinging the worker", () => {
beforeEach(() => {
vi.mocked(ping).mockClear();
Expand Down
16 changes: 16 additions & 0 deletions src/component/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ export async function enqueueOperations(
if (newestEntry) {
return;
}
if (env.WORKER_SCHEDULE_PING === "true") {
await ctx.scheduler.runAfter(0, internal.worker.pingWorker, {});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When the worker is in its final empty cycle, this scheduled ping can see running and no-op before the park commits. Since the enqueue no longer reads worker status, it cannot retry and schedule another ping, so the pending row can remain stranded; preserve the status dependency or make ping/parking coordination retry-safe.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/component/worker.ts, line 95:

<comment>When the worker is in its final empty cycle, this scheduled ping can see `running` and no-op before the park commits. Since the enqueue no longer reads worker status, it cannot retry and schedule another ping, so the pending row can remain stranded; preserve the status dependency or make ping/parking coordination retry-safe.</comment>

<file context>
@@ -92,16 +92,7 @@ export async function enqueueOperations(
-    if (!status || status.kind === "idle") {
-      await ctx.scheduler.runAfter(0, internal.worker.pingWorker, {});
-    }
+    await ctx.scheduler.runAfter(0, internal.worker.pingWorker, {});
     return;
   }
</file context>
Suggested change
await ctx.scheduler.runAfter(0, internal.worker.pingWorker, {});
const status = await ctx.runQuery(components.batchWorker.lib.status, {
name: OPS_WORKER_NAME,
});
if (!status || status.kind === "idle") {
await ctx.scheduler.runAfter(0, internal.worker.pingWorker, {});
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case the worker cycle should retry, as it takes a read dependency on pendingUpdates if the snapshot query returned empty. When we enqueue operations, that will cause a conflict.

return;
}
await pingBatchWorker(ctx);
}

async function pingBatchWorker(ctx: MutationCtx) {
await ping(ctx, components.batchWorker, {
// TODO: explore separate queues by namespace
name: OPS_WORKER_NAME,
Expand All @@ -103,6 +111,14 @@ export async function enqueueOperations(
});
}

export const pingWorker = internalMutation({
args: {},
returns: v.null(),
handler: async (ctx) => {
await pingBatchWorker(ctx);
},
});

const vBatchEntry = v.object({
id: v.id("pendingOperations"),
commitTs: v.int64(),
Expand Down