15767714253 commented on issue #68771: URL: https://github.com/apache/doris/issues/68771#issuecomment-6094630691
> Breakwater-GitHub-Analysis-Slot: slot_841d0d77defc > > **Initial assessment:** This strongly matches a known `TimeSharingTaskExecutor` queue-accounting defect. The defect is present in the upstream **4.1.2** source; the matching fix was merged as [PR #63568](https://github.com/apache/doris/pull/63568), with the 4.1 backport in [PR #64855](https://github.com/apache/doris/pull/64855). I verified that backport commit `1f197f6c931217f546463371865c4d756b1a6058` is absent from the 4.1.2 ancestry and present in 4.1.3. Confidence is high for the source-level defect and its match to these symptoms; confirming the exact running BE builds is still necessary. > > **Verified code mechanism:** I inspected upstream tag `4.1.2` (commit `aec169d20256a788de448f46737b5fb53ee3b4e3`) without changing the checkout. > > * With `enable_task_executor_in_internal_table=true` (the default), `WorkloadGroup` creates `TaskExecutorSimplifiedScanScheduler` for `ls_<workload_group>`, using `doris_scanner_thread_pool_queue_size` as its queue limit. See [workload_group.cpp](https://github.com/apache/doris/blob/aec169d20256a788de448f46737b5fb53ee3b4e3/be/src/runtime/workload_group/workload_group.cpp#L561-L573). > * The metric hook reads `get_queue_size()`, which returns `_total_queued_tasks`, rather than the actual split-queue size. `_do_submit()` increments that count when offering a split; a worker decrements it when taking the split. See [executor metric/submission code](https://github.com/apache/doris/blob/aec169d20256a788de448f46737b5fb53ee3b4e3/be/src/exec/scan/task_executor/time_sharing/time_sharing_task_executor.cpp#L242-L254) and [queue-size getter](https://github.com/apache/doris/blob/aec169d20256a788de448f46737b5fb53ee3b4e3/be/src/exec/scan/task_executor/time_sharing/time_sharing_task_executor.h#L208-L211). > * `ScannerContext::stop_scanners()` and its destructor call `remove_task()`. That function closes the task handle and physically removes its queued splits with `remove_all()`, **without decrementing `_total_queued_tasks`**. Workers decide whether to sleep from the real split-queue size, so they can be idle while the reported count retains removed entries. See [scanner cleanup](https://github.com/apache/doris/blob/aec169d20256a788de448f46737b5fb53ee3b4e3/be/src/exec/scan/scanner_context.cpp#L366-L385), [remove_task](https://github.com/apache/doris/blob/aec169d20256a788de448f46737b5fb53ee3b4e3/be/src/exec/scan/task_executor/time_sharing/time_sharing_task_executor.cpp#L746-L775), and [worker dispatch](https://github.com/apache/doris/blob/aec169d20256a788de448f46737b5fb53ee3b4e3/be/src/exec/scan/task_executor/time_sharing/time_sharing_task_executor.cpp#L503-L547). > > A minimal sequence is therefore: enqueue a split (+1), remove its task before a worker takes it (real queue -1, accounting unchanged), repeat. This explains accumulated phantom entries and why continuing to execute unrelated scan tasks does not drain the leaked offset. It does not require token shutdown itself to omit a decrement. > > This can affect query execution, not just the dashboard. The [capacity check](https://github.com/apache/doris/blob/aec169d20256a788de448f46737b5fb53ee3b4e3/be/src/exec/scan/task_executor/time_sharing/time_sharing_task_executor.cpp#L383-L391) uses: > > `capacity_remaining = max_threads - active_threads + max_queue_size - _total_queued_tasks` > > It rejects submissions and increments `submit_failed` when this is below 1. Thus the exact boundary depends on active workers; with 48 maximum workers and zero active workers, it is 102448, rather than exactly 102400. The projected exhaustion date remains an estimate. > > **What remains unverified:** The supplied samples support this mechanism but do not identify which queries removed queued splits. Equal execution rates do not imply equal cancellation/cleanup histories. The supposedly healthy BE's flat, nonzero count of 19030 could also be an older leaked offset. Also, active-thread and queue gauges are read under separate lock acquisitions, so one `active=0` sample alone is not conclusive; sustained observations and source evidence are stronger. > > Please provide: > > 1. Full BE version/build Git hashes for all five nodes, any local patches, and effective `enable_task_executor_in_internal_table`, scanner queue limit, and workload-group scan-thread settings. > 2. Several timestamped raw `/metrics` captures retaining all labels, including pool `id` and BE identity, plus the dashboard PromQL. This will exclude aggregation across different pool instances. > 3. Representative query IDs, sanitized SQL/reproduction steps, and BE/FE logs showing cancellation, timeout, early scan termination, or cleanup around count increases. Please also include any `TooManyTasks` / `Thread pool ... is at capacity` errors and workload/configuration changes near the reported onset. These events are candidates, not established triggers here. > > **Recommended next steps:** Use a tested maintenance build containing the 4.1 backport; upstream 4.1.3 contains this specific fix, but verify the packaged BE hash. The patch subtracts only entries actually removed from the executor queue, centralizes offer accounting, and handles the idle transition/notification. Subtracting the entire task's split list would be incorrect because it can include running or otherwise nonqueued splits. > > For validation, use the existing [`TimeSharingTaskExecutorTest.test_remove_task_clears_queued_task_count`](https://github.com/apache/doris/blob/1f197f6c931217f546463371865c4d756b1a6058/be/test/exec/executor/time_sharing/time_sharing_task_executor_test.cpp#L373-L423) from the backport, then exercise scan cancellation/cleanup in staging and check that an idle pool returns to zero and accepts subsequent submissions. I inspected that test; I did not build or run the BE suite or reproduce this against your cluster. A BE restart recreates the counter and can temporarily restore headroom, but does not remove the defect from an unpatched binary. Increasing the queue limit only postpones rejection. > > If the deployed build already includes the fix and still exhibits growth, keep this issue open for further investigation: obtain a maintainer-assisted snapshot of the actual split-queue size and `_total_queued_tasks` under the same executor lock, plus worker thread stacks. That comparison distinguishes an accounting mismatch from a real queue that is failing to dispatch. -- 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]
