You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/10/19 22:16:10 UTC

[7/7] kudu git commit: consensus: remove bits of dead code

consensus: remove bits of dead code

We had some old code path in which PeerMessageQueue::AppendOperations()
would previously return ServiceUnavailable indicating some kind of
memory limit. However, I traced through the code and could only find one
case in which it would return ServiceUnavailable: if the log itself is
shut down.

We expect that the lifecycle of the log is controlled such that this
will never happen. So, the code path that tried to handle this is now
just a CHECK, and I was able to remove some code that was only called in
this (dead) case.

Additionally removes some other unused typedefs and methods in
ReplicaState.

Change-Id: I8dc0a129f4f136256083a31e4b4b27e6a673dbee
Reviewed-on: http://gerrit.cloudera.org:8080/4712
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Reviewed-by: Mike Percy <mp...@apache.org>


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

Branch: refs/heads/master
Commit: af2ba10c215bd15d6d9bd6d3942c63b1961ebd52
Parents: 03d967a
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Oct 12 19:54:57 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Wed Oct 19 22:08:00 2016 +0000

----------------------------------------------------------------------
 src/kudu/consensus/raft_consensus.cc       | 20 ++++----------------
 src/kudu/consensus/raft_consensus_state.cc | 19 -------------------
 src/kudu/consensus/raft_consensus_state.h  | 12 ------------
 3 files changed, 4 insertions(+), 47 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/af2ba10c/src/kudu/consensus/raft_consensus.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus.cc b/src/kudu/consensus/raft_consensus.cc
index 67021a5..90ab3df 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -592,22 +592,10 @@ Status RaftConsensus::AppendNewRoundToQueueUnlocked(const scoped_refptr<Consensu
   *round->replicate_msg()->mutable_id() = queue_->GetNextOpId();
   RETURN_NOT_OK(AddPendingOperationUnlocked(round));
 
-  Status s = queue_->AppendOperation(round->replicate_scoped_refptr());
-
-  // Handle Status::ServiceUnavailable(), which means the queue is full.
-  if (PREDICT_FALSE(s.IsServiceUnavailable())) {
-    gscoped_ptr<OpId> id(round->replicate_msg()->release_id());
-    // Cancel the operation that we started.
-    state_->CancelPendingOperation(*id);
-    LOG_WITH_PREFIX_UNLOCKED(WARNING) << ": Could not append replicate request "
-                 << "to the queue. Queue is Full. "
-                 << "Queue metrics: " << queue_->ToString();
-    // TODO(todd) count of number of ops failed due to consensus queue overflow.
-  } else if (PREDICT_FALSE(s.IsIOError())) {
-    // This likely came from the log.
-    LOG(FATAL) << "IO error appending to the queue: " << s.ToString();
-  }
-  RETURN_NOT_OK_PREPEND(s, "Unable to append operation to consensus queue");
+  // The only reasons for a bad status would be if the log itself were shut down,
+  // or if we had an actual IO error, which we currently don't handle.
+  CHECK_OK_PREPEND(queue_->AppendOperation(round->replicate_scoped_refptr()),
+                   Substitute("$0: could not append to queue", LogPrefixUnlocked()));
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/af2ba10c/src/kudu/consensus/raft_consensus_state.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus_state.cc b/src/kudu/consensus/raft_consensus_state.cc
index 9494f68..522e728 100644
--- a/src/kudu/consensus/raft_consensus_state.cc
+++ b/src/kudu/consensus/raft_consensus_state.cc
@@ -361,15 +361,6 @@ Status ReplicaState::CancelPendingTransactions() {
   return Status::OK();
 }
 
-void ReplicaState::GetUncommittedPendingOperationsUnlocked(
-    vector<scoped_refptr<ConsensusRound> >* ops) {
-  for (const IndexToRoundMap::value_type& entry : pending_txns_) {
-    if (entry.first > last_committed_op_id_.index()) {
-      ops->push_back(entry.second);
-    }
-  }
-}
-
 void ReplicaState::AbortOpsAfterUnlocked(int64_t index) {
   DCHECK(update_lock_.is_locked());
   LOG_WITH_PREFIX_UNLOCKED(INFO) << "Aborting all transactions after (but not including): "
@@ -509,16 +500,6 @@ OpId ReplicaState::GetLastPendingTransactionOpIdUnlocked() const {
 }
 
 
-void ReplicaState::CancelPendingOperation(const OpId& id) {
-  OpId previous = id;
-  previous.set_index(previous.index() - 1);
-  DCHECK(update_lock_.is_locked());
-  CHECK_EQ(GetCurrentTermUnlocked(), id.term());
-
-  scoped_refptr<ConsensusRound> round = EraseKeyReturnValuePtr(&pending_txns_, id.index());
-  DCHECK(round);
-}
-
 string ReplicaState::LogPrefix() {
   ReplicaState::UniqueLock lock;
   CHECK_OK(LockForRead(&lock));

http://git-wip-us.apache.org/repos/asf/kudu/blob/af2ba10c/src/kudu/consensus/raft_consensus_state.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus_state.h b/src/kudu/consensus/raft_consensus_state.h
index e95b499..dc875b5 100644
--- a/src/kudu/consensus/raft_consensus_state.h
+++ b/src/kudu/consensus/raft_consensus_state.h
@@ -91,10 +91,6 @@ class ReplicaState {
 
   typedef std::map<int64_t, scoped_refptr<ConsensusRound> > IndexToRoundMap;
 
-  typedef std::set<int64_t> OutstandingCommits;
-
-  typedef IndexToRoundMap::value_type IndexToRoundEntry;
-
   ReplicaState(ConsensusOptions options, std::string peer_uuid,
                std::unique_ptr<ConsensusMetadata> cmeta);
 
@@ -232,9 +228,6 @@ class ReplicaState {
 
   const ConsensusOptions& GetOptions() const;
 
-  // Returns the operations that are not consensus committed.
-  void GetUncommittedPendingOperationsUnlocked(std::vector<scoped_refptr<ConsensusRound> >* ops);
-
   // Aborts pending operations after, but not including 'index'. The OpId with 'index'
   // will become our new last received id. If there are pending operations with indexes
   // higher than 'index' those operations are aborted.
@@ -272,11 +265,6 @@ class ReplicaState {
   // to complete. This does not cancel transactions being applied.
   Status CancelPendingTransactions();
 
-  // Used when, for some reason, an operation that failed before it could be considered
-  // a part of the state machine. Basically restores the id gen to the state it was before
-  // generating 'id'.
-  void CancelPendingOperation(const OpId& id);
-
   // Returns the number of transactions that are currently in the pending state
   // i.e. transactions for which Prepare() is done or under way.
   int GetNumPendingTxnsUnlocked() const;