github-actions[bot] commented on code in PR #68610:
URL: https://github.com/apache/doris/pull/68610#discussion_r4130378110
##########
be/src/util/threadpool.cpp:
##########
@@ -521,8 +528,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;
+ // shutdown() may be waiting for the last pending thread to go
away.
+ _no_threads_cond.notify_all();
+ // If we have no threads, we can't do any work. Callers treat
a failed submit as
+ // a task that will never run (for example, an RPC handler
completes its closure
+ // itself), so withdraw the task queued above before returning
the error.
+ LOG(WARNING) << "Thread pool " << _name
Review Comment:
[P1] Keep accepted tasks runnable when concurrent worker starts fail. With
zero workers and at least two allowed threads, submit A queues a task and
reserves a start, then B does the same. If A's `create_thread()` fails first,
B's pending start makes A skip this withdrawal and return OK. If B also fails,
B withdraws only its own task, leaving A queued with no worker or pending
start. The default scan scheduler can then leave a Context asleep, and
`multiget_data_v2` can leave its accepted RPC closure uncalled. Please handle
all queued accepted tasks when the final start fails (or start a replacement
worker), and test overlapping failures. This is distinct from the existing
thread about an error return retaining the failed submit's own callback.
##########
be/src/exec/scan/scanner.cpp:
##########
@@ -88,6 +88,20 @@ Status Scanner::init(RuntimeState* state, const
VExprContextSPtrs& conjuncts) {
}
Status Scanner::get_block_after_projects(RuntimeState* state, Block* block,
bool* eos) {
+ RETURN_IF_ERROR(_get_block_after_projects(state, block, eos));
+ // Publish progress to the shared counter so peer scanners can observe it.
Only rows that leave
+ // the scanner are charged: rows still held in _padding_block are not
charged, because once
+ // the counter is exhausted the context may finish without running this
scanner again. The
+ // counter may go negative when several scanners subtract concurrently;
that is harmless
+ // because the operator's reached_limit() makes the final cut.
+ if (_shared_scan_limit && block->rows() > 0) {
Review Comment:
[P1] Bound reads while projected rows wait in padding. With a projected
`LIMIT 2` and two scanners over selective predicates, each scanner can buffer
one matching row, then read a long filtered tail. Both rows sit below the
half-batch padding threshold, so this new charge never runs; the shared counter
remains 2, and each scanner's private row count remains below its limit of 2.
The padding loop can therefore scan the rest of both ranges before returning
the already sufficient rows. Before this move, `get_block()` charged the
matching rows and the next read observed exhaustion. Please flush or expose
small-LIMIT padding progress without counting rows that might be dropped, and
test a long filtered tail. This is a read-amplification case distinct from the
existing missing-row thread.
--
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]
