This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 756d21660b4 [improve](http) Save the requested url in http execution
error (#43855)
756d21660b4 is described below
commit 756d21660b464576101d03ae3545470826ea318c
Author: walter <[email protected]>
AuthorDate: Fri Nov 15 19:56:39 2024 +0800
[improve](http) Save the requested url in http execution error (#43855)
Save the requested URL in HTTP execution erro
---
be/src/http/http_client.cpp | 22 +++++++++++++++----
be/src/http/http_client.h | 3 ++-
be/src/olap/single_replica_compaction.cpp | 14 +++++--------
be/src/olap/single_replica_compaction.h | 1 -
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, 66 insertions(+), 28 deletions(-)
diff --git a/be/src/http/http_client.cpp b/be/src/http/http_client.cpp
index c842a4fe2dd..fc4c997fce8 100644
--- a/be/src/http/http_client.cpp
+++ b/be/src/http/http_client.cpp
@@ -27,6 +27,7 @@
#include "http/http_headers.h"
#include "http/http_status.h"
#include "runtime/exec_env.h"
+#include "util/security.h"
#include "util/stack_util.h"
namespace doris {
@@ -205,9 +206,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();
}
@@ -275,13 +278,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;
@@ -293,7 +305,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 fb692c50268..c0c8863a9b0 100644
--- a/be/src/http/http_client.h
+++ b/be/src/http/http_client.h
@@ -164,7 +164,8 @@ public:
Status _escape_url(const std::string& url, std::string* escaped_url);
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 7470afe0ef6..458f3949b17 100644
--- a/be/src/olap/single_replica_compaction.cpp
+++ b/be/src/olap/single_replica_compaction.cpp
@@ -39,6 +39,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"
@@ -373,7 +374,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="
- << _mask_token(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));
@@ -438,10 +439,10 @@ 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: "
- << _mask_token(remote_file_url) << " to: " << local_file_path
+ << 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);
@@ -453,7 +454,7 @@ 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=" <<
_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");
@@ -585,9 +586,4 @@ Status SingleReplicaCompaction::_finish_clone(const string&
clone_dir,
return res;
}
-std::string SingleReplicaCompaction::_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/single_replica_compaction.h
b/be/src/olap/single_replica_compaction.h
index 67f5527dd7b..10ec65ec3f0 100644
--- a/be/src/olap/single_replica_compaction.h
+++ b/be/src/olap/single_replica_compaction.h
@@ -62,7 +62,6 @@ private:
const std::string& local_path);
Status _release_snapshot(const std::string& ip, int port, const
std::string& snapshot_path);
Status _finish_clone(const std::string& clone_dir, const Version& version);
- std::string _mask_token(const std::string& str);
CompactionType _compaction_type;
std::vector<PendingRowsetGuard> _pending_rs_guards;
diff --git a/be/src/olap/task/engine_clone_task.cpp
b/be/src/olap/task/engine_clone_task.cpp
index fc3a69fd5cd..75cbcf68e95 100644
--- a/be/src/olap/task/engine_clone_task.cpp
+++ b/be/src/olap/task/engine_clone_task.cpp
@@ -32,7 +32,6 @@
#include <memory>
#include <mutex>
#include <ostream>
-#include <regex>
#include <set>
#include <shared_mutex>
#include <system_error>
@@ -64,6 +63,7 @@
#include "util/debug_points.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"
@@ -415,7 +415,7 @@ Status
EngineCloneTask::_make_and_download_snapshots(DataDir& data_dir,
status = _download_files(&data_dir, remote_url_prefix,
local_data_path);
if (!status.ok()) [[unlikely]] {
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);
continue; // Try another BE
}
@@ -552,11 +552,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);
@@ -572,7 +572,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");
@@ -600,7 +600,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 version,
bool is_incremental_clone) {
@@ -864,9 +864,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 9290ed9552e..a11d4c742f4 100644
--- a/be/src/olap/task/engine_clone_task.h
+++ b/be/src/olap/task/engine_clone_task.h
@@ -86,8 +86,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:
StorageEngine& _engine;
const TCloneReq& _clone_req;
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: [email protected]
For additional commands, e-mail: [email protected]
