From 1fe4ea4c7cafda31d2e5fd8f076ed0f95bf06718 Mon Sep 17 00:00:00 2001 From: yiguolei Date: Sun, 10 Apr 2022 00:16:43 +0800 Subject: [PATCH] [Refactor-step1] Add OLAPInternalError to status (#8900) --- be/CMakeLists.txt | 4 ++++ be/src/common/signal_handler.h | 1 - be/src/common/status.cpp | 14 +++++++---- be/src/common/status.h | 43 ++++++++++++++++++++++++++++++---- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt index 5e076c310f..174df0ec35 100644 --- a/be/CMakeLists.txt +++ b/be/CMakeLists.txt @@ -49,6 +49,10 @@ elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang") set (COMPILER_CLANG 1) endif () +# Set boost/stacktrace use backtrace api to unwind +add_definitions(-DBOOST_STACKTRACE_USE_BACKTRACE) +add_definitions(-DPRINT_ALL_ERR_STATUS_STACKTRACE) + option(GLIBC_COMPATIBILITY "Enable compatibility with older glibc libraries." ON) message(STATUS "GLIBC_COMPATIBILITY is ${GLIBC_COMPATIBILITY}") diff --git a/be/src/common/signal_handler.h b/be/src/common/signal_handler.h index 68c7737d59..9faae8cc88 100644 --- a/be/src/common/signal_handler.h +++ b/be/src/common/signal_handler.h @@ -31,7 +31,6 @@ // // Implementation of InstallFailureSignalHandler(). -#define BOOST_STACKTRACE_USE_BACKTRACE #include #include #include diff --git a/be/src/common/status.cpp b/be/src/common/status.cpp index 01002be1e0..bb38e40be4 100644 --- a/be/src/common/status.cpp +++ b/be/src/common/status.cpp @@ -10,26 +10,32 @@ namespace doris { Status::Status(const TStatus& s) { if (s.status_code != TStatusCode::OK) { + // It is ok to set precise code == 1 here, because we do not know the precise code + // just from thrift's TStatus if (s.error_msgs.empty()) { assemble_state(s.status_code, Slice(), 1, Slice()); } else { assemble_state(s.status_code, s.error_msgs[0], 1, Slice()); } } else { - set_ok(); + _length = 0; } } +// TODO yiguolei, maybe should init PStatus's precise code because OLAPInternal Error may +// tranfer precise code during BRPC Status::Status(const PStatus& s) { TStatusCode::type code = (TStatusCode::type)s.status_code(); if (code != TStatusCode::OK) { + // It is ok to set precise code == 1 here, because we do not know the precise code + // just from thrift's TStatus if (s.error_msgs_size() == 0) { assemble_state(code, Slice(), 1, Slice()); } else { assemble_state(code, s.error_msgs(0), 1, Slice()); } } else { - set_ok(); + _length = 0; } } @@ -152,7 +158,7 @@ Slice Status::message() const { return Slice(); } - return Slice(_state + HEADER_LEN, _length); + return Slice(_state + HEADER_LEN, _length - HEADER_LEN); } Status Status::clone_and_prepend(const Slice& msg) const { @@ -169,4 +175,4 @@ Status Status::clone_and_append(const Slice& msg) const { return Status(code(), message(), precise_code(), msg); } -} // namespace doris +} // namespace doris \ No newline at end of file diff --git a/be/src/common/status.h b/be/src/common/status.h index a1632bd4b5..9e419800f1 100644 --- a/be/src/common/status.h +++ b/be/src/common/status.h @@ -4,9 +4,13 @@ #pragma once +#include #include #include +#include +#include + #include "common/compiler_util.h" #include "common/logging.h" #include "gen_cpp/Status_types.h" // for TStatus @@ -38,7 +42,7 @@ public: // same as copy c'tor Status& operator=(const Status& rhs) { if (rhs._length) { - memcpy(_state, rhs._state, rhs._length + Status::HEADER_LEN); + memcpy(_state, rhs._state, rhs._length); } else { _length = 0; } @@ -151,8 +155,18 @@ public: return Status(TStatusCode::DATA_QUALITY_ERROR, msg, precise_code, msg2); } + // A wrapper for OLAPStatus + // Precise code is for OLAPStatus's enum value + // All Status Error is treated as Internal Error + static Status OLAPInternalError(int16_t precise_code, const Slice& msg = Slice()) { + #ifdef PRINT_ALL_ERR_STATUS_STACKTRACE + LOG(WARNING) << "Error occurred, error code = " << precise_code << ", with message: " << msg + << "\n" << boost::stacktrace::stacktrace(); + #endif + return Status(TStatusCode::INTERNAL_ERROR, Slice(), precise_code, msg); + } + bool ok() const { return _length == 0; } - void set_ok() { _length = 0; } bool is_cancelled() const { return code() == TStatusCode::CANCELLED; } bool is_mem_limit_exceeded() const { return code() == TStatusCode::MEM_LIMIT_EXCEEDED; } @@ -240,8 +254,17 @@ public: /// trailing message. Status clone_and_append(const Slice& msg) const; + // if(!status) or if (status) will use this operator operator bool() const { return this->ok(); } + // Used like if (res == Status::OK()) + // if the state is ok, then both code and precise code is not initialized properly, so that should check ok state + // ignore error messages during comparison + bool operator == (const Status& st) { return ok() ? st.ok() : code() == st.code() && precise_code() == st.precise_code(); } + + // Used like if (res != Status::OK()) + bool operator != (const Status& st) { return ok() ? !st.ok() : code() != st.code() || precise_code() != st.precise_code(); } + private: void assemble_state(TStatusCode::type code, const Slice& msg, int16_t precise_code, const Slice& msg2) { DCHECK(code != TStatusCode::OK); @@ -262,7 +285,7 @@ private: size = MESSAGE_LEN; } - _length = size; + _length = size + HEADER_LEN; _code = (char)code; _precise_code = precise_code; @@ -294,7 +317,10 @@ private: char _state[STATE_CAPACITY]; struct { - int64_t _length : 32; // message length + // Message length == HEADER(7 bytes) + message size + // Sometimes error message is empty, so that we could not use length==0 to indicate + // whether there is error happens + int64_t _length : 32; int64_t _code : 8; int64_t _precise_code : 16; int64_t _message : 8; // save message since here @@ -302,6 +328,13 @@ private: }; }; +// Override the << operator, it is used during LOG(INFO) << "xxxx" << status; +// Add inline here to dedup many includes +inline std::ostream & operator << (std::ostream & ostr, const Status & param) +{ + return ostr << param.to_string(); +} + // some generally useful macros #define RETURN_IF_ERROR(stmt) \ do { \ @@ -360,4 +393,4 @@ private: #undef WARN_UNUSED_RESULT #endif -#define WARN_UNUSED_RESULT __attribute__((warn_unused_result)) +#define WARN_UNUSED_RESULT __attribute__((warn_unused_result)) \ No newline at end of file