Fix autoscale and disable-prefetch for green worker pools#10361
Fix autoscale and disable-prefetch for green worker pools#10361arkadiuszbach wants to merge 3 commits into
Conversation
a4217e8 to
ab7d9d3
Compare
Fix gevent and eventlet worker pools so autoscale grow/shrink keeps Celery's pool limit and the underlying green-pool capacity in sync. For gevent, report configured capacity from num_processes instead of active greenlets. This also fixes worker_disable_prefetch for gevent workers, which relies on pool.num_processes when deciding whether more tasks can be consumed. For both gevent and eventlet, wake spawn waiters when autoscale grows the pool instead of only changing semaphore counters.
ab7d9d3 to
798a7ca
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10361 +/- ##
==========================================
- Coverage 88.36% 88.34% -0.02%
==========================================
Files 153 153
Lines 19787 19793 +6
Branches 2296 2298 +2
==========================================
+ Hits 17484 17487 +3
- Misses 2004 2007 +3
Partials 299 299
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
auvipy
left a comment
auvipy
left a comment
There was a problem hiding this comment.
can you please check if the test coverage can be improved?
There was a problem hiding this comment.
Pull request overview
This PR fixes autoscale (and the worker_disable_prefetch decision that depends on it) for the gevent and eventlet worker pools, addressing the long-standing issue #4003. The autoscaler reads pool.num_processes as current capacity; for gevent this previously returned len(self._pool) (active greenlets) rather than the configured pool size, so scaling decisions were based on active work instead of capacity. The change also makes green-pool grow/shrink keep Celery's limit in sync with the pool size and, importantly, wake greenthreads/greenlets already blocked in spawn() when the pool grows, instead of only mutating semaphore counters.
Changes:
- gevent:
num_processesnow returnsself.limit;grow/shrinkkeeplimitand_pool.sizein sync andgrowreleases the semaphore per new slot to wake blocked spawns. - eventlet:
grow/shrinkreplace_pool.resize(...)with explicitsemcounter/release()handling plussize/limitsync so blockedspawn()waiters are notified on growth. - Tests: expanded gevent/eventlet unit tests for capacity/notify behavior, plus a real-eventlet integration test verifying
grow()wakes a blockedspawn()waiter.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| celery/concurrency/gevent.py | num_processes returns capacity; grow/shrink sync limit/size and release semaphore to wake waiters. |
| celery/concurrency/eventlet.py | grow/shrink replace resize with direct sem handling and release() to notify blocked waiters. |
| t/unit/concurrency/test_gevent.py | Updated test_pool and added tests for capacity, shrink, num_processes, and autoscaler scaling. |
| t/unit/concurrency/test_eventlet.py | Updated grow/shrink tests and added capacity, autoscaler, and a real-eventlet grow-wakes-waiter test. |
| assert len(testMap) == 0 | ||
|
|
||
|
|
||
| class test_TaskPool_eventlet: |
auvipy
left a comment
auvipy
left a comment
There was a problem hiding this comment.
should we also consider smoke tests or integration tests for this, beside inproving test coverage?
Summary
Fixes green worker pool capacity tracking for autoscale.
num_processesto report configured pool capacity instead of active greenlets.worker_disable_prefetchbehavior for gevent workers, since it relies onpool.num_processes.limitduring autoscale grow/shrink.spawn()waiters when gevent/eventlet pools grow, instead of only mutating semaphore counters.Why
The autoscaler uses
pool.num_processesas the current pool capacity. For gevent, this previously returnedlen(self._pool), which is the number of tracked/running greenlets rather than configured capacity. That caused autoscale decisions to be based on active work instead of the pool size.This resolves the long-standing autoscale failure reported in #4003 for gevent and eventlet workers. Prefork already scales correctly, but the green pools had stale/incorrect capacity accounting and did not wake blocked task submissions when autoscale increased the pool size.
worker_disable_prefetchalso relies onpool.num_processeswhen deciding whether the worker can consume more tasks. With gevent, the incorrect active-greenlet count could cause disable-prefetch to make the wrong consume/stop decision.Both gevent and eventlet pool growth adjusted semaphore counters directly. That updates capacity numerically, but it does not notify tasks already blocked waiting for a pool slot. The fix releases the relevant semaphore slots during grow so blocked submissions can continue immediately.