This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch branch-1.18.x in repository https://gitbox.apache.org/repos/asf/kudu.git
commit dcde98c7cb130c126458fb3c4ccf7cc7eeedb503 Author: Alexey Serbin <[email protected]> AuthorDate: Fri Nov 15 15:01:54 2024 -0800 [fs] simplify FsErrorManager This patch updates the code in FsErrorManager to use * std::array instead of std::unordered_map * std::mutex instead of Mutex Also, {Master,TabletServer}::ShutdownImpl() now unregister all the error handlers that have been registered. Change-Id: I0a6f82f4da173949f2fc40517ebacd34be3a0434 Reviewed-on: http://gerrit.cloudera.org:8080/22074 Tested-by: Kudu Jenkins Reviewed-by: Yifan Zhang <[email protected]> (cherry picked from commit 0c30c723e419f0a659cb7b809e3a11c539bf227b) Reviewed-on: http://gerrit.cloudera.org:8080/22305 Reviewed-by: Abhishek Chennaka <[email protected]> Tested-by: Alexey Serbin <[email protected]> --- src/kudu/fs/error_manager.cc | 15 +++++++-------- src/kudu/fs/error_manager.h | 36 +++++++++++++++++++++--------------- src/kudu/master/master.cc | 3 ++- src/kudu/tserver/tablet_server.cc | 3 ++- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/kudu/fs/error_manager.cc b/src/kudu/fs/error_manager.cc index 6ef1aaad8..44b74371a 100644 --- a/src/kudu/fs/error_manager.cc +++ b/src/kudu/fs/error_manager.cc @@ -20,8 +20,6 @@ #include <string> #include <utility> -#include "kudu/gutil/map-util.h" - using std::string; namespace kudu { @@ -32,26 +30,27 @@ static void DoNothingErrorNotification(const string& /* uuid */, const string& /* tenant_id */) {} FsErrorManager::FsErrorManager() { - InsertOrDie(&callbacks_, ErrorHandlerType::DISK_ERROR, &DoNothingErrorNotification); - InsertOrDie(&callbacks_, ErrorHandlerType::NO_AVAILABLE_DISKS, &DoNothingErrorNotification); - InsertOrDie(&callbacks_, ErrorHandlerType::CFILE_CORRUPTION, &DoNothingErrorNotification); + callbacks_.fill(&DoNothingErrorNotification); } void FsErrorManager::SetErrorNotificationCb(ErrorHandlerType e, ErrorNotificationCb cb) { + DCHECK_LT(e, callbacks_.max_size()); std::lock_guard l(lock_); - EmplaceOrUpdate(&callbacks_, e, std::move(cb)); + callbacks_[e] = std::move(cb); } void FsErrorManager::UnsetErrorNotificationCb(ErrorHandlerType e) { + DCHECK_LT(e, callbacks_.max_size()); std::lock_guard l(lock_); - EmplaceOrUpdate(&callbacks_, e, &DoNothingErrorNotification); + callbacks_[e] = &DoNothingErrorNotification; } void FsErrorManager::RunErrorNotificationCb(ErrorHandlerType e, const string& uuid, const string& tenant_id) const { + DCHECK_LT(e, callbacks_.max_size()); std::lock_guard l(lock_); - FindOrDie(callbacks_, e)(uuid, tenant_id); + callbacks_[e](uuid, tenant_id); } } // namespace fs diff --git a/src/kudu/fs/error_manager.h b/src/kudu/fs/error_manager.h index 8c89195a0..04c9ac6ad 100644 --- a/src/kudu/fs/error_manager.h +++ b/src/kudu/fs/error_manager.h @@ -16,18 +16,19 @@ // under the License. #pragma once +#include <array> +#include <cstdint> #include <functional> +#include <mutex> #include <string> -#include <unordered_map> -#include <utility> #include <glog/logging.h> #include "kudu/fs/dir_manager.h" #include "kudu/fs/dir_util.h" +#include "kudu/gutil/macros.h" #include "kudu/gutil/port.h" #include "kudu/gutil/ref_counted.h" -#include "kudu/util/mutex.h" namespace kudu { namespace fs { @@ -85,9 +86,9 @@ typedef std::function<void(const std::string&, const std::string&)> ErrorNotific } \ } while (0) -enum ErrorHandlerType { +enum ErrorHandlerType : uint8_t { // For disk failures. - DISK_ERROR, + DISK_ERROR = 0, // For errors that caused by no data dirs being available (e.g. if all disks // are full or failed when creating a block). @@ -112,13 +113,16 @@ enum ErrorHandlerType { // is the subsequent failure to create a block because all disks have been // marked as failed) must complete before ERROR2 can be returned to its // caller. - NO_AVAILABLE_DISKS, + NO_AVAILABLE_DISKS = 1, // For CFile corruptions. - CFILE_CORRUPTION, + CFILE_CORRUPTION = 2, // For broken invariants caused by KUDU-2233. - KUDU_2233_CORRUPTION, + KUDU_2233_CORRUPTION = 3, + + // Update ERROR_HANDLER_TYPE_MAX if adding new elements into the enum. + ERROR_HANDLER_TYPE_MAX = KUDU_2233_CORRUPTION, }; // When certain operations fail, the side effects of the error can span multiple @@ -133,6 +137,7 @@ enum ErrorHandlerType { class FsErrorManager : public RefCountedThreadSafe<FsErrorManager> { public: FsErrorManager(); + ~FsErrorManager() = default; // Sets the error notification callback. // @@ -166,15 +171,16 @@ class FsErrorManager : public RefCountedThreadSafe<FsErrorManager> { } private: - friend class RefCountedThreadSafe<FsErrorManager>; - ~FsErrorManager() {} + // Callbacks to be run when an error occurs. + std::array<ErrorNotificationCb, + ErrorHandlerType::ERROR_HANDLER_TYPE_MAX + 1> callbacks_; - // Callbacks to be run when an error occurs. - std::unordered_map<ErrorHandlerType, ErrorNotificationCb, std::hash<int>> callbacks_; + // Protects calls to notifications, enforcing that a single callback runs at + // a time. Since callbacks might lead to IO and memory allocation, using a + // busy-waiting primitive isn't an option here. + mutable std::mutex lock_; - // Protects calls to notifications, enforcing that a single callback runs at - // a time. - mutable Mutex lock_; + DISALLOW_COPY_AND_ASSIGN(FsErrorManager); }; } // namespace fs diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc index 1c2ba417a..484094704 100644 --- a/src/kudu/master/master.cc +++ b/src/kudu/master/master.cc @@ -544,8 +544,9 @@ void Master::ShutdownImpl() { init_pool_->Shutdown(); maintenance_manager_->Shutdown(); catalog_manager_->Shutdown(); - fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::DISK_ERROR); + fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::KUDU_2233_CORRUPTION); fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::CFILE_CORRUPTION); + fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::DISK_ERROR); // 3. Shut down generic subsystems. KuduServer::Shutdown(); diff --git a/src/kudu/tserver/tablet_server.cc b/src/kudu/tserver/tablet_server.cc index 247d86069..37bf958da 100644 --- a/src/kudu/tserver/tablet_server.cc +++ b/src/kudu/tserver/tablet_server.cc @@ -183,8 +183,9 @@ void TabletServer::ShutdownImpl() { // 2. Shut down the tserver's subsystems. maintenance_manager_->Shutdown(); WARN_NOT_OK(heartbeater_->Stop(), "Failed to stop TS Heartbeat thread"); - fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::DISK_ERROR); + fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::KUDU_2233_CORRUPTION); fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::CFILE_CORRUPTION); + fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::DISK_ERROR); tablet_manager_->Shutdown(); client_initializer_->Shutdown();
