From 3398b45eb776fd09271f329f3844656c822fcad4 Mon Sep 17 00:00:00 2001 From: Shinichi Umegane Date: Wed, 23 Oct 2024 13:50:50 +0900 Subject: [PATCH] Umegane dev 965 (#58) Temporarily replace exception throwing with logging and process termination --- docs/internal/exception_handling_ja.md | 20 ++ include/limestone/api/cursor.h | 3 + include/limestone/api/datastore.h | 15 ++ include/limestone/api/log_channel.h | 24 ++ include/limestone/api/snapshot.h | 3 + src/limestone/cursor.cpp | 7 +- src/limestone/datastore.cpp | 270 ++++++++++--------- src/limestone/limestone_exception_helper.cpp | 21 ++ src/limestone/limestone_exception_helper.h | 31 ++- src/limestone/log_channel.cpp | 137 ++++++---- src/limestone/snapshot.cpp | 9 +- test/limestone/log/log_dir_test.cpp | 2 + test/limestone/log/rotate_test.cpp | 3 + 13 files changed, 365 insertions(+), 180 deletions(-) create mode 100644 src/limestone/limestone_exception_helper.cpp diff --git a/docs/internal/exception_handling_ja.md b/docs/internal/exception_handling_ja.md index bfe3d1ac..5cee6a95 100644 --- a/docs/internal/exception_handling_ja.md +++ b/docs/internal/exception_handling_ja.md @@ -49,3 +49,23 @@ Limnestoneの処理の中で、I/Oエラーが発生すると、std::runtime_err * datastore::recover() * datastore::shutdown() +## 暫定対処 + +現在API呼び出し側で、limestone_exceptionをキャッチする仕組みが未実装である。 +このままだと、I/Oエラーが発生した場合に、プロセスがストールしてしまうことが +あるため、limestone_exceptionをスローする可能性があるAPIについて、 +limestone_exceptionをスローするのではなく、FATALのログを出力して +abortする対応をする。 + +また、APIのDoxygenコメントに以下の記述を追加する。 + +``` + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. +``` + + + + + diff --git a/include/limestone/api/cursor.h b/include/limestone/api/cursor.h index 58bc57bf..e2d9bc9a 100644 --- a/include/limestone/api/cursor.h +++ b/include/limestone/api/cursor.h @@ -55,6 +55,9 @@ class cursor { * @brief change the current cursor to point to the next entry * @attention this function is not thread-safe. * @exception limestone_exception if an error occurs while reading the log entry + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @return true if the next entry exists, false otherwise */ bool next(); diff --git a/include/limestone/api/datastore.h b/include/limestone/api/datastore.h index 7af35d2f..492292a7 100644 --- a/include/limestone/api/datastore.h +++ b/include/limestone/api/datastore.h @@ -69,6 +69,9 @@ class datastore { * @brief create an object with the given configuration * @param conf a reference to a configuration object used in the object construction * @exception limestone_exception if an I/O error occurs during construction + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. */ explicit datastore(configuration const& conf); @@ -114,6 +117,9 @@ class datastore { * @brief transition this object to an operational state * @details after this method is called, create_channel() can be invoked. * @exception limestone_io_exception Thrown if an I/O error occurs. + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @attention this function is not thread-safe, and the from directory must not contain any files other than log files. */ void ready(); @@ -153,6 +159,9 @@ class datastore { * @brief change the current epoch ID * @param new epoch id which must be greater than current epoch ID. * @exception limestone_io_exception Thrown if an I/O error occurs. + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @attention this function should be called after the ready() is called. */ void switch_epoch(epoch_id_type epoch_id); @@ -194,6 +203,9 @@ class datastore { * @brief start backup operation * @detail a backup object is created, which contains a list of log files. * @exception limestone_io_exception Thrown if an I/O error occurs. + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @return a reference to the backup object. */ backup& begin_backup(); @@ -203,6 +215,9 @@ class datastore { * @brief start backup operation * @detail a backup_detail object is created, which contains a list of log entry. * @exception limestone_io_exception Thrown if an I/O error occurs. + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @return a reference to the backup_detail object. */ std::unique_ptr begin_backup(backup_type btype); diff --git a/include/limestone/api/log_channel.h b/include/limestone/api/log_channel.h index 1c05b32e..492ad6d4 100644 --- a/include/limestone/api/log_channel.h +++ b/include/limestone/api/log_channel.h @@ -47,6 +47,9 @@ class log_channel { * @brief join a persistence session for the current epoch in this channel * @attention this function is not thread-safe. * @exception limestone_exception if I/O error occurs + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @note the current epoch is the last epoch specified by datastore::switch_epoch() * @note datastore::switch_epoch() and this function can be called simultaneously. * If these functions are invoked at the same time, the result will be as if one of them was called first, @@ -58,6 +61,9 @@ class log_channel { * @brief notifies the completion of an operation in this channel for the current persistent session the channel is participating in * @attention this function is not thread-safe. * @exception limestone_exception if I/O error occurs + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @note when all channels that have participated in the current persistent session call end_session() and the current epoch is * greater than the session's epoch, the persistent session itself is complete */ @@ -76,6 +82,9 @@ class log_channel { * @param value the value byte string for the entry to be added * @param write_version (optional) the write version of the entry to be added. If omitted, the default value is used * @exception limestone_exception if I/O error occurs + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @attention this function is not thread-safe. */ void add_entry(storage_id_type storage_id, std::string_view key, std::string_view value, write_version_type write_version); @@ -88,6 +97,9 @@ class log_channel { * @param write_version (optional) the write version of the entry to be added. If omitted, the default value is used * @param large_objects (optional) the list of large objects associated with the entry to be added * @exception limestone_exception if I/O error occurs + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @attention this function is not thread-safe. */ void add_entry(storage_id_type storage_id, std::string_view key, std::string_view value, write_version_type write_version, const std::vector& large_objects); @@ -98,6 +110,9 @@ class log_channel { * @param key the key byte string for the entry to be deleted * @param write_version the write version of the entry to be removed * @exception limestone_exception if I/O error occurs + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @attention this function is not thread-safe. * @note no deletion operation is performed on the entry that has been added to the current persistent session, instead, * the entries to be deleted are treated as if they do not exist in a recover() operation from a log stored in the current persistent session @@ -109,6 +124,9 @@ class log_channel { * @param storage_id the storage ID of the entry to be added * @param write_version the write version of the entry to be added * @exception limestone_exception if I/O error occurs + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @attention this function is not thread-safe. * @impl this operation may be ignored. */ @@ -119,6 +137,9 @@ class log_channel { * @param storage_id the storage ID of the entry to be removed * @param write_version the write version of the entry to be removed * @exception limestone_exception if I/O error occurs + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @attention this function is not thread-safe. * @note no deletion operation is performed on the entry that has been added to the current persistent session, instead, * the target entries are treated as if they do not exist in the recover() operation from the log stored in the current persistent session. @@ -130,6 +151,9 @@ class log_channel { * @param storage_id the storage ID of the entry to be removed * @param write_version the write version of the entry to be removed * @exception limestone_exception if I/O error occurs + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @attention this function is not thread-safe. * @note no deletion operation is performed on the entry that has been added to the current persistent session, instead, * the target entries are treated as if they do not exist in the recover() operation from the log stored in the current persistent session. diff --git a/include/limestone/api/snapshot.h b/include/limestone/api/snapshot.h index 197ffb33..7bbf5440 100644 --- a/include/limestone/api/snapshot.h +++ b/include/limestone/api/snapshot.h @@ -57,6 +57,9 @@ class snapshot { * @details the returned cursor points to the first element by calling cursor::next(). * @attention this function is thread-safe. * @exception limestone_exception if the file stream of the cursor is not good. + * @note Currently, this function does not throw an exception but logs the error and aborts the process. + * However, throwing an exception is the intended behavior, and this will be restored in future versions. + * Therefore, callers of this API must handle the exception properly as per the original design. * @return unique pointer of the cursor */ [[nodiscard]] std::unique_ptr get_cursor() const; diff --git a/src/limestone/cursor.cpp b/src/limestone/cursor.cpp index 5fc0d30f..73f85933 100644 --- a/src/limestone/cursor.cpp +++ b/src/limestone/cursor.cpp @@ -38,7 +38,12 @@ cursor::~cursor() noexcept { } bool cursor::next() { - return pimpl->next(); + try { + return pimpl->next(); + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + throw; // Unreachable, but required to satisfy the compiler + } } storage_id_type cursor::storage() const noexcept { diff --git a/src/limestone/datastore.cpp b/src/limestone/datastore.cpp index ecbfa273..ea68b086 100644 --- a/src/limestone/datastore.cpp +++ b/src/limestone/datastore.cpp @@ -40,55 +40,59 @@ using namespace limestone::internal; datastore::datastore() noexcept = default; datastore::datastore(configuration const& conf) : location_(conf.data_locations_.at(0)) { - LOG(INFO) << "/:limestone:config:datastore setting log location = " << location_.string(); - boost::system::error_code error; - const bool result_check = boost::filesystem::exists(location_, error); - boost::filesystem::path manifest_path = boost::filesystem::path(location_) / std::string(internal::manifest_file_name); - boost::filesystem::path compaction_catalog_path= boost::filesystem::path(location_) / compaction_catalog::get_catalog_filename(); - if (!result_check || error) { - const bool result_mkdir = boost::filesystem::create_directory(location_, error); - if (!result_mkdir || error) { - LOG_AND_THROW_IO_EXCEPTION("fail to create directory: " + location_.string(), error); - } - internal::setup_initial_logdir(location_); - add_file(manifest_path); - } else { - int count = 0; - // use existing log-dir - for (const boost::filesystem::path& p : boost::filesystem::directory_iterator(location_)) { - if (!boost::filesystem::is_directory(p)) { - count++; - add_file(p); + try { + LOG(INFO) << "/:limestone:config:datastore setting log location = " << location_.string(); + boost::system::error_code error; + const bool result_check = boost::filesystem::exists(location_, error); + boost::filesystem::path manifest_path = boost::filesystem::path(location_) / std::string(internal::manifest_file_name); + boost::filesystem::path compaction_catalog_path= boost::filesystem::path(location_) / compaction_catalog::get_catalog_filename(); + if (!result_check || error) { + const bool result_mkdir = boost::filesystem::create_directory(location_, error); + if (!result_mkdir || error) { + LOG_AND_THROW_IO_EXCEPTION("fail to create directory: " + location_.string(), error); } - } - if (count == 0) { internal::setup_initial_logdir(location_); add_file(manifest_path); + } else { + int count = 0; + // use existing log-dir + for (const boost::filesystem::path& p : boost::filesystem::directory_iterator(location_)) { + if (!boost::filesystem::is_directory(p)) { + count++; + add_file(p); + } + } + if (count == 0) { + internal::setup_initial_logdir(location_); + add_file(manifest_path); + } } - } - internal::check_and_migrate_logdir_format(location_); - add_file(compaction_catalog_path); - compaction_catalog_ = std::make_unique(compaction_catalog::from_catalog_file(location_)); - - // XXX: prusik era - // TODO: read rotated epoch files if main epoch file does not exist - epoch_file_path_ = location_ / boost::filesystem::path(std::string(limestone::internal::epoch_file_name)); - const bool result = boost::filesystem::exists(epoch_file_path_, error); - if (!result || error) { - FILE* strm = fopen(epoch_file_path_.c_str(), "a"); // NOLINT(*-owning-memory) - if (!strm) { - LOG_AND_THROW_IO_EXCEPTION("does not have write permission for the log_location directory, path: " + location_.string(), errno); - } - if (fclose(strm) != 0) { // NOLINT(*-owning-memory) - LOG_AND_THROW_IO_EXCEPTION("fclose failed", errno); + internal::check_and_migrate_logdir_format(location_); + add_file(compaction_catalog_path); + compaction_catalog_ = std::make_unique(compaction_catalog::from_catalog_file(location_)); + + // XXX: prusik era + // TODO: read rotated epoch files if main epoch file does not exist + epoch_file_path_ = location_ / boost::filesystem::path(std::string(limestone::internal::epoch_file_name)); + const bool result = boost::filesystem::exists(epoch_file_path_, error); + if (!result || error) { + FILE* strm = fopen(epoch_file_path_.c_str(), "a"); // NOLINT(*-owning-memory) + if (!strm) { + LOG_AND_THROW_IO_EXCEPTION("does not have write permission for the log_location directory, path: " + location_.string(), errno); + } + if (fclose(strm) != 0) { // NOLINT(*-owning-memory) + LOG_AND_THROW_IO_EXCEPTION("fclose failed", errno); + } + add_file(epoch_file_path_); } - add_file(epoch_file_path_); - } - recover_max_parallelism_ = conf.recover_max_parallelism_; - LOG(INFO) << "/:limestone:config:datastore setting the number of recover process thread = " << recover_max_parallelism_; + recover_max_parallelism_ = conf.recover_max_parallelism_; + LOG(INFO) << "/:limestone:config:datastore setting the number of recover process thread = " << recover_max_parallelism_; - VLOG_LP(log_debug) << "datastore is created, location = " << location_.string(); + VLOG_LP(log_debug) << "datastore is created, location = " << location_.string(); + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } datastore::~datastore() noexcept{ @@ -104,9 +108,13 @@ void datastore::recover() const noexcept { } void datastore::ready() { - create_snapshot(); - online_compaction_worker_future_ = std::async(std::launch::async, &datastore::online_compaction_worker, this); - state_ = state::ready; + try { + create_snapshot(); + online_compaction_worker_future_ = std::async(std::launch::async, &datastore::online_compaction_worker, this); + state_ = state::ready; + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } std::unique_ptr datastore::get_snapshot() const { @@ -132,15 +140,19 @@ log_channel& datastore::create_channel(const boost::filesystem::path& location) epoch_id_type datastore::last_epoch() const noexcept { return static_cast(epoch_id_informed_.load()); } void datastore::switch_epoch(epoch_id_type new_epoch_id) { - check_after_ready(static_cast(__func__)); - rotation_task_helper::attempt_task_execution_from_queue(); - auto neid = static_cast(new_epoch_id); - if (auto switched = epoch_id_switched_.load(); neid <= switched) { - LOG_LP(WARNING) << "switch to epoch_id_type of " << neid << " (<=" << switched << ") is curious"; - } + try { + check_after_ready(static_cast(__func__)); + rotation_task_helper::attempt_task_execution_from_queue(); + auto neid = static_cast(new_epoch_id); + if (auto switched = epoch_id_switched_.load(); neid <= switched) { + LOG_LP(WARNING) << "switch to epoch_id_type of " << neid << " (<=" << switched << ") is curious"; + } - epoch_id_switched_.store(neid); - update_min_epoch_id(true); + epoch_id_switched_.store(neid); + update_min_epoch_id(true); + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } void datastore::update_min_epoch_id(bool from_switch_epoch) { // NOLINT(readability-function-cognitive-complexity) @@ -240,92 +252,102 @@ std::future datastore::shutdown() noexcept { // old interface backup& datastore::begin_backup() { - auto tmp_files = get_files(); - backup_ = std::unique_ptr(new backup(tmp_files)); - return *backup_; + try { + auto tmp_files = get_files(); + backup_ = std::unique_ptr(new backup(tmp_files)); + return *backup_; + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } + return *backup_; // Required to satisfy the compiler } std::unique_ptr datastore::begin_backup(backup_type btype) { // NOLINT(readability-function-cognitive-complexity) - rotation_result result = rotate_log_files(); +try { + rotation_result result = rotate_log_files(); - // LOG-0: all files are log file, so all files are selected in both standard/transaction mode. - (void) btype; + // LOG-0: all files are log file, so all files are selected in both standard/transaction mode. + (void) btype; - // calculate files_ minus active-files - std::set inactive_files(result.get_rotation_end_files()); - inactive_files.erase(epoch_file_path_); - for (const auto& lc : log_channels_) { - if (lc->registered_) { - inactive_files.erase(lc->file_path()); + // calculate files_ minus active-files + std::set inactive_files(result.get_rotation_end_files()); + inactive_files.erase(epoch_file_path_); + for (const auto& lc : log_channels_) { + if (lc->registered_) { + inactive_files.erase(lc->file_path()); + } } - } - // build entries - std::vector entries; - for (auto & ent : inactive_files) { - // LOG-0: assume files are located flat in logdir. - std::string filename = ent.filename().string(); - auto dst = filename; - switch (filename[0]) { - case 'p': { - if (filename.find("wal", 1) == 1) { - // "pwal" - // pwal files are type:logfile, detached - - // skip an "inactive" file with the name of active file, - // it will cause some trouble if a file (that has the name of mutable files) is saved as immutable file. - // but, by skip, backup files may be imcomplete. - if (filename.length() == 9) { // FIXME: too adohoc check - boost::system::error_code error; - bool result = boost::filesystem::is_empty(ent, error); - if (!error && !result) { - LOG_LP(ERROR) << "skip the file with the name like active files: " << filename; + // build entries + std::vector entries; + for (auto & ent : inactive_files) { + // LOG-0: assume files are located flat in logdir. + std::string filename = ent.filename().string(); + auto dst = filename; + switch (filename[0]) { + case 'p': { + if (filename.find("wal", 1) == 1) { + // "pwal" + // pwal files are type:logfile, detached + + // skip an "inactive" file with the name of active file, + // it will cause some trouble if a file (that has the name of mutable files) is saved as immutable file. + // but, by skip, backup files may be imcomplete. + if (filename.length() == 9) { // FIXME: too adohoc check + boost::system::error_code error; + bool result = boost::filesystem::is_empty(ent, error); + if (!error && !result) { + LOG_LP(ERROR) << "skip the file with the name like active files: " << filename; + } + continue; } - continue; + entries.emplace_back(ent.string(), dst, false, false); + } else { + // unknown type } - entries.emplace_back(ent.string(), dst, false, false); - } else { - // unknown type + break; } - break; - } - case 'e': { - if (filename.find("poch", 1) == 1) { - // "epoch" - // epoch file(s) are type:logfile, the last rotated file is non-detached - - // skip active file - if (filename.length() == 5) { // FIXME: too adohoc check - continue; - } + case 'e': { + if (filename.find("poch", 1) == 1) { + // "epoch" + // epoch file(s) are type:logfile, the last rotated file is non-detached + + // skip active file + if (filename.length() == 5) { // FIXME: too adohoc check + continue; + } - // TODO: only last epoch file is not-detached - entries.emplace_back(ent.string(), dst, false, false); - } else { - // unknown type + // TODO: only last epoch file is not-detached + entries.emplace_back(ent.string(), dst, false, false); + } else { + // unknown type + } + break; } - break; - } - case 'l': { - if (filename == internal::manifest_file_name) { - entries.emplace_back(ent.string(), dst, true, false); - } else { - // unknown type + case 'l': { + if (filename == internal::manifest_file_name) { + entries.emplace_back(ent.string(), dst, true, false); + } else { + // unknown type + } + break; } - break; - } - case 'c': { - if (filename == compaction_catalog::get_catalog_filename()) { - entries.emplace_back(ent.string(), dst, false, false); + case 'c': { + if (filename == compaction_catalog::get_catalog_filename()) { + entries.emplace_back(ent.string(), dst, false, false); + } + break; + } + default: { + // unknown type } - break; - } - default: { - // unknown type } } + return std::unique_ptr(new backup_detail(entries, epoch_id_switched_.load())); + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + throw; // Unreachable, but required to satisfy the compiler } - return std::unique_ptr(new backup_detail(entries, epoch_id_switched_.load())); } tag_repository& datastore::epoch_tag_repository() noexcept { @@ -414,7 +436,7 @@ int64_t datastore::current_unix_epoch_in_millis() { } void datastore::online_compaction_worker() { - VLOG(log_info) << "online compaction worker started..." << std::endl; + LOG_LP(INFO) << "online compaction worker started..." << std::endl; boost::filesystem::path ctrl_dir = location_ / "ctrl"; boost::filesystem::path start_file = ctrl_dir / "start_compaction"; @@ -423,7 +445,7 @@ void datastore::online_compaction_worker() { ensure_directory_exists(ctrl_dir); if (!boost::filesystem::exists(ctrl_dir)) { if (!boost::filesystem::create_directory(ctrl_dir)) { - VLOG(log_error) << "failed to create directory: " << ctrl_dir.string(); + LOG_LP(INFO) << "failed to create directory: " << ctrl_dir.string(); return; } } diff --git a/src/limestone/limestone_exception_helper.cpp b/src/limestone/limestone_exception_helper.cpp new file mode 100644 index 00000000..a1fb3674 --- /dev/null +++ b/src/limestone/limestone_exception_helper.cpp @@ -0,0 +1,21 @@ +/* + * Copyright 2022-2024 Project Tsurugi. + * + * Licensed 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 "limestone_exception_helper.h" + +namespace limestone::testing { + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + bool enable_exception_throwing = false; +} diff --git a/src/limestone/limestone_exception_helper.h b/src/limestone/limestone_exception_helper.h index a08c5153..1ceaaaaf 100644 --- a/src/limestone/limestone_exception_helper.h +++ b/src/limestone/limestone_exception_helper.h @@ -23,6 +23,12 @@ #pragma once + namespace limestone::testing { + // This flag controls whether exceptions are thrown in tests or the process is aborted + // NOLINTNEXTLINE: non-const global variable is intentional + extern bool enable_exception_throwing; + } + namespace limestone { using limestone::api::limestone_exception; @@ -65,7 +71,30 @@ inline std::string extract_filename(const std::string& path) { throw limestone_io_exception(full_message + " (at " + extract_filename(__FILE__) + ":" + std::to_string(__LINE__) + ")", (error_code)); \ } +// helper function to handle exceptions and abort +inline void handle_exception_and_abort(std::string_view func_name) { + try { + throw; + } catch (const limestone_exception& e) { + if (limestone::testing::enable_exception_throwing) { + throw; + } + VLOG_LP(google::FATAL) << "Fatal error in " << func_name << ": " << e.what(); + std::abort(); // Safety measure: this should never be reached due to VLOG_LP(google::FATAL) + } catch (const std::runtime_error& e) { + VLOG_LP(google::FATAL) << "Runtime error in " << func_name << ": " << e.what(); + std::abort(); // Safety measure: this should never be reached due to VLOG_LP(google::FATAL) + } catch (const std::exception& e) { + VLOG_LP(google::FATAL) << "Unexpected exception in " << func_name << ": " << e.what(); + std::abort(); // Safety measure: this should never be reached due to VLOG_LP(google::FATAL) + } catch (...) { + VLOG_LP(google::FATAL) << "Unknown exception in " << func_name; + std::abort(); // Safety measure: this should never be reached due to VLOG_LP(google::FATAL) + } +} - +// macro to handle exceptions and abort +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) +#define HANDLE_EXCEPTION_AND_ABORT() handle_exception_and_abort(static_cast(__func__)) } // namespace limestone \ No newline at end of file diff --git a/src/limestone/log_channel.cpp b/src/limestone/log_channel.cpp index ce64cb45..525982bc 100644 --- a/src/limestone/log_channel.cpp +++ b/src/limestone/log_channel.cpp @@ -40,62 +40,77 @@ log_channel::log_channel(boost::filesystem::path location, std::size_t id, datas } void log_channel::begin_session() { - do { - current_epoch_id_.store(envelope_.epoch_id_switched_.load()); - std::atomic_thread_fence(std::memory_order_acq_rel); - } while (current_epoch_id_.load() != envelope_.epoch_id_switched_.load()); - latest_ession_epoch_id_.store(static_cast(current_epoch_id_.load())); - - auto log_file = file_path(); - strm_ = fopen(log_file.c_str(), "a"); // NOLINT(*-owning-memory) - if (!strm_) { - LOG_AND_THROW_IO_EXCEPTION("cannot make file on " + location_.string(), errno); - } - setvbuf(strm_, nullptr, _IOFBF, 128L * 1024L); // NOLINT, NB. glibc may ignore size when _IOFBF and buffer=NULL - if (!registered_) { - envelope_.add_file(log_file); - registered_ = true; - } - log_entry::begin_session(strm_, static_cast(current_epoch_id_.load())); - { - std::lock_guard lock(session_mutex_); - waiting_epoch_ids_.insert(latest_ession_epoch_id_); + try { + do { + current_epoch_id_.store(envelope_.epoch_id_switched_.load()); + std::atomic_thread_fence(std::memory_order_acq_rel); + } while (current_epoch_id_.load() != envelope_.epoch_id_switched_.load()); + latest_ession_epoch_id_.store(static_cast(current_epoch_id_.load())); + + auto log_file = file_path(); + strm_ = fopen(log_file.c_str(), "a"); // NOLINT(*-owning-memory) + if (!strm_) { + LOG_AND_THROW_IO_EXCEPTION("cannot make file on " + location_.string(), errno); + } + setvbuf(strm_, nullptr, _IOFBF, 128L * 1024L); // NOLINT, NB. glibc may ignore size when _IOFBF and buffer=NULL + if (!registered_) { + envelope_.add_file(log_file); + registered_ = true; + } + log_entry::begin_session(strm_, static_cast(current_epoch_id_.load())); + { + std::lock_guard lock(session_mutex_); + waiting_epoch_ids_.insert(latest_ession_epoch_id_); + } + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); } } void log_channel::end_session() { - if (fflush(strm_) != 0) { - LOG_AND_THROW_IO_EXCEPTION("fflush failed", errno); - } - if (fsync(fileno(strm_)) != 0) { - LOG_AND_THROW_IO_EXCEPTION("fsync failed", errno); - } - finished_epoch_id_.store(current_epoch_id_.load()); - current_epoch_id_.store(UINT64_MAX); - envelope_.update_min_epoch_id(); - - if (fclose(strm_) != 0) { // NOLINT(*-owning-memory) - LOG_AND_THROW_IO_EXCEPTION("fclose failed", errno); - } - - // Remove current_epoch_id_ from waiting_epoch_ids_ - { - std::lock_guard lock(session_mutex_); - waiting_epoch_ids_.erase(latest_ession_epoch_id_.load()); - // Notify waiting threads - session_cv_.notify_all(); + try { + if (fflush(strm_) != 0) { + LOG_AND_THROW_IO_EXCEPTION("fflush failed", errno); + } + if (fsync(fileno(strm_)) != 0) { + LOG_AND_THROW_IO_EXCEPTION("fsync failed", errno); + } + finished_epoch_id_.store(current_epoch_id_.load()); + current_epoch_id_.store(UINT64_MAX); + envelope_.update_min_epoch_id(); + + if (fclose(strm_) != 0) { // NOLINT(*-owning-memory) + LOG_AND_THROW_IO_EXCEPTION("fclose failed", errno); + } + + // Remove current_epoch_id_ from waiting_epoch_ids_ + { + std::lock_guard lock(session_mutex_); + waiting_epoch_ids_.erase(latest_ession_epoch_id_.load()); + // Notify waiting threads + session_cv_.notify_all(); + } + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); } } - void log_channel::abort_session([[maybe_unused]] status status_code, [[maybe_unused]] const std::string& message) noexcept { - LOG_LP(ERROR) << "not implemented"; - std::abort(); // FIXME + try { + LOG_LP(ERROR) << "not implemented"; + std::abort(); // FIXME + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } void log_channel::add_entry(storage_id_type storage_id, std::string_view key, std::string_view value, write_version_type write_version) { - log_entry::write(strm_, storage_id, key, value, write_version); - write_version_ = write_version; + try { + log_entry::write(strm_, storage_id, key, value, write_version); + write_version_ = write_version; + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } void log_channel::add_entry([[maybe_unused]] storage_id_type storage_id, [[maybe_unused]] std::string_view key, [[maybe_unused]] std::string_view value, [[maybe_unused]] write_version_type write_version, [[maybe_unused]] const std::vector& large_objects) { @@ -103,23 +118,39 @@ void log_channel::add_entry([[maybe_unused]] storage_id_type storage_id, [[maybe }; void log_channel::remove_entry(storage_id_type storage_id, std::string_view key, write_version_type write_version) { - log_entry::write_remove(strm_, storage_id, key, write_version); - write_version_ = write_version; + try { + log_entry::write_remove(strm_, storage_id, key, write_version); + write_version_ = write_version; + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } void log_channel::add_storage(storage_id_type storage_id, write_version_type write_version) { - log_entry::write_add_storage(strm_, storage_id, write_version); - write_version_ = write_version; + try { + log_entry::write_add_storage(strm_, storage_id, write_version); + write_version_ = write_version; + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } void log_channel::remove_storage(storage_id_type storage_id, write_version_type write_version) { - log_entry::write_remove_storage(strm_, storage_id, write_version); - write_version_ = write_version; + try { + log_entry::write_remove_storage(strm_, storage_id, write_version); + write_version_ = write_version; + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } void log_channel::truncate_storage(storage_id_type storage_id, write_version_type write_version) { - log_entry::write_clear_storage(strm_, storage_id, write_version); - write_version_ = write_version; + try { + log_entry::write_clear_storage(strm_, storage_id, write_version); + write_version_ = write_version; + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + } } boost::filesystem::path log_channel::file_path() const noexcept { diff --git a/src/limestone/snapshot.cpp b/src/limestone/snapshot.cpp index ab189e2f..76d63751 100644 --- a/src/limestone/snapshot.cpp +++ b/src/limestone/snapshot.cpp @@ -20,6 +20,7 @@ #include "compaction_catalog.h" #include "logging_helper.h" #include "snapshot_impl.h" +#include "limestone_exception_helper.h" namespace limestone::api { // FIXME fill implementation @@ -30,8 +31,14 @@ snapshot::snapshot(boost::filesystem::path location, : pimpl(std::make_unique(std::move(location), std::move(clear_storage))) { } + std::unique_ptr snapshot::get_cursor() const { - return pimpl->get_cursor(); + try { + return pimpl->get_cursor(); + } catch (...) { + HANDLE_EXCEPTION_AND_ABORT(); + throw; // Unreachable, but required to satisfy the compiler + } } std::unique_ptr snapshot::find([[maybe_unused]] storage_id_type storage_id, [[maybe_unused]] std::string_view entry_key) const noexcept { diff --git a/test/limestone/log/log_dir_test.cpp b/test/limestone/log/log_dir_test.cpp index 3eef8fa9..2a1e73f8 100644 --- a/test/limestone/log/log_dir_test.cpp +++ b/test/limestone/log/log_dir_test.cpp @@ -30,6 +30,7 @@ const boost::filesystem::path manifest_path = boost::filesystem::path(location) const boost::filesystem::path compaction_catalog_path = boost::filesystem::path(location) / "compaction_catalog"; void SetUp() { + limestone::testing::enable_exception_throwing = true; boost::filesystem::remove_all(location); if (!boost::filesystem::create_directory(location)) { std::cerr << "cannot make directory" << std::endl; @@ -46,6 +47,7 @@ const boost::filesystem::path compaction_catalog_path = boost::filesystem::path( } void TearDown() { + limestone::testing::enable_exception_throwing = false; datastore_ = nullptr; boost::filesystem::remove_all(location); } diff --git a/test/limestone/log/rotate_test.cpp b/test/limestone/log/rotate_test.cpp index c4ce1ebc..92061917 100644 --- a/test/limestone/log/rotate_test.cpp +++ b/test/limestone/log/rotate_test.cpp @@ -12,6 +12,7 @@ #include "test_root.h" #include "limestone/api/limestone_exception.h" +#include "limestone_exception_helper.h" using limestone::api::limestone_exception; #include "rotation_task.h" @@ -26,6 +27,7 @@ inline constexpr const char* location = "/tmp/rotate_test"; class rotate_test : public ::testing::Test { public: void SetUp() { + limestone::testing::enable_exception_throwing = true; boost::filesystem::remove_all(location); if (!boost::filesystem::create_directory(location)) { std::cerr << "cannot make directory" << std::endl; @@ -47,6 +49,7 @@ class rotate_test : public ::testing::Test { } void TearDown() { + limestone::testing::enable_exception_throwing = false; datastore_ = nullptr; boost::filesystem::remove_all(location); }