github-actions[bot] commented on code in PR #70305:
URL: https://github.com/apache/doris/pull/70305#discussion_r4487768733


##########
be/src/exec/runtime_filter/runtime_filter_mgr.cpp:
##########
@@ -54,6 +54,43 @@ namespace doris {
 
 namespace {
 
+bool needs_global_size_sync(const TRuntimeFilterDesc& desc) {
+    // Only Bloom-backed, non-broadcast producers wait for a query-wide size. 
A MIN/MAX
+    // producer never sends a size RPC even when the optional flag is present.
+    return !desc.is_broadcast_join && desc.__isset.build_bf_by_runtime_size &&
+           desc.build_bf_by_runtime_size &&
+           (desc.type == TRuntimeFilterType::BLOOM || desc.type == 
TRuntimeFilterType::IN_OR_BLOOM);
+}
+
+template <typename Response>
+class MergeDispatchCallback final : public HandleErrorBrpcCallback<Response> {
+public:
+    MergeDispatchCallback(std::weak_ptr<QueryContext> context,
+                          std::shared_ptr<GlobalMergeContext::Completion> 
completion)
+            : HandleErrorBrpcCallback<Response>(std::move(context)),
+              _completion(std::move(completion)) {}
+
+    void call() override {
+        bool failed = this->cntl_->Failed();
+        if constexpr (HasStatus<Response>) {
+            Status status = Status::create(this->response_->status());
+            // A producer can finish before a delayed size-sync reaches it; 
its EOF is a
+            // normal acknowledgement, unlike a transport failure or another 
RPC error.
+            failed |= !status.ok() && !status.is<ErrorCode::END_OF_FILE>();
+        }
+        HandleErrorBrpcCallback<Response>::call();
+        // Publish the failure before the last pending callback disappears. 
The cleaner
+        // cannot mistake an unsuccessful dispatch for an acknowledged RF.
+        if (failed) {
+            _completion->failed.store(true, std::memory_order_release);

Review Comment:
   [P1] Respect `ignore_runtime_filter_error` when releasing the delayed 
QueryContext. `MergeDispatchCallback::call()` sets `Completion::failed` for 
every transport/non-EOF error even when the callback was created with an empty 
weak context because `ignore_runtime_filter_error` is true. 
`can_release_query_context()` then rejects the context whenever `failed` is 
set, while `_query_ctx_map_delay_delete` owns the last strong reference. A 
single-use query whose RF publish or size RPC fails therefore completes with 
the error intentionally ignored but keeps the full QueryContext and 
runtime-filter state until timeout/cancel. Make an ignored failure an 
acknowledged terminal dispatch for cleanup while still draining callbacks, or 
distinguish it from an unresolved dispatch failure in the release predicate.



-- 
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]

Reply via email to