feat: add maxConcurrentRuns to worker (#162 step 3/4)#173
Conversation
Add maxConcurrentRuns option to createDurably() for parallel run processing. Worker maintains a slot-based concurrency pool: fills slots eagerly, refills immediately when work completes, and falls back to delayed polling when idle. - Default: 1 (sequential, no behavioral change for existing users) - Validated as positive integer - stop() uses Promise.allSettled to handle rejected in-flight runs - Concurrency key exclusion still enforced by storage layer - Docs, examples, and tests updated Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 40 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Worker as Worker (Orchestrator)
participant SlotFiller as Slot Filler (fillSlots)
participant Processor as Job Processor (processOne)
participant Storage as Storage
participant Idle as Idle Maintenance
participant Scheduler as Polling Scheduler
Client->>Worker: start()
activate Worker
Worker->>SlotFiller: fillSlots()
activate SlotFiller
loop while activeCount < maxConcurrentRuns
SlotFiller->>Processor: processOne()
activate Processor
Processor->>Storage: claim/process job
alt job found
Storage-->>Processor: jobData
Processor->>Storage: mark complete
else no job
Storage-->>Processor: false
end
deactivate Processor
SlotFiller->>SlotFiller: track activePromises/activeCount
end
deactivate SlotFiller
alt all slots idle and processOne returned false
Worker->>Idle: runIdleMaintenanceSafe()
Idle-->>Worker: onIdle done
Worker->>Scheduler: scheduleDelayedPoll(pollingIntervalMs)
else slot freed mid-run
SlotFiller->>Processor: claim next immediately
end
Client->>Worker: stop()
Worker->>Worker: await Promise.allSettled(activePromises + idleMaintenance)
deactivate Worker
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
website/public/llms.txt (1)
577-577: ブラウザ例の空行について確認してください。Line 577 で
pollingIntervalMs: 100,の後に空行が追加されていますが、Node.js 例(Line 63-64)ではmaxConcurrentRunsが追加されている一方、ブラウザ例では追加されていません。一貫性のため、ブラウザ例にもmaxConcurrentRunsを追加するか、空行を削除することを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/public/llms.txt` at line 577, ブラウザ例の設定で "pollingIntervalMs: 100," の後に空行があるため、Node.js 例(該当箇所で "maxConcurrentRuns" を追加している)と不一致になっています。ブラウザ例にも同じ構成プロパティ "maxConcurrentRuns" を追加して Node.js 例と揃えるか、不要であれば空行を削除して両例のフォーマットを一致させてください(参照する識別子: pollingIntervalMs, maxConcurrentRuns)。packages/durably/docs/llms.md (1)
577-577: ブラウザ例にmaxConcurrentRunsが欠落しています。Node.js 例では
maxConcurrentRuns: 1が追加されていますが、ブラウザ例では空行のみが追加されています。一貫性のため、ブラウザ例にも同様のオプションとコメントを追加することを検討してください。📝 提案される修正
const durably = createDurably({ dialect, pollingIntervalMs: 100, - + maxConcurrentRuns: 1, // Concurrent runs processed by the worker (default: 1) leaseRenewIntervalMs: 500,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/durably/docs/llms.md` at line 577, ブラウザ用の例に Node.js 例で追加されたオプションが欠けているため、ブラウザのサンプルに maxConcurrentRuns: 1 を追加し、Node.js と同じ説明コメント(例: "// Limit to one concurrent run for deterministic behavior" など)を入れて一貫性を保ってください; 対象となるサンプルはドキュメント内のブラウザ例の設定オブジェクト(参照: maxConcurrentRuns, ブラウザ例の初期化ブロック)を更新します。packages/durably/tests/shared/concurrency.shared.ts (1)
124-147: ローカルに作る Durably インスタンスもfinallyで teardown した方が安全です。この 3 ケースは末尾でしか
dp.stop()/d.db.destroy()を呼んでいないので、途中のwaitForやexpectが失敗すると polling timer と DB handle が残ります。try/finallyか suite-level cleanup に寄せておくと、後続テストへの波及を防げます。Also applies to: 164-203, 256-281
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/durably/tests/shared/concurrency.shared.ts` around lines 124 - 147, Wrap the Durably test setup and teardown in a try/finally (or use suite-level afterEach) so dp.stop() and d.db.destroy() always run even if assertions or waitFor fail: locate the creation calls (createDurably, d.migrate, d.register, dp.start) and ensure dp.stop() and d.db.destroy() are invoked in the finally block (or centralized cleanup) to prevent leftover polling timers/DB handles; apply the same change to the other two test blocks referenced (the blocks around lines 164-203 and 256-281).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/durably/src/durably.ts`:
- Around line 104-110: The validateMaxConcurrentRuns function currently uses
Number.isInteger which allows unsafe integers; change the validation in
validateMaxConcurrentRuns to use Number.isSafeInteger(value) and keep the >= 1
check (e.g., if (!Number.isSafeInteger(value) || value < 1) throw new
ValidationError('maxConcurrentRuns must be a positive safe integer')); update
the error message accordingly so callers and the worker loop (while (running &&
activeCount < maxConcurrentRuns)) cannot receive unsafe large integers that
would cause excessive synchronous Promise creation.
In `@packages/durably/src/worker.ts`:
- Around line 81-89: The current branch for "!didProcess && activeCount > 0"
never reschedules a poll, causing freed slots to be lost while long-running runs
remain; update the logic in the worker loop to call scheduleDelayedPoll (or
otherwise enqueue a poll) when didProcess is false but activeCount > 0 so empty
slots will trigger another poll; specifically modify the branch around
didProcess/activeCount/running (references: didProcess, activeCount, running,
runIdleMaintenanceSafe, scheduleDelayedPoll) to ensure scheduleDelayedPoll() is
invoked in the "!didProcess && activeCount > 0" case so available capacity is
reused.
- Around line 107-113: The promise returned by p.finally(...) must also be
handled to avoid an unhandled rejection when the original p rejects; update the
code that currently calls void p.finally(() => { activePromises.delete(p) }) so
that the finally-returned promise is captured and has a catch handler (e.g.,
chain .catch(...) on the result) or chain .catch(...) before .finally; reference
the promise variable p and the cleanup action activePromises.delete(p) (and the
surrounding processSlotCycle/processOne flow) when implementing the fix.
---
Nitpick comments:
In `@packages/durably/docs/llms.md`:
- Line 577: ブラウザ用の例に Node.js 例で追加されたオプションが欠けているため、ブラウザのサンプルに maxConcurrentRuns:
1 を追加し、Node.js と同じ説明コメント(例: "// Limit to one concurrent run for deterministic
behavior" など)を入れて一貫性を保ってください; 対象となるサンプルはドキュメント内のブラウザ例の設定オブジェクト(参照:
maxConcurrentRuns, ブラウザ例の初期化ブロック)を更新します。
In `@packages/durably/tests/shared/concurrency.shared.ts`:
- Around line 124-147: Wrap the Durably test setup and teardown in a try/finally
(or use suite-level afterEach) so dp.stop() and d.db.destroy() always run even
if assertions or waitFor fail: locate the creation calls (createDurably,
d.migrate, d.register, dp.start) and ensure dp.stop() and d.db.destroy() are
invoked in the finally block (or centralized cleanup) to prevent leftover
polling timers/DB handles; apply the same change to the other two test blocks
referenced (the blocks around lines 164-203 and 256-281).
In `@website/public/llms.txt`:
- Line 577: ブラウザ例の設定で "pollingIntervalMs: 100," の後に空行があるため、Node.js 例(該当箇所で
"maxConcurrentRuns" を追加している)と不一致になっています。ブラウザ例にも同じ構成プロパティ "maxConcurrentRuns"
を追加して Node.js 例と揃えるか、不要であれば空行を削除して両例のフォーマットを一致させてください(参照する識別子:
pollingIntervalMs, maxConcurrentRuns)。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a94c70b7-1174-4ec9-8c34-99cb7c8a61f9
📒 Files selected for processing (11)
packages/durably/docs/llms.mdpackages/durably/src/durably.tspackages/durably/src/worker.tspackages/durably/tests/node/types.test.tspackages/durably/tests/node/worker.test.tspackages/durably/tests/shared/concurrency.shared.tspackages/durably/tests/shared/durably.shared.tspackages/durably/tests/shared/worker.shared.tswebsite/api/create-durably.mdwebsite/api/index.mdwebsite/public/llms.txt
- Use Number.isSafeInteger instead of Number.isInteger to prevent unsafe large integers from causing excessive Promise creation - Fix lost slot bug: schedule delayed poll when !didProcess regardless of activeCount, so idle slots re-poll for new work while other slots are still running - Fix unhandled rejection: chain .finally().catch() instead of separate .catch() and .finally() on the same promise Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/durably/src/durably.ts`:
- Around line 96-110: The validateMaxConcurrentRuns function currently only
checks Number.isSafeInteger and >0, which allows impractically large values; add
an upper cap constant (e.g., MAX_CONCURRENT_RUNS = 1000) and update
validateMaxConcurrentRuns to throw if value > MAX_CONCURRENT_RUNS, and adjust
DEFAULTS.maxConcurrentRuns if needed to use a safe default under that cap;
reference validateMaxConcurrentRuns and DEFAULTS to locate and enforce the new
upper bound so worker.ts's fillSlots loop cannot allocate millions of slots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3725483-73d1-4cfd-9149-1125983116c5
📒 Files selected for processing (14)
examples/fullstack-react-router/app/lib/durably.server.tsexamples/fullstack-vercel-turso/app/lib/durably.server.tsexamples/server-node-postgres/lib/durably.tsexamples/server-node/lib/durably.tsexamples/spa-react-router/app/lib/durably.tsexamples/spa-vite-react/src/lib/durably.tspackages/durably-react/docs/llms.mdpackages/durably/src/durably.tspackages/durably/src/worker.tspackages/durably/tests/node/worker.test.tswebsite/api/durably-react/fullstack.mdwebsite/api/durably-react/spa.mdwebsite/guide/deployment.mdwebsite/public/llms.txt
✅ Files skipped from review due to trivial changes (4)
- examples/spa-react-router/app/lib/durably.ts
- website/guide/deployment.md
- website/public/llms.txt
- packages/durably-react/docs/llms.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ns removal Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ensures dp.stop() and d.db.destroy() always run even if assertions or waitFor timeout, preventing leftover polling timers and DB handles. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…afe integer checks Add structured checklist to spec-review covering: - Input validation: exact valid domain for numeric options (safe integer) - Concurrency safety AND liveness (idle slots keep polling, freed slots reused) - Detached promise rejection ownership (no unhandled rejections) - stop()/shutdown must use allSettled pattern - Behavioral regression criteria Sync matching items to spec-draft quality checklist. Learned from #173 CodeRabbit review: idle slot loss, unhandled rejection, and unsafe integer validation were missed at spec stage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…afe integer checks (#175) Add structured checklist to spec-review covering: - Input validation: exact valid domain for numeric options (safe integer) - Concurrency safety AND liveness (idle slots keep polling, freed slots reused) - Detached promise rejection ownership (no unhandled rejections) - stop()/shutdown must use allSettled pattern - Behavioral regression criteria Sync matching items to spec-draft quality checklist. Learned from #173 CodeRabbit review: idle slot loss, unhandled rejection, and unsafe integer validation were missed at spec stage. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
maxConcurrentRunsoption tocreateDurably()for parallel run processing1(sequential — no behavioral change for existing users)stop()usesPromise.allSettledto gracefully handle rejected in-flight runsChanges
worker.tsfillSlots()→processSlotCycle()→ refill/idledurably.tsmaxConcurrentRunsinDurablyOptions, validated as positive integerworker.test.tsworker.shared.ts,concurrency.shared.ts,durably.shared.tsllms.md,website/api/,website/guide/, examplesContext
Step 3/4 of #162 (core internal restructuring). Storage layer already supports concurrent claims (
SKIP LOCKED,lease_generation,concurrency_key), so the change is isolated to the worker polling loop.Test plan
pnpm validatepasses (format, lint, typecheck, tests)🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
改善
テスト
ドキュメント