This is an automated email from the ASF dual-hosted git repository.
w41ter pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 514a17e5d77 [improve](http) Save the requested url in http execution
error #43855 (#44105)
514a17e5d77 is described below
commit 514a17e5d7742c3a35f02d8ff0c3d64d6958960d
Author: walter <maoch...@selectdb.com>
AuthorDate: Mon Nov 18 15:05:57 2024 +0800
[improve](http) Save the requested url in http execution error #43855
(#44105)
cherry pick from #43855
---
be/src/http/http_client.cpp | 22 +++++++++++++++----
be/src/http/http_client.h | 3 ++-
be/src/olap/single_replica_compaction.cpp | 12 ++++++-----
be/src/olap/single_replica_compaction.h | 4 ++--
be/src/olap/task/engine_clone_task.cpp | 17 ++++++---------
be/src/olap/task/engine_clone_task.h | 2 --
be/src/util/security.h | 35 +++++++++++++++++++++++++++++++
7 files changed, 70 insertions(+), 25 deletions(-)
diff --git a/be/src/http/http_client.cpp b/be/src/http/http_client.cpp
index 3cad98e11b7..3fc2fc85a25 100644
--- a/be/src/http/http_client.cpp
+++ b/be/src/http/http_client.cpp
@@ -26,6 +26,7 @@
#include "common/config.h"
#include "http/http_headers.h"
#include "http/http_status.h"
+#include "util/security.h"
#include "util/stack_util.h"
namespace doris {
@@ -198,9 +199,11 @@ Status HttpClient::execute(const std::function<bool(const
void* data, size_t len
_callback = &callback;
auto code = curl_easy_perform(_curl);
if (code != CURLE_OK) {
+ std::string url = mask_token(_get_url());
LOG(WARNING) << "fail to execute HTTP client, errmsg=" <<
_to_errmsg(code)
- << ", trace=" << get_stack_trace();
- return Status::HttpError(_to_errmsg(code));
+ << ", trace=" << get_stack_trace() << ", url=" << url;
+ std::string errmsg = fmt::format("{}, url={}", _to_errmsg(code), url);
+ return Status::HttpError(std::move(errmsg));
}
return Status::OK();
}
@@ -268,13 +271,22 @@ Status HttpClient::execute(std::string* response) {
return execute(callback);
}
-const char* HttpClient::_to_errmsg(CURLcode code) {
+const char* HttpClient::_to_errmsg(CURLcode code) const {
if (_error_buf[0] == 0) {
return curl_easy_strerror(code);
}
return _error_buf;
}
+const char* HttpClient::_get_url() const {
+ const char* url = nullptr;
+ curl_easy_getinfo(_curl, CURLINFO_EFFECTIVE_URL, &url);
+ if (!url) {
+ url = "<unknown>";
+ }
+ return url;
+}
+
Status HttpClient::execute_with_retry(int retry_times, int sleep_time,
const
std::function<Status(HttpClient*)>& callback) {
Status status;
@@ -286,7 +298,9 @@ Status HttpClient::execute_with_retry(int retry_times, int
sleep_time,
if (http_status == 200) {
return status;
} else {
- auto error_msg = fmt::format("http status code is not 200,
code={}", http_status);
+ std::string url = mask_token(client._get_url());
+ auto error_msg = fmt::format("http status code is not 200,
code={}, url={}",
+ http_status, url);
LOG(WARNING) << error_msg;
return Status::HttpError(error_msg);
}
diff --git a/be/src/http/http_client.h b/be/src/http/http_client.h
index a6ab49c1e8a..a5fe2d3f48a 100644
--- a/be/src/http/http_client.h
+++ b/be/src/http/http_client.h
@@ -147,7 +147,8 @@ public:
size_t on_response_data(const void* data, size_t length);
private:
- const char* _to_errmsg(CURLcode code);
+ const char* _to_errmsg(CURLcode code) const;
+ const char* _get_url() const;
private:
CURL* _curl = nullptr;
diff --git a/be/src/olap/single_replica_compaction.cpp
b/be/src/olap/single_replica_compaction.cpp
index fdccc78816f..c190796f6bc 100644
--- a/be/src/olap/single_replica_compaction.cpp
+++ b/be/src/olap/single_replica_compaction.cpp
@@ -37,6 +37,7 @@
#include "task/engine_clone_task.h"
#include "util/brpc_client_cache.h"
#include "util/doris_metrics.h"
+#include "util/security.h"
#include "util/thrift_rpc_helper.h"
#include "util/trace.h"
@@ -390,7 +391,7 @@ Status SingleReplicaCompaction::_download_files(DataDir*
data_dir,
// then it will try to clone from BE 2, but it will find the file 1
already exist, but file 1 with same
// name may have different versions.
VLOG_DEBUG << "single replica compaction begin to download files, remote
path="
- << remote_url_prefix << " local_path=" << local_path;
+ << mask_token(remote_url_prefix) << " local_path=" <<
local_path;
RETURN_IF_ERROR(io::global_local_filesystem()->delete_directory(local_path));
RETURN_IF_ERROR(io::global_local_filesystem()->create_directory(local_path));
@@ -448,9 +449,9 @@ Status SingleReplicaCompaction::_download_files(DataDir*
data_dir,
std::string local_file_path = local_path + file_name;
- LOG(INFO) << "single replica compaction begin to download file from: "
<< remote_file_url
- << " to: " << local_file_path << ". size(B): " << file_size
- << ", timeout(s): " << estimate_timeout;
+ LOG(INFO) << "single replica compaction begin to download file from: "
+ << mask_token(remote_file_url) << " to: " << local_file_path
+ << ". size(B): " << file_size << ", timeout(s): " <<
estimate_timeout;
auto download_cb = [&remote_file_url, estimate_timeout,
&local_file_path,
file_size](HttpClient* client) {
@@ -462,7 +463,8 @@ Status SingleReplicaCompaction::_download_files(DataDir*
data_dir,
uint64_t local_file_size =
std::filesystem::file_size(local_file_path);
if (local_file_size != file_size) {
LOG(WARNING) << "download file length error"
- << ", remote_path=" << remote_file_url << ",
file_size=" << file_size
+ << ", remote_path=" << mask_token(remote_file_url)
+ << ", file_size=" << file_size
<< ", local_file_size=" << local_file_size;
return Status::InternalError("downloaded file size is not
equal");
}
diff --git a/be/src/olap/single_replica_compaction.h
b/be/src/olap/single_replica_compaction.h
index ae013b3748d..a51954ac93e 100644
--- a/be/src/olap/single_replica_compaction.h
+++ b/be/src/olap/single_replica_compaction.h
@@ -58,10 +58,10 @@ private:
Status _download_files(DataDir* data_dir, const std::string&
remote_url_prefix,
const std::string& local_path);
Status _release_snapshot(const std::string& ip, int port, const
std::string& snapshot_path);
- Status _finish_clone(const string& clone_dir, const Version& version);
+ Status _finish_clone(const std::string& clone_dir, const Version& version);
CompactionType _compaction_type;
DISALLOW_COPY_AND_ASSIGN(SingleReplicaCompaction);
};
-} // namespace doris
\ No newline at end of file
+} // namespace doris
diff --git a/be/src/olap/task/engine_clone_task.cpp
b/be/src/olap/task/engine_clone_task.cpp
index 0b077076e57..ad9196f5a5e 100644
--- a/be/src/olap/task/engine_clone_task.cpp
+++ b/be/src/olap/task/engine_clone_task.cpp
@@ -31,7 +31,6 @@
#include <memory>
#include <mutex>
#include <ostream>
-#include <regex>
#include <set>
#include <shared_mutex>
#include <system_error>
@@ -63,6 +62,7 @@
#include "runtime/thread_context.h"
#include "util/defer_op.h"
#include "util/network_util.h"
+#include "util/security.h"
#include "util/stopwatch.hpp"
#include "util/thrift_rpc_helper.h"
#include "util/trace.h"
@@ -410,7 +410,7 @@ Status
EngineCloneTask::_make_and_download_snapshots(DataDir& data_dir,
_clone_req.table_id, _clone_req.partition_id,
_clone_req.schema_hash);
} else {
LOG_WARNING("failed to download snapshot from remote BE")
- .tag("url", _mask_token(remote_url_prefix))
+ .tag("url", mask_token(remote_url_prefix))
.error(status);
}
@@ -554,11 +554,11 @@ Status EngineCloneTask::_download_files(DataDir*
data_dir, const std::string& re
std::string local_file_path = local_path + "/" + file_name;
- LOG(INFO) << "clone begin to download file from: " <<
_mask_token(remote_file_url)
+ LOG(INFO) << "clone begin to download file from: " <<
mask_token(remote_file_url)
<< " to: " << local_file_path << ". size(B): " << file_size
<< ", timeout(s): " << estimate_timeout;
- auto download_cb = [this, &remote_file_url, estimate_timeout,
&local_file_path,
+ auto download_cb = [&remote_file_url, estimate_timeout,
&local_file_path,
file_size](HttpClient* client) {
RETURN_IF_ERROR(client->init(remote_file_url));
client->set_timeout_ms(estimate_timeout * 1000);
@@ -574,7 +574,7 @@ Status EngineCloneTask::_download_files(DataDir* data_dir,
const std::string& re
}
if (local_file_size != file_size) {
LOG(WARNING) << "download file length error"
- << ", remote_path=" <<
_mask_token(remote_file_url)
+ << ", remote_path=" << mask_token(remote_file_url)
<< ", file_size=" << file_size
<< ", local_file_size=" << local_file_size;
return Status::InternalError("downloaded file size is not
equal");
@@ -602,7 +602,7 @@ Status EngineCloneTask::_download_files(DataDir* data_dir,
const std::string& re
/// This method will only be called if tablet already exist in this BE when
doing clone.
/// This method will do the following things:
-/// 1. Linke all files from CLONE dir to tablet dir if file does not exist in
tablet dir
+/// 1. Link all files from CLONE dir to tablet dir if file does not exist in
tablet dir
/// 2. Call _finish_xx_clone() to revise the tablet meta.
Status EngineCloneTask::_finish_clone(Tablet* tablet, const std::string&
clone_dir,
int64_t committed_version, bool
is_incremental_clone) {
@@ -867,9 +867,4 @@ Status EngineCloneTask::_finish_full_clone(Tablet* tablet,
// TODO(plat1ko): write cooldown meta to remote if this replica is
cooldown replica
}
-std::string EngineCloneTask::_mask_token(const std::string& str) {
- std::regex pattern("token=[\\w|-]+");
- return regex_replace(str, pattern, "token=******");
-}
-
} // namespace doris
diff --git a/be/src/olap/task/engine_clone_task.h
b/be/src/olap/task/engine_clone_task.h
index 08afc47283a..da8f824d870 100644
--- a/be/src/olap/task/engine_clone_task.h
+++ b/be/src/olap/task/engine_clone_task.h
@@ -85,8 +85,6 @@ private:
Status _release_snapshot(const std::string& ip, int port, const
std::string& snapshot_path);
- std::string _mask_token(const std::string& str);
-
private:
const TCloneReq& _clone_req;
vector<TTabletInfo>* _tablet_infos;
diff --git a/be/src/util/security.h b/be/src/util/security.h
new file mode 100644
index 00000000000..d2201b1b297
--- /dev/null
+++ b/be/src/util/security.h
@@ -0,0 +1,35 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <regex>
+#include <string>
+
+namespace doris {
+
+inline std::string mask_token(const std::string& str) {
+ std::regex pattern("token=[\\w|-]+");
+ return std::regex_replace(str, pattern, "token=******");
+}
+
+inline std::string mask_token(const char* str) {
+ std::regex pattern("token=[\\w|-]+");
+ return std::regex_replace(str, pattern, "token=******");
+}
+
+} // namespace doris
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
