Skip to content

[chore](status) unify error code between thrift,pb, status.h #34397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yiguolei merged 8 commits into from
May 7, 2024
Merged

[chore](status) unify error code between thrift,pb, status.h #34397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fdcfa95
[status](checker) add check for tstatus and status
Doris-Extras May 2, 2024
ad3704d
f
Doris-Extras May 5, 2024
1f78cc3
f
Doris-Extras May 5, 2024
ac60a08
f
Doris-Extras May 5, 2024
b2f1710
f
Doris-Extras May 5, 2024
5db3657
f
Doris-Extras May 6, 2024
47ce3be
f
Doris-Extras May 6, 2024
22fbd98
f
Doris-Extras May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion be/src/common/status.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 : ""));
Expand Down
133 changes: 81 additions & 52 deletions be/src/common/status.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,58 +30,49 @@ class PStatus;
namespace ErrorCode {

// E thrift_error_name, print_stacktrace
#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(MEM_LIMIT_EXCEEDED, false); \
TStatusError(THRIFT_RPC_ERROR, true); \
TStatusError(TIMEOUT, true); \
TStatusError(TOO_MANY_TASKS, true); \
TStatusError(UNINITIALIZED, false); \
TStatusError(ABORTED, true); \
TStatusError(DATA_QUALITY_ERROR, false); \
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);
// 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); \
Expand Down Expand Up @@ -118,6 +109,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); \
Expand Down Expand Up @@ -301,15 +306,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;
Expand Down
5 changes: 4 additions & 1 deletion be/src/service/doris_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,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) {
Expand Down
38 changes: 38 additions & 0 deletions be/test/common/status_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
38 changes: 20 additions & 18 deletions gensrc/thrift/Status.thrift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -105,6 +105,8 @@ enum TStatusCode {

// 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 {
Expand Down
2 changes: 2 additions & 0 deletions regression-test/suites/external_table_p0/hive/ddl/test_hive_write_type.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]"))
}
Expand All @@ -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]"))
}

Expand Down