[kudu] branch master updated: [consensus] small cleanup on consensus::Peer

[al...@apache.org]

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 1dfd025  [consensus] small cleanup on consensus::Peer
1dfd025 is described below

commit 1dfd0253d375b3012b0cd67a93dd111871814a0c
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Mar 26 00:13:25 2021 -0700

    [consensus] small cleanup on consensus::Peer
    
    Peer::LogPrefixUnlocked() was constructing a new string every time it's
    called, but all the information in the prefix is immutable for a Peer
    instance.  Also, it's possible to set "tablet_id", "caller_uuid", and
    "dest_uuid" fields of the Peer::request_ PB structure only once.
    
    Change-Id: Iefce6791d12fbd126ee30104d1005209ad3de3ff
    Reviewed-on: http://gerrit.cloudera.org:8080/17236
    Tested-by: Kudu Jenkins
    Reviewed-by: Bankim Bhavsar <ba...@cloudera.com>
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/consensus/consensus_peers.cc | 27 ++++++++++++++++-----------
 src/kudu/consensus/consensus_peers.h  |  6 +++---
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/kudu/consensus/consensus_peers.cc b/src/kudu/consensus/consensus_peers.cc
index 758a95d..a025cbc 100644
--- a/src/kudu/consensus/consensus_peers.cc
+++ b/src/kudu/consensus/consensus_peers.cc
@@ -135,6 +135,9 @@ Peer::Peer(RaftPeerPB peer_pb,
     : tablet_id_(std::move(tablet_id)),
       leader_uuid_(std::move(leader_uuid)),
       peer_pb_(std::move(peer_pb)),
+      log_prefix_(Substitute("T $0 P $1 -> Peer $2 ($3:$4): ",
+                  tablet_id_, leader_uuid_, peer_pb_.permanent_uuid(),
+                  peer_pb_.last_known_addr().host(), peer_pb_.last_known_addr().port())),
       proxy_(std::move(proxy)),
       queue_(queue),
       failed_attempts_(0),
@@ -208,7 +211,6 @@ void Peer::SendNextRequest(bool even_if_queue_empty) {
   // negotiation round.
   if (!has_sent_first_request_) {
     even_if_queue_empty = true;
-    has_sent_first_request_ = true;
   }
 
   // If our last request generated an error, and this is not a normal
@@ -259,10 +261,6 @@ void Peer::SendNextRequest(bool even_if_queue_empty) {
     return;
   }
 
-  request_.set_tablet_id(tablet_id_);
-  request_.set_caller_uuid(leader_uuid_);
-  request_.set_dest_uuid(peer_pb_.permanent_uuid());
-
   bool req_has_ops = request_.ops_size() > 0 || (commit_index_after > commit_index_before);
   // If the queue is empty, check if we were told to send a status-only
   // message, if not just return.
@@ -275,6 +273,14 @@ void Peer::SendNextRequest(bool even_if_queue_empty) {
     heartbeater_->Snooze();
   }
 
+  if (!has_sent_first_request_) {
+    // Set the 'immutable' fields in the request only once upon first request.
+    request_.set_tablet_id(tablet_id_);
+    request_.set_caller_uuid(leader_uuid_);
+    request_.set_dest_uuid(peer_pb_.permanent_uuid());
+    has_sent_first_request_ = true;
+  }
+
   MAYBE_FAULT(FLAGS_fault_crash_on_leader_request_fraction);
 
   VLOG_WITH_PREFIX_UNLOCKED(2) << "Sending to peer " << peer_pb().permanent_uuid() << ": "
@@ -390,8 +396,9 @@ void Peer::ProcessResponse() {
     }
   });
   if (PREDICT_FALSE(!s.ok())) {
-    LOG_WITH_PREFIX_UNLOCKED(WARNING) << "Unable to process peer response: " << s.ToString()
-        << ": " << SecureShortDebugString(response_);
+    LOG_WITH_PREFIX_UNLOCKED(WARNING) << Substitute(
+        "unable to process peer response: $0: $1",
+         s.ToString(), SecureShortDebugString(response_));
     request_pending_ = false;
   }
 }
@@ -482,10 +489,8 @@ void Peer::ProcessResponseErrorUnlocked(const Status& status) {
   request_pending_ = false;
 }
 
-string Peer::LogPrefixUnlocked() const {
-  return Substitute("T $0 P $1 -> Peer $2 ($3:$4): ",
-                    tablet_id_, leader_uuid_, peer_pb_.permanent_uuid(),
-                    peer_pb_.last_known_addr().host(), peer_pb_.last_known_addr().port());
+const string& Peer::LogPrefixUnlocked() const {
+  return log_prefix_;
 }
 
 void Peer::Close() {
diff --git a/src/kudu/consensus/consensus_peers.h b/src/kudu/consensus/consensus_peers.h
index 912ad64..5be351a 100644
--- a/src/kudu/consensus/consensus_peers.h
+++ b/src/kudu/consensus/consensus_peers.h
@@ -154,14 +154,14 @@ class Peer :
   // Signals there was an error sending the request to the peer.
   void ProcessResponseErrorUnlocked(const Status& status);
 
-  std::string LogPrefixUnlocked() const;
+  const std::string& LogPrefixUnlocked() const;
 
   const std::string& tablet_id() const { return tablet_id_; }
 
   const std::string tablet_id_;
   const std::string leader_uuid_;
-
-  RaftPeerPB peer_pb_;
+  const RaftPeerPB peer_pb_;
+  const std::string log_prefix_;
 
   std::unique_ptr<PeerProxy> proxy_;