morningman commented on PR #68713:
URL: https://github.com/apache/doris/pull/68713#issuecomment-6080975144
@Gabriel39 Thanks — confirmed, and it is reachable earlier than either of us
described. Fixed in cddb40bec0b: no thread waits at the gate any more.
**Where it bites.** The binding limit is not the pool's thread cap
(`max(512, 10 x cores)`, which my reply to the bot leaned on) but the context's
admission: once a workload group's remote scan scheduler has
`min_active_file_scan_threads` (8 x cores by default) tasks active or queued —
and a thread blocked in `acquire()` counts as active — each context is held to
`_min_scan_concurrency` (1 for file scans) in flight
(`ScannerContext::_get_margin`, `can_admit_scan_task`). A holder whose block
the operator consumed then goes back to `_pending_tasks` while its context
still has a waiter in flight, and nothing reconsiders it until that waiter
completes, which needs the holder's share. On a 16-core BE, nine concurrent
`SELECT *` over a fluss primary-key table of 16 declaring buckets get there;
the waits then run out together and every waiter opens above the budget.
**What changed.**
- `JniScanHeapGate::request(bytes, stop_waiting)` returns an `Admission` at
once: admitted when the reader fits, stopped when its scan already stopped,
otherwise waiting with a `SharedListenableFuture<Void>`. The gate has a thread
of its own that completes those futures — never inline on the thread that gave
a share back, which may hold its context's lock (`stop_scanners()` closing
scanners) — and every 100 ms polls stopped scans (`IOContext::should_stop`, set
by `try_stop()`), the wait limit and a changed budget.
- A JNI reader that has to wait opens nothing and reports it through
`waiting_for()` (`TableReader::waiting_for()`, forwarded by the hybrid readers;
V1 `JniReader` likewise). `FileScannerV2` / `FileScanner` end the turn without
a block, and `_scanner_scan` parks the task: `ScanTask::State::PARKED`, out of
`_in_flight_tasks_num`, so it holds neither a worker nor a slot, and the
context schedules again since that slot may now admit a pending holder. When
the future is done the task is scheduled like a task whose block was just
consumed (`schedule_scan_task(ctx, task, lock)`), on either scheduler, and the
reader then opens its Java scanner — or ends its split if the scan stopped
first.
**Tests.**
- The scheduler interaction you asked for:
`ScannerContextTest.{thread_pool,task_executor}_heap_waiters_park_and_leave_the_worker_to_the_holder`
run three scanners that each declare the whole budget on a one-worker pool,
and `thread_pool_heap_waiters_leave_the_busy_pools_slot_to_the_holder` holds
the context to one task in flight after the waiters have parked. Every scanner
finishes, with `admitted_after_wait_limit() == 0` and the wait limit at 10
minutes; with a blocking wait each of them would have stalled until it.
`thread_pool_stopping_a_context_ends_its_parked_tasks` covers the other way
out: a context stopped while its scanners wait sees them leave the line without
a share, and nothing runs again.
- The burst:
`JniScanHeapGateTest.ReadersThatWaitedTooLongTogetherAreAllAdmittedAboveTheBudget`
pins it down (four waiters past the limit are admitted together, 1300 MB on a
500 MB budget), and `FuturesAreCompletedOnTheGatesOwnThread` the threading rule
above.
- 269 BE tests in 31 suites pass (scan scheduling, both file scanners, the
JNI readers, the paimon/fluss/hudi readers, the task executor); the new cases
also 20 times, shuffled. A `JniTableReaderTest` pair checks the reader side: a
split that does not fit returns from `prepare_split()` without touching the
JVM, and one whose scan stops while it waits opens nothing.
**Why the wait limit stays.** Parking removes the dependency on workers and
on the context's slot, but not one on a consumer: in a join whose two sides
both declare, the probe side's scanners take shares, produce a block each and
keep the shares while the probe waits for the build, and the build side's later
splits queue behind them. Only the limit ends that, so "without timeout
admission" holds for the worker/slot case your test describes, not for this
one. The config now says the limit is per split, and that the wait holds no
thread.
I'm also reproducing it end to end on a local BE - the old and the new build
with `min_active_file_scan_threads` lowered - and will add the numbers here.
The parked state is new to the scan scheduler, so I'd value your eyes on
`ScannerContext::park_scan_task` / `_resume_parked_task` in particular.
--
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]
