feat(spider-scheduler): Reschedule task assignments returned by lost execution managers. - #395
Merged
LinZhihao-723 merged 4 commits intoJul 14, 2026
Conversation
Contributor
Walkthrough
ChangesScheduler assignment rescheduling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ExecutionManager
participant RescheduleQueue
participant RoundRobin
participant DispatchQueue
ExecutionManager->>RescheduleQueue: return TaskAssignment
RescheduleQueue->>RoundRobin: provide assignment on tick
RoundRobin->>RoundRobin: filter session and classify task
RoundRobin->>DispatchQueue: enqueue assignment for dispatch
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
LinZhihao-723
marked this pull request as ready for review
July 13, 2026 20:21
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/spider-scheduler/src/core_impl/round_robin/implementation.rs (2)
540-542: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging dropped stale-session assignments.
Stale rescheduled assignments are discarded silently here, whereas comparable ignore paths (e.g.
enqueue_ready_entriesfor finalizing jobs) emit a trace. Since rescheduling is a lost-EM recovery path, adebug/traceline recording the droppedjob_id/task_idand both session IDs would aid incident diagnosis without changing behaviour.🤖 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 `@components/spider-scheduler/src/core_impl/round_robin/implementation.rs` around lines 540 - 542, Add a debug or trace log before the continue in the stale-assignment branch, recording the dropped assignment’s job_id, task_id, assignment.session_id, and self.storage_session_id. Preserve the existing discard behavior and follow the logging style used by enqueue_ready_entries.
207-207: 🩺 Stability & Availability | 🔵 TrivialUnbounded reschedule queue: consider backpressure/monitoring. The
reschedule()snapshot bounds per-tick work, but the channel itself is unbounded, so sustained EM loss (or a redispatch path that keeps re-queuing) can grow this queue without limit and pressure scheduler memory. Consider emitting a gauge/metric for the queue depth and/or an alert threshold so operators can detect a runaway reschedule backlog before it degrades the scheduler.🤖 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 `@components/spider-scheduler/src/core_impl/round_robin/implementation.rs` at line 207, Instrument the unbounded reschedule queue represented by reschedule_queue_reader with a queue-depth gauge, updating it whenever items are enqueued or drained; add an alert or threshold for sustained excessive depth so operators can detect runaway backlog. Preserve the existing per-tick reschedule() work bound and scheduling behavior.
🤖 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 `@components/spider-scheduler/Cargo.toml`:
- Line 34: Update the rand dependency declaration in Cargo.toml from 0.9.1 to
version 0.9.3 or later, ensuring the resolved dependency is outside the advisory
range.
---
Nitpick comments:
In `@components/spider-scheduler/src/core_impl/round_robin/implementation.rs`:
- Around line 540-542: Add a debug or trace log before the continue in the
stale-assignment branch, recording the dropped assignment’s job_id, task_id,
assignment.session_id, and self.storage_session_id. Preserve the existing
discard behavior and follow the logging style used by enqueue_ready_entries.
- Line 207: Instrument the unbounded reschedule queue represented by
reschedule_queue_reader with a queue-depth gauge, updating it whenever items are
enqueued or drained; add an alert or threshold for sustained excessive depth so
operators can detect runaway backlog. Preserve the existing per-tick
reschedule() work bound and scheduling behavior.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 99e834d3-c561-43d4-a47d-b55a06fd12a3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
components/spider-core/src/types/scheduler.rscomponents/spider-execution-manager/src/runtime.rscomponents/spider-proto-rust/src/assignment.rscomponents/spider-scheduler/Cargo.tomlcomponents/spider-scheduler/src/core.rscomponents/spider-scheduler/src/core_impl/round_robin/implementation.rscomponents/spider-scheduler/src/core_impl/round_robin/tests.rscomponents/spider-scheduler/src/dispatch_queue.rscomponents/spider-scheduler/src/execution_manager_registry.rscomponents/spider-scheduler/src/grpc.rscomponents/spider-scheduler/src/runtime.rscomponents/spider-scheduler/src/service.rstests/huntsman/em-runtime/tests/test_runtime.rs
| [dev-dependencies] | ||
| anyhow = "1.0.102" | ||
| dashmap = "6.1.0" | ||
| rand = "0.9.1" |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if rand 0.9.1 exists on crates.io
curl -s https://crates.io/api/v1/crates/rand/0.9.1 | jq '.version.num, .version.yanked'
# Check for security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: RUST, package: "rand") {
nodes {
advisory { summary severity publishedAt }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}'Repository: y-scope/spider
Length of output: 880
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Raw crates.io response for rand 0.9.1:"
curl -s https://crates.io/api/v1/crates/rand/0.9.1 | jq .
echo
echo "Searching published rand versions for 0.9.1:"
curl -s "https://crates.io/api/v1/crates/rand" | jq -r '.versions[].num' | grep -x '0.9.1' || trueRepository: y-scope/spider
Length of output: 599
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Trying crates.io sparse index entry for rand:"
curl -s https://index.crates.io/ra/nd/rand | sed -n '1,120p'Repository: y-scope/spider
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
curl -s https://index.crates.io/ra/nd/rand | grep -m1 '"vers":"0.9.1"' || trueRepository: y-scope/spider
Length of output: 1542
Bump rand to 0.9.3 or later rand 0.9.1 is published, but it falls in the advisory range >= 0.9.0, < 0.9.3. components/spider-scheduler/Cargo.toml:34
🤖 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 `@components/spider-scheduler/Cargo.toml` at line 34, Update the rand
dependency declaration in Cargo.toml from 0.9.1 to version 0.9.3 or later,
ensuring the resolved dependency is outside the advisory range.
sitaowang1998
approved these changes
Jul 14, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When an execution manager is lost, the execution-manager registry already pushes that manager's outstanding task assignments onto the scheduler's reschedule queue, but nothing consumed that queue: its reader was parked, unused, on the
Runtime. This PR wires the reschedule queue's reader into the scheduler core and implements reschedule ingestion in the round-robin core, so a lost manager's assignments are re-placed and re-dispatched instead of being silently dropped.The enabling change is that the storage session is moved onto each
TaskAssignment. A rescheduled assignment travels the reschedule queue as a bareTaskAssignment, with no session paired alongside it, so it could not otherwise be checked against the scheduler's current session. Carryingsession_idon the assignment makes it self-describing, which both lets the core drop assignments from a superseded session and collapses the several places that previously tracked the session next to the assignment down to a single source of truth.Wire the reschedule queue reader into the scheduler core (
core.rs,runtime.rs)SchedulerCore::rungains areschedule_queue_reader: tokio::sync::mpsc::UnboundedReceiver<TaskAssignment>parameter, threaded throughRoundRobinCoreintoRoundRobin.create_runtimenow hands the reschedule queue's receiver to the spawned core instead of parking it onRuntime. The execution-manager registry continues to hold the sender and pushes a dead manager's outstanding assignments onto it.Carry the storage session on each
TaskAssignment(spider-core, dispatch queue, proto, execution manager)session_id: SessionIdtoTaskAssignment(the scheduler's view of storage's session when the assignment was produced), and removed the now-redundantSchedulerResponse::session_idso the assignment is the single source of truth.DispatchQueueSource::dequeuenow returns a bareTaskAssignmentinstead of a(SessionId, TaskAssignment)pair;SchedulerServiceState::next_taskandmake_next_task_responsefollow suit. The dispatch-queue reader keeps its session read-guard, so the queue's drain-and-invalidate behavior onbump_session_idis unchanged.SchedulerAssignmentalready carriessession_id, so only thespider-proto-rustconversion changes to route it intotask_assignment.session_id. The execution manager now readsresponse.task_assignment.session_id.Reschedule ingestion in the round-robin core (
implementation.rs)tick()now runs a newreschedule()step after consuming the inbound poll and before making scheduling decisions, so rescheduled tasks are filtered against the up-to-date session and dispatched in the same tick.reschedule()drains a bounded snapshot of the reschedule queue (assignments pushed concurrently by the registry during the drain are deferred to the next tick rather than extending the loop), drops any assignment from a superseded session (session_id < storage_session_id), groups the survivors by task kind, and loads them through the same commit-ready / cleanup-ready / ready enqueue helpers as the inbound path — so finalizing semantics and buffered-task de-duplication apply identically. Re-dispatched assignments receive a fresh id and the current session.storage_session_id.Tests (
round_robin/tests.rs)tick()-driven tests: a rescheduled ready task is re-dispatched with the current session and a fresh id; a stale-session assignment is dropped; and a rescheduled task is de-duplicated against a concurrently-buffered inbound copy of the same task.run()loop, randomly pushes a subset of dispatched assignments back onto the reschedule queue (each task at most once, using a realrandsource), and asserts that every task eventually completes and that each rescheduled task is dispatched exactly twice.randas a dev-dependency ofspider-scheduler.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
New Features
Improvements
Bug Fixes