github-actions[bot] commented on code in PR #66985:
URL: https://github.com/apache/doris/pull/66985#discussion_r3837626053
##########
be/src/util/threadpool.cpp:
##########
@@ -521,13 +537,14 @@ Status ThreadPool::do_submit(std::shared_ptr<Runnable> r,
ThreadPoolToken* token
l.lock();
_num_threads_pending_start--;
if (_num_threads + _num_threads_pending_start == 0) {
- // If we have no threads, we can't do any work.
- return status;
+ _no_threads_cond.notify_all();
+ }
+ if (_pool_status.ok()) {
+ // The task was published only because another worker can
execute it.
+ DORIS_CHECK_GT(_num_threads + _num_threads_pending_start, 0);
Review Comment:
[P1] Keep an executor alive across concurrent max-thread shrink
This assertion is reachable for an accepted *additional*-worker submission.
With one active worker and max two, submitting task B increments
`_num_threads_pending_start`, publishes B, and calls `create_thread()` outside
`_lock`. A concurrent runtime reset to max one can then make the active worker
retire at the `live + pending > max` check, because it counts the presumed
replacement. If that creation subsequently fails, this path decrements pending
to zero and aborts here while B is still queued.
`ThreadPoolSimplifiedScanScheduler::reset_thread_num()` can race with scan
submissions, and the new test covers only a pool that starts with zero workers.
Please preserve a live executor (or transactionally roll back the accepted
task) across this resize/failure interleaving and add deterministic coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
