-
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
[fix](brpc_client_cache) resolve hostname in DNS cache before passing to brpc (#40074) #40786
Conversation
… to brpc (apache#40074) Currently brpc does not support resloving IPv6 hostnames, errors will be returned on `brpc::Channel::Init`. The brpc client cache may return `nullptr` on its `get_client` or `get_new_client_no_cache` APIs. This PR made the following changes: 1. Resolve hostnames from DNS cache before passing it to brpc. 2. Callers should check nullptr after get client, in case of failures.
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
run buildall |
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.
clang-tidy made some suggestions
if (!is_valid_ip(host)) { | ||
Status status = ExecEnv::GetInstance()->dns_cache()->get(host, &realhost); | ||
std::string realhost = host; | ||
auto dns_cache = ExecEnv::GetInstance()->dns_cache(); |
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: 'auto dns_cache' can be declared as 'auto *dns_cache' [readability-qualified-auto]
auto dns_cache = ExecEnv::GetInstance()->dns_cache(); | |
auto *dns_cache = ExecEnv::GetInstance()->dns_cache(); |
@@ -71,11 +71,26 @@ Status transmit_block_httpv2(ExecEnv* exec_env, std::unique_ptr<Closure> closure | |||
TNetworkAddress brpc_dest_addr) { | |||
RETURN_IF_ERROR(request_embed_attachment_contain_blockv2(closure->request_.get(), closure)); | |||
|
|||
std::string host = brpc_dest_addr.hostname; | |||
auto dns_cache = ExecEnv::GetInstance()->dns_cache(); |
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: 'auto dns_cache' can be declared as 'auto *dns_cache' [readability-qualified-auto]
auto dns_cache = ExecEnv::GetInstance()->dns_cache(); | |
auto *dns_cache = ExecEnv::GetInstance()->dns_cache(); |
@@ -700,11 +700,30 @@ void VNodeChannel::try_send_pending_block(RuntimeState* state) { | |||
return; | |||
} | |||
|
|||
std::string host = _node_info.host; | |||
auto dns_cache = ExecEnv::GetInstance()->dns_cache(); |
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: 'auto dns_cache' can be declared as 'auto *dns_cache' [readability-qualified-auto]
auto dns_cache = ExecEnv::GetInstance()->dns_cache(); | |
auto *dns_cache = ExecEnv::GetInstance()->dns_cache(); |
TeamCity be ut coverage result: |
backport #40074