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
commit 8ffa18196dcd3917ae624f9dc9679040e65967cb
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Sun May 23 09:00:15 2021 -0700
[consensus] a small optimisation on log prefix
This patch introduces a small optimisation into the implementation of
the RaftConsensus::LogPrefixThreadSafe() method: since the prefix
it produces doesn't change, it's possible to construct the string once
in the constructor and then return a constant reference.
I also updated PendingRound class to store a reference to the prefix as
well since instances of the PendingRound class have shorter live span
than RaftConsensus objects which create PendingRound objects.
Change-Id: I6a5082fe07449ef24827696638ddd8e925a08264
Reviewed-on: http://gerrit.cloudera.org:8080/17489
Tested-by: Alexey Serbin <aser...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/consensus/pending_rounds.cc | 8 +++-----
src/kudu/consensus/pending_rounds.h | 8 +++++---
src/kudu/consensus/raft_consensus.cc | 10 ++--------
src/kudu/consensus/raft_consensus.h | 7 ++++++-
4 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/kudu/consensus/pending_rounds.cc
b/src/kudu/consensus/pending_rounds.cc
index 9200b07..cd275dc 100644
--- a/src/kudu/consensus/pending_rounds.cc
+++ b/src/kudu/consensus/pending_rounds.cc
@@ -47,14 +47,12 @@ namespace consensus {
// PendingRounds
//------------------------------------------------------------
-PendingRounds::PendingRounds(string log_prefix, TimeManager* time_manager)
- : log_prefix_(std::move(log_prefix)),
+PendingRounds::PendingRounds(const string& log_prefix,
+ TimeManager* time_manager)
+ : log_prefix_(log_prefix),
last_committed_op_id_(MinimumOpId()),
time_manager_(time_manager) {}
-PendingRounds::~PendingRounds() {
-}
-
Status PendingRounds::CancelPendingOps() {
ThreadRestrictions::AssertWaitAllowed();
if (pending_ops_.empty()) {
diff --git a/src/kudu/consensus/pending_rounds.h
b/src/kudu/consensus/pending_rounds.h
index 742dbeb..530db2f 100644
--- a/src/kudu/consensus/pending_rounds.h
+++ b/src/kudu/consensus/pending_rounds.h
@@ -42,8 +42,8 @@ class TimeManager;
// operations.
class PendingRounds {
public:
- PendingRounds(std::string log_prefix, TimeManager* time_manager);
- ~PendingRounds();
+ PendingRounds(const std::string& log_prefix, TimeManager* time_manager);
+ ~PendingRounds() = default;
// Set the committed op during startup. This should be done after appending
// any of the pending ops, and will take care of triggering any that are now
@@ -99,7 +99,9 @@ class PendingRounds {
private:
const std::string& LogPrefix() const { return log_prefix_; }
- const std::string log_prefix_;
+ // Storing just a reference here: the RaftConsensus class which instantiates
+ // a PendingRound object keeps the ownership of the referenced string.
+ const std::string& log_prefix_;
// Index=>Round map that manages pending ops, i.e. operations for which we've
// received a replicate message from the leader but have yet to be committed.
diff --git a/src/kudu/consensus/raft_consensus.cc
b/src/kudu/consensus/raft_consensus.cc
index 0daf78a..7849c6d 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -183,6 +183,7 @@ RaftConsensus::RaftConsensus(
ServerContext server_ctx)
: options_(std::move(options)),
local_peer_pb_(std::move(local_peer_pb)),
+ log_prefix_(Substitute("T $0 P $1: ", options_.tablet_id, peer_uuid())),
cmeta_manager_(DCHECK_NOTNULL(std::move(cmeta_manager))),
server_ctx_(std::move(server_ctx)),
state_(kNew),
@@ -289,7 +290,6 @@ Status RaftConsensus::Start(const ConsensusBootstrapInfo&
info,
queue.get(),
raft_pool_token_.get(),
log_));
-
unique_ptr<PendingRounds> pending(new PendingRounds(
LogPrefixThreadSafe(), time_manager_.get()));
@@ -2662,7 +2662,7 @@ void RaftConsensus::ElectionCallback(ElectionReason
reason, const ElectionResult
auto self = shared_from_this();
Status s = raft_pool_token_->Submit([=]() { self->DoElectionCallback(reason,
result); });
if (!s.ok()) {
- static const char* msg = "unable to run election callback";
+ static const char* const msg = "unable to run election callback";
CHECK(s.IsServiceUnavailable()) << LogPrefixThreadSafe() << msg;
WARN_NOT_OK(s, LogPrefixThreadSafe() + msg);
}
@@ -3226,12 +3226,6 @@ string RaftConsensus::LogPrefixUnlocked() const {
return Substitute("T $0 P $1$2: ", options_.tablet_id, peer_uuid(),
cmeta_info);
}
-string RaftConsensus::LogPrefixThreadSafe() const {
- return Substitute("T $0 P $1: ",
- options_.tablet_id,
- peer_uuid());
-}
-
string RaftConsensus::ToString() const {
ThreadRestrictions::AssertWaitAllowed();
LockGuard l(lock_);
diff --git a/src/kudu/consensus/raft_consensus.h
b/src/kudu/consensus/raft_consensus.h
index 743789e..adb0bad 100644
--- a/src/kudu/consensus/raft_consensus.h
+++ b/src/kudu/consensus/raft_consensus.h
@@ -838,7 +838,9 @@ class RaftConsensus : public
std::enable_shared_from_this<RaftConsensus>,
// A variant of LogPrefix which does not take the lock. This is a slightly
// less thorough prefix which only includes immutable (and thus thread-safe)
// information, but does not require the lock.
- std::string LogPrefixThreadSafe() const;
+ const std::string& LogPrefixThreadSafe() const {
+ return log_prefix_;
+ }
std::string ToString() const;
std::string ToStringUnlocked() const;
@@ -850,6 +852,9 @@ class RaftConsensus : public
std::enable_shared_from_this<RaftConsensus>,
// Information about the local peer, including the local UUID.
const RaftPeerPB local_peer_pb_;
+ // Log prefix for this peer.
+ const std::string log_prefix_;
+
// Consensus metadata service.
const scoped_refptr<ConsensusMetadataManager> cmeta_manager_;
