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 <[email protected]> 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 <[email protected]> Reviewed-by: Alexey Serbin <[email protected]> Reviewed-by: Andrew Wong <[email protected]> --- 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));
