lide-reed opened a new pull request, #68002:
URL: https://github.com/apache/doris/pull/68002
### Proposed changes
Fix #67997
**Problem**
`RuntimeFilter::serialize()` marks the outgoing filter as `disabled` for any
state other
than `READY`:
```cpp
auto state = _wrapper->get_state();
if (state != RuntimeFilterWrapper::State::READY) {
request->set_disabled(true);
return Status::OK();
}
```
`UNINITED` means "no filter content has been produced yet", not "this filter
was
cancelled", but the consumer cannot distinguish the two. It treats
`disabled` as an
absorbing state (`set_state(DISABLED, "get disabled from remote")`) and
permanently gives
up the filter, so the probe side degrades to a full scan.
The merger decides readiness only by producer count, which is independent of
whether any
producer really produced content:
```cpp
// RuntimeFilterMerger::merge_from()
*ready = _received_producer_num == _expected_producer_num;
if (_received_producer_num == _expected_producer_num) {
_rf_state = State::READY;
}
if (_wrapper->get_state() == RuntimeFilterWrapper::State::UNINITED) {
_wrapper = other->_wrapper; // may still be UNINITED
return Status::OK();
}
```
So when all producers report without content, `merge_from()` returns `ready
== true` while
the merged wrapper is still `UNINITED`, and the caller publishes it as
`disabled` to every
consumer.
**Fix**
Align with the legacy (branch-3.1) semantics: only a really disabled filter
(`max_in_num` / join spill / rpc error -> `State::DISABLED`) is published as
`disabled`; a
filter that is simply not ready is skipped and consumers wait until
`runtime_filter_wait_time_ms`.
| File | Change |
| --- | --- |
| `runtime_filter.h` | `serialize()`: only `DISABLED` sets `disabled=true`;
`UNINITED` publishes nothing (DCHECK + WARNING + `OK`) |
| `runtime_filter_merger.h` | new `is_wrapper_uninited()` helper |
| `runtime_filter_producer.cpp` | `do_merge`: skip send when merged wrapper
is `UNINITED` |
| `runtime_filter_mgr.cpp` | `_send_rf_to_target()`: skip broadcast when
merged wrapper is `UNINITED` |
**Why `UNINITED` returns `OK` instead of `InternalError`**
The old behavior for `UNINITED` was a silent performance degradation (filter
disabled),
not an error. Turning it into `InternalError` would escalate a performance
issue into a
query failure. Publishing nothing keeps the failure mode conservative: the
filter is lost,
the consumer times out, and at worst the probe side falls back to a full
scan. `DCHECK` is
kept so unexpected paths still fail fast in debug builds.
### Checklist
- [ ] Issue is created and linked
- [ ] No new configuration or user-facing interface
- [ ] No behavior change for correctly produced filters
### Further comments
Silent performance regression is hard to notice in production; the fix
restores the
previous (branch-3.1) semantics and makes the worst case a filter timeout
rather than a
permanently disabled filter.
--
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]
