Repository navigation
Add an env var to schedule the batch worker ping - #269
Conversation
commit: |
cab9b72 to
05abdfd
Compare
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="README.md">
<violation number="1" location="README.md:810">
P2: When callers use `enqueueBatch`, this setting schedules one ping for the entire batch, not one scheduled function per queued operation. Describe the cost per non-empty enqueue call so scheduler-load guidance matches the implementation.</violation>
</file>
<file name="src/component/worker.ts">
<violation number="1" location="src/component/worker.ts:95">
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.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| await ctx.db.insert("pendingOperations", entry); | ||
| } | ||
| if (env.WORKER_SCHEDULE_PING === "true") { | ||
| await ctx.scheduler.runAfter(0, internal.worker.pingWorker, {}); |
There was a problem hiding this comment.
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>
| 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, {}); | |
| } |
There was a problem hiding this comment.
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.
|
|
||
| Queuing a write then touches the worker component not at all — no read to | ||
| conflict on and nothing written — and the ping runs in its own transaction | ||
| afterwards. The cost is one scheduled function per queued write, whether or not |
There was a problem hiding this comment.
P2: When callers use enqueueBatch, this setting schedules one ping for the entire batch, not one scheduled function per queued operation. Describe the cost per non-empty enqueue call so scheduler-load guidance matches the implementation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 810:
<comment>When callers use `enqueueBatch`, this setting schedules one ping for the entire batch, not one scheduled function per queued operation. Describe the cost per non-empty enqueue call so scheduler-load guidance matches the implementation.</comment>
<file context>
@@ -805,12 +805,11 @@ app.use(aggregate, {
-reach for this when it won't.
+Queuing a write then touches the worker component not at all — no read to
+conflict on and nothing written — and the ping runs in its own transaction
+afterwards. The cost is one scheduled function per queued write, whether or not
+the worker needed waking, so this trades contention for scheduler load. Prefer a
+longer cooldown where that will do, and reach for this when it won't.
</file context>
| afterwards. The cost is one scheduled function per queued write, whether or not | |
| afterwards. The cost is one scheduled function per non-empty enqueue call, whether or not |
There was a problem hiding this comment.
2 existing issues remain and no new issues found across 5 files
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
be65f9e to
2b826d8
Compare
c59f3a0 to
f366aac
Compare
2b826d8 to
54d147e
Compare
f366aac to
78abeb5
Compare
78abeb5 to
a31c1ef
Compare
Queuing a write pings the batch worker from inside the queuing mutation, which reads the record holding the worker's run state and, when the worker is parked, writes it too. A burst arriving at a parked worker therefore has every mutation in it writing one record, and all but one losing the race. Under `WORKER_SCHEDULE_PING`, the ping is scheduled instead, so queuing a write touches the worker component not at all: no read to conflict on and nothing written. The scheduled ping runs after the mutation commits and does the same work it always did, so nothing can be stranded by racing the worker as it parks. Over a bursty workload this takes the reads made from queuing mutations from 31 to 0 and the writes from 6 to 0, leaving the same 12 writes in the worker's own transactions. Off by default: it costs one scheduled function per queued write, whether or not the worker needed waking, which is a poor trade for a deployment that can just poll for longer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
a31c1ef to
d2a4416
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Change: Feature Merge Risk: ⚪ Minimal · up to
|
Uh oh!
There was an error while loading. Please reload this page.