kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [2/8] kudu git commit: raft_consensus: clean up overrides, protected methods
Date Thu, 06 Oct 2016 00:05:58 GMT
raft_consensus: clean up overrides, protected methods

RaftConsensus used to have a subclass mock in raft_consensus-test, but
that test is now removed. Given this, we can make some
previously-protected methods private and non-virtual.

I also took this opportunity to modernize the 'override' style to C++11,
and also to remove a function which was essentially duplicate code.

Change-Id: Ib52d3ac40ed68a4ff8b1738897f6ef62f94a843b
Reviewed-on: http://gerrit.cloudera.org:8080/4620
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/bda9b94e
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/bda9b94e
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/bda9b94e

Branch: refs/heads/master
Commit: bda9b94e88b2602dcc029e763c8a9e0eecff8e08
Parents: 4bcbb4a
Author: Todd Lipcon <todd@apache.org>
Authored: Tue Oct 4 14:34:10 2016 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Wed Oct 5 21:41:36 2016 +0000

----------------------------------------------------------------------
 src/kudu/consensus/raft_consensus.cc            |  9 +-
 src/kudu/consensus/raft_consensus.h             | 98 +++++++++-----------
 .../consensus/raft_consensus_quorum-test.cc     |  2 +-
 3 files changed, 45 insertions(+), 64 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/bda9b94e/src/kudu/consensus/raft_consensus.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus.cc b/src/kudu/consensus/raft_consensus.cc
index 0cb724e..3e4a89b 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -1511,12 +1511,6 @@ void RaftConsensus::Shutdown() {
   shutdown_.Store(true, kMemOrderRelease);
 }
 
-RaftPeerPB::Role RaftConsensus::GetActiveRole() const {
-  ReplicaState::UniqueLock lock;
-  CHECK_OK(state_->LockForRead(&lock));
-  return state_->GetActiveRoleUnlocked();
-}
-
 OpId RaftConsensus::GetLatestOpIdFromLog() {
   OpId id;
   log_->GetLatestEntryOpId(&id);
@@ -1675,8 +1669,7 @@ Status RaftConsensus::RequestVoteRespondVoteGranted(const VoteRequestPB*
request
 RaftPeerPB::Role RaftConsensus::role() const {
   ReplicaState::UniqueLock lock;
   CHECK_OK(state_->LockForRead(&lock));
-  return GetConsensusRole(state_->GetPeerUuid(),
-                          state_->ConsensusStateUnlocked(CONSENSUS_CONFIG_ACTIVE));
+  return state_->GetActiveRoleUnlocked();
 }
 
 std::string RaftConsensus::LogPrefixUnlocked() {

http://git-wip-us.apache.org/repos/asf/kudu/blob/bda9b94e/src/kudu/consensus/raft_consensus.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus.h b/src/kudu/consensus/raft_consensus.h
index 887201b..07ef186 100644
--- a/src/kudu/consensus/raft_consensus.h
+++ b/src/kudu/consensus/raft_consensus.h
@@ -91,69 +91,64 @@ class RaftConsensus : public Consensus,
 
   virtual ~RaftConsensus();
 
-  virtual Status Start(const ConsensusBootstrapInfo& info) OVERRIDE;
+  Status Start(const ConsensusBootstrapInfo& info) override;
 
-  virtual bool IsRunning() const OVERRIDE;
+  bool IsRunning() const override;
 
   // Emulates an election by increasing the term number and asserting leadership
   // in the configuration by sending a NO_OP to other peers.
   // This is NOT safe to use in a distributed configuration with failure detection
   // enabled, as it could result in a split-brain scenario.
-  virtual Status EmulateElection() OVERRIDE;
+  Status EmulateElection() override;
 
-  virtual Status StartElection(ElectionMode mode) OVERRIDE;
+  Status StartElection(ElectionMode mode) override;
 
-  virtual Status WaitUntilLeaderForTests(const MonoDelta& timeout) OVERRIDE;
+  Status WaitUntilLeaderForTests(const MonoDelta& timeout) override;
 
-  virtual Status StepDown(LeaderStepDownResponsePB* resp) OVERRIDE;
+  Status StepDown(LeaderStepDownResponsePB* resp) override;
 
   // Call StartElection(), log a warning if the call fails (usually due to
   // being shut down).
   void ReportFailureDetected(const std::string& name, const Status& msg);
 
-  virtual Status Replicate(const scoped_refptr<ConsensusRound>& round) OVERRIDE;
+  Status Replicate(const scoped_refptr<ConsensusRound>& round) override;
 
-  virtual Status CheckLeadershipAndBindTerm(const scoped_refptr<ConsensusRound>&
round) OVERRIDE;
+  Status CheckLeadershipAndBindTerm(const scoped_refptr<ConsensusRound>& round)
override;
 
-  virtual Status Update(const ConsensusRequestPB* request,
-                        ConsensusResponsePB* response) OVERRIDE;
+  Status Update(const ConsensusRequestPB* request,
+                ConsensusResponsePB* response) override;
 
-  virtual Status RequestVote(const VoteRequestPB* request,
-                             VoteResponsePB* response) OVERRIDE;
+  Status RequestVote(const VoteRequestPB* request,
+                     VoteResponsePB* response) override;
 
-  virtual Status ChangeConfig(const ChangeConfigRequestPB& req,
-                              const StatusCallback& client_cb,
-                              boost::optional<tserver::TabletServerErrorPB::Code>*
error_code)
-                              OVERRIDE;
+  Status ChangeConfig(const ChangeConfigRequestPB& req,
+                      const StatusCallback& client_cb,
+                      boost::optional<tserver::TabletServerErrorPB::Code>* error_code)
override;
 
-  virtual RaftPeerPB::Role role() const OVERRIDE;
+  Status GetLastOpId(OpIdType type, OpId* id) override;
 
-  virtual std::string peer_uuid() const OVERRIDE;
+  RaftPeerPB::Role role() const override;
 
-  virtual std::string tablet_id() const OVERRIDE;
+  std::string peer_uuid() const override;
 
-  virtual ConsensusStatePB ConsensusState(ConsensusConfigType type) const OVERRIDE;
+  std::string tablet_id() const override;
 
-  virtual RaftConfigPB CommittedConfig() const OVERRIDE;
+  ConsensusStatePB ConsensusState(ConsensusConfigType type) const override;
 
-  virtual void DumpStatusHtml(std::ostream& out) const OVERRIDE;
+  RaftConfigPB CommittedConfig() const override;
 
-  virtual void Shutdown() OVERRIDE;
+  void DumpStatusHtml(std::ostream& out) const override;
 
-  // Makes this peer advance it's term (and step down if leader), for tests.
-  virtual Status AdvanceTermForTests(int64_t new_term);
+  void Shutdown() override;
 
-  // Return the active (as opposed to committed) role.
-  RaftPeerPB::Role GetActiveRole() const;
+  // Makes this peer advance it's term (and step down if leader), for tests.
+  Status AdvanceTermForTests(int64_t new_term);
 
   // Returns the replica state for tests. This should never be used outside of
   // tests, in particular calling the LockFor* methods on the returned object
   // can cause consensus to deadlock.
   ReplicaState* GetReplicaStateForTests();
 
-  virtual Status GetLastOpId(OpIdType type, OpId* id) OVERRIDE;
-
-
   //------------------------------------------------------------
   // PeerMessageQueueObserver implementation
   //------------------------------------------------------------
@@ -171,25 +166,6 @@ class RaftConsensus : public Consensus,
 
   log::RetentionIndexes GetRetentionIndexes() override;
 
- protected:
-  // Trigger that a non-Transaction ConsensusRound has finished replication.
-  // If the replication was successful, an status will be OK. Otherwise, it
-  // may be Aborted or some other error status.
-  // If 'status' is OK, write a Commit message to the local WAL based on the
-  // type of message it is.
-  // The 'client_cb' will be invoked at the end of this execution.
-  virtual void NonTxRoundReplicationFinished(ConsensusRound* round,
-                                             const StatusCallback& client_cb,
-                                             const Status& status);
-
-  // As a leader, append a new ConsensusRound to the queue.
-  // Only virtual and protected for mocking purposes.
-  virtual Status AppendNewRoundToQueueUnlocked(const scoped_refptr<ConsensusRound>&
round);
-
-  // As a follower, start a consensus round not associated with a Transaction.
-  // Only virtual and protected for mocking purposes.
-  virtual Status StartConsensusOnlyRoundUnlocked(const ReplicateRefPtr& msg);
-
  private:
   friend class ReplicaState;
   friend class RaftConsensusQuorumTest;
@@ -288,12 +264,6 @@ class RaftConsensus : public Consensus,
   // and also truncate the LogCache accordingly.
   void TruncateAndAbortOpsAfterUnlocked(int64_t truncate_after_index);
 
-  // Pushes a new Raft configuration to a majority of peers. Contrary to write operations,
-  // this actually waits for the commit round to reach a majority of peers, so that we know
-  // we can proceed. If this returns Status::OK(), a majority of peers have accepted the
new
-  // configuration. The peer cannot perform any additional operations until this succeeds.
-  Status PushConfigurationToPeersUnlocked(const RaftConfigPB& new_config);
-
   // Returns the most recent OpId written to the Log.
   OpId GetLatestOpIdFromLog();
 
@@ -432,6 +402,24 @@ class RaftConsensus : public Consensus,
                              const RaftConfigPB& committed_config,
                              const std::string& reason);
 
+  // Trigger that a non-Transaction ConsensusRound has finished replication.
+  // If the replication was successful, an status will be OK. Otherwise, it
+  // may be Aborted or some other error status.
+  // If 'status' is OK, write a Commit message to the local WAL based on the
+  // type of message it is.
+  // The 'client_cb' will be invoked at the end of this execution.
+  void NonTxRoundReplicationFinished(ConsensusRound* round,
+                                     const StatusCallback& client_cb,
+                                     const Status& status);
+
+  // As a leader, append a new ConsensusRound to the queue.
+  // Only virtual and protected for mocking purposes.
+  Status AppendNewRoundToQueueUnlocked(const scoped_refptr<ConsensusRound>& round);
+
+  // As a follower, start a consensus round not associated with a Transaction.
+  // Only virtual and protected for mocking purposes.
+  Status StartConsensusOnlyRoundUnlocked(const ReplicateRefPtr& msg);
+
   // Threadpool for constructing requests to peers, handling RPC callbacks,
   // etc.
   gscoped_ptr<ThreadPool> thread_pool_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/bda9b94e/src/kudu/consensus/raft_consensus_quorum-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus_quorum-test.cc b/src/kudu/consensus/raft_consensus_quorum-test.cc
index 9d63266..362a230 100644
--- a/src/kudu/consensus/raft_consensus_quorum-test.cc
+++ b/src/kudu/consensus/raft_consensus_quorum-test.cc
@@ -78,7 +78,7 @@ void DoNothing(const string& s) {
 Status WaitUntilLeaderForTests(RaftConsensus* raft) {
   MonoTime deadline = MonoTime::Now() + MonoDelta::FromSeconds(15);
   while (MonoTime::Now() < deadline) {
-    if (raft->GetActiveRole() == RaftPeerPB::LEADER) {
+    if (raft->role() == RaftPeerPB::LEADER) {
       return Status::OK();
     }
     SleepFor(MonoDelta::FromMilliseconds(10));


Mime
View raw message