Doris-Breakwater commented on issue #69902: URL: https://github.com/apache/doris/issues/69902#issuecomment-6526640860
Breakwater-GitHub-Analysis-Slot: slot_1b02b24482a9 **Assessment:** The reported race is strongly supported by the source: a hash-join build that has consumed all input can still lose its runtime filters during downstream-driven early termination. This merits BE pipeline/runtime-filter investigation as a performance issue. The supplied excerpts establish disabled filters and excess scan volume; they do not independently establish which producer disabled the filters or the exact scheduling sequence. I have not reproduced the race on a running cluster. The issue currently has no labels. I checked the local `4.1.5-release` source at `917b20be83878e98f94e3ebf7efccc80346e3e50`, and the corresponding lifecycle remains in the local upstream-master snapshot `2a4191c1dc94e789e1725d6cb345f46c31a17272` (October 29). These are source checks, not confirmation of the reporter's deployed build or a tested fix. **Verified code chain:** 1. At build EOS, `sink_impl()` sends filter sizes, builds the hash table, initializes probe short-circuiting, then releases the probe dependency. RF construction/publication remains in `close()`. An empty, non-mark `RIGHT_OUTER_JOIN` sets `short_circuit_for_probe`; `pull()` immediately returns EOS. See [build sink](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/operator/hashjoin_build_sink.cpp#L868) and [probe](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/operator/hashjoin_probe_operator.cpp#L190). 2. When all tasks of the downstream pipeline close, `decrement_running_task()` propagates upstream through the pipeline DAG. `make_all_runnable()` marks upstream tasks for early wakeup and makes their dependencies ready; `PipelineTask::execute()` then invokes operator termination even if its execution has already reached EOS. This is conditional on pipeline/destination completion, not merely one arbitrary probe task finishing. See [completion propagation](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/pipeline/pipeline_fragment_context.cpp#L1930), [upstream wakeup](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/pipeline/pipeline.cpp#L122), and [task completion](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/pipeline/pipeline_task.cpp#L475). 3. Build-sink `terminate()` calls `skip_process()` without distinguishing successfully completed input from incomplete input. That helper marks unpublished filters `DISABLED` with `skip all rf process` and publishes them; normal `close()` skips RF processing once terminated. See [terminate](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/operator/hashjoin_build_sink.cpp#L118) and [skip_process](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/runtime_filter/runtime_filter_producer_helper.cpp#L151). 4. `RuntimeFilterWrapper::merge()` propagates a disabled contribution. On the consumer, `APPLIED` does **not** mean a filtering predicate was installed: `_apply_ready_expr()` sets that lifecycle state and returns without adding predicates for a disabled wrapper. This explains the apparent `DISABLED`/`APPLIED` combination. See [merge](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/runtime_filter/runtime_filter_wrapper.cpp#L127) and [consumer](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/runtime_filter/runtime_filter_consumer.cpp#L28). The described build/probe direction is reasonable: selective `orders` produces filters for the large `order_detail` scan. The supplied comparison points toward RF lifecycle loss, rather than evidence of a bad join order. The 175 ms wait is a readiness wait, not scan CPU. Full profiles are needed to attribute the remaining 10–30 seconds among scan, exchange, and scheduling costs. The UNINITED serialization change proposed in [#68002](https://github.com/apache/doris/pull/68002) would not address this explicit DISABLED path. **Constraints on a fix:** Distinguishing completed builds is sensible, but an EOS-only `build(); publish();` change in `terminate()` is insufficient. Early termination forces finish dependencies ready; that does not prove the size-sync reply arrived. `insert()` requires `WAITING_FOR_DATA`, and a late `set_synced_size()` callback expects a disabled wrapper if the producer has already advanced past that transition. Even an empty build, which skips insertion, needs correct callback handling. Publication also releases the producer wrapper, so duplicate publication must be prevented. See the [producer state machine](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/runtime_filter/runtime_filter_producer.h#L143) and [size-sync completion](https://github.com/apache/doris/blob/917b20be83878e98f94e3ebf7efccc80346e3e50/be/src/exec/runtime_filter/runtime_filter_producer.cpp#L189). Preserve the incomplete-build size-reporting guarantee introduced by [#55942](https://github.com/apache/doris/commit/06bfd0c3d238a028c611c495f33136d134b91aa4). Do not change merge semantics to ignore DISABLED contributions: an unfinished producer may have unobserved keys. A completed empty contribution is different; for the IN filter, merging it with the other instance's `{key}` should retain `{key}`, not disable the filter or make the global filter empty. Delaying the empty probe's release until RF publication is worth evaluating, with cancellation and dependency-cycle tests; it is not a validated patch here. **Requested evidence and next steps:** - Provide exact FE/BE build identifiers, sanitized DDL and data-generation SQL, session settings, and full `EXPLAIN` output confirming the two-instance partitioned plan. - Attach paired fast/slow full text profiles with query IDs and per-instance detail: actual build input rows, build `RuntimeFilterInfo/SkipProcess`, task `WakeUpEarly`, RF states, finish-dependency waits, and scan rows/active timers. Include correlated BE logs for cancellation, spill, or RF RPC errors, which can also disable filters. - Add a deterministic regression that pauses the empty build after successful EOS but before publication, permits probe completion, and controls size-sync delivery. Cover sync enabled/disabled, late callbacks, both producer arrival orders, incomplete-build termination, cancellation, all-empty input, VARCHAR/null keys, and shared-build/spill paths if the change affects them. Assert result correctness, no hangs/state errors, one contribution per producer, and preservation of the nonempty global IN filter. Increasing RF wait cannot restore an explicitly disabled filter. Disabling size synchronization removes one wait but leaves the release-to-publication window, consistent with the reported residual failure; treat the observed improvement as partial mitigation, not a fix. No verified fixed version or complete workaround was established. No repository code was changed. -- 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]
