This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 32cbd4a583449d2fae3fcf2f59a995f454b9e679
Author: yiguolei <676222...@qq.com>
AuthorDate: Tue May 7 19:45:38 2024 +0800
[chore](status) unify error code between thrift,pb, status.h (#34397)
Co-authored-by: yiguolei <yiguo...@gmail.com>
---
be/src/common/status.cpp | 6 +-
be/src/common/status.h | 111 ++++++++++++++++-----
be/src/service/doris_main.cpp | 5 +-
be/test/common/status_test.cpp | 38 +++++++
gensrc/thrift/Status.thrift | 41 ++++----
.../hive/ddl/test_hive_write_type.groovy | 2 +
6 files changed, 157 insertions(+), 46 deletions(-)
diff --git a/be/src/common/status.cpp b/be/src/common/status.cpp
index 7741aaa58df..d17e18951c5 100644
--- a/be/src/common/status.cpp
+++ b/be/src/common/status.cpp
@@ -20,7 +20,7 @@ namespace doris {
namespace ErrorCode {
ErrorCodeState error_states[MAX_ERROR_CODE_DEFINE_NUM];
-ErrorCodeInitializer error_code_init;
+ErrorCodeInitializer error_code_init(10);
} // namespace ErrorCode
void Status::to_thrift(TStatus* s) const {
@@ -29,6 +29,10 @@ void Status::to_thrift(TStatus* s) const {
s->status_code = TStatusCode::OK;
return;
}
+ // Currently, there are many error codes, not defined in thrift and need
pass to FE.
+ // DCHECK(_code > 0)
+ // << "The error code has to > 0 because TStatusCode need it > 0,
it's actual value is "
+ // << _code;
s->status_code = (int16_t)_code > 0 ? (TStatusCode::type)_code :
TStatusCode::INTERNAL_ERROR;
s->error_msgs.push_back(fmt::format("({})[{}]{}",
BackendOptions::get_localhost(),
code_as_string(), _err_msg ?
_err_msg->_msg : ""));
diff --git a/be/src/common/status.h b/be/src/common/status.h
index 2c3b462b645..58bd7270b89 100644
--- a/be/src/common/status.h
+++ b/be/src/common/status.h
@@ -30,6 +30,7 @@ class PStatus;
namespace ErrorCode {
// E thrift_error_name, print_stacktrace
+<<<<<<< HEAD
#define APPLY_FOR_THRIFT_ERROR_CODES(TStatusError) \
TStatusError(PUBLISH_TIMEOUT, false); \
TStatusError(MEM_ALLOC_FAILED, true); \
@@ -56,31 +57,51 @@ namespace ErrorCode {
TStatusError(LABEL_ALREADY_EXISTS, true); \
TStatusError(NOT_AUTHORIZED, true); \
TStatusError(HTTP_ERROR, true);
+=======
+#define APPLY_FOR_THRIFT_ERROR_CODES(TStatusError) \
+ TStatusError(PUBLISH_TIMEOUT, false); \
+ TStatusError(MEM_ALLOC_FAILED, true); \
+ TStatusError(BUFFER_ALLOCATION_FAILED, true); \
+ TStatusError(INVALID_ARGUMENT, false); \
+ TStatusError(INVALID_JSON_PATH, false); \
+ TStatusError(MINIMUM_RESERVATION_UNAVAILABLE, true); \
+ TStatusError(CORRUPTION, true); \
+ TStatusError(IO_ERROR, true); \
+ TStatusError(NOT_FOUND, true); \
+ TStatusError(ALREADY_EXIST, true); \
+ TStatusError(NOT_IMPLEMENTED_ERROR, true); \
+ TStatusError(END_OF_FILE, false); \
+ TStatusError(INTERNAL_ERROR, true); \
+ TStatusError(RUNTIME_ERROR, true); \
+ TStatusError(CANCELLED, false); \
+ TStatusError(ANALYSIS_ERROR, false); \
+ TStatusError(MEM_LIMIT_EXCEEDED, false); \
+ TStatusError(THRIFT_RPC_ERROR, true); \
+ TStatusError(TIMEOUT, true); \
+ TStatusError(TOO_MANY_TASKS, true); \
+ TStatusError(UNINITIALIZED, false); \
+ TStatusError(INCOMPLETE, false); \
+ TStatusError(OLAP_ERR_VERSION_ALREADY_MERGED, false); \
+ TStatusError(ABORTED, true); \
+ TStatusError(DATA_QUALITY_ERROR, false); \
+ TStatusError(LABEL_ALREADY_EXISTS, true); \
+ TStatusError(NOT_AUTHORIZED, true); \
+ TStatusError(BINLOG_DISABLE, false); \
+ TStatusError(BINLOG_TOO_OLD_COMMIT_SEQ, false); \
+ TStatusError(BINLOG_TOO_NEW_COMMIT_SEQ, false); \
+ TStatusError(BINLOG_NOT_FOUND_DB, false); \
+ TStatusError(BINLOG_NOT_FOUND_TABLE, false); \
+ TStatusError(NETWORK_ERROR, false); \
+ TStatusError(ILLEGAL_STATE, false); \
+ TStatusError(SNAPSHOT_NOT_EXIST, true); \
+ TStatusError(HTTP_ERROR, true); \
+ TStatusError(TABLET_MISSING, true); \
+ TStatusError(NOT_MASTER, true); \
+ TStatusError(DELETE_BITMAP_LOCK_ERROR, false);
+>>>>>>> 91701ddadd ([chore](status) unify error code between thrift,pb,
status.h (#34397))
// E error_name, error_code, print_stacktrace
#define APPLY_FOR_OLAP_ERROR_CODES(E) \
E(OK, 0, false); \
- E(OS_ERROR, -100, true); \
- E(DIR_NOT_EXIST, -101, true); \
- E(FILE_NOT_EXIST, -102, true); \
- E(CREATE_FILE_ERROR, -103, true); \
- E(STL_ERROR, -105, true); \
- E(MUTEX_ERROR, -107, true); \
- E(PTHREAD_ERROR, -108, true); \
- E(NETWORK_ERROR, -109, true); \
- E(UB_FUNC_ERROR, -110, true); \
- E(COMPRESS_ERROR, -111, true); \
- E(DECOMPRESS_ERROR, -112, true); \
- E(UNKNOWN_COMPRESSION_TYPE, -113, true); \
- E(MMAP_ERROR, -114, true); \
- E(CANNOT_CREATE_DIR, -117, true); \
- E(UB_NETWORK_ERROR, -118, true); \
- E(FILE_FORMAT_ERROR, -119, true); \
- E(EVAL_CONJUNCTS_ERROR, -120, true); \
- E(COPY_FILE_ERROR, -121, true); \
- E(FILE_ALREADY_EXIST, -122, true); \
- E(BAD_CAST, -123, true); \
- E(ARITHMETIC_OVERFLOW_ERRROR, -124, false); \
- E(PERMISSION_DENIED, -125, false); \
E(CALL_SEQUENCE_ERROR, -202, true); \
E(BUFFER_OVERFLOW, -204, true); \
E(CONFIG_ERROR, -205, true); \
@@ -117,6 +138,20 @@ namespace ErrorCode {
E(ALREADY_CLOSED, -239, false); \
E(SERVICE_UNAVAILABLE, -240, true); \
E(NEED_SEND_AGAIN, -241, false); \
+ E(OS_ERROR, -242, true); \
+ E(DIR_NOT_EXIST, -243, true); \
+ E(FILE_NOT_EXIST, -244, true); \
+ E(CREATE_FILE_ERROR, -245, true); \
+ E(STL_ERROR, -246, true); \
+ E(MUTEX_ERROR, -247, true); \
+ E(PTHREAD_ERROR, -248, true); \
+ E(UB_FUNC_ERROR, -250, true); \
+ E(COMPRESS_ERROR, -251, true); \
+ E(DECOMPRESS_ERROR, -252, true); \
+ E(FILE_ALREADY_EXIST, -253, true); \
+ E(BAD_CAST, -254, true); \
+ E(ARITHMETIC_OVERFLOW_ERRROR, -255, false); \
+ E(PERMISSION_DENIED, -256, false); \
E(CE_CMD_PARAMS_ERROR, -300, true); \
E(CE_BUFFER_TOO_SMALL, -301, true); \
E(CE_CMD_NOT_VALID, -302, true); \
@@ -298,15 +333,39 @@ extern ErrorCodeState
error_states[MAX_ERROR_CODE_DEFINE_NUM];
class ErrorCodeInitializer {
public:
- ErrorCodeInitializer() {
-#define M(NAME, ENABLESTACKTRACE) error_states[TStatusCode::NAME].stacktrace =
ENABLESTACKTRACE;
+ ErrorCodeInitializer(int temp) : signal_value(temp) {
+ for (int i = 0; i < MAX_ERROR_CODE_DEFINE_NUM; ++i) {
+ error_states[i].error_code = 0;
+ }
+#define M(NAME, ENABLESTACKTRACE) \
+ error_states[TStatusCode::NAME].stacktrace = ENABLESTACKTRACE; \
+ error_states[TStatusCode::NAME].description = #NAME; \
+ error_states[TStatusCode::NAME].error_code = TStatusCode::NAME;
APPLY_FOR_THRIFT_ERROR_CODES(M)
#undef M
-#define M(NAME, ERRORCODE, ENABLESTACKTRACE) \
- error_states[abs(ERRORCODE)].stacktrace = ENABLESTACKTRACE;
+// In status.h, if error code > 0, then it means it will be used in
TStatusCode and will
+// also be used in FE.
+// Other error codes that with error code < 0, will only be used in BE.
+// We use abs(error code) as the index in error_states, so that these two
kinds of error
+// codes MUST not have overlap.
+// Add an assert here to make sure the code in TStatusCode and other error
code are not
+// overlapped.
+#define M(NAME, ERRORCODE, ENABLESTACKTRACE) \
+ assert(error_states[abs(ERRORCODE)].error_code == 0); \
+ error_states[abs(ERRORCODE)].stacktrace = ENABLESTACKTRACE; \
+ error_states[abs(ERRORCODE)].error_code = ERRORCODE;
APPLY_FOR_OLAP_ERROR_CODES(M)
#undef M
}
+
+ void check_init() {
+ //the signal value is 0, it means the global error states not inited,
it's logical error
+ // DO NOT use dcheck here, because dcheck depend on glog, and glog
maybe not inited at this time.
+ assert(signal_value != 0);
+ }
+
+private:
+ int signal_value = 0;
};
extern ErrorCodeInitializer error_code_init;
diff --git a/be/src/service/doris_main.cpp b/be/src/service/doris_main.cpp
index 00aa4761467..38b8ba21d77 100644
--- a/be/src/service/doris_main.cpp
+++ b/be/src/service/doris_main.cpp
@@ -308,7 +308,10 @@ int main(int argc, char** argv) {
doris::signal::InstallFailureSignalHandler();
// create StackTraceCache Instance, at the beginning, other static
destructors may use.
StackTrace::createCache();
-
+ extern doris::ErrorCode::ErrorCodeInitializer error_code_init;
+ // Some developers will modify status.h and we use a very ticky logic to
init error_states
+ // and it maybe not inited. So add a check here.
+ doris::ErrorCode::error_code_init.check_init();
// check if print version or help
if (argc > 1) {
if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-v") == 0) {
diff --git a/be/test/common/status_test.cpp b/be/test/common/status_test.cpp
index 0371346e3cd..c1197dad0b1 100644
--- a/be/test/common/status_test.cpp
+++ b/be/test/common/status_test.cpp
@@ -46,6 +46,44 @@ TEST_F(StatusTest, OK) {
}
}
+// This test is used to make sure TStatusCode is defined in status.h
+TEST_F(StatusTest, TStatusCodeWithStatus) {
+ // The definition in status.h
+ //extern ErrorCode::ErrorCodeState
error_states[ErrorCode::MAX_ERROR_CODE_DEFINE_NUM];
+ extern ErrorCode::ErrorCodeState error_states;
+ extern ErrorCode::ErrorCodeInitializer error_code_init;
+ // The definition in Status_types.h
+ extern const std::map<int, const char*> _TStatusCode_VALUES_TO_NAMES;
+ ErrorCode::error_code_init.check_init();
+ // Check if all code in status.h with error_code > 0, is in Status_types.h
+ for (int i = 0; i < ErrorCode::MAX_ERROR_CODE_DEFINE_NUM; ++i) {
+ //std::cout << "begin to check number " << i << ", error status "
+ // << ErrorCode::error_states[i].error_code << std::endl;
+ if (ErrorCode::error_states[i].error_code > 0) {
+ //std::cout << "Status info " <<
ErrorCode::error_states[i].error_code << ","
+ // << ErrorCode::error_states[i].description << ","
+ // << ::doris::_TStatusCode_VALUES_TO_NAMES.at(i) <<
std::endl;
+ EXPECT_TRUE(::doris::_TStatusCode_VALUES_TO_NAMES.find(i) !=
+ ::doris::_TStatusCode_VALUES_TO_NAMES.end());
+ // also check name is equal
+ EXPECT_TRUE(ErrorCode::error_states[i].description.compare(
+ ::doris::_TStatusCode_VALUES_TO_NAMES.at(i))
== 0);
+ }
+ }
+ // Check all code in Status_types.h in status.h
+ for (auto& tstatus_st : _TStatusCode_VALUES_TO_NAMES) {
+ // std::cout << "TStatusCode info " << tstatus_st.first << "," <<
tstatus_st.second
+ // << std::endl;
+ if (tstatus_st.first < 1) {
+ // OK with error code == 0, is not defined with tstatus, ignore it.
+ continue;
+ }
+ EXPECT_TRUE(tstatus_st.first < ErrorCode::MAX_ERROR_CODE_DEFINE_NUM);
+
EXPECT_TRUE(ErrorCode::error_states[tstatus_st.first].description.compare(
+ tstatus_st.second) == 0);
+ }
+}
+
TEST_F(StatusTest, Error) {
// default
Status st = Status::InternalError("123");
diff --git a/gensrc/thrift/Status.thrift b/gensrc/thrift/Status.thrift
index cb80350fb6e..c377634df65 100644
--- a/gensrc/thrift/Status.thrift
+++ b/gensrc/thrift/Status.thrift
@@ -43,20 +43,20 @@ enum TStatusCode {
INTERNAL_ERROR = 6,
THRIFT_RPC_ERROR = 7,
TIMEOUT = 8,
- KUDU_NOT_ENABLED = 9, // Deprecated
- KUDU_NOT_SUPPORTED_ON_OS = 10, // Deprecated
+ //KUDU_NOT_ENABLED = 9, // Deprecated
+ //KUDU_NOT_SUPPORTED_ON_OS = 10, // Deprecated
MEM_ALLOC_FAILED = 11,
BUFFER_ALLOCATION_FAILED = 12,
MINIMUM_RESERVATION_UNAVAILABLE = 13,
PUBLISH_TIMEOUT = 14,
LABEL_ALREADY_EXISTS = 15,
TOO_MANY_TASKS = 16,
- ES_INTERNAL_ERROR = 17,
- ES_INDEX_NOT_FOUND = 18,
- ES_SHARD_NOT_FOUND = 19,
- ES_INVALID_CONTEXTID = 20,
- ES_INVALID_OFFSET = 21,
- ES_REQUEST_ERROR = 22,
+ //ES_INTERNAL_ERROR = 17,
+ //ES_INDEX_NOT_FOUND = 18,
+ //ES_SHARD_NOT_FOUND = 19,
+ //ES_INVALID_CONTEXTID = 20,
+ //ES_INVALID_OFFSET = 21,
+ //ES_REQUEST_ERROR = 22,
END_OF_FILE = 30,
NOT_FOUND = 31,
@@ -68,23 +68,23 @@ enum TStatusCode {
ILLEGAL_STATE = 37,
NOT_AUTHORIZED = 38,
ABORTED = 39,
- REMOTE_ERROR = 40,
+ //REMOTE_ERROR = 40,
//SERVICE_UNAVAILABLE = 41, // Not used any more
UNINITIALIZED = 42,
- CONFIGURATION_ERROR = 43,
+ //CONFIGURATION_ERROR = 43,
INCOMPLETE = 44,
OLAP_ERR_VERSION_ALREADY_MERGED = 45,
DATA_QUALITY_ERROR = 46,
INVALID_JSON_PATH = 47,
- VEC_EXCEPTION = 50,
- VEC_LOGIC_ERROR = 51,
- VEC_ILLEGAL_DIVISION = 52,
- VEC_BAD_CAST = 53,
- VEC_CANNOT_ALLOCATE_MEMORY = 54,
- VEC_CANNOT_MUNMAP = 55,
- VEC_CANNOT_MREMAP = 56,
- VEC_BAD_ARGUMENTS = 57,
+ //VEC_EXCEPTION = 50,
+ //VEC_LOGIC_ERROR = 51,
+ //VEC_ILLEGAL_DIVISION = 52,
+ //VEC_BAD_CAST = 53,
+ //VEC_CANNOT_ALLOCATE_MEMORY = 54,
+ //VEC_CANNOT_MUNMAP = 55,
+ //VEC_CANNOT_MREMAP = 56,
+ //VEC_BAD_ARGUMENTS = 57,
// Binlog Related from 60
BINLOG_DISABLE = 60,
@@ -102,6 +102,11 @@ enum TStatusCode {
TABLET_MISSING = 72,
NOT_MASTER = 73,
+
+ // used for cloud
+ DELETE_BITMAP_LOCK_ERROR = 100,
+ // Not be larger than 200, see status.h
+ // And all error code defined here, should also be defined in status.h
}
struct TStatus {
diff --git
a/regression-test/suites/external_table_p0/hive/ddl/test_hive_write_type.groovy
b/regression-test/suites/external_table_p0/hive/ddl/test_hive_write_type.groovy
index 2a524e18a04..5b8870d4b71 100644
---
a/regression-test/suites/external_table_p0/hive/ddl/test_hive_write_type.groovy
+++
b/regression-test/suites/external_table_p0/hive/ddl/test_hive_write_type.groovy
@@ -195,6 +195,7 @@ suite("test_hive_write_type",
"p0,external,hive,external_docker,external_docker_
(true, 123, 987654321099, 'abcdefghij', 3.1214, 63.28,
123.4567, 'varcharval', 'stringval');
"""
} catch (Exception e) {
+ log.info(e.getMessage())
// BE err msg need use string contains to check
assertTrue(e.getMessage().contains("[E-124]Arithmetic
overflow, convert failed from 1234567, expected data is [-999999, 999999]"))
}
@@ -206,6 +207,7 @@ suite("test_hive_write_type",
"p0,external,hive,external_docker,external_docker_
('1', 123, 987654319, 'abcdefghij', '3.15', '6.28',
123.4567, 432, 'stringval');
"""
} catch (Exception e) {
+ log.info(e.getMessage())
assertTrue(e.getMessage().contains("[E-124]Arithmetic
overflow, convert failed from 1234567, expected data is [-999999, 999999]"))
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
