(kudu) 01/02: [consensus] micro-optimisation on RequestVote()

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 806bba5b2995e50c37d14cc6cb67f166dd27c7fb
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Jan 24 16:18:06 2025 -0800

    [consensus] micro-optimisation on RequestVote()
    
    This patch simplifies the logic around 'update_guard' in
    RaftConsensus::RequestVote() since unique_lock<>::try_lock() already
    reports if the underlying locking primitive has been acquired.  It also
    fixes compilation warning issued by GCC13/14 about ignoring the return
    value of the unique_lock<>::try_lock() method.
    
    This patch doesn't contain any functional modifications.
    
    Change-Id: I115d88b90ce6ec94737bb85f1003cf943e1ab9e0
    Reviewed-on: http://gerrit.cloudera.org:8080/22387
    Tested-by: Kudu Jenkins
    Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
    (cherry picked from commit 47af08154d1522ae4910ea306cf77c2b0e8e4f81)
    Reviewed-on: http://gerrit.cloudera.org:8080/22402
    Tested-by: Alexey Serbin <al...@apache.org>
    Reviewed-by: Gabriella Lotz <lo...@gmail.com>
---
 src/kudu/consensus/raft_consensus.cc | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/kudu/consensus/raft_consensus.cc b/src/kudu/consensus/raft_consensus.cc
index 6be58bbf8..5ec7aea89 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -1749,19 +1749,19 @@ Status RaftConsensus::RequestVote(const VoteRequestPB* request,
   // Lock ordering: update_lock_ must be acquired before lock_.
   std::unique_lock<simple_spinlock> update_guard(update_lock_, std::defer_lock);
   if (PREDICT_TRUE(FLAGS_enable_leader_failure_detection)) {
-    update_guard.try_lock();
+    if (!update_guard.try_lock()) {
+      // There is another vote or update concurrent with the vote. In that case, that
+      // other request is likely to reset the timer, and we'll end up just voting
+      // "NO" after waiting. To avoid starving RPC handlers and causing cascading
+      // timeouts, just vote a quick NO.
+      return RequestVoteRespondIsBusy(request, response);
+    }
   } else {
     // If failure detection is not enabled, then we can't just reject the vote,
     // because there will be no automatic retry later. So, block for the lock.
     update_guard.lock();
   }
-  if (!update_guard.owns_lock()) {
-    // There is another vote or update concurrent with the vote. In that case, that
-    // other request is likely to reset the timer, and we'll end up just voting
-    // "NO" after waiting. To avoid starving RPC handlers and causing cascading
-    // timeouts, just vote a quick NO.
-    return RequestVoteRespondIsBusy(request, response);
-  }
+  DCHECK(update_guard.owns_lock());
 
   // Acquire the replica state lock so we can read / modify the consensus state.
   ThreadRestrictions::AssertWaitAllowed();