You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2020/06/05 16:15:03 UTC

[kudu] branch master updated: [consensus] use std::atomic in raft_consensus.{cc, h}

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 df1a5d9  [consensus] use std::atomic in raft_consensus.{cc,h}
df1a5d9 is described below

commit df1a5d90f68730af9c89ad47cb668516b6822d5b
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu Jun 4 22:50:20 2020 -0700

    [consensus] use std::atomic in raft_consensus.{cc,h}
    
    There was a mix of std::atomic<> and AtomicBool and others in
    raft_consensus.{cc,h}.  This patch switches all instances all
    atomic varilables there to std::atomic.
    
    Change-Id: I14214ca486b86b241f1b6fcf8842867a6bf1459a
    Reviewed-on: http://gerrit.cloudera.org:8080/16035
    Tested-by: Kudu Jenkins
    Reviewed-by: Grant Henke <gr...@apache.org>
---
 src/kudu/consensus/raft_consensus.cc | 16 +++++++++-------
 src/kudu/consensus/raft_consensus.h  |  9 ++++-----
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/kudu/consensus/raft_consensus.cc b/src/kudu/consensus/raft_consensus.cc
index 2bc566c..b9418e4 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -623,12 +623,12 @@ Status RaftConsensus::TransferLeadership(const boost::optional<string>& new_lead
 Status RaftConsensus::BeginLeaderTransferPeriodUnlocked(
     const boost::optional<string>& successor_uuid) {
   DCHECK(lock_.is_locked());
-  if (leader_transfer_in_progress_.CompareAndSwap(false, true)) {
+  if (std::atomic_exchange(&leader_transfer_in_progress_, true)) {
     return Status::ServiceUnavailable(
         Substitute("leadership transfer for $0 already in progress",
                    options_.tablet_id));
   }
-  leader_transfer_in_progress_.Store(true, kMemOrderAcquire);
+  leader_transfer_in_progress_ = true;
   queue_->BeginWatchForSuccessor(successor_uuid);
   transfer_period_timer_->Start();
   return Status::OK();
@@ -637,7 +637,7 @@ Status RaftConsensus::BeginLeaderTransferPeriodUnlocked(
 void RaftConsensus::EndLeaderTransferPeriod() {
   transfer_period_timer_->Stop();
   queue_->EndWatchForSuccessor();
-  leader_transfer_in_progress_.Store(false, kMemOrderRelease);
+  leader_transfer_in_progress_ = false;
 }
 
 scoped_refptr<ConsensusRound> RaftConsensus::NewRound(
@@ -1017,7 +1017,7 @@ void RaftConsensus::TryStartElectionOnPeerTask(const string& peer_uuid) {
 
 Status RaftConsensus::Update(const ConsensusRequestPB* request,
                              ConsensusResponsePB* response) {
-  update_calls_for_tests_.Increment();
+  ++update_calls_for_tests_;
 
   if (PREDICT_FALSE(FLAGS_follower_reject_update_consensus_requests)) {
     return Status::IllegalState("Rejected: --follower_reject_update_consensus_requests "
@@ -2227,14 +2227,16 @@ void RaftConsensus::Shutdown() {
   // ThreadRestrictions assertions in the case where the RaftConsensus
   // destructor runs on the reactor thread due to an election callback being
   // the last outstanding reference.
-  if (shutdown_.Load(kMemOrderAcquire)) return;
+  if (shutdown_) {
+    return;
+  }
 
   Stop();
   {
     LockGuard l(lock_);
     SetStateUnlocked(kShutdown);
   }
-  shutdown_.Store(true, kMemOrderRelease);
+  shutdown_ = true;
 }
 
 Status RaftConsensus::StartConsensusOnlyRoundUnlocked(const ReplicateRefPtr& msg) {
@@ -3002,7 +3004,7 @@ Status RaftConsensus::CheckActiveLeaderUnlocked() const {
       // Check for the consistency of the information in the consensus metadata
       // and the state of the consensus queue.
       DCHECK(queue_->IsInLeaderMode());
-      if (leader_transfer_in_progress_.Load()) {
+      if (leader_transfer_in_progress_) {
         return Status::ServiceUnavailable("leader transfer in progress");
       }
       return Status::OK();
diff --git a/src/kudu/consensus/raft_consensus.h b/src/kudu/consensus/raft_consensus.h
index 3d05a27..33ec160 100644
--- a/src/kudu/consensus/raft_consensus.h
+++ b/src/kudu/consensus/raft_consensus.h
@@ -42,7 +42,6 @@
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/tablet/metadata.pb.h"
 #include "kudu/tserver/tserver.pb.h"
-#include "kudu/util/atomic.h"
 #include "kudu/util/locks.h"
 #include "kudu/util/make_shared.h"
 #include "kudu/util/metrics.h"
@@ -359,7 +358,7 @@ class RaftConsensus : public std::enable_shared_from_this<RaftConsensus>,
   Status AdvanceTermForTests(int64_t new_term);
 
   int update_calls_for_tests() const {
-    return update_calls_for_tests_.Load();
+    return update_calls_for_tests_;
   }
 
   //------------------------------------------------------------
@@ -888,7 +887,7 @@ class RaftConsensus : public std::enable_shared_from_this<RaftConsensus>,
 
   std::shared_ptr<rpc::PeriodicTimer> failure_detector_;
 
-  AtomicBool leader_transfer_in_progress_;
+  std::atomic<bool> leader_transfer_in_progress_;
   boost::optional<std::string> designated_successor_uuid_;
   std::shared_ptr<rpc::PeriodicTimer> transfer_period_timer_;
 
@@ -912,10 +911,10 @@ class RaftConsensus : public std::enable_shared_from_this<RaftConsensus>,
   // A flag to help us avoid taking a lock on the reactor thread if the object
   // is already in kShutdown state.
   // TODO(mpercy): Try to get rid of this extra flag.
-  AtomicBool shutdown_;
+  std::atomic<bool> shutdown_;
 
   // The number of times Update() has been called, used for some test assertions.
-  AtomicInt<int32_t> update_calls_for_tests_;
+  std::atomic<int32_t> update_calls_for_tests_;
 
   // The wrapping into std::atomic<> is to simplify the synchronization between
   // consensus-related writers and readers of the attached metric gauge.