This is an automated email from the ASF dual-hosted git repository. kxiao pushed a commit to branch branch-3.0 in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.0 by this push: new 46bd816f42d [opt](inverted index) Optimize the codes exception handling process #44205 #44601 (#44862) 46bd816f42d is described below commit 46bd816f42dacf1665a0df56d947542de0e4431e Author: zzzxl <yangs...@selectdb.com> AuthorDate: Mon Dec 9 08:57:44 2024 +0800 [opt](inverted index) Optimize the codes exception handling process #44205 #44601 (#44862) Co-authored-by: Sun Chenyang <suncheny...@selectdb.com> --- .../olap/rowset/segment_v2/inverted_index_common.h | 103 +++++++ .../segment_v2/inverted_index_file_writer.cpp | 86 +++--- .../rowset/segment_v2/inverted_index_file_writer.h | 8 +- .../rowset/segment_v2/inverted_index_writer.cpp | 45 +-- .../common/inverted_index_common_test.cpp | 343 +++++++++++++++++++++ .../test_inverted_index_writer_exception.groovy | 89 ++++++ 6 files changed, 595 insertions(+), 79 deletions(-) diff --git a/be/src/olap/rowset/segment_v2/inverted_index_common.h b/be/src/olap/rowset/segment_v2/inverted_index_common.h new file mode 100644 index 00000000000..1fdb7df2931 --- /dev/null +++ b/be/src/olap/rowset/segment_v2/inverted_index_common.h @@ -0,0 +1,103 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include <CLucene.h> // IWYU pragma: keep + +#include <memory> + +#include "common/logging.h" + +namespace lucene::store { +class Directory; +} // namespace lucene::store + +namespace doris::segment_v2 { + +struct DirectoryDeleter { + void operator()(lucene::store::Directory* ptr) const { _CLDECDELETE(ptr); } +}; + +struct ErrorContext { + std::string err_msg; + std::exception_ptr eptr; +}; + +template <typename T> +concept HasClose = requires(T t) { + { t->close() }; +}; + +template <typename PtrType> + requires HasClose<PtrType> +void finally_close(PtrType& resource, ErrorContext& error_context) { + if (resource) { + try { + resource->close(); + } catch (CLuceneError& err) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("Error occurred while closing resource: "); + error_context.err_msg.append(err.what()); + LOG(ERROR) << error_context.err_msg; + } catch (...) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("Error occurred while closing resource"); + LOG(ERROR) << error_context.err_msg; + } + } +} + +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-macros" +#endif + +#define FINALLY_CLOSE(resource) \ + { \ + static_assert(sizeof(error_context) > 0, \ + "error_context must be defined before using FINALLY macro!"); \ + finally_close(resource, error_context); \ + } + +// Return ERROR after finally +#define FINALLY(finally_block) \ + { \ + static_assert(sizeof(error_context) > 0, \ + "error_context must be defined before using FINALLY macro!"); \ + finally_block; \ + if (error_context.eptr) { \ + return Status::Error<ErrorCode::INVERTED_INDEX_CLUCENE_ERROR>(error_context.err_msg); \ + } \ + } + +// Re-throw the exception after finally +#define FINALLY_EXCEPTION(finally_block) \ + { \ + static_assert(sizeof(error_context) > 0, \ + "error_context must be defined before using FINALLY macro!"); \ + finally_block; \ + if (error_context.eptr) { \ + std::rethrow_exception(error_context.eptr); \ + } \ + } + +#if defined(__clang__) +#pragma clang diagnostic pop +#endif + +} // namespace doris::segment_v2 \ No newline at end of file diff --git a/be/src/olap/rowset/segment_v2/inverted_index_file_writer.cpp b/be/src/olap/rowset/segment_v2/inverted_index_file_writer.cpp index 2d50730daff..bb373be5ee9 100644 --- a/be/src/olap/rowset/segment_v2/inverted_index_file_writer.cpp +++ b/be/src/olap/rowset/segment_v2/inverted_index_file_writer.cpp @@ -243,10 +243,9 @@ void InvertedIndexFileWriter::copyFile(const char* fileName, lucene::store::Dire Status InvertedIndexFileWriter::write_v1() { int64_t total_size = 0; - std::string err_msg; - lucene::store::Directory* out_dir = nullptr; - std::exception_ptr eptr; + std::unique_ptr<lucene::store::Directory, DirectoryDeleter> out_dir = nullptr; std::unique_ptr<lucene::store::IndexOutput> output = nullptr; + ErrorContext error_context; for (const auto& entry : _indices_dirs) { const int64_t index_id = entry.first.first; const auto& index_suffix = entry.first.second; @@ -262,7 +261,7 @@ Status InvertedIndexFileWriter::write_v1() { // Create output stream auto result = create_output_stream_v1(index_id, index_suffix); - out_dir = result.first; + out_dir = std::move(result.first); output = std::move(result.second); size_t start = output->getFilePointer(); @@ -275,23 +274,19 @@ Status InvertedIndexFileWriter::write_v1() { total_size += compound_file_size; add_index_info(index_id, index_suffix, compound_file_size); } catch (CLuceneError& err) { - eptr = std::current_exception(); + error_context.eptr = std::current_exception(); auto index_path = InvertedIndexDescriptor::get_index_file_path_v1( _index_path_prefix, index_id, index_suffix); - err_msg = "CLuceneError occur when write_v1 idx file " + index_path + - " error msg: " + err.what(); - } - - // Close and clean up - finalize_output_dir(out_dir); - if (output) { - output->close(); - } - - if (eptr) { - LOG(ERROR) << err_msg; - return Status::Error<ErrorCode::INVERTED_INDEX_CLUCENE_ERROR>(err_msg); + error_context.err_msg.append("CLuceneError occur when write_v1 idx file: "); + error_context.err_msg.append(index_path); + error_context.err_msg.append(", error msg: "); + error_context.err_msg.append(err.what()); + LOG(ERROR) << error_context.err_msg; } + FINALLY({ + FINALLY_CLOSE(output); + FINALLY_CLOSE(out_dir); + }) } _total_file_size = total_size; @@ -299,10 +294,9 @@ Status InvertedIndexFileWriter::write_v1() { } Status InvertedIndexFileWriter::write_v2() { - std::string err_msg; - lucene::store::Directory* out_dir = nullptr; + std::unique_ptr<lucene::store::Directory, DirectoryDeleter> out_dir = nullptr; std::unique_ptr<lucene::store::IndexOutput> compound_file_output = nullptr; - std::exception_ptr eptr; + ErrorContext error_context; try { // Calculate header length and initialize offset int64_t current_offset = headerLength(); @@ -311,7 +305,7 @@ Status InvertedIndexFileWriter::write_v2() { // Create output stream auto result = create_output_stream_v2(); - out_dir = result.first; + out_dir = std::move(result.first); compound_file_output = std::move(result.second); // Write version and number of indices @@ -326,22 +320,18 @@ Status InvertedIndexFileWriter::write_v2() { _total_file_size = compound_file_output->getFilePointer(); _file_info.set_index_size(_total_file_size); } catch (CLuceneError& err) { - eptr = std::current_exception(); + error_context.eptr = std::current_exception(); auto index_path = InvertedIndexDescriptor::get_index_file_path_v2(_index_path_prefix); - err_msg = "CLuceneError occur when close idx file " + index_path + - " error msg: " + err.what(); - } - - // Close and clean up - finalize_output_dir(out_dir); - if (compound_file_output) { - compound_file_output->close(); - } - - if (eptr) { - LOG(ERROR) << err_msg; - return Status::Error<ErrorCode::INVERTED_INDEX_CLUCENE_ERROR>(err_msg); + error_context.err_msg.append("CLuceneError occur when close idx file: "); + error_context.err_msg.append(index_path); + error_context.err_msg.append(", error msg: "); + error_context.err_msg.append(err.what()); + LOG(ERROR) << error_context.err_msg; } + FINALLY({ + FINALLY_CLOSE(compound_file_output); + FINALLY_CLOSE(out_dir); + }) return Status::OK(); } @@ -369,13 +359,6 @@ std::vector<FileInfo> InvertedIndexFileWriter::prepare_sorted_files( return sorted_files; } -void InvertedIndexFileWriter::finalize_output_dir(lucene::store::Directory* out_dir) { - if (out_dir != nullptr) { - out_dir->close(); - _CLDECDELETE(out_dir) - } -} - void InvertedIndexFileWriter::add_index_info(int64_t index_id, const std::string& index_suffix, int64_t compound_file_size) { InvertedIndexFileInfo_IndexInfo index_info; @@ -424,7 +407,8 @@ std::pair<int64_t, int32_t> InvertedIndexFileWriter::calculate_header_length( return {header_length, header_file_count}; } -std::pair<lucene::store::Directory*, std::unique_ptr<lucene::store::IndexOutput>> +std::pair<std::unique_ptr<lucene::store::Directory, DirectoryDeleter>, + std::unique_ptr<lucene::store::IndexOutput>> InvertedIndexFileWriter::create_output_stream_v1(int64_t index_id, const std::string& index_suffix) { io::Path cfs_path(InvertedIndexDescriptor::get_index_file_path_v1(_index_path_prefix, index_id, @@ -434,6 +418,7 @@ InvertedIndexFileWriter::create_output_stream_v1(int64_t index_id, auto* out_dir = DorisFSDirectoryFactory::getDirectory(_fs, idx_path.c_str()); out_dir->set_file_writer_opts(_opts); + std::unique_ptr<lucene::store::Directory, DirectoryDeleter> out_dir_ptr(out_dir); auto* out = out_dir->createOutput(idx_name.c_str()); DBUG_EXECUTE_IF("InvertedIndexFileWriter::write_v1_out_dir_createOutput_nullptr", @@ -443,9 +428,9 @@ InvertedIndexFileWriter::create_output_stream_v1(int64_t index_id, "output is nullptr."; _CLTHROWA(CL_ERR_IO, "Create CompoundDirectory output error"); } - std::unique_ptr<lucene::store::IndexOutput> output(out); - return {out_dir, std::move(output)}; + + return {std::move(out_dir_ptr), std::move(output)}; } void InvertedIndexFileWriter::write_header_and_data_v1(lucene::store::IndexOutput* output, @@ -483,15 +468,20 @@ void InvertedIndexFileWriter::write_header_and_data_v1(lucene::store::IndexOutpu } } -std::pair<lucene::store::Directory*, std::unique_ptr<lucene::store::IndexOutput>> +std::pair<std::unique_ptr<lucene::store::Directory, DirectoryDeleter>, + std::unique_ptr<lucene::store::IndexOutput>> InvertedIndexFileWriter::create_output_stream_v2() { io::Path index_path {InvertedIndexDescriptor::get_index_file_path_v2(_index_path_prefix)}; + auto* out_dir = DorisFSDirectoryFactory::getDirectory(_fs, index_path.parent_path().c_str()); out_dir->set_file_writer_opts(_opts); + std::unique_ptr<lucene::store::Directory, DirectoryDeleter> out_dir_ptr(out_dir); + DCHECK(_idx_v2_writer != nullptr) << "inverted index file writer v2 is nullptr"; auto compound_file_output = std::unique_ptr<lucene::store::IndexOutput>( out_dir->createOutputV2(_idx_v2_writer.get())); - return std::make_pair(out_dir, std::move(compound_file_output)); + + return {std::move(out_dir_ptr), std::move(compound_file_output)}; } void InvertedIndexFileWriter::write_version_and_indices_count(lucene::store::IndexOutput* output) { diff --git a/be/src/olap/rowset/segment_v2/inverted_index_file_writer.h b/be/src/olap/rowset/segment_v2/inverted_index_file_writer.h index 3a2fcc1e6ac..ba42ffdceb1 100644 --- a/be/src/olap/rowset/segment_v2/inverted_index_file_writer.h +++ b/be/src/olap/rowset/segment_v2/inverted_index_file_writer.h @@ -29,6 +29,7 @@ #include "io/fs/file_system.h" #include "io/fs/file_writer.h" #include "io/fs/local_file_system.h" +#include "olap/rowset/segment_v2/inverted_index_common.h" #include "olap/rowset/segment_v2/inverted_index_desc.h" #include "runtime/exec_env.h" @@ -105,21 +106,22 @@ private: void sort_files(std::vector<FileInfo>& file_infos); void copyFile(const char* fileName, lucene::store::Directory* dir, lucene::store::IndexOutput* output, uint8_t* buffer, int64_t bufferLength); - void finalize_output_dir(lucene::store::Directory* out_dir); void add_index_info(int64_t index_id, const std::string& index_suffix, int64_t compound_file_size); int64_t headerLength(); // Helper functions specific to write_v1 std::pair<int64_t, int32_t> calculate_header_length(const std::vector<FileInfo>& sorted_files, lucene::store::Directory* directory); - std::pair<lucene::store::Directory*, std::unique_ptr<lucene::store::IndexOutput>> + virtual std::pair<std::unique_ptr<lucene::store::Directory, DirectoryDeleter>, + std::unique_ptr<lucene::store::IndexOutput>> create_output_stream_v1(int64_t index_id, const std::string& index_suffix); virtual void write_header_and_data_v1(lucene::store::IndexOutput* output, const std::vector<FileInfo>& sorted_files, lucene::store::Directory* directory, int64_t header_length, int32_t header_file_count); // Helper functions specific to write_v2 - std::pair<lucene::store::Directory*, std::unique_ptr<lucene::store::IndexOutput>> + virtual std::pair<std::unique_ptr<lucene::store::Directory, DirectoryDeleter>, + std::unique_ptr<lucene::store::IndexOutput>> create_output_stream_v2(); void write_version_and_indices_count(lucene::store::IndexOutput* output); struct FileMetadata { diff --git a/be/src/olap/rowset/segment_v2/inverted_index_writer.cpp b/be/src/olap/rowset/segment_v2/inverted_index_writer.cpp index a4f3ca55dd1..86a8f89e4c9 100644 --- a/be/src/olap/rowset/segment_v2/inverted_index_writer.cpp +++ b/be/src/olap/rowset/segment_v2/inverted_index_writer.cpp @@ -51,6 +51,7 @@ #include "olap/rowset/segment_v2/common.h" #include "olap/rowset/segment_v2/inverted_index/analyzer/analyzer.h" #include "olap/rowset/segment_v2/inverted_index/char_filter/char_filter_factory.h" +#include "olap/rowset/segment_v2/inverted_index_common.h" #include "olap/rowset/segment_v2/inverted_index_desc.h" #include "olap/rowset/segment_v2/inverted_index_file_writer.h" #include "olap/rowset/segment_v2/inverted_index_fs_directory.h" @@ -63,11 +64,6 @@ #include "util/slice.h" #include "util/string_util.h" -#define FINALLY_CLOSE_OUTPUT(x) \ - try { \ - if (x != nullptr) x->close(); \ - } catch (...) { \ - } namespace doris::segment_v2 { const int32_t MAX_FIELD_LEN = 0x7FFFFFFFL; const int32_t MERGE_FACTOR = 100000000; @@ -138,13 +134,6 @@ public: } } - void close() { - if (_index_writer) { - _index_writer->close(); - _index_writer.reset(); - } - } - void close_on_error() override { try { DBUG_EXECUTE_IF("InvertedIndexColumnWriter::close_on_error_throw_exception", @@ -618,7 +607,6 @@ public: buf.resize(size); _null_bitmap.write(reinterpret_cast<char*>(buf.data()), false); null_bitmap_out->writeBytes(buf.data(), size); - null_bitmap_out->close(); } } @@ -628,6 +616,7 @@ public: std::unique_ptr<lucene::store::IndexOutput> data_out = nullptr; std::unique_ptr<lucene::store::IndexOutput> index_out = nullptr; std::unique_ptr<lucene::store::IndexOutput> meta_out = nullptr; + ErrorContext error_context; try { // write bkd file if constexpr (field_is_numeric_type(field_type)) { @@ -656,16 +645,11 @@ public: << "Inverted index writer create output error occurred: nullptr"; _CLTHROWA(CL_ERR_IO, "Create output error with nullptr"); } - meta_out->close(); - data_out->close(); - index_out->close(); - _dir->close(); } else if constexpr (field_is_slice_type(field_type)) { null_bitmap_out = std::unique_ptr< lucene::store::IndexOutput>(_dir->createOutput( InvertedIndexDescriptor::get_temporary_null_bitmap_file_name())); write_null_bitmap(null_bitmap_out.get()); - close(); DBUG_EXECUTE_IF( "InvertedIndexWriter._throw_clucene_error_in_fulltext_writer_close", { _CLTHROWA(CL_ERR_IO, @@ -673,19 +657,24 @@ public: }); } } catch (CLuceneError& e) { - FINALLY_CLOSE_OUTPUT(null_bitmap_out) - FINALLY_CLOSE_OUTPUT(meta_out) - FINALLY_CLOSE_OUTPUT(data_out) - FINALLY_CLOSE_OUTPUT(index_out) + error_context.eptr = std::current_exception(); + error_context.err_msg.append("Inverted index writer finish error occurred: "); + error_context.err_msg.append(e.what()); + LOG(ERROR) << error_context.err_msg; + } + FINALLY({ + FINALLY_CLOSE(null_bitmap_out); + FINALLY_CLOSE(meta_out); + FINALLY_CLOSE(data_out); + FINALLY_CLOSE(index_out); if constexpr (field_is_numeric_type(field_type)) { - FINALLY_CLOSE_OUTPUT(_dir) + FINALLY_CLOSE(_dir); } else if constexpr (field_is_slice_type(field_type)) { - FINALLY_CLOSE_OUTPUT(_index_writer); + FINALLY_CLOSE(_index_writer); + // After closing the _index_writer, it needs to be reset to null to prevent issues of not closing it or closing it multiple times. + _index_writer.reset(); } - LOG(WARNING) << "Inverted index writer finish error occurred: " << e.what(); - return Status::Error<doris::ErrorCode::INVERTED_INDEX_CLUCENE_ERROR>( - "Inverted index writer finish error occurred:{}", e.what()); - } + }) return Status::OK(); } diff --git a/be/test/olap/rowset/segment_v2/inverted_index/common/inverted_index_common_test.cpp b/be/test/olap/rowset/segment_v2/inverted_index/common/inverted_index_common_test.cpp new file mode 100644 index 00000000000..96624260521 --- /dev/null +++ b/be/test/olap/rowset/segment_v2/inverted_index/common/inverted_index_common_test.cpp @@ -0,0 +1,343 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "olap/rowset/segment_v2/inverted_index_common.h" + +#include <gtest/gtest.h> + +#include "common/status.h" + +namespace doris::segment_v2 { + +class InvertedIndexCommonTest : public testing::Test { +public: + void SetUp() override {} + + void TearDown() override {} + + InvertedIndexCommonTest() = default; + ~InvertedIndexCommonTest() override = default; +}; + +TEST_F(InvertedIndexCommonTest, TestFinallyClose) { + class InvertedIndexBase { + public: + InvertedIndexBase(int32_t& count) : count_(count) {} + + void close() { count_++; } + void clear() { count_++; } + + int32_t& count_; + }; + { + int32_t count = 0; + { + ErrorContext error_context; + auto ptr = std::make_shared<InvertedIndexBase>(count); + finally_close(ptr, error_context); + } + EXPECT_EQ(count, 1); + } + { + int32_t count = 0; + { + ErrorContext error_context; + auto ptr = std::shared_ptr<InvertedIndexBase>(new InvertedIndexBase(count), + [](InvertedIndexBase* p) { + if (p) { + p->clear(); + delete p; + p = nullptr; + } + }); + finally_close(ptr, error_context); + } + EXPECT_EQ(count, 2); + } + { + int32_t count = 0; + { + ErrorContext error_context; + auto ptr = std::make_unique<InvertedIndexBase>(count); + finally_close(ptr, error_context); + } + EXPECT_EQ(count, 1); + } + { + struct Deleter { + void operator()(InvertedIndexBase* p) const { + if (p) { + p->clear(); + delete p; + p = nullptr; + } + } + }; + + int32_t count = 0; + { + ErrorContext error_context; + auto ptr = std::unique_ptr<InvertedIndexBase, Deleter>(new InvertedIndexBase(count)); + finally_close(ptr, error_context); + } + EXPECT_EQ(count, 2); + } +} + +TEST_F(InvertedIndexCommonTest, TestTryBlockException) { + class InvertedIndexBase { + public: + void add() { _CLTHROWA(CL_ERR_IO, "test add error"); } + void close() {} + }; + + // return error + { + auto func = []() -> Status { + auto base_ptr = std::make_unique<InvertedIndexBase>(); + ErrorContext error_context; + try { + base_ptr->add(); + } catch (CLuceneError& e) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("error: "); + error_context.err_msg.append(e.what()); + } + FINALLY({ + EXPECT_TRUE(error_context.eptr); + FINALLY_CLOSE(base_ptr); + }) + return Status::OK(); + }; + auto ret = func(); + EXPECT_EQ(ret.code(), ErrorCode::INVERTED_INDEX_CLUCENE_ERROR); + } + + // throw exception + { + auto func = []() { + auto base_ptr = std::make_unique<InvertedIndexBase>(); + ErrorContext error_context; + try { + base_ptr->add(); + } catch (CLuceneError& e) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("error: "); + error_context.err_msg.append(e.what()); + } + FINALLY_EXCEPTION({ + EXPECT_TRUE(error_context.eptr); + FINALLY_CLOSE(base_ptr); + }) + }; + bool is_exception = false; + try { + func(); + } catch (CLuceneError& e) { + EXPECT_EQ(e.number(), CL_ERR_IO); + is_exception = true; + } + EXPECT_TRUE(is_exception); + } +} + +TEST_F(InvertedIndexCommonTest, TestFinallyBlockException) { + class InvertedIndexBase { + public: + void add() {} + void close() { _CLTHROWA(CL_ERR_Runtime, "test close error"); } + }; + + // return error + { + auto func = []() -> Status { + auto base_ptr = std::make_unique<InvertedIndexBase>(); + ErrorContext error_context; + try { + base_ptr->add(); + } catch (CLuceneError& e) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("error: "); + error_context.err_msg.append(e.what()); + } + FINALLY({ + EXPECT_FALSE(error_context.eptr); + FINALLY_CLOSE(base_ptr); + EXPECT_TRUE(error_context.eptr); + }) + return Status::OK(); + }; + auto ret = func(); + EXPECT_EQ(ret.code(), ErrorCode::INVERTED_INDEX_CLUCENE_ERROR); + } + + // throw exception + { + auto func = []() { + auto base_ptr = std::make_unique<InvertedIndexBase>(); + ErrorContext error_context; + try { + base_ptr->add(); + } catch (CLuceneError& e) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("error: "); + error_context.err_msg.append(e.what()); + } + FINALLY_EXCEPTION({ + EXPECT_FALSE(error_context.eptr); + FINALLY_CLOSE(base_ptr); + EXPECT_TRUE(error_context.eptr); + }) + }; + bool is_exception = false; + try { + func(); + } catch (CLuceneError& e) { + EXPECT_EQ(e.number(), CL_ERR_Runtime); + is_exception = true; + } + EXPECT_TRUE(is_exception); + } +} + +TEST_F(InvertedIndexCommonTest, TestTryAndFinallyBlockException) { + class InvertedIndexBase { + public: + void add() { _CLTHROWA(CL_ERR_IO, "test add error"); } + void close() { _CLTHROWA(CL_ERR_Runtime, "test close error"); } + }; + + // return error + { + auto func = []() -> Status { + auto base_ptr = std::make_unique<InvertedIndexBase>(); + ErrorContext error_context; + try { + base_ptr->add(); + } catch (CLuceneError& e) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("error: "); + error_context.err_msg.append(e.what()); + } + FINALLY({ + EXPECT_TRUE(error_context.eptr); + FINALLY_CLOSE(base_ptr); + EXPECT_TRUE(error_context.eptr); + }) + return Status::OK(); + }; + auto ret = func(); + EXPECT_EQ(ret.code(), ErrorCode::INVERTED_INDEX_CLUCENE_ERROR); + } + + // throw exception + { + auto func = []() { + auto base_ptr = std::make_unique<InvertedIndexBase>(); + ErrorContext error_context; + try { + base_ptr->add(); + } catch (CLuceneError& e) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("error: "); + error_context.err_msg.append(e.what()); + } + FINALLY_EXCEPTION({ + EXPECT_TRUE(error_context.eptr); + FINALLY_CLOSE(base_ptr); + EXPECT_TRUE(error_context.eptr); + }) + }; + bool is_exception = false; + try { + func(); + } catch (CLuceneError& e) { + EXPECT_EQ(e.number(), CL_ERR_Runtime); + is_exception = true; + } + EXPECT_TRUE(is_exception); + } +} + +TEST_F(InvertedIndexCommonTest, TestRawPointerException) { + class InvertedIndexBase { + public: + void add() { _CLTHROWA(CL_ERR_IO, "test add error"); } + void close() { _CLTHROWA(CL_ERR_Runtime, "test close error"); } + }; + + // return error + { + auto func = []() -> Status { + auto* base_ptr = new InvertedIndexBase(); + ErrorContext error_context; + try { + base_ptr->add(); + } catch (CLuceneError& e) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("error: "); + error_context.err_msg.append(e.what()); + } + FINALLY({ + EXPECT_TRUE(error_context.eptr); + FINALLY_CLOSE(base_ptr); + if (base_ptr) { + delete base_ptr; + base_ptr = nullptr; + } + EXPECT_TRUE(error_context.eptr); + }) + return Status::OK(); + }; + auto ret = func(); + EXPECT_EQ(ret.code(), ErrorCode::INVERTED_INDEX_CLUCENE_ERROR); + } + + // throw exception + { + auto func = []() { + auto* base_ptr = new InvertedIndexBase(); + ErrorContext error_context; + try { + base_ptr->add(); + } catch (CLuceneError& e) { + error_context.eptr = std::current_exception(); + error_context.err_msg.append("error: "); + error_context.err_msg.append(e.what()); + } + FINALLY_EXCEPTION({ + EXPECT_TRUE(error_context.eptr); + FINALLY_CLOSE(base_ptr); + if (base_ptr) { + delete base_ptr; + base_ptr = nullptr; + } + EXPECT_TRUE(error_context.eptr); + }) + }; + bool is_exception = false; + try { + func(); + } catch (CLuceneError& e) { + EXPECT_EQ(e.number(), CL_ERR_Runtime); + is_exception = true; + } + EXPECT_TRUE(is_exception); + } +} + +} // namespace doris::segment_v2 \ No newline at end of file diff --git a/regression-test/suites/inverted_index_p0/test_inverted_index_writer_exception.groovy b/regression-test/suites/inverted_index_p0/test_inverted_index_writer_exception.groovy new file mode 100644 index 00000000000..ced1c8d74ae --- /dev/null +++ b/regression-test/suites/inverted_index_p0/test_inverted_index_writer_exception.groovy @@ -0,0 +1,89 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import java.sql.SQLException + +suite("test_inverted_index_writer_exception", "nonConcurrent"){ + def indexTbName1 = "test_inverted_index_writer_exception" + + sql "DROP TABLE IF EXISTS ${indexTbName1}" + + sql """ + CREATE TABLE ${indexTbName1} ( + `@timestamp` int(11) NULL COMMENT "", + `clientip` varchar(20) NULL COMMENT "", + `request` text NULL COMMENT "", + `status` int(11) NULL COMMENT "", + `size` int(11) NULL COMMENT "", + INDEX request_idx (`request`) USING INVERTED PROPERTIES("parser" = "english") COMMENT '' + ) ENGINE=OLAP + DUPLICATE KEY(`@timestamp`) + COMMENT "OLAP" + DISTRIBUTED BY RANDOM BUCKETS 1 + PROPERTIES ( + "replication_allocation" = "tag.location.default: 1" + ); + """ + + try { + GetDebugPoint().enableDebugPointForAllBEs("InvertedIndexWriter._throw_clucene_error_in_fulltext_writer_close") + try { + sql """ INSERT INTO ${indexTbName1} VALUES (1, "40.135.0.0", "GET /images/hm_bg.jpg HTTP/1.0", 1, 2); """ + } catch (SQLException e) { + if (e.message.contains("E-6002")) { + log.info("Test passed 1: Encountered expected exception [E-6002].") + } else { + throw e + } + } + } finally { + GetDebugPoint().disableDebugPointForAllBEs("InvertedIndexWriter._throw_clucene_error_in_fulltext_writer_close") + } + + try { + GetDebugPoint().enableDebugPointForAllBEs("DorisFSDirectory::close_close_with_error") + try { + sql """ INSERT INTO ${indexTbName1} VALUES (2, "40.135.0.0", "GET /images/hm_bg.jpg HTTP/1.0", 1, 2); """ + } catch (SQLException e) { + if (e.message.contains("E-6002")) { + log.info("Test passed 2: Encountered expected exception [E-6002].") + } else { + throw e + } + } + } finally { + GetDebugPoint().disableDebugPointForAllBEs("DorisFSDirectory::close_close_with_error") + } + + try { + GetDebugPoint().enableDebugPointForAllBEs("InvertedIndexWriter._throw_clucene_error_in_fulltext_writer_close") + GetDebugPoint().enableDebugPointForAllBEs("DorisFSDirectory::close_close_with_error") + + try { + sql """ INSERT INTO ${indexTbName1} VALUES (3, "40.135.0.0", "GET /images/hm_bg.jpg HTTP/1.0", 1, 2); """ + } catch (SQLException e) { + if (e.message.contains("E-6002")) { + log.info("Test passed 3: Encountered expected exception [E-6002].") + } else { + throw e + } + } + } finally { + GetDebugPoint().disableDebugPointForAllBEs("InvertedIndexWriter._throw_clucene_error_in_fulltext_writer_close") + GetDebugPoint().disableDebugPointForAllBEs("DorisFSDirectory::close_close_with_error") + } +} \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org