[Enhancement](Log) Reduce usage of log fatal(PART I) (apache#42344)

## Proposed changes Issue Number: close apache#40835  use `throw Exception` to replace them which not in `if constexpr`， and change part of `INTERNAL_ERROR` in this [pr](https://github.com/apache/doris/pull/38144/files)(file `aggregate_function_reader_first_last.h` and `aggregate_function_window.h`) to `FatalError`. for those in `if constexpr else{...}`, use `static_assert` about template argument which used in that judgement to advance them to compile time but there seems to be some bugs with the template parameter instantiation in the files `comparison_predicate.h`, `set_probe_sink_operator.cpp`, `set_sink_operator.cpp`, `comparison_predicate.h`, `in_list_predicate.h` and `set_source_operator.cpp` that I haven't modified yet. --------- Co-authored-by: wyxxxcat <[email protected]>
englefly · Dec 19, 2024 · 55c26e0 · 55c26e0
1 parent 6cce408
commit 55c26e0
Show file tree

Hide file tree

Showing 71 changed files with 250 additions and 282 deletions.
diff --git a/be/src/common/status.h b/be/src/common/status.h
@@ -293,7 +293,8 @@ namespace ErrorCode {
     E(ENTRY_NOT_FOUND, -7002, false);                        \
     E(INVALID_TABLET_STATE, -7211, false);                   \
     E(ROWSETS_EXPIRED, -7311, false);                        \
-    E(CGROUP_ERROR, -7411, false);
+    E(CGROUP_ERROR, -7411, false);                           \
+    E(FATAL_ERROR, -7412, false);
 
 // Define constexpr int error_code_name = error_code_value
 #define M(NAME, ERRORCODE, ENABLESTACKTRACE) constexpr int NAME = ERRORCODE;
@@ -446,6 +447,14 @@ class [[nodiscard]] Status {
 
     static Status OK() { return {}; }
 
+    template <bool stacktrace = true, typename... Args>
+    static Status FatalError(std::string_view msg, Args&&... args) {
+#ifndef NDEBUG
+        LOG(FATAL) << fmt::format(msg, std::forward<Args>(args)...);
+#endif
+        return Error<ErrorCode::FATAL_ERROR, stacktrace>(msg, std::forward<Args>(args)...);
+    }
+
 // default have stacktrace. could disable manually.
 #define ERROR_CTOR(name, code)                                                       \
     template <bool stacktrace = true, typename... Args>                              \

diff --git a/be/src/gutil/strings/escaping.cc b/be/src/gutil/strings/escaping.cc
@@ -10,6 +10,8 @@
 #include <limits>
 #include <ostream>
 
+#include "common/exception.h"
+
 using std::numeric_limits;
 #include <vector>
 
@@ -1084,7 +1086,8 @@ int Base64UnescapeInternal(const char* src, int szsrc, char* dest, int szdest,
 
     default:
         // state should have no other values at this point.
-        LOG(FATAL) << "This can't happen; base64 decoder state = " << state;
+        throw doris::Exception(
+                doris::Status::FatalError("This can't happen; base64 decoder state = {}", state));
     }
 
     // The remainder of the string should be all whitespace, mixed with

diff --git a/be/src/gutil/strings/numbers.cc b/be/src/gutil/strings/numbers.cc
@@ -19,6 +19,8 @@
 #include <limits>
 #include <ostream>
 
+#include "common/exception.h"
+
 using std::numeric_limits;
 #include <string>
 
@@ -772,8 +774,8 @@ uint64 atoi_kmgt(const char* s) {
             scale = GG_ULONGLONG(1) << 40;
             break;
         default:
-            LOG(FATAL) << "Invalid mnemonic: `" << c << "';"
-                       << " should be one of `K', `M', `G', and `T'.";
+            throw doris::Exception(doris::Status::FatalError(
+                    "Invalid mnemonic: `{}'; should be one of `K', `M', `G', and `T'.", c));
         }
     }
     return n * scale;

diff --git a/be/src/gutil/strings/util.cc b/be/src/gutil/strings/util.cc
@@ -19,6 +19,8 @@
 #include <mutex>
 #include <ostream>
 
+#include "common/exception.h"
+
 using std::copy;
 using std::max;
 using std::min;
@@ -489,8 +491,7 @@ const char* strstr_delimited(const char* haystack, const char* needle, char deli
             ++haystack;
         }
     }
-    LOG(FATAL) << "Unreachable statement";
-    return nullptr;
+    throw doris::Exception(doris::Status::FatalError("Unreachable statement"));
 }
 
 // ----------------------------------------------------------------------

diff --git a/be/src/gutil/threading/thread_collision_warner.cc b/be/src/gutil/threading/thread_collision_warner.cc
@@ -4,6 +4,9 @@
 
 #include "gutil/threading/thread_collision_warner.h"
 
+#include "common/exception.h"
+#include "common/status.h"
+
 #ifdef __linux__
 #include <syscall.h>
 #else
@@ -19,8 +22,9 @@
 namespace base {
 
 void DCheckAsserter::warn(int64_t previous_thread_id, int64_t current_thread_id) {
-    LOG(FATAL) << "Thread Collision! Previous thread id: " << previous_thread_id
-               << ", current thread id: " << current_thread_id;
+    throw doris::Exception(doris::Status::FatalError(
+            "Thread Collision! Previous thread id: {}, current thread id: {}", previous_thread_id,
+            current_thread_id));
 }
 
 static subtle::Atomic64 CurrentThread() {

diff --git a/be/src/io/file_factory.h b/be/src/io/file_factory.h
@@ -118,10 +118,9 @@ class FileFactory {
         case TStorageBackendType::HDFS:
             return TFileType::FILE_HDFS;
         default:
-            LOG(FATAL) << "not match type to convert, from type:" << type;
+            throw Exception(Status::FatalError("not match type to convert, from type:{}", type));
         }
-        LOG(FATAL) << "__builtin_unreachable";
-        __builtin_unreachable();
+        throw Exception(Status::FatalError("__builtin_unreachable"));
     }
 };
 

diff --git a/be/src/olap/block_column_predicate.h b/be/src/olap/block_column_predicate.h
@@ -74,25 +74,21 @@ class BlockColumnPredicate {
     }
 
     virtual bool can_do_apply_safely(PrimitiveType input_type, bool is_null) const {
-        LOG(FATAL) << "should not reach here";
-        return true;
+        throw Exception(Status::FatalError("should not reach here"));
     }
 
     virtual bool support_zonemap() const { return true; }
 
     virtual bool evaluate_and(const std::pair<WrapperField*, WrapperField*>& statistic) const {
-        LOG(FATAL) << "should not reach here";
-        return true;
+        throw Exception(Status::FatalError("should not reach here"));
     }
 
     virtual bool evaluate_and(const segment_v2::BloomFilter* bf) const {
-        LOG(FATAL) << "should not reach here";
-        return true;
+        throw Exception(Status::FatalError("should not reach here"));
     }
 
     virtual bool evaluate_and(const StringRef* dict_words, const size_t dict_num) const {
-        LOG(FATAL) << "should not reach here";
-        return true;
+        throw Exception(Status::FatalError("should not reach here"));
     }
 
     virtual bool can_do_bloom_filter(bool ngram) const { return false; }

diff --git a/be/src/olap/data_dir.cpp b/be/src/olap/data_dir.cpp
@@ -316,10 +316,10 @@ Status DataDir::_check_incompatible_old_format_tablet() {
                                           std::string_view value) -> bool {
         // if strict check incompatible old format, then log fatal
         if (config::storage_strict_check_incompatible_old_format) {
-            LOG(FATAL)
-                    << "There are incompatible old format metas, current version does not support "
-                    << "and it may lead to data missing!!! "
-                    << "tablet_id = " << tablet_id << " schema_hash = " << schema_hash;
+            throw Exception(Status::FatalError(
+                    "There are incompatible old format metas, current version does not support and "
+                    "it may lead to data missing!!! tablet_id = {} schema_hash = {}",
+                    tablet_id, schema_hash));
         } else {
             LOG(WARNING)
                     << "There are incompatible old format metas, current version does not support "
@@ -451,7 +451,8 @@ Status DataDir::load() {
                      << ", loaded tablet: " << tablet_ids.size()
                      << ", error tablet: " << failed_tablet_ids.size() << ", path: " << _path;
         if (!config::ignore_load_tablet_failure) {
-            LOG(FATAL) << "load tablets encounter failure. stop BE process. path: " << _path;
+            throw Exception(Status::FatalError(
+                    "load tablets encounter failure. stop BE process. path: {}", _path));
         }
     }
     if (!load_tablet_status) {
@@ -495,10 +496,9 @@ Status DataDir::load() {
         }
     }
     if (rowset_partition_id_eq_0_num > config::ignore_invalid_partition_id_rowset_num) {
-        LOG(FATAL) << fmt::format(
+        throw Exception(Status::FatalError(
                 "roswet partition id eq 0 is {} bigger than config {}, be exit, plz check be.INFO",
-                rowset_partition_id_eq_0_num, config::ignore_invalid_partition_id_rowset_num);
-        exit(-1);
+                rowset_partition_id_eq_0_num, config::ignore_invalid_partition_id_rowset_num));
     }
 
     // traverse rowset

diff --git a/be/src/olap/key_coder.h b/be/src/olap/key_coder.h
@@ -109,8 +109,8 @@ class KeyCoderTraits<
             case 16:
                 return BigEndian::FromHost128(val);
             default:
-                LOG(FATAL) << "Invalid type to big endian, type=" << int(field_type)
-                           << ", size=" << sizeof(UnsignedCppType);
+                throw Exception(Status::FatalError("Invalid type to big endian, type={}, size={}",
+                                                   int(field_type), sizeof(UnsignedCppType)));
             }
         }
     }
@@ -300,8 +300,7 @@ class KeyCoderTraits<FieldType::OLAP_FIELD_TYPE_CHAR> {
     }
 
     static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
-        LOG(FATAL) << "decode_ascending is not implemented";
-        return Status::OK();
+        throw Exception(Status::FatalError("decode_ascending is not implemented"));
     }
 };
 
@@ -320,8 +319,7 @@ class KeyCoderTraits<FieldType::OLAP_FIELD_TYPE_VARCHAR> {
     }
 
     static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
-        LOG(FATAL) << "decode_ascending is not implemented";
-        return Status::OK();
+        throw Exception(Status::FatalError("decode_ascending is not implemented"));
     }
 };
 
@@ -340,8 +338,7 @@ class KeyCoderTraits<FieldType::OLAP_FIELD_TYPE_STRING> {
     }
 
     static Status decode_ascending(Slice* encoded_key, size_t index_size, uint8_t* cell_ptr) {
-        LOG(FATAL) << "decode_ascending is not implemented";
-        return Status::OK();
+        throw Exception(Status::FatalError("decode_ascending is not implemented"));
     }
 };
 

diff --git a/be/src/olap/like_column_predicate.h b/be/src/olap/like_column_predicate.h
@@ -128,8 +128,8 @@ class LikeColumnPredicate : public ColumnPredicate {
                     }
                 }
             } else {
-                LOG(FATAL) << "vectorized (not) like predicates should be dict column";
-                __builtin_unreachable();
+                throw Exception(Status::FatalError(
+                        "vectorized (not) like predicates should be dict column"));
             }
         } else {
             if (column.is_column_dictionary()) {
@@ -153,8 +153,8 @@ class LikeColumnPredicate : public ColumnPredicate {
                     }
                 }
             } else {
-                LOG(FATAL) << "vectorized (not) like predicates should be dict column";
-                __builtin_unreachable();
+                throw Exception(Status::FatalError(
+                        "vectorized (not) like predicates should be dict column"));
             }
         }
     }

diff --git a/be/src/olap/match_predicate.h b/be/src/olap/match_predicate.h
@@ -55,8 +55,7 @@ class MatchPredicate : public ColumnPredicate {
     //evaluate predicate on Bitmap
     Status evaluate(BitmapIndexIterator* iterator, uint32_t num_rows,
                     roaring::Roaring* roaring) const override {
-        LOG(FATAL) << "Not Implemented MatchPredicate::evaluate";
-        __builtin_unreachable();
+        throw Exception(Status::FatalError("Not Implemented MatchPredicate::evaluate"));
     }
 
     //evaluate predicate on inverted

diff --git a/be/src/olap/null_predicate.h b/be/src/olap/null_predicate.h
@@ -87,8 +87,8 @@ class NullPredicate : public ColumnPredicate {
         if (_is_null) {
             return bf->test_bytes(nullptr, 0);
         } else {
-            LOG(FATAL) << "Bloom filter is not supported by predicate type: is_null=" << _is_null;
-            return true;
+            throw Exception(Status::FatalError(
+                    "Bloom filter is not supported by predicate type: is_null="));
         }
     }
 

diff --git a/be/src/olap/olap_common.h b/be/src/olap/olap_common.h
@@ -36,6 +36,7 @@
 #include <utility>
 
 #include "common/config.h"
+#include "common/exception.h"
 #include "io/io_common.h"
 #include "olap/olap_define.h"
 #include "olap/rowset/rowset_fwd.h"
@@ -419,7 +420,8 @@ struct RowsetId {
                     LOG(WARNING) << "failed to init rowset id: " << rowset_id_str;
                     high = next_rowset_id().hi;
                 } else {
-                    LOG(FATAL) << "failed to init rowset id: " << rowset_id_str;
+                    throw Exception(
+                            Status::FatalError("failed to init rowset id: {}", rowset_id_str));
                 }
             }
             init(1, high, 0, 0);
@@ -440,7 +442,7 @@ struct RowsetId {
     void init(int64_t id_version, int64_t high, int64_t middle, int64_t low) {
         version = id_version;
         if (UNLIKELY(high >= MAX_ROWSET_ID)) {
-            LOG(FATAL) << "inc rowsetid is too large:" << high;
+            throw Exception(Status::FatalError("inc rowsetid is too large:{}", high));
         }
         hi = (id_version << 56) + (high & LOW_56_BITS);
         mi = middle;

diff --git a/be/src/olap/page_cache.h b/be/src/olap/page_cache.h
@@ -176,11 +176,9 @@ class StoragePageCache {
             return _pk_index_page_cache.get();
         }
         default:
-            LOG(FATAL) << "get error type page cache";
-            __builtin_unreachable();
+            throw Exception(Status::FatalError("get error type page cache"));
         }
-        LOG(FATAL) << "__builtin_unreachable";
-        __builtin_unreachable();
+        throw Exception(Status::FatalError("__builtin_unreachable"));
     }
 };
 

diff --git a/be/src/olap/rowset/beta_rowset_writer_v2.h b/be/src/olap/rowset/beta_rowset_writer_v2.h
@@ -99,8 +99,7 @@ class BetaRowsetWriterV2 : public RowsetWriter {
     };
 
     RowsetSharedPtr manual_build(const RowsetMetaSharedPtr& rowset_meta) override {
-        LOG(FATAL) << "not implemeted";
-        return nullptr;
+        throw Exception(Status::FatalError("not implemeted"));
     }
 
     PUniqueId load_id() override { return _context.load_id; }

diff --git a/be/src/olap/rowset/rowset_writer.h b/be/src/olap/rowset/rowset_writer.h
@@ -170,7 +170,9 @@ class RowsetWriter {
 
     virtual int32_t allocate_segment_id() = 0;
 
-    virtual void set_segment_start_id(int num_segment) { LOG(FATAL) << "not supported!"; }
+    virtual void set_segment_start_id(int num_segment) {
+        throw Exception(Status::FatalError("not supported!"));
+    }
 
     virtual int64_t delete_bitmap_ns() { return 0; }
 

diff --git a/be/src/olap/rowset/segment_v2/hierarchical_data_reader.cpp b/be/src/olap/rowset/segment_v2/hierarchical_data_reader.cpp
@@ -80,8 +80,7 @@ Status HierarchicalDataReader::init(const ColumnIteratorOptions& opts) {
 }
 
 Status HierarchicalDataReader::seek_to_first() {
-    LOG(FATAL) << "Not implemented";
-    __builtin_unreachable();
+    throw Exception(Status::FatalError("Not implemented"));
 }
 
 Status HierarchicalDataReader::seek_to_ordinal(ordinal_t ord) {
@@ -159,8 +158,7 @@ Status ExtractReader::init(const ColumnIteratorOptions& opts) {
 }
 
 Status ExtractReader::seek_to_first() {
-    LOG(FATAL) << "Not implemented";
-    __builtin_unreachable();
+    throw Exception(Status::FatalError("Not implemented"));
 }
 
 Status ExtractReader::seek_to_ordinal(ordinal_t ord) {

diff --git a/be/src/olap/storage_policy.cpp b/be/src/olap/storage_policy.cpp
@@ -141,8 +141,10 @@ std::vector<std::pair<std::string, int64_t>> get_storage_resource_ids() {
 namespace {
 
 [[noreturn]] void exit_at_unknown_path_version(std::string_view resource_id, int64_t path_version) {
-    LOG(FATAL) << "unknown path version, please upgrade BE or drop this storage vault. resource_id="
-               << resource_id << " path_version=" << path_version;
+    throw Exception(
+            Status::FatalError("unknown path version, please upgrade BE or drop this storage "
+                               "vault. resource_id={} path_version={}",
+                               resource_id, path_version));
 }
 
 } // namespace

diff --git a/be/src/olap/tablet_reader.cpp b/be/src/olap/tablet_reader.cpp
@@ -61,7 +61,7 @@ using namespace ErrorCode;
 
 void TabletReader::ReaderParams::check_validation() const {
     if (UNLIKELY(version.first == -1 && is_segcompaction == false)) {
-        LOG(FATAL) << "version is not set. tablet=" << tablet->tablet_id();
+        throw Exception(Status::FatalError("version is not set. tablet={}", tablet->tablet_id()));
     }
 }
 

diff --git a/be/src/pipeline/dependency.h b/be/src/pipeline/dependency.h
@@ -723,8 +723,7 @@ inline std::string get_exchange_type_name(ExchangeType idx) {
     case ExchangeType::LOCAL_MERGE_SORT:
         return "LOCAL_MERGE_SORT";
     }
-    LOG(FATAL) << "__builtin_unreachable";
-    __builtin_unreachable();
+    throw Exception(Status::FatalError("__builtin_unreachable"));
 }
 
 struct DataDistribution {

diff --git a/be/src/pipeline/exec/exchange_sink_buffer.cpp b/be/src/pipeline/exec/exchange_sink_buffer.cpp
@@ -422,8 +422,7 @@ void ExchangeSinkBuffer::_ended(InstanceLoId id) {
         }
         LOG(INFO) << ss.str();
 
-        LOG(FATAL) << "not find the instance id";
-        __builtin_unreachable();
+        throw Exception(Status::FatalError("not find the instance id"));
     } else {
         std::unique_lock<std::mutex> lock(*_instance_to_package_queue_mutex[id]);
         _running_sink_count[id]--;