You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by kx...@apache.org on 2023/07/20 14:37:25 UTC
[doris] 07/25: [Enhancement](http) Add HttpError to HttpClient::execute_with_retry (#21989)
This is an automated email from the ASF dual-hosted git repository.
kxiao pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
commit dbfed0056fcba35893bf7cc99631356f5cd7f69c
Author: Jack Drogon <ja...@gmail.com>
AuthorDate: Thu Jul 20 10:43:05 2023 +0800
[Enhancement](http) Add HttpError to HttpClient::execute_with_retry (#21989)
---
be/src/common/status.h | 2 ++
be/src/http/action/download_binlog_action.cpp | 33 +++++++++++++++++----------
be/src/http/http_client.cpp | 10 +++++++-
gensrc/thrift/Status.thrift | 3 +++
4 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/be/src/common/status.h b/be/src/common/status.h
index 1ccf5bec5f..b2d5cf7d67 100644
--- a/be/src/common/status.h
+++ b/be/src/common/status.h
@@ -57,6 +57,7 @@ TStatusError(ABORTED);
TStatusError(DATA_QUALITY_ERROR);
TStatusError(LABEL_ALREADY_EXISTS);
TStatusError(NOT_AUTHORIZED);
+TStatusError(HTTP_ERROR);
#undef TStatusError
// BE internal errors
E(OS_ERROR, -100);
@@ -400,6 +401,7 @@ public:
ERROR_CTOR(Aborted, ABORTED)
ERROR_CTOR(DataQualityError, DATA_QUALITY_ERROR)
ERROR_CTOR(NotAuthorized, NOT_AUTHORIZED)
+ ERROR_CTOR(HttpError, HTTP_ERROR)
#undef ERROR_CTOR
template <int code>
diff --git a/be/src/http/action/download_binlog_action.cpp b/be/src/http/action/download_binlog_action.cpp
index 1e2ea0e36f..7548328a83 100644
--- a/be/src/http/action/download_binlog_action.cpp
+++ b/be/src/http/action/download_binlog_action.cpp
@@ -77,10 +77,19 @@ void handle_get_binlog_info(HttpRequest* req) {
auto tablet = get_tablet(tablet_id);
const auto& [rowset_id, num_segments] = tablet->get_binlog_info(binlog_version);
- auto binlog_info_msg = fmt::format("{}:{}", rowset_id, num_segments);
- HttpChannel::send_reply(req, binlog_info_msg);
+ if (rowset_id.empty()) {
+ HttpChannel::send_reply(
+ req, HttpStatus::NOT_FOUND,
+ fmt::format("get binlog info failed, binlog_version={}", binlog_version));
+ } else if (num_segments < 0) {
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR,
+ fmt::format("invalid num_segments: {}", num_segments));
+ } else {
+ auto binlog_info_msg = fmt::format("{}:{}", rowset_id, num_segments);
+ HttpChannel::send_reply(req, binlog_info_msg);
+ }
} catch (const std::exception& e) {
- HttpChannel::send_reply(req, e.what());
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR, e.what());
LOG(WARNING) << "get binlog info failed, error: " << e.what();
return;
}
@@ -97,7 +106,7 @@ void handle_get_segment_file(HttpRequest* req) {
const auto& segment_index = get_http_param(req, kSegmentIndexParameter);
segment_file_path = tablet->get_segment_filepath(rowset_id, segment_index);
} catch (const std::exception& e) {
- HttpChannel::send_reply(req, e.what());
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR, e.what());
LOG(WARNING) << "get download file path failed, error: " << e.what();
return;
}
@@ -107,12 +116,12 @@ void handle_get_segment_file(HttpRequest* req) {
bool exists = false;
Status status = io::global_local_filesystem()->exists(segment_file_path, &exists);
if (!status.ok()) {
- HttpChannel::send_reply(req, status.to_string());
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR, status.to_string());
LOG(WARNING) << "check file exists failed, error: " << status.to_string();
return;
}
if (!exists) {
- HttpChannel::send_reply(req, "file not exist.");
+ HttpChannel::send_reply(req, HttpStatus::NOT_FOUND, "file not exist.");
LOG(WARNING) << "file not exist, file path: " << segment_file_path;
return;
}
@@ -127,14 +136,13 @@ void handle_get_rowset_meta(HttpRequest* req) {
const auto& binlog_version = get_http_param(req, kBinlogVersionParameter);
auto rowset_meta = tablet->get_binlog_rowset_meta(binlog_version, rowset_id);
if (rowset_meta.empty()) {
- // TODO(Drogon): send error
- HttpChannel::send_reply(req,
+ HttpChannel::send_reply(req, HttpStatus::NOT_FOUND,
fmt::format("get rowset meta failed, rowset_id={}", rowset_id));
} else {
HttpChannel::send_reply(req, rowset_meta);
}
} catch (const std::exception& e) {
- HttpChannel::send_reply(req, e.what());
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR, e.what());
LOG(WARNING) << "get download file path failed, error: " << e.what();
}
}
@@ -147,7 +155,8 @@ void DownloadBinlogAction::handle(HttpRequest* req) {
VLOG_CRITICAL << "accept one download binlog request " << req->debug_string();
if (!config::enable_feature_binlog) {
- HttpChannel::send_reply(req, "binlog feature is not enabled.");
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR,
+ "binlog feature is not enabled.");
return;
}
@@ -157,7 +166,7 @@ void DownloadBinlogAction::handle(HttpRequest* req) {
// FIXME(Drogon): support check token
// status = _check_token(req);
if (!status.ok()) {
- HttpChannel::send_reply(req, status.to_string());
+ HttpChannel::send_reply(req, HttpStatus::UNAUTHORIZED, status.to_string());
return;
}
}
@@ -175,7 +184,7 @@ void DownloadBinlogAction::handle(HttpRequest* req) {
} else {
auto error_msg = fmt::format("invalid method: {}", method);
LOG(WARNING) << error_msg;
- HttpChannel::send_reply(req, error_msg);
+ HttpChannel::send_reply(req, HttpStatus::NOT_IMPLEMENTED, error_msg);
}
}
diff --git a/be/src/http/http_client.cpp b/be/src/http/http_client.cpp
index 0919d21d38..9c3298632c 100644
--- a/be/src/http/http_client.cpp
+++ b/be/src/http/http_client.cpp
@@ -24,6 +24,7 @@
#include <ostream>
#include "common/config.h"
+#include "http/http_status.h"
#include "util/stack_util.h"
namespace doris {
@@ -229,7 +230,14 @@ Status HttpClient::execute_with_retry(int retry_times, int sleep_time,
HttpClient client;
status = callback(&client);
if (status.ok()) {
- return status;
+ auto http_status = static_cast<HttpStatus>(client.get_http_status());
+ if (http_status == 200) {
+ return status;
+ } else {
+ auto error_msg = fmt::format("http status code is not 200, status={}", http_status);
+ LOG(WARNING) << error_msg;
+ return Status::HttpError(error_msg);
+ }
}
sleep(sleep_time);
}
diff --git a/gensrc/thrift/Status.thrift b/gensrc/thrift/Status.thrift
index 0342f9bce0..0d1931d8ec 100644
--- a/gensrc/thrift/Status.thrift
+++ b/gensrc/thrift/Status.thrift
@@ -94,6 +94,9 @@ enum TStatusCode {
// Snapshot Related from 70
SNAPSHOT_NOT_EXIST = 70,
+
+ // BE Status HTTP_ERROR
+ HTTP_ERROR = 71,
}
struct TStatus {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org