Gabriel39 commented on PR #68713: URL: https://github.com/apache/doris/pull/68713#issuecomment-6016448398
Concurrency concern at `99e33c47bd19fe75e8477231f74e9dabfaf1c853`: **blocking heap admission can starve the scan workers needed by readers that already hold permits.** `JniTableReader::_open_jni_scanner()` synchronously calls the gate before opening the Java scanner ([call site](https://github.com/apache/doris/blob/99e33c47bd19fe75e8477231f74e9dabfaf1c853/be/src/format_v2/jni/jni_table_reader.cpp#L464-L511)). The condition-variable wait releases the gate mutex, but it still occupies the calling scan worker. Meanwhile, a permit remains held across scan attempts until the reader closes: producing a block does not release it. The scanner publishes a completed task, and consuming that block schedules its next attempt ([execution](https://github.com/apache/doris/blob/99e33c47bd19fe75e8477231f74e9dabfaf1c853/be/src/exec/scan/scanner_scheduler.cpp#L244-L333), [rescheduling](https://github.com/apache/doris/blob/99e33c47bd19fe75e8477231f74e9dabfaf1c853/be/src/exec/scan/scanner_context.cpp#L385-L425)). A possible interleaving with a saturated, finite scan pool is: 1. Reader A holds a permit, produces a non-final block, and yields its worker. 2. Readers that cannot acquire heap permits occupy all available workers inside `acquire()`. 3. A's continuation is queued on the same pool. It cannot finish and release its permit because no worker is available; the workers are waiting for permits to be released. This is a **resource-dependency / thread-pool starvation risk**, not a confirmed permanent mutex deadlock. The default 60-second timeout can break the wait, but it does so by admitting a reader even when the budget does not fit ([timeout path](https://github.com/apache/doris/blob/99e33c47bd19fe75e8477231f74e9dabfaf1c853/be/src/util/jni_scan_heap_gate.cpp#L79-L108)). Several similarly timed waiters may consequently open above budget in a burst, turning a prolonged stall into JVM heap pressure or OOM. That timeout is per admission attempt, not a bound on the entire query's delay. The explicit bypass for the nested Fluss tail-key read already avoids one same-thread permit dependency. The concern here is the separate dependency between scan worker availability and permit-holder progress. Please add an integration test with a bounded scan pool: let a permit holder yield after its first block, fill the workers with heap waiters, and verify that the holder can resume and finish **without relying on timeout admission**. The gate's standalone tests use independent threads and do not exercise that scheduler interaction. A separate test with multiple waiters timing out together would characterize the over-budget burst. Consider integrating admission with scheduler suspension/resumption so waiting readers relinquish their worker and are rescheduled when eligible, or otherwise guarantee progress for permit holders. Simply shortening the timeout would bypass the memory budget sooner. This comment is based on static call-path analysis; I have not run a concurrent reproduction. The exact occurrence depends on pool saturation and scheduling order. -- 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]
