kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [2/2] kudu git commit: KUDU-2342. consensus: use tighter bound for non-voter promotion
Date Thu, 15 Mar 2018 06:52:10 GMT
KUDU-2342. consensus: use tighter bound for non-voter promotion

This patch replaces the NON_VOTER promotion heuristic based on the
--consensus_promotion_max_wal_entries_behind gflag with a heuristic that
roughly matches the promotion criteria outlined in Diego Ongaro's
dissertation.

The approach is that we track the operation count of the last batch
successfully sent from leader to peer. If the peer is within that number
of ops of the commit index, the peer is eligible for promotion.

The aforementioned gflag has been removed.

This patch passes existing tests and did well on manual testing. I will
follow up on this patch with an automated test.

Change-Id: Iff517f01d6dc25eb15d01593dd57b7dc0dd25956
Reviewed-on: http://gerrit.cloudera.org:8080/9627
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: f2479e21d5a3002ebf5b1012fde83a6cffc2db82
Parents: 4c1788e
Author: Mike Percy <mpercy@apache.org>
Authored: Wed Mar 14 15:39:16 2018 -0700
Committer: Alexey Serbin <aserbin@cloudera.com>
Committed: Thu Mar 15 06:51:25 2018 +0000

----------------------------------------------------------------------
 src/kudu/consensus/consensus_queue.cc | 35 ++++++++++++++++++------------
 1 file changed, 21 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/f2479e21/src/kudu/consensus/consensus_queue.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/consensus_queue.cc b/src/kudu/consensus/consensus_queue.cc
index f85589b..804739c 100644
--- a/src/kudu/consensus/consensus_queue.cc
+++ b/src/kudu/consensus/consensus_queue.cc
@@ -71,13 +71,6 @@ DEFINE_int32(consensus_inject_latency_ms_in_notifications, 0,
 TAG_FLAG(consensus_inject_latency_ms_in_notifications, hidden);
 TAG_FLAG(consensus_inject_latency_ms_in_notifications, unsafe);
 
-DEFINE_int32(consensus_promotion_max_wal_entries_behind, 100,
-             "The number of WAL entries that a NON_VOTER with PROMOTE=true can "
-             "be behind the leader's committed index and be considered ready "
-             "for promotion to VOTER.");
-TAG_FLAG(consensus_promotion_max_wal_entries_behind, advanced);
-TAG_FLAG(consensus_promotion_max_wal_entries_behind, experimental);
-
 DECLARE_int32(consensus_rpc_timeout_ms);
 DECLARE_bool(safe_time_advancement_without_writes);
 DECLARE_bool(raft_prepare_replacement_before_eviction);
@@ -970,9 +963,8 @@ void PeerMessageQueue::UpdateExchangeStatus(TrackedPeer* peer,
 void PeerMessageQueue::PromoteIfNeeded(TrackedPeer* peer, const TrackedPeer& prev_peer_state,
                                        const ConsensusStatusPB& status) {
   DCHECK(queue_lock_.is_locked());
-  int64_t entries_behind = queue_state_.committed_index - peer->last_received.index();
   if (queue_state_.mode != PeerMessageQueue::LEADER ||
-      entries_behind > FLAGS_consensus_promotion_max_wal_entries_behind) {
+      peer->last_exchange_status != PeerStatus::OK) {
     return;
   }
 
@@ -982,13 +974,28 @@ void PeerMessageQueue::PromoteIfNeeded(TrackedPeer* peer, const TrackedPeer&
pre
   Status s = GetRaftConfigMember(DCHECK_NOTNULL(queue_state_.active_config.get()),
                                  peer->uuid(), &peer_pb);
   if (s.ok() &&
-      peer_pb &&
       peer_pb->member_type() == RaftPeerPB::NON_VOTER &&
       peer_pb->attrs().promote()) {
-    // This peer is ready to promote.
-    //
-    // TODO(mpercy): Should we introduce a function SafeToPromote() that
-    // does the same calculation as SafeToEvict() but for adding a VOTER?
+
+    // Only promote the peer if it is within one round-trip of being fully
+    // caught-up with the current commit index, as measured by recent
+    // UpdateConsensus() operation batch sizes.
+
+    // If we had never previously contacted this peer, wait until the second
+    // time we contact them to try to promote them.
+    if (prev_peer_state.last_received.index() == 0) return;
+
+    int64_t last_batch_size =
+        std::max<int64_t>(0, peer->last_received.index() - prev_peer_state.last_received.index());
+    bool peer_caught_up =
+        !OpIdEquals(status.last_received_current_leader(), MinimumOpId()) &&
+        status.last_received_current_leader().index() + last_batch_size
+            >= queue_state_.committed_index;
+    if (!peer_caught_up) return;
+
+    // TODO(mpercy): Implement a SafeToPromote() check to ensure that we only
+    // try to promote a NON_VOTER to VOTER if we will be able to commit the
+    // resulting config change operation.
     NotifyObserversOfPeerToPromote(peer->uuid());
   }
 }


Mime
View raw message