You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2020/03/30 18:56:39 UTC
[kudu] 05/07: fs: remove kudu::Bind usage from ErrorManager
This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit ea9243a10cfa7d67547d3e1996af35b0455d1e97
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Sat Mar 28 01:42:01 2020 -0700
fs: remove kudu::Bind usage from ErrorManager
Change-Id: I15df4eda0e205535df5ec3700f66164840a1b183
Reviewed-on: http://gerrit.cloudera.org:8080/15579
Tested-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/fs/error_manager-test.cc | 36 +++++++++++++++++++----------------
src/kudu/fs/error_manager.cc | 11 +++++------
src/kudu/fs/error_manager.h | 5 ++---
src/kudu/fs/fs_manager.cc | 9 +++++----
src/kudu/fs/log_block_manager-test.cc | 9 +++++----
src/kudu/master/master.cc | 5 ++---
src/kudu/tserver/tablet_server.cc | 15 +++++++++------
7 files changed, 48 insertions(+), 42 deletions(-)
diff --git a/src/kudu/fs/error_manager-test.cc b/src/kudu/fs/error_manager-test.cc
index 33a4866..538d3db 100644
--- a/src/kudu/fs/error_manager-test.cc
+++ b/src/kudu/fs/error_manager-test.cc
@@ -15,11 +15,14 @@
// specific language governing permissions and limitations
// under the License.
+#include "kudu/fs/error_manager.h"
+
+#include <cstdlib>
+#include <functional>
#include <map>
#include <memory>
#include <set>
#include <sstream>
-#include <stdlib.h>
#include <string>
#include <thread>
#include <vector>
@@ -27,9 +30,6 @@
#include <glog/logging.h>
#include <gtest/gtest.h>
-#include "kudu/fs/error_manager.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/strings/join.h"
#include "kudu/gutil/threading/thread_collision_warner.h"
@@ -129,9 +129,10 @@ TEST_F(FsErrorManagerTest, TestBasicRegistration) {
// Register a callback to update the first '-1' entry in test_vec_ to '0'
// after waiting a random amount of time.
- em()->SetErrorNotificationCb(ErrorHandlerType::DISK_ERROR,
- Bind(&FsErrorManagerTest::SleepAndWriteFirstEmptyCb,
- Unretained(this), ErrorHandlerType::DISK_ERROR));
+ em()->SetErrorNotificationCb(
+ ErrorHandlerType::DISK_ERROR, [this](const string& uuid) {
+ this->SleepAndWriteFirstEmptyCb(ErrorHandlerType::DISK_ERROR, uuid);
+ });
em()->RunErrorNotificationCb(ErrorHandlerType::DISK_ERROR, "");
ASSERT_EQ(0, FindFirst(ErrorHandlerType::DISK_ERROR));
@@ -141,9 +142,10 @@ TEST_F(FsErrorManagerTest, TestBasicRegistration) {
ASSERT_EQ(-1, FindFirst(ErrorHandlerType::NO_AVAILABLE_DISKS));
// Now register another callback.
- em()->SetErrorNotificationCb(ErrorHandlerType::NO_AVAILABLE_DISKS,
- Bind(&FsErrorManagerTest::SleepAndWriteFirstEmptyCb,
- Unretained(this), ErrorHandlerType::NO_AVAILABLE_DISKS));
+ em()->SetErrorNotificationCb(
+ ErrorHandlerType::NO_AVAILABLE_DISKS, [this](const string& uuid) {
+ this->SleepAndWriteFirstEmptyCb(ErrorHandlerType::NO_AVAILABLE_DISKS, uuid);
+ });
em()->RunErrorNotificationCb(ErrorHandlerType::NO_AVAILABLE_DISKS, "");
ASSERT_EQ(1, FindFirst(ErrorHandlerType::NO_AVAILABLE_DISKS));
@@ -162,12 +164,14 @@ TEST_F(FsErrorManagerTest, TestBasicRegistration) {
// Test that the callbacks get run serially.
TEST_F(FsErrorManagerTest, TestSerialization) {
- em()->SetErrorNotificationCb(ErrorHandlerType::DISK_ERROR,
- Bind(&FsErrorManagerTest::SleepAndWriteFirstEmptyCb,
- Unretained(this), ErrorHandlerType::DISK_ERROR));
- em()->SetErrorNotificationCb(ErrorHandlerType::NO_AVAILABLE_DISKS,
- Bind(&FsErrorManagerTest::SleepAndWriteFirstEmptyCb,
- Unretained(this), ErrorHandlerType::NO_AVAILABLE_DISKS));
+ em()->SetErrorNotificationCb(
+ ErrorHandlerType::DISK_ERROR, [this](const string& uuid) {
+ this->SleepAndWriteFirstEmptyCb(ErrorHandlerType::DISK_ERROR, uuid);
+ });
+ em()->SetErrorNotificationCb(
+ ErrorHandlerType::NO_AVAILABLE_DISKS, [this](const string& uuid) {
+ this->SleepAndWriteFirstEmptyCb(ErrorHandlerType::NO_AVAILABLE_DISKS, uuid);
+ });
// Swap back and forth between error-handler type.
const auto IntToEnum = [&] (int i) {
diff --git a/src/kudu/fs/error_manager.cc b/src/kudu/fs/error_manager.cc
index 0e2bb41..55b78db 100644
--- a/src/kudu/fs/error_manager.cc
+++ b/src/kudu/fs/error_manager.cc
@@ -21,7 +21,6 @@
#include <string>
#include <utility>
-#include "kudu/gutil/bind.h"
#include "kudu/gutil/map-util.h"
using std::string;
@@ -33,9 +32,9 @@ namespace fs {
static void DoNothingErrorNotification(const string& /* uuid */) {}
FsErrorManager::FsErrorManager() {
- InsertOrDie(&callbacks_, ErrorHandlerType::DISK_ERROR, Bind(DoNothingErrorNotification));
- InsertOrDie(&callbacks_, ErrorHandlerType::NO_AVAILABLE_DISKS, Bind(DoNothingErrorNotification));
- InsertOrDie(&callbacks_, ErrorHandlerType::CFILE_CORRUPTION, Bind(DoNothingErrorNotification));
+ InsertOrDie(&callbacks_, ErrorHandlerType::DISK_ERROR, &DoNothingErrorNotification);
+ InsertOrDie(&callbacks_, ErrorHandlerType::NO_AVAILABLE_DISKS, &DoNothingErrorNotification);
+ InsertOrDie(&callbacks_, ErrorHandlerType::CFILE_CORRUPTION, &DoNothingErrorNotification);
}
void FsErrorManager::SetErrorNotificationCb(ErrorHandlerType e, ErrorNotificationCb cb) {
@@ -45,12 +44,12 @@ void FsErrorManager::SetErrorNotificationCb(ErrorHandlerType e, ErrorNotificatio
void FsErrorManager::UnsetErrorNotificationCb(ErrorHandlerType e) {
std::lock_guard<Mutex> l(lock_);
- EmplaceOrUpdate(&callbacks_, e, Bind(DoNothingErrorNotification));
+ EmplaceOrUpdate(&callbacks_, e, &DoNothingErrorNotification);
}
void FsErrorManager::RunErrorNotificationCb(ErrorHandlerType e, const string& uuid) const {
std::lock_guard<Mutex> l(lock_);
- FindOrDie(callbacks_, e).Run(uuid);
+ FindOrDie(callbacks_, e)(uuid);
}
} // namespace fs
diff --git a/src/kudu/fs/error_manager.h b/src/kudu/fs/error_manager.h
index 6348971..b045223 100644
--- a/src/kudu/fs/error_manager.h
+++ b/src/kudu/fs/error_manager.h
@@ -14,9 +14,9 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-
#pragma once
+#include <functional>
#include <string>
#include <unordered_map>
#include <utility>
@@ -25,7 +25,6 @@
#include "kudu/fs/dir_manager.h"
#include "kudu/fs/dir_util.h"
-#include "kudu/gutil/callback.h"
#include "kudu/gutil/port.h"
#include "kudu/util/mutex.h"
@@ -37,7 +36,7 @@ namespace fs {
//
// e.g. the ErrorNotificationCb for disk failure handling takes the UUID of a
// directory, marks it failed, and shuts down the tablets in that directory.
-typedef Callback<void(const std::string&)> ErrorNotificationCb;
+typedef std::function<void(const std::string&)> ErrorNotificationCb;
// Evaluates the expression and handles it if it results in an error.
// Returns if the status is an error.
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index 09f623a..3b866f8 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -19,6 +19,7 @@
#include <cinttypes>
#include <ctime>
+#include <functional>
#include <initializer_list>
#include <iostream>
#include <unordered_map>
@@ -37,8 +38,6 @@
#include "kudu/fs/fs.pb.h"
#include "kudu/fs/fs_report.h"
#include "kudu/fs/log_block_manager.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/stringprintf.h"
@@ -423,8 +422,10 @@ Status FsManager::Open(FsReport* report) {
}
// Set an initial error handler to mark data directories as failed.
- error_manager_->SetErrorNotificationCb(ErrorHandlerType::DISK_ERROR,
- Bind(&DataDirManager::MarkDirFailedByUuid, Unretained(dd_manager_.get())));
+ error_manager_->SetErrorNotificationCb(
+ ErrorHandlerType::DISK_ERROR, [this](const string& uuid) {
+ this->dd_manager_->MarkDirFailedByUuid(uuid);
+ });
// Finally, initialize and open the block manager if needed.
if (!opts_.skip_block_manager) {
diff --git a/src/kudu/fs/log_block_manager-test.cc b/src/kudu/fs/log_block_manager-test.cc
index e1e2676..37c13ec 100644
--- a/src/kudu/fs/log_block_manager-test.cc
+++ b/src/kudu/fs/log_block_manager-test.cc
@@ -22,6 +22,7 @@
#include <cstdlib>
#include <cstring>
#include <deque>
+#include <functional>
#include <initializer_list>
#include <memory>
#include <ostream>
@@ -45,8 +46,6 @@
#include "kudu/fs/fs.pb.h"
#include "kudu/fs/fs_report.h"
#include "kudu/fs/log_block_manager-test-util.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/casts.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/ref_counted.h"
@@ -1689,8 +1688,10 @@ TEST_F(LogBlockManagerTest, TestOpenWithFailedDirectories) {
DataDirManagerOptions(), &dd_manager_));
// Wire in a callback to fail data directories.
- error_manager_.SetErrorNotificationCb(ErrorHandlerType::DISK_ERROR,
- Bind(&DataDirManager::MarkDirFailedByUuid, Unretained(dd_manager_.get())));
+ error_manager_.SetErrorNotificationCb(
+ ErrorHandlerType::DISK_ERROR, [this](const string& uuid) {
+ this->dd_manager_->MarkDirFailedByUuid(uuid);
+ });
bm_.reset(CreateBlockManager(nullptr));
// Fail one of the directories, chosen randomly.
diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc
index 0226a72..f55cc6a 100644
--- a/src/kudu/master/master.cc
+++ b/src/kudu/master/master.cc
@@ -35,7 +35,6 @@
#include "kudu/consensus/raft_consensus.h"
#include "kudu/fs/error_manager.h"
#include "kudu/fs/fs_manager.h"
-#include "kudu/gutil/bind.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/master/catalog_manager.h"
@@ -169,9 +168,9 @@ Status Master::Start() {
Status Master::StartAsync() {
CHECK_EQ(kInitialized, state_);
fs_manager_->SetErrorNotificationCb(ErrorHandlerType::DISK_ERROR,
- Bind(&Master::CrashMasterOnDiskError));
+ &CrashMasterOnDiskError);
fs_manager_->SetErrorNotificationCb(ErrorHandlerType::CFILE_CORRUPTION,
- Bind(&Master::CrashMasterOnCFileCorruption));
+ &CrashMasterOnCFileCorruption);
RETURN_NOT_OK(maintenance_manager_->Start());
diff --git a/src/kudu/tserver/tablet_server.cc b/src/kudu/tserver/tablet_server.cc
index b82d359..dd33ecb 100644
--- a/src/kudu/tserver/tablet_server.cc
+++ b/src/kudu/tserver/tablet_server.cc
@@ -17,6 +17,7 @@
#include "kudu/tserver/tablet_server.h"
+#include <functional>
#include <memory>
#include <ostream>
#include <utility>
@@ -27,8 +28,6 @@
#include "kudu/cfile/block_cache.h"
#include "kudu/fs/error_manager.h"
#include "kudu/fs/fs_manager.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/rpc/service_if.h"
#include "kudu/server/rpc_server.h"
@@ -122,10 +121,14 @@ Status TabletServer::WaitInited() {
Status TabletServer::Start() {
CHECK_EQ(kInitialized, state_);
- fs_manager_->SetErrorNotificationCb(ErrorHandlerType::DISK_ERROR,
- Bind(&TSTabletManager::FailTabletsInDataDir, Unretained(tablet_manager_.get())));
- fs_manager_->SetErrorNotificationCb(ErrorHandlerType::CFILE_CORRUPTION,
- Bind(&TSTabletManager::FailTabletAndScheduleShutdown, Unretained(tablet_manager_.get())));
+ fs_manager_->SetErrorNotificationCb(
+ ErrorHandlerType::DISK_ERROR, [this](const string& uuid) {
+ this->tablet_manager_->FailTabletsInDataDir(uuid);
+ });
+ fs_manager_->SetErrorNotificationCb(
+ ErrorHandlerType::CFILE_CORRUPTION, [this](const string& uuid) {
+ this->tablet_manager_->FailTabletAndScheduleShutdown(uuid);
+ });
unique_ptr<ServiceIf> ts_service(new TabletServiceImpl(this));
unique_ptr<ServiceIf> admin_service(new TabletServiceAdminImpl(this));