This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 0c30c723e [fs] simplify FsErrorManager
0c30c723e is described below
commit 0c30c723e419f0a659cb7b809e3a11c539bf227b
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]>
---
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();
