Skip to content

Commit

Permalink
[improve](http) Save the requested url in http execution error (#43855)
Browse files Browse the repository at this point in the history
Save the requested URL in HTTP execution erro
  • Loading branch information
w41ter committed Nov 18, 2024
1 parent f20aff5 commit 51ae64d
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 25 deletions.
22 changes: 18 additions & 4 deletions be/src/http/http_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion be/src/http/http_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ class HttpClient {
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;
Expand Down
12 changes: 7 additions & 5 deletions be/src/olap/single_replica_compaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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) {
Expand All @@ -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");
}
Expand Down
4 changes: 2 additions & 2 deletions be/src/olap/single_replica_compaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ class SingleReplicaCompaction : public Compaction {
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
} // namespace doris
17 changes: 6 additions & 11 deletions be/src/olap/task/engine_clone_task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include <memory>
#include <mutex>
#include <ostream>
#include <regex>
#include <set>
#include <shared_mutex>
#include <system_error>
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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");
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
2 changes: 0 additions & 2 deletions be/src/olap/task/engine_clone_task.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ class EngineCloneTask : public EngineTask {

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;
Expand Down
35 changes: 35 additions & 0 deletions be/src/util/security.h
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 51ae64d

Please sign in to comment.