Uh oh!
There was an error while loading. Please reload this page.
[fix](be) keep FlushToken running count symmetric on cancel - #61684
Conversation
Thearas
commented
Mar 24, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
14e58fd to
ca240bcCompareThere was a problem hiding this comment.
Pull request overview
Fixes a shutdown/cancel edge case where queued flush tasks could decrement flush_running_count without ever incrementing it, potentially driving the counter negative and causing cancel() to wait indefinitely.
Changes:
- Move
flush_running_count++earlier in_flush_memtable()so every exit path has symmetric increment/decrement. - Ensure the deferred cleanup always corresponds to a prior increment, even when
_is_shutdown()is already true.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
ca240bc to
fe5cc86Compareliaoxin01
commented
Mar 24, 2026
run buildall |
doris-robot
commented
Mar 24, 2026
TPC-H: Total hot run time: 26367 ms |
doris-robot
commented
Mar 24, 2026
TPC-DS: Total hot run time: 169088 ms |
hello-stephen
commented
Mar 24, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 24, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
PR approved by anyone and no changes requested. |
PR approved by at least one committer and no changes requested. |
Uh oh!
There was an error while loading. Please reload this page.
## Problem When a queued memtable flush task starts after `FlushToken::cancel()` has already marked the token shutdown, `_flush_memtable()` can return before the old `flush_running_count++` path but still run the deferred `flush_running_count--`. This can drive the counter below zero and make `cancel()` wait forever for `flush_running_count == 0`. Related PR: #53481 ## Fix Move `flush_running_count++` to the top of `_flush_memtable()` before registering the deferred cleanup so the running-count accounting stays symmetric on every exit path. ## Validation - Reasoned from production stack and gdb evidence showing `flush_running_count = -1` while cancel was blocked in `_wait_running_task_finish()` - Not run locally: the BE UT environment in this workspace would require a full initial `ut_build_ASAN` build
## Problem When a queued memtable flush task starts after `FlushToken::cancel()` has already marked the token shutdown, `_flush_memtable()` can return before the old `flush_running_count++` path but still run the deferred `flush_running_count--`. This can drive the counter below zero and make `cancel()` wait forever for `flush_running_count == 0`. Related PR: #53481 ## Fix Move `flush_running_count++` to the top of `_flush_memtable()` before registering the deferred cleanup so the running-count accounting stays symmetric on every exit path. ## Validation - Reasoned from production stack and gdb evidence showing `flush_running_count = -1` while cancel was blocked in `_wait_running_task_finish()` - Not run locally: the BE UT environment in this workspace would require a full initial `ut_build_ASAN` build
…1684) ## Problem When a queued memtable flush task starts after `FlushToken::cancel()` has already marked the token shutdown, `_flush_memtable()` can return before the old `flush_running_count++` path but still run the deferred `flush_running_count--`. This can drive the counter below zero and make `cancel()` wait forever for `flush_running_count == 0`. Related PR: apache#53481 ## Fix Move `flush_running_count++` to the top of `_flush_memtable()` before registering the deferred cleanup so the running-count accounting stays symmetric on every exit path. ## Validation - Reasoned from production stack and gdb evidence showing `flush_running_count = -1` while cancel was blocked in `_wait_running_task_finish()` - Not run locally: the BE UT environment in this workspace would require a full initial `ut_build_ASAN` build
Problem
When a queued memtable flush task starts after
FlushToken::cancel()has already marked the token shutdown,_flush_memtable()can return before the oldflush_running_count++path but still run the deferredflush_running_count--. This can drive the counter below zero and makecancel()wait forever forflush_running_count == 0.Related PR: #53481
Fix
Move
flush_running_count++to the top of_flush_memtable()before registering the deferred cleanup so the running-count accounting stays symmetric on every exit path.Validation
flush_running_count = -1while cancel was blocked in_wait_running_task_finish()ut_build_ASANbuild