HappenLee commented on PR #68684:
URL: https://github.com/apache/doris/pull/68684#issuecomment-6038331583
Suggested refinement for e30c9b41908: keep shared producer inputs read-only,
but make copying conditional on the input's ownership. This would preserve the
mixed-target correctness fix while avoiding the two RPC copies discussed above.
The caller can identify ordinary local consumers using the actual
registrations:
```cpp
bool has_plain_local_consumer =
!state->local_runtime_filter_mgr()->get_consume_filters(filter_id).empty();
bool mixed_targets = _need_do_merge(state) && has_plain_local_consumer;
```
`has_local_targets` alone is insufficient: targets that require a local
merge are also local targets. A pure local filter that does not enter the
merger needs no copy.
Pass an explicit input ownership policy (e.g. `Shared` / `Transferable`)
through `RuntimeFilterMerger::merge_from()` and `RuntimeFilterWrapper::merge()`:
- **Shared:** the producer has ordinary local consumers, or its wrapper is
shared by multiple broadcast-join producers. The merger must preserve this
input's filter data.
- **Transferable:** the caller guarantees there are no other readers or
subsequent writers. This includes an unshared local producer with no ordinary
local consumers, and the global receiver's RPC-local `tmp_filter`.
Broadcast sharing must be accounted for separately. Checking only the
current producer's local consumers is unsafe because a sibling producer may use
the same wrapper. The sharing property can be recorded when `build(...,
use_shared_table, ...)` installs/reuses the wrapper. A smaller conservative
implementation could classify local broadcast-join inputs as shared.
RPC-deserialized inputs remain transferable regardless of the join descriptor.
Apply the policy at both ownership acquisition points:
1. For the first merger input, clone a shared wrapper; adopt a transferable
wrapper.
2. When an IN accumulator receives a Bloom input, clone a shared Bloom
directory; adopt a transferable directory, then insert the accumulated IN
values.
Other IN-set union, Bloom OR and MinMax merge operations can continue to
read the source and modify the accumulator using the existing logic. The caller
must relinquish use of transferred data; `shared_ptr::use_count()` should not
be used to infer this guarantee.
This keeps copying on the merger side, so multiple ordinary local
consumers/producers do not each require a separate clone. It also avoids
introducing a general COW mechanism or changing the FE/BE protocol.
Suggested tests: mixed-target consumers retain their original data after
later merges/overflow; broadcast producers retain an unchanged shared wrapper,
including a disabled sibling; first RPC Bloom input is adopted and survives
destruction of `tmp_filter`; later RPC IN-to-Bloom conversion reuses the
incoming directory and preserves both inputs' values and NULL semantics.
--
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]
