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
The following commit(s) were added to refs/heads/master by this push:
new 8d1b6e1 consensus: remove kudu::Bind usage from dirty callbacks
8d1b6e1 is described below
commit 8d1b6e191011a7eb6aa52930e2f2f59dc80e3c44
Author: Adar Dembo <[email protected]>
AuthorDate: Sat Mar 28 01:47:04 2020 -0700
consensus: remove kudu::Bind usage from dirty callbacks
And from various other miscellaneous places.
Change-Id: I2b18c3d37816d9314e74ec6c039461041fc87ddd
Reviewed-on: http://gerrit.cloudera.org:8080/15582
Reviewed-by: Andrew Wong <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Adar Dembo <[email protected]>
---
src/kudu/consensus/leader_election.cc | 9 ++-------
src/kudu/consensus/log.h | 1 -
src/kudu/consensus/raft_consensus.cc | 6 +++---
src/kudu/consensus/raft_consensus.h | 9 ++++-----
src/kudu/consensus/raft_consensus_quorum-test.cc | 3 +--
src/kudu/master/catalog_manager.cc | 4 +---
src/kudu/master/sys_catalog.cc | 11 +++++------
src/kudu/master/sys_catalog.h | 8 ++------
src/kudu/tablet/tablet_metadata.h | 1 -
src/kudu/tablet/tablet_replica-test.cc | 9 ++++-----
src/kudu/tablet/tablet_replica.cc | 5 +++--
src/kudu/tablet/tablet_replica.h | 9 +++------
src/kudu/tserver/tablet_copy_source_session-test.cc | 10 +++++-----
src/kudu/tserver/ts_tablet_manager.cc | 11 ++++-------
src/kudu/util/net/dns_resolver.cc | 1 -
src/kudu/util/net/dns_resolver.h | 1 -
16 files changed, 37 insertions(+), 61 deletions(-)
diff --git a/src/kudu/consensus/leader_election.cc
b/src/kudu/consensus/leader_election.cc
index 917d03e..4c81918 100644
--- a/src/kudu/consensus/leader_election.cc
+++ b/src/kudu/consensus/leader_election.cc
@@ -24,14 +24,11 @@
#include <utility>
#include <vector>
-#include <boost/bind.hpp> // IWYU pragma: keep
#include <glog/logging.h>
#include "kudu/common/wire_protocol.h"
#include "kudu/consensus/consensus_peers.h"
#include "kudu/consensus/metadata.pb.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/callback.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/strings/join.h"
@@ -251,6 +248,7 @@ void LeaderElection::Run() {
// The rest of the code below is for a typical multi-node configuration.
vector<string> other_voter_info;
other_voter_info.reserve(other_voter_uuids.size());
+ scoped_refptr<LeaderElection> self(this);
for (const auto& voter_uuid : other_voter_uuids) {
VoterState* state = nullptr;
{
@@ -285,10 +283,7 @@ void LeaderElection::Run() {
state->request,
&state->response,
&state->rpc,
- // We use gutil Bind() for the refcounting and boost::bind to adapt the
- // gutil Callback to a thunk.
- boost::bind(&Closure::Run,
- Bind(&LeaderElection::VoteResponseRpcCallback, this,
voter_uuid)));
+ [self, voter_uuid]() { self->VoteResponseRpcCallback(voter_uuid); });
}
LOG_WITH_PREFIX(INFO) << Substitute("Requested $0vote from peers $1",
request_.is_pre_election() ? "pre-" : "",
diff --git a/src/kudu/consensus/log.h b/src/kudu/consensus/log.h
index 68a24c2..a0358f7 100644
--- a/src/kudu/consensus/log.h
+++ b/src/kudu/consensus/log.h
@@ -37,7 +37,6 @@
#include "kudu/consensus/log_util.h"
#include "kudu/consensus/opid.pb.h"
#include "kudu/consensus/ref_counted_replicate.h"
-#include "kudu/gutil/callback.h" // IWYU pragma: keep
#include "kudu/gutil/macros.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/util/blocking_queue.h"
diff --git a/src/kudu/consensus/raft_consensus.cc
b/src/kudu/consensus/raft_consensus.cc
index 0c2e473..93699d6 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -227,14 +227,14 @@ Status RaftConsensus::Start(const ConsensusBootstrapInfo&
info,
unique_ptr<TimeManager> time_manager,
ConsensusRoundHandler* round_handler,
const scoped_refptr<MetricEntity>& metric_entity,
- Callback<void(const string& reason)>
mark_dirty_clbk) {
+ MarkDirtyCallback cb) {
DCHECK(metric_entity);
peer_proxy_factory_ = DCHECK_NOTNULL(std::move(peer_proxy_factory));
log_ = DCHECK_NOTNULL(std::move(log));
time_manager_ = DCHECK_NOTNULL(std::move(time_manager));
round_handler_ = DCHECK_NOTNULL(round_handler);
- mark_dirty_clbk_ = std::move(mark_dirty_clbk);
+ mark_dirty_clbk_ = std::move(cb);
term_metric_ = metric_entity->FindOrCreateGauge(&METRIC_raft_term,
CurrentTerm(),
@@ -2774,7 +2774,7 @@ log::RetentionIndexes
RaftConsensus::GetRetentionIndexes() {
}
void RaftConsensus::MarkDirty(const string& reason) {
- WARN_NOT_OK(raft_pool_token_->Submit([=]() {
this->mark_dirty_clbk_.Run(reason); }),
+ WARN_NOT_OK(raft_pool_token_->Submit([=]() { this->mark_dirty_clbk_(reason);
}),
LogPrefixThreadSafe() + "Unable to run MarkDirty callback");
}
diff --git a/src/kudu/consensus/raft_consensus.h
b/src/kudu/consensus/raft_consensus.h
index 912da5c..41e283c 100644
--- a/src/kudu/consensus/raft_consensus.h
+++ b/src/kudu/consensus/raft_consensus.h
@@ -18,6 +18,7 @@
#include <atomic>
#include <cstdint>
+#include <functional>
#include <iosfwd>
#include <memory>
#include <mutex>
@@ -36,7 +37,6 @@
#include "kudu/consensus/metadata.pb.h"
#include "kudu/consensus/opid.pb.h"
#include "kudu/consensus/ref_counted_replicate.h"
-#include "kudu/gutil/callback.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/ref_counted.h"
@@ -58,8 +58,6 @@ typedef std::unique_ptr<Lock> ScopedLock;
class Status;
class ThreadPool;
class ThreadPoolToken;
-template <typename Sig>
-class Callback;
namespace rpc {
class PeriodicTimer;
@@ -106,6 +104,7 @@ struct TabletVotingState {
typedef int64_t ConsensusTerm;
typedef StatusCallback ConsensusReplicatedCallback;
+typedef std::function<void(const std::string& reason)> MarkDirtyCallback;
class RaftConsensus : public std::enable_shared_from_this<RaftConsensus>,
public enable_make_shared<RaftConsensus>,
@@ -162,7 +161,7 @@ class RaftConsensus : public
std::enable_shared_from_this<RaftConsensus>,
std::unique_ptr<TimeManager> time_manager,
ConsensusRoundHandler* round_handler,
const scoped_refptr<MetricEntity>& metric_entity,
- Callback<void(const std::string& reason)> mark_dirty_clbk);
+ MarkDirtyCallback cb);
// Returns true if RaftConsensus is running.
bool IsRunning() const;
@@ -908,7 +907,7 @@ class RaftConsensus : public
std::enable_shared_from_this<RaftConsensus>,
// This is used to calculate back-off of the election timeout.
int64_t failed_elections_since_stable_leader_;
- Callback<void(const std::string& reason)> mark_dirty_clbk_;
+ MarkDirtyCallback mark_dirty_clbk_;
// A flag to help us avoid taking a lock on the reactor thread if the object
// is already in kShutdown state.
diff --git a/src/kudu/consensus/raft_consensus_quorum-test.cc
b/src/kudu/consensus/raft_consensus_quorum-test.cc
index ebdb8cc..4f740d2 100644
--- a/src/kudu/consensus/raft_consensus_quorum-test.cc
+++ b/src/kudu/consensus/raft_consensus_quorum-test.cc
@@ -54,7 +54,6 @@
#include "kudu/consensus/raft_consensus.h"
#include "kudu/consensus/time_manager.h"
#include "kudu/fs/fs_manager.h"
-#include "kudu/gutil/bind.h"
#include "kudu/gutil/casts.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/ref_counted.h"
@@ -227,7 +226,7 @@ class RaftConsensusQuorumTest : public KuduTest {
std::move(time_manager),
txn_factories_.back().get(),
metric_entity_,
- Bind(&DoNothing)));
+ &DoNothing));
}
return Status::OK();
}
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index 14e4173..19218ce 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -84,8 +84,6 @@
#include "kudu/fs/fs_manager.h"
#include "kudu/gutil/atomicops.h"
#include "kudu/gutil/basictypes.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/port.h"
@@ -1305,7 +1303,7 @@ Status CatalogManager::VisitTablesAndTablets() {
Status CatalogManager::InitSysCatalogAsync(bool is_first_run) {
std::lock_guard<LockType> l(lock_);
unique_ptr<SysCatalogTable> new_catalog(new SysCatalogTable(
- master_, Bind(&CatalogManager::ElectedAsLeaderCb, Unretained(this))));
+ master_, [this]() -> Status { return this->ElectedAsLeaderCb(); }));
if (is_first_run) {
RETURN_NOT_OK(new_catalog->CreateNew(master_->fs_manager()));
} else {
diff --git a/src/kudu/master/sys_catalog.cc b/src/kudu/master/sys_catalog.cc
index 5d02dc9..571af54 100644
--- a/src/kudu/master/sys_catalog.cc
+++ b/src/kudu/master/sys_catalog.cc
@@ -54,8 +54,6 @@
#include "kudu/consensus/quorum_util.h"
#include "kudu/consensus/raft_consensus.h"
#include "kudu/fs/fs_manager.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/strings/join.h"
#include "kudu/gutil/strings/substitute.h"
@@ -365,7 +363,7 @@ void SysCatalogTable::SysCatalogStateChanged(const string&
tablet_id, const stri
LOG_WITH_PREFIX(INFO) << "This master's current role is: "
<< RaftPeerPB::Role_Name(new_role);
if (new_role == RaftPeerPB::LEADER) {
- Status s = leader_cb_.Run();
+ Status s = leader_cb_();
// Callback errors are non-fatal only if the catalog manager has shut down.
if (!s.ok()) {
@@ -393,14 +391,15 @@ Status SysCatalogTable::SetupTablet(
// TODO(matteo): handle crash mid-creation of tablet? do we ever end up with
// a partially created tablet here?
+ const auto& tablet_id = metadata->tablet_id();
tablet_replica_.reset(new TabletReplica(
metadata,
cmeta_manager_,
local_peer_pb_,
master_->tablet_apply_pool(),
- Bind(&SysCatalogTable::SysCatalogStateChanged,
- Unretained(this),
- metadata->tablet_id())));
+ [this, tablet_id](const string& reason) {
+ this->SysCatalogStateChanged(tablet_id, reason);
+ }));
RETURN_NOT_OK_SHUTDOWN(tablet_replica_->Init({ /*quiescing*/nullptr,
master_->num_raft_leaders(),
master_->raft_pool() }),
diff --git a/src/kudu/master/sys_catalog.h b/src/kudu/master/sys_catalog.h
index 0b31d1f..35b52a0 100644
--- a/src/kudu/master/sys_catalog.h
+++ b/src/kudu/master/sys_catalog.h
@@ -14,8 +14,7 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-#ifndef KUDU_MASTER_SYS_CATALOG_H_
-#define KUDU_MASTER_SYS_CATALOG_H_
+#pragma once
#include <cstddef>
#include <cstdint>
@@ -29,7 +28,6 @@
#include "kudu/common/schema.h"
#include "kudu/consensus/metadata.pb.h"
-#include "kudu/gutil/callback.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/ref_counted.h"
@@ -141,7 +139,7 @@ class SysCatalogTable {
// The row ID of the latest notification log entry in the sys catalog table.
static const char* const kLatestNotificationLogEntryIdRowId;
- typedef Callback<Status()> ElectedLeaderCallback;
+ typedef std::function<Status()> ElectedLeaderCallback;
enum CatalogEntryType {
TABLES_ENTRY = 1,
@@ -382,5 +380,3 @@ class SysCatalogTable {
} // namespace master
} // namespace kudu
-
-#endif
diff --git a/src/kudu/tablet/tablet_metadata.h
b/src/kudu/tablet/tablet_metadata.h
index 384a9ca..6700c87 100644
--- a/src/kudu/tablet/tablet_metadata.h
+++ b/src/kudu/tablet/tablet_metadata.h
@@ -29,7 +29,6 @@
#include "kudu/common/partition.h"
#include "kudu/fs/block_id.h"
#include "kudu/gutil/atomicops.h"
-#include "kudu/gutil/callback.h" // IWYU pragma: keep
#include "kudu/gutil/macros.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/tablet/metadata.pb.h"
diff --git a/src/kudu/tablet/tablet_replica-test.cc
b/src/kudu/tablet/tablet_replica-test.cc
index 1139b66..b4d1750 100644
--- a/src/kudu/tablet/tablet_replica-test.cc
+++ b/src/kudu/tablet/tablet_replica-test.cc
@@ -47,8 +47,6 @@
#include "kudu/consensus/opid_util.h"
#include "kudu/consensus/raft_consensus.h"
#include "kudu/fs/fs_manager.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/rpc/messenger.h"
@@ -144,14 +142,15 @@ class TabletReplicaTest : public KuduTabletTest {
}
// "Bootstrap" and start the TabletReplica.
+ const auto& tablet_id = tablet()->tablet_id();
tablet_replica_.reset(
new TabletReplica(tablet()->shared_metadata(),
cmeta_manager_,
*config_peer,
apply_pool_.get(),
-
Bind(&TabletReplicaTest::TabletReplicaStateChangedCallback,
- Unretained(this),
- tablet()->tablet_id())));
+ [this, tablet_id](const string& reason) {
+ this->TabletReplicaStateChangedCallback(tablet_id,
reason);
+ }));
ASSERT_OK(tablet_replica_->Init({ /*quiescing*/nullptr,
/*num_leaders*/nullptr,
raft_pool_.get() }));
diff --git a/src/kudu/tablet/tablet_replica.cc
b/src/kudu/tablet/tablet_replica.cc
index ff93e09..28599d9 100644
--- a/src/kudu/tablet/tablet_replica.cc
+++ b/src/kudu/tablet/tablet_replica.cc
@@ -102,6 +102,7 @@ namespace tablet {
using consensus::ConsensusBootstrapInfo;
using consensus::ConsensusOptions;
using consensus::ConsensusRound;
+using consensus::MarkDirtyCallback;
using consensus::OpId;
using consensus::PeerProxyFactory;
using consensus::RaftConfigPB;
@@ -129,13 +130,13 @@ TabletReplica::TabletReplica(
scoped_refptr<consensus::ConsensusMetadataManager> cmeta_manager,
consensus::RaftPeerPB local_peer_pb,
ThreadPool* apply_pool,
- Callback<void(const std::string& reason)> mark_dirty_clbk)
+ MarkDirtyCallback cb)
: meta_(DCHECK_NOTNULL(std::move(meta))),
cmeta_manager_(DCHECK_NOTNULL(std::move(cmeta_manager))),
local_peer_pb_(std::move(local_peer_pb)),
log_anchor_registry_(new LogAnchorRegistry()),
apply_pool_(apply_pool),
- mark_dirty_clbk_(std::move(mark_dirty_clbk)),
+ mark_dirty_clbk_(std::move(cb)),
state_(NOT_INITIALIZED),
last_status_("Tablet initializing...") {
}
diff --git a/src/kudu/tablet/tablet_replica.h b/src/kudu/tablet/tablet_replica.h
index a7e115c..b120803 100644
--- a/src/kudu/tablet/tablet_replica.h
+++ b/src/kudu/tablet/tablet_replica.h
@@ -30,7 +30,6 @@
#include "kudu/consensus/metadata.pb.h"
#include "kudu/consensus/raft_consensus.h"
#include "kudu/fs/fs_manager.h"
-#include "kudu/gutil/callback.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/tablet/metadata.pb.h"
@@ -51,8 +50,6 @@ class MaintenanceOp;
class MonoDelta;
class ThreadPool;
class ThreadPoolToken;
-template <typename Sig>
-class Callback;
namespace consensus {
class ConsensusMetadataManager;
@@ -91,7 +88,7 @@ class TabletReplica : public
RefCountedThreadSafe<TabletReplica>,
scoped_refptr<consensus::ConsensusMetadataManager>
cmeta_manager,
consensus::RaftPeerPB local_peer_pb,
ThreadPool* apply_pool,
- Callback<void(const std::string& reason)> mark_dirty_clbk);
+ consensus::MarkDirtyCallback cb);
// Initializes RaftConsensus.
// This must be called before publishing the instance to other threads.
@@ -290,7 +287,7 @@ class TabletReplica : public
RefCountedThreadSafe<TabletReplica>,
// Marks the tablet as dirty so that it's included in the next heartbeat.
void MarkTabletDirty(const std::string& reason) {
- mark_dirty_clbk_.Run(reason);
+ mark_dirty_clbk_(reason);
}
// Return the total on-disk size of this tablet replica, in bytes.
@@ -347,7 +344,7 @@ class TabletReplica : public
RefCountedThreadSafe<TabletReplica>,
//
// Must be called whenever cluster membership or leadership changes, or when
// the tablet's schema changes.
- const Callback<void(const std::string& reason)> mark_dirty_clbk_;
+ const consensus::MarkDirtyCallback mark_dirty_clbk_;
TabletStatePB state_;
Status error_;
diff --git a/src/kudu/tserver/tablet_copy_source_session-test.cc
b/src/kudu/tserver/tablet_copy_source_session-test.cc
index ff94519..e528662 100644
--- a/src/kudu/tserver/tablet_copy_source_session-test.cc
+++ b/src/kudu/tserver/tablet_copy_source_session-test.cc
@@ -18,6 +18,7 @@
#include "kudu/tserver/tablet_copy_source_session.h"
#include <cstdint>
+#include <functional>
#include <memory>
#include <ostream>
#include <string>
@@ -43,8 +44,6 @@
#include "kudu/fs/block_id.h"
#include "kudu/fs/block_manager.h"
#include "kudu/fs/fs_manager.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/gutil/strings/fastmem.h"
#include "kudu/gutil/strings/substitute.h"
@@ -158,14 +157,15 @@ class TabletCopyTest : public KuduTabletTest {
new ConsensusMetadataManager(fs_manager()));
ASSERT_OK(cmeta_manager->Create(tablet()->tablet_id(), config,
kMinimumTerm));
+ const auto& tablet_id = tablet()->tablet_id();
tablet_replica_.reset(
new TabletReplica(tablet()->metadata(),
cmeta_manager,
*config_peer,
apply_pool_.get(),
-
Bind(&TabletCopyTest::TabletReplicaStateChangedCallback,
- Unretained(this),
- tablet()->tablet_id())));
+ [this, tablet_id](const string& reason) {
+ this->TabletReplicaStateChangedCallback(tablet_id,
reason);
+ }));
ASSERT_OK(tablet_replica_->Init({ /*quiescing*/nullptr,
/*num_leaders*/nullptr,
raft_pool_.get() }));
diff --git a/src/kudu/tserver/ts_tablet_manager.cc
b/src/kudu/tserver/ts_tablet_manager.cc
index 7a1481a..706c2e5 100644
--- a/src/kudu/tserver/ts_tablet_manager.cc
+++ b/src/kudu/tserver/ts_tablet_manager.cc
@@ -26,7 +26,6 @@
#include <utility>
#include <vector>
-#include <boost/bind.hpp> // IWYU pragma: keep
#include <boost/optional/optional.hpp>
#include <gflags/gflags.h>
#include <glog/logging.h>
@@ -46,8 +45,6 @@
#include "kudu/consensus/raft_consensus.h"
#include "kudu/fs/data_dirs.h"
#include "kudu/fs/fs_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/strings/substitute.h"
@@ -830,15 +827,15 @@ Status TSTabletManager::CreateAndRegisterTabletReplica(
scoped_refptr<TabletMetadata> meta,
RegisterTabletReplicaMode mode,
scoped_refptr<TabletReplica>* replica_out) {
- const string& tablet_id = meta->tablet_id();
+ const auto& tablet_id = meta->tablet_id();
scoped_refptr<TabletReplica> replica(
new TabletReplica(std::move(meta),
cmeta_manager_,
local_peer_pb_,
server_->tablet_apply_pool(),
- Bind(&TSTabletManager::MarkTabletDirty,
- Unretained(this),
- tablet_id)));
+ [this, tablet_id](const string& reason) {
+ this->MarkTabletDirty(tablet_id, reason);
+ }));
Status s = replica->Init({ server_->mutable_quiescing(),
server_->num_raft_leaders(),
server_->raft_pool() });
diff --git a/src/kudu/util/net/dns_resolver.cc
b/src/kudu/util/net/dns_resolver.cc
index 75a12b7..179dc50 100644
--- a/src/kudu/util/net/dns_resolver.cc
+++ b/src/kudu/util/net/dns_resolver.cc
@@ -24,7 +24,6 @@
#include <gflags/gflags.h>
#include <glog/logging.h>
-#include "kudu/gutil/callback.h" // IWYU pragma: keep
#include "kudu/gutil/port.h"
#include "kudu/util/flag_tags.h"
#include "kudu/util/malloc.h"
diff --git a/src/kudu/util/net/dns_resolver.h b/src/kudu/util/net/dns_resolver.h
index 93cf130..6cde473 100644
--- a/src/kudu/util/net/dns_resolver.h
+++ b/src/kudu/util/net/dns_resolver.h
@@ -14,7 +14,6 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-
#pragma once
#include <cstddef>