github-actions[bot] commented on code in PR #65837:
URL: https://github.com/apache/doris/pull/65837#discussion_r3764805557
##########
be/src/exec/operator/scan_operator.cpp:
##########
@@ -73,23 +73,24 @@ bool ScanLocalState<Derived>::should_run_serial() const {
Status ScanLocalStateBase::update_late_arrival_runtime_filter(RuntimeState*
state,
int&
arrived_rf_num) {
- // Lock needed because _conjuncts can be accessed concurrently by multiple
scanner threads
- LockGuard lock(_conjuncts_lock);
- size_t conjuncts_before = _conjuncts.size();
- RETURN_IF_ERROR(_helper.try_append_late_arrival_runtime_filter(state,
_parent->row_descriptor(),
-
arrived_rf_num, _conjuncts));
- if (state->enable_adjust_conjunct_order_by_cost()) {
- std::ranges::stable_sort(_conjuncts, [](const auto& a, const auto& b) {
- return a->execute_cost() < b->execute_cost();
- });
- };
- // Only re-run partition pruning when
try_append_late_arrival_runtime_filter
- // actually appended new conjuncts. Otherwise this hook would re-scan all
- // partition boundaries on every scheduler pass while there are still
- // unapplied RFs (Scanner::_applied_rf_num is not advanced here), wasting
- // CPU re-evaluating the same set of RFs against the same boundaries.
- if (_conjuncts.size() > conjuncts_before) {
- RETURN_IF_ERROR(_on_runtime_filter_update());
+ VExprContextSPtrs new_conjuncts;
+ {
+ // Lock needed because _conjuncts can be accessed concurrently by
multiple scanner threads.
+ LockGuard lock(_conjuncts_lock);
+ size_t conjuncts_before = _conjuncts.size();
+ RETURN_IF_ERROR(_helper.try_append_late_arrival_runtime_filter(
+ state, _parent->row_descriptor(), arrived_rf_num, _conjuncts));
+ if (_conjuncts.size() > conjuncts_before) {
+ new_conjuncts.assign(_conjuncts.begin() + conjuncts_before,
_conjuncts.end());
+ }
+ if (state->enable_adjust_conjunct_order_by_cost()) {
+ std::ranges::stable_sort(_conjuncts, [](const auto& a, const auto&
b) {
+ return a->execute_cost() < b->execute_cost();
+ });
+ }
+ }
+ if (!new_conjuncts.empty()) {
+ RETURN_IF_ERROR(_on_runtime_filter_update(new_conjuncts));
Review Comment:
[P1] Prevent projection from racing with expression cloning
After this lock is released, scanner A can execute a non-slot runtime-filter
target during partition projection while scanner B enters
`clone_conjunct_ctxs()` for the newly published filter. `VExprContext::clone()`
shares the original `_root` and calls `open()` on it, so it writes shared node
state while A reads it; for example, `VInPredicate::open()` updates
`_is_args_all_constant` while execution uses that field to select its argument
layout (and the lifecycle flags race as well). The old code kept
`_on_runtime_filter_update()` under `_conjuncts_lock`, excluding this
interleaving. Please keep the filter unpublished/serialized through projection
or give pruning a genuinely independent expression tree; an ordinary
`VExprContext::clone()` still shares `_root`.
--
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]
