feat: add opt-in CoDel load shedding#450
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughAdds opt-in per-queue Classic CoDel load shedding to the scheduler, routes shed jobs to the DLQ with ChangesCoDel load shedding
Runtime configuration wiring
Java SDK configuration
Node and Python SDK configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SDK
participant Worker
participant Scheduler
participant CodelState
participant DLQ
SDK->>Worker: provide per-queue CoDel settings
Worker->>Scheduler: register_queue_codel
Scheduler->>CodelState: evaluate job sojourn time before dispatch
CodelState-->>Scheduler: return drop decision
Scheduler->>DLQ: move shed job with codel: reason
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/taskito-core/src/scheduler/maintenance.rs (1)
313-335: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPrevent CoDel entries from starving ordinary DLQ retries.
The 50-row candidate limit is applied before this filter. If the selected window contains only permanent
codel:entries, every sweep skips the same rows and eligible non-CoDel entries behind them are never retried. Filter CoDel rows inlist_dead_for_retry, or page past skipped rows until retryable candidates are found; add a regression test with 50 CoDel entries followed by a normal retryable entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/taskito-core/src/scheduler/maintenance.rs` around lines 313 - 335, The retry sweep in the maintenance flow must not let skipped CoDel entries consume the fixed candidate window. Update list_dead_for_retry or the surrounding pagination logic to exclude codel: entries before applying the 50-row limit, or continue fetching pages until retryable candidates are collected; preserve existing retry behavior and add a regression test covering 50 CoDel entries followed by one retryable entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java`:
- Around line 1112-1113: The worker currently captures queue configurations when
the builder is created instead of when it starts. In DefaultTaskito, pass a
late-bound queue-config source; update Worker’s construction and
option-serialization flow to retain that source and resolve the latest
configurations immediately before serialization, and add the CodelTest
regression covering builder creation before Taskito.codel(...) configuration
followed by startup and shedding verification.
In `@sdks/node/src/types.ts`:
- Around line 176-182: Update Queue.configureQueue to validate codel.targetMs
and codel.intervalMs before forwarding them to native code, requiring each value
to be a positive finite integer and rejecting zero, negatives, fractions, NaN,
and infinities. Reuse the existing queue option validation pattern and add Node
tests covering invalid CoDel inputs.
---
Outside diff comments:
In `@crates/taskito-core/src/scheduler/maintenance.rs`:
- Around line 313-335: The retry sweep in the maintenance flow must not let
skipped CoDel entries consume the fixed candidate window. Update
list_dead_for_retry or the surrounding pagination logic to exclude codel:
entries before applying the 50-row limit, or continue fetching pages until
retryable candidates are collected; preserve existing retry behavior and add a
regression test covering 50 CoDel entries followed by one retryable entry.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8b809ad-6c81-4113-aee2-57ea11760ca4
📒 Files selected for processing (20)
crates/taskito-core/src/scheduler/codel.rscrates/taskito-core/src/scheduler/maintenance.rscrates/taskito-core/src/scheduler/mod.rscrates/taskito-core/src/scheduler/poller.rscrates/taskito-java/src/convert.rscrates/taskito-java/src/worker.rscrates/taskito-node/src/config.rscrates/taskito-node/src/convert/mod.rscrates/taskito-node/src/convert/task_config.rscrates/taskito-node/src/worker.rscrates/taskito-python/src/py_queue/worker.rssdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.javasdks/java/src/main/java/org/byteveda/taskito/Taskito.javasdks/java/src/main/java/org/byteveda/taskito/worker/Worker.javasdks/java/src/test/java/org/byteveda/taskito/core/CodelTest.javasdks/node/src/types.tssdks/node/src/worker.tssdks/node/test/core/codel.test.tssdks/python/taskito/mixins/runtime_config.pysdks/python/tests/core/test_codel.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdks/node/src/queue.ts`:
- Around line 397-404: Update the limits validation and storage flow around the
codel checks to snapshot the configuration after validation. Store a
shallow-copied limits object and a separately copied codel object so later
caller mutations to targetMs or intervalMs cannot affect runWorker().
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69a851df-3d17-4858-9b29-66691c9fd999
📒 Files selected for processing (5)
sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.javasdks/java/src/main/java/org/byteveda/taskito/worker/Worker.javasdks/java/src/test/java/org/byteveda/taskito/core/CodelTest.javasdks/node/src/queue.tssdks/node/test/core/codel.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java
1 participant
Summary
Opt-in CoDel (Controlled Delay, RFC 8289) load shedding per queue (S27, Tier 4). Under sustained overload — a job's wait past its eligibility staying above
targetfor a fullinterval— the dispatcher sheds the stalest jobs to the DLQ instead of running them stale. A transient spike is never shed; that's the whole point of CoDel over a fixed max-age cutoff. Off by default.Design
scheduler/codel.rs— the CoDel controller (should_drop,interval / sqrt(count)control law), unit-tested with explicit time (no wall clock).codel_admit): the job is stillPending, somove_to_dlqis uniform across backends and leaves no dangling execution claim. Sojourn is measured fromscheduled_at, so an intentional delay is never counted as staleness.codel:; the DLQ auto-retry sweep skips those entries so dropped jobs aren't resurrected.QueueConfig(its owncodel_configsmap +register_queue_codel) so the common no-CoDel path pays nothing.Cross-SDK surface
set_queue_codel(queue, target_ms, interval_ms).configureQueue(q, { codel: { targetMs, intervalMs } }).Taskito.codel(queue, targetMs, intervalMs)(adds the per-queue config path to the Java worker options).Tests
Summary by CodeRabbit
codel:-prefixed reason.