-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature](runtime-filter) cancel query when runtime_filter's rpc failed #43627
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -305,7 +305,8 @@ Status RuntimeFilterMergeControllerEntity::init(UniqueId query_id, | |
return Status::OK(); | ||
} | ||
|
||
Status RuntimeFilterMergeControllerEntity::send_filter_size(const PSendFilterSizeRequest* request) { | ||
Status RuntimeFilterMergeControllerEntity::send_filter_size(std::weak_ptr<QueryContext> query_ctx, | ||
const PSendFilterSizeRequest* request) { | ||
SCOPED_CONSUME_MEM_TRACKER(_mem_tracker); | ||
std::shared_ptr<RuntimeFilterCntlVal> cnt_val; | ||
|
||
|
@@ -326,6 +327,8 @@ Status RuntimeFilterMergeControllerEntity::send_filter_size(const PSendFilterSiz | |
|
||
Status st = Status::OK(); | ||
if (cnt_val->source_addrs.size() == cnt_val->producer_size) { | ||
auto ctx = query_ctx.lock()->ignore_runtime_filter_error() ? std::weak_ptr<QueryContext> {} | ||
: query_ctx; | ||
for (auto addr : cnt_val->source_addrs) { | ||
std::shared_ptr<PBackendService_Stub> stub( | ||
ExecEnv::GetInstance()->brpc_internal_client_cache()->get_client(addr)); | ||
|
@@ -339,7 +342,7 @@ Status RuntimeFilterMergeControllerEntity::send_filter_size(const PSendFilterSiz | |
auto closure = AutoReleaseClosure<PSyncFilterSizeRequest, | ||
DummyBrpcCallback<PSyncFilterSizeResponse>>:: | ||
create_unique(std::make_shared<PSyncFilterSizeRequest>(), | ||
DummyBrpcCallback<PSyncFilterSizeResponse>::create_shared()); | ||
DummyBrpcCallback<PSyncFilterSizeResponse>::create_shared(), ctx); | ||
|
||
auto* pquery_id = closure->request_->mutable_query_id(); | ||
pquery_id->set_hi(_state->query_id.hi()); | ||
|
@@ -377,7 +380,8 @@ Status RuntimeFilterMgr::sync_filter_size(const PSyncFilterSizeRequest* request) | |
} | ||
|
||
// merge data | ||
Status RuntimeFilterMergeControllerEntity::merge(const PMergeFilterRequest* request, | ||
Status RuntimeFilterMergeControllerEntity::merge(std::weak_ptr<QueryContext> query_ctx, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: function 'merge' exceeds recommended size/complexity thresholds [readability-function-size] Status RuntimeFilterMergeControllerEntity::merge(std::weak_ptr<QueryContext> query_ctx,
^ Additional contextbe/src/runtime/runtime_filter_mgr.cpp:381: 118 lines including whitespace and comments (threshold 80) Status RuntimeFilterMergeControllerEntity::merge(std::weak_ptr<QueryContext> query_ctx,
^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: function 'merge' exceeds recommended size/complexity thresholds [readability-function-size] Status RuntimeFilterMergeControllerEntity::merge(std::weak_ptr<QueryContext> query_ctx,
^ Additional contextbe/src/runtime/runtime_filter_mgr.cpp:382: 119 lines including whitespace and comments (threshold 80) Status RuntimeFilterMergeControllerEntity::merge(std::weak_ptr<QueryContext> query_ctx,
^ |
||
const PMergeFilterRequest* request, | ||
butil::IOBufAsZeroCopyInputStream* attach_data) { | ||
SCOPED_CONSUME_MEM_TRACKER(_mem_tracker); | ||
std::shared_ptr<RuntimeFilterCntlVal> cnt_val; | ||
|
@@ -444,12 +448,14 @@ Status RuntimeFilterMergeControllerEntity::merge(const PMergeFilterRequest* requ | |
has_attachment = true; | ||
} | ||
|
||
auto ctx = query_ctx.lock()->ignore_runtime_filter_error() ? std::weak_ptr<QueryContext> {} | ||
: query_ctx; | ||
std::vector<TRuntimeFilterTargetParamsV2>& targets = cnt_val->targetv2_info; | ||
for (auto& target : targets) { | ||
auto closure = AutoReleaseClosure<PPublishFilterRequestV2, | ||
DummyBrpcCallback<PPublishFilterResponse>>:: | ||
create_unique(std::make_shared<PPublishFilterRequestV2>(apply_request), | ||
DummyBrpcCallback<PPublishFilterResponse>::create_shared()); | ||
DummyBrpcCallback<PPublishFilterResponse>::create_shared(), ctx); | ||
|
||
closure->request_->set_filter_id(request->filter_id()); | ||
closure->request_->set_merge_time(merge_time); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,9 @@ | |
#include <google/protobuf/stubs/common.h> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: 'google/protobuf/stubs/common.h' file not found [clang-diagnostic-error] #include <google/protobuf/stubs/common.h>
^ |
||
|
||
#include <atomic> | ||
#include <utility> | ||
|
||
#include "runtime/query_context.h" | ||
#include "runtime/thread_context.h" | ||
#include "service/brpc.h" | ||
#include "util/ref_count_closure.h" | ||
|
@@ -79,8 +81,9 @@ class AutoReleaseClosure : public google::protobuf::Closure { | |
ENABLE_FACTORY_CREATOR(AutoReleaseClosure); | ||
|
||
public: | ||
AutoReleaseClosure(std::shared_ptr<Request> req, std::shared_ptr<Callback> callback) | ||
: request_(req), callback_(callback) { | ||
AutoReleaseClosure(std::shared_ptr<Request> req, std::shared_ptr<Callback> callback, | ||
std::weak_ptr<QueryContext> context = {}) | ||
: request_(req), callback_(callback), context_(std::move(context)) { | ||
this->cntl_ = callback->cntl_; | ||
this->response_ = callback->response_; | ||
} | ||
|
@@ -113,12 +116,22 @@ class AutoReleaseClosure : public google::protobuf::Closure { | |
|
||
protected: | ||
virtual void _process_if_rpc_failed() { | ||
LOG(WARNING) << "RPC meet failed: " << cntl_->ErrorText(); | ||
std::string error_msg = "RPC meet failed: " + cntl_->ErrorText(); | ||
if (auto ctx = context_.lock(); ctx) { | ||
ctx->cancel(Status::NetworkError(error_msg)); | ||
} else { | ||
LOG(WARNING) << error_msg; | ||
} | ||
} | ||
|
||
virtual void _process_if_meet_error_status(const Status& status) { | ||
// no need to log END_OF_FILE, reduce the unlessful log | ||
if (!status.is<ErrorCode::END_OF_FILE>()) { | ||
if (status.is<ErrorCode::END_OF_FILE>()) { | ||
// no need to log END_OF_FILE, reduce the unlessful log | ||
return; | ||
} | ||
if (auto ctx = context_.lock(); ctx) { | ||
ctx->cancel(status); | ||
} else { | ||
LOG(WARNING) << "RPC meet error status: " << status; | ||
} | ||
} | ||
|
@@ -136,6 +149,7 @@ class AutoReleaseClosure : public google::protobuf::Closure { | |
// Use a weak ptr to keep the callback, so that the callback can be deleted if the main | ||
// thread is freed. | ||
Weak callback_; | ||
std::weak_ptr<QueryContext> context_; | ||
}; | ||
|
||
} // namespace doris |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function 'publish' has cognitive complexity of 77 (threshold 50) [readability-function-cognitive-complexity]
Additional context
be/src/exprs/runtime_filter.cpp:995: nesting level increased to 1
auto send_to_remote = [&](IRuntimeFilter* filter) { ^
be/src/exprs/runtime_filter.cpp:998: +2, including nesting penalty of 1, nesting level increased to 2
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:998: +3, including nesting penalty of 2, nesting level increased to 3
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1001: nesting level increased to 1
auto send_to_local = [&](std::shared_ptr<RuntimePredicateWrapper> wrapper) { ^
be/src/exprs/runtime_filter.cpp:1003: +2, including nesting penalty of 1, nesting level increased to 2
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1003: +3, including nesting penalty of 2, nesting level increased to 3
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1013: nesting level increased to 1
auto do_local_merge = [&]() { ^
be/src/exprs/runtime_filter.cpp:1015: +2, including nesting penalty of 1, nesting level increased to 2
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1015: +3, including nesting penalty of 2, nesting level increased to 3
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1018: +2, including nesting penalty of 1, nesting level increased to 2
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1018: +3, including nesting penalty of 2, nesting level increased to 3
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1020: +2, including nesting penalty of 1, nesting level increased to 2
be/src/exprs/runtime_filter.cpp:1021: +3, including nesting penalty of 2, nesting level increased to 3
if (_has_local_target) { ^
be/src/exprs/runtime_filter.cpp:1022: +4, including nesting penalty of 3, nesting level increased to 4
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1022: +5, including nesting penalty of 4, nesting level increased to 5
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1023: +1, nesting level increased to 3
} else { ^
be/src/exprs/runtime_filter.cpp:1024: +4, including nesting penalty of 3, nesting level increased to 4
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1024: +5, including nesting penalty of 4, nesting level increased to 5
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1030: +1, including nesting penalty of 0, nesting level increased to 1
if (_need_local_merge && _has_local_target) { ^
be/src/exprs/runtime_filter.cpp:1030: +1
if (_need_local_merge && _has_local_target) { ^
be/src/exprs/runtime_filter.cpp:1031: +2, including nesting penalty of 1, nesting level increased to 2
RETURN_IF_ERROR(do_local_merge()); ^
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1031: +3, including nesting penalty of 2, nesting level increased to 3
RETURN_IF_ERROR(do_local_merge()); ^
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1032: +1, nesting level increased to 1
be/src/exprs/runtime_filter.cpp:1033: +2, including nesting penalty of 1, nesting level increased to 2
RETURN_IF_ERROR(send_to_local(_wrapper)); ^
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1033: +3, including nesting penalty of 2, nesting level increased to 3
RETURN_IF_ERROR(send_to_local(_wrapper)); ^
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1034: +1, nesting level increased to 1
be/src/exprs/runtime_filter.cpp:1035: +2, including nesting penalty of 1, nesting level increased to 2
if (_is_broadcast_join || _state->be_exec_version < USE_NEW_SERDE) { ^
be/src/exprs/runtime_filter.cpp:1035: +1
if (_is_broadcast_join || _state->be_exec_version < USE_NEW_SERDE) { ^
be/src/exprs/runtime_filter.cpp:1036: +3, including nesting penalty of 2, nesting level increased to 3
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1036: +4, including nesting penalty of 3, nesting level increased to 4
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1037: +1, nesting level increased to 2
} else { ^
be/src/exprs/runtime_filter.cpp:1038: +3, including nesting penalty of 2, nesting level increased to 3
RETURN_IF_ERROR(do_local_merge()); ^
be/src/common/status.h:632: expanded from macro 'RETURN_IF_ERROR'
do { \ ^
be/src/exprs/runtime_filter.cpp:1038: +4, including nesting penalty of 3, nesting level increased to 4
RETURN_IF_ERROR(do_local_merge()); ^
be/src/common/status.h:634: expanded from macro 'RETURN_IF_ERROR'
if (UNLIKELY(!_status_.ok())) { \ ^
be/src/exprs/runtime_filter.cpp:1040: +1, nesting level increased to 1
} else { ^