Doris-Breakwater commented on issue #67997: URL: https://github.com/apache/doris/issues/67997#issuecomment-5675817346
Breakwater-GitHub-Analysis-Slot: slot_5c6c307cf791 ## Preliminary assessment **Triage: needs reproduction / reported root cause is not yet confirmed on 4.1.3.** The issue identifies a real defensive weakness in `RuntimeFilter::serialize()`, but the described production path does not appear able to deliver an `UNINITED` wrapper to serialization in the 4.1.3 source. The observable consequence is valid **if** that invariant is violated: [`serialize()` maps every non-`READY` state to `disabled=true`](https://github.com/apache/doris/blob/4.1.3/be/src/exec/runtime_filter/runtime_filter.h#L56-L68); the remote [`assign()` maps that bit to `DISABLED` with the generic reason `get disabled from remote`](https://github.com/apache/doris/blob/4.1.3/be/src/exec/runtime_filter/runtime_filter_wrapper.cpp#L619-L630); and the consumer is signaled as ready but installs no predicate for a disabled wrapper. That would safely preserve results while losing runtime-filter pruning. However, the claimed `UNINITED` source is contradicted by the normal producer lifecycle: - Hash-join finalization calls `build()` before `publish()`. `build()` sets every producer wrapper to `READY` even when the build block is null/empty or contains only the hash join's mock row; insertion is conditional, readiness is not. See [`RuntimeFilterProducerHelper::build()`](https://github.com/apache/doris/blob/4.1.3/be/src/exec/runtime_filter/runtime_filter_producer_helper.cpp#L92-L141). - Cross-join and set-operation producer helpers also explicitly set `READY` before publishing, including the zero-cardinality set path. - Early termination/spill uses `skip_process()`, which explicitly sets `DISABLED` before publishing. Cancelled or terminated hash-join close paths skip the normal build/publish path. - `RuntimeFilterProducer::publish()` requires producer state `READY_TO_PUBLISH`; the production transition to that state is paired with setting the wrapper to `READY` or `DISABLED`. - There are only two production callers of `RuntimeFilterMerger::merge_from()`. The local caller receives the guarded producer above. The global merge-controller caller first reconstructs a temporary producer via `assign()`, which yields only `READY` or `DISABLED`, never `UNINITED`, before merging. - A local consumer signal also validates that the incoming wrapper is `READY` or `DISABLED`; an `UNINITED` local wrapper would fail loudly rather than silently become disabled. Therefore, producer completion is not counted independently of producer finalization in the way described: a producer reports to the merger only from `publish()`, after its wrapper has been made terminal. Zero filter rows are represented as a finalized `READY` filter, not as `UNINITED`. The merger does keep its arrival state separately from wrapper state, but that is currently backed by this caller invariant. The profile string `get disabled from remote` is not proof that the sender was `UNINITED`. It is also used for legitimate `DISABLED` messages (for example max-IN threshold, spill/skip, or an RPC/size-sync failure), because the original disable reason is not serialized. The branch-3.1 comparison also needs qualification: the legacy implementation checks explicit `ignored`/`disabled` flags before serialization, but it has no directly equivalent `UNINITED` wrapper state on this wire path. It does not establish that a terminal zero-row producer should remain unpublished. The issue currently has no labels, assignee, milestone, comments, concrete SQL reproducer, logs, or profile attached. ## Information needed to confirm the bug Please provide: 1. The exact deployed build identity (full `SELECT VERSION()` output and whether FE/BEs are mixed builds), plus the relevant runtime-filter session variables. 2. A minimal SQL/DDL/data reproducer and `EXPLAIN VERBOSE`, including join distribution, fragment instance count, and build-side row counts per producer. 3. The complete query profile from an affected run and, ideally, a fast comparison run. Include all build-sink and scan-node `RuntimeFilterInfo` sections and row counters. 4. FE and BE logs for the query ID and time window, especially runtime-filter RPC/size-sync failures, spill or early-termination messages, and max-IN disable events. 5. Direct sender-side evidence immediately before `merge_from()` and `serialize()`: filter ID, stage, producer state, wrapper state and reason, and merger expected/received counts. A targeted debug log or debug-point reproduction is sufficient. ## Recommended next steps Because the reporter is willing to submit a PR, the most useful first artifact is a failing BE test that exercises the **production lifecycle**, not a direct call that manually passes an `UNINITED` wrapper: - cover a zero-row producer through helper `build()` -> `publish()` -> local/global merge -> serialization; - cover multiple producers with empty and non-empty inputs; - assert the wrapper state before serialization and the received consumer state/reason. If such a test finds a real path to `UNINITED`, fix the missing terminal transition at that source. Merely skipping the send in `serialize()` is not a complete fix after all expected producers have reported: no producer remains to publish later, so consumers would only wait until timeout. The intended terminal semantics for an empty filter should be defined explicitly. Independently, the serialization boundary should be hardened so the three states are not conflated: serialize content for `READY`, publish `disabled=true` only for `DISABLED`, and treat `UNINITED` as an invariant violation (or handle it explicitly at a caller if maintainers decide it is a valid state there), with a regression test and diagnostic context. This would turn the latent silent-degradation risk into actionable evidence without assuming the issue's proposed root cause. -- 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]
