pool: park scaled-down workers on a queue that never drains

[mittalrishabh]

What is changed and how it works

WorkerThread::pop spin-pops the local queue and returns as soon as a task is ready. pop_or_sleep — the only place that compares the worker id against core_thread_count — is therefore never reached while the queue has work in it.

The result is that scaling in has no effect exactly when it is needed. Under a sustained backlog every worker keeps finding a task, so scale_workers(n) lowers core_thread_count but the surplus threads never park. We saw this on a TiKV unified read pool: the pool reported 7 core threads while measuring 8.8 cores of CPU, with 7k–12k tasks queued continuously.

This moves the check ahead of the spin-pop loop: a worker above core_thread_count pauses and parks straight away. Once parked, the existing logic in pop_or_sleep's validate callback keeps it asleep until it is back within the core count, and unpark_one's park-token filter already declines to wake it.

Custom queues are exempt. Only custom::LocalQueue::pop can return PopResult::Pending, and that deadline is observed on the spin-pop path, so scaled-down workers on a custom queue keep the old behaviour (test_scaled_down_pending_timeout_wakes_core_worker covers it). The SingleLevel/Multilevel/Priority queues convert through Option<Pop<T>> and can never produce Pending.

Check List

Tests

• Unit test: test_scaled_down_worker_parks_while_tasks_are_queued — a worker above core_thread_count facing 8 queued tasks must park with handle == 0. Without the fix it runs all 8.

Side effects

• None expected. Behaviour changes only for workers whose id is above core_thread_count, which are already meant to be idle.

Summary by CodeRabbit

• Bug Fixes

  • Improved worker scaling so surplus workers pause promptly when the system scales down, even while tasks remain queued.
  • Preserved timely processing for custom queues that rely on scheduled task deadlines.
  • Improved responsiveness during scale-in operations without interrupting eligible queued work.

• Tests

  • Added coverage verifying that scaled-down workers pause while queued tasks are still present.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 144aac8c-f9dd-4204-bd40-c3a0b4d58ce2

📥 Commits

Reviewing files that changed from the base of the PR and between 7dd6ef0 and ae6ed26.

📒 Files selected for processing (1)

• src/pool/worker.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The change adds queue deferral detection and uses it to park scaled-down workers before they pop tasks. A test verifies that a surplus worker parks while tasks remain queued.

Changes

Scaled-down worker parking

Layer / File(s)	Summary
Parking decision src/queue.rs, src/pool/spawn.rs	LocalQueue::may_defer identifies custom queues that can return Pending. Local::should_park_before_pop parks scaled-down workers only when the queue does not defer work.
Worker pop integration and validation src/pool/worker.rs	WorkerThread::pop parks qualifying workers before the spin-pop loop. The test verifies that a surplus worker handles no queued tasks.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to ae6ed

Scaled-down workers now stop taking ordinary queued work and park promptly, improving scale-in behavior. During a concurrent reduction, a worker may still begin one additional task after observing the previous worker limit, creating a bounded scheduling and resource-containment risk that should have explicit owner awareness or follow-up.

Sequence Diagram(s)

sequenceDiagram
  participant WorkerThread
  participant Local
  participant LocalQueue
  participant Runner
  WorkerThread->>Local: should_park_before_pop()
  Local->>LocalQueue: may_defer()
  Local-->>WorkerThread: Return park decision
  WorkerThread->>Runner: pause()
  WorkerThread->>WorkerThread: pop_or_sleep(None)
  WorkerThread->>Runner: resume()

Loading

Suggested reviewers: lcwangchao

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: parking scaled-down workers when a queue does not drain.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mittalrishabh]

/retest

[glorv]

rest LGTM