Mailing-List: contact commits-help@kudu.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kudu.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: mpercy@apache.org
To: commits@kudu.apache.org
Date: Sat, 02 Dec 2017 05:29:45 -0000
Message-Id: <be6160e55f214c06b5049569e94b911c@git.apache.org>
In-Reply-To: <5fa3cfc1acf44f9c8e9e74e9e5345a64@git.apache.org>
References: <5fa3cfc1acf44f9c8e9e74e9e5345a64@git.apache.org>
Subject: [3/3] kudu git commit: KUDU-1097. Only add/evict when processing
 leader tablet reports
archived-at: Sat, 02 Dec 2017 05:29:45 -0000

KUDU-1097. Only add/evict when processing leader tablet reports

The leader replica is the only replica that sends a health report. The
master needs the health report to make an eviction decision. Thus, we
should only consider eviction or addition of a new replica if the tablet
report was received from the leader.

There is no standalone test for this but this patch is required for the
following patch, "Never evict a leader", to pass due to the DCHECK
introduced in that patch.

Change-Id: If7e318e042cd27ff4544bc5aae76db4ab9e51da1
Reviewed-on: http://gerrit.cloudera.org:8080/8682
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/8735
Tested-by: Mike Percy <mpercy@apache.org>


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

Branch: refs/heads/branch-1.6.x
Commit: 8a570e7720d6947f678b41df018413db80821a14
Parents: d0d6994
Author: Mike Percy <mpercy@apache.org>
Authored: Wed Nov 29 04:24:13 2017 -0800
Committer: Mike Percy <mpercy@apache.org>
Committed: Sat Dec 2 05:29:22 2017 +0000

----------------------------------------------------------------------
 src/kudu/master/catalog_manager.cc | 44 ++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/8a570e77/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index beb4302..8144d74 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -3488,32 +3488,36 @@ Status CatalogManager::ProcessTabletReport(
       // 7e. Make tablet configuration change depending on the mode the server
       // is running with. The choice between two alternative modes is controlled
       // by the 'raft_prepare_replacement_before_eviction' run-time flag.
-      if (FLAGS_raft_prepare_replacement_before_eviction) {
-        // An alternative scheme of managing tablet replicas: the catalog
-        // manager processes the health-related info on replicas from the tablet
-        // report and initiates appropriate modifications for the tablet Raft
-        // configuration: evict an already-replaced failed voter replica or add
-        // a new non-voter replica marked for promotion as a replacement.
+      if (!FLAGS_raft_prepare_replacement_before_eviction) {
+        if (consensus_state_updated &&
+            FLAGS_master_add_server_when_underreplicated &&
+            CountVoters(cstate.committed_config()) < replication_factor) {
+          // Add a server to the config if it is under-replicated.
+          //
+          // This is an idempotent operation due to a CAS enforced on the
+          // committed config's opid_index.
+          rpcs.emplace_back(new AsyncAddReplicaTask(
+              master_, tablet, cstate, RaftPeerPB::VOTER, &rng_));
+        }
+
+      // When --raft_prepare_replacement_before_eviction is enabled, we
+      // consider whether to add or evict replicas based on the health report
+      // included in the leader's tablet report. Since only the leader tracks
+      // health, we ignore reports from non-leaders in this case.
+      } else if (!cstate.leader_uuid().empty() &&
+                 ts_desc->permanent_uuid() == cstate.leader_uuid()) {
         const RaftConfigPB& config = cstate.committed_config();
         string to_evict;
-        if (IsUnderReplicated(config, replication_factor)) {
-          rpcs.emplace_back(new AsyncAddReplicaTask(
-              master_, tablet, cstate, RaftPeerPB::NON_VOTER, &rng_));
-        } else if (PREDICT_TRUE(FLAGS_catalog_manager_evict_excess_replicas) &&
-                   CanEvictReplica(config, replication_factor, &to_evict)) {
+        if (PREDICT_TRUE(FLAGS_catalog_manager_evict_excess_replicas) &&
+                         CanEvictReplica(config, replication_factor, &to_evict)) {
           DCHECK(!to_evict.empty());
           rpcs.emplace_back(new AsyncEvictReplicaTask(
               master_, tablet, cstate, std::move(to_evict)));
+        } else if (FLAGS_master_add_server_when_underreplicated &&
+                   IsUnderReplicated(config, replication_factor)) {
+          rpcs.emplace_back(new AsyncAddReplicaTask(
+              master_, tablet, cstate, RaftPeerPB::NON_VOTER, &rng_));
         }
-      } else if (consensus_state_updated &&
-                 FLAGS_master_add_server_when_underreplicated &&
-                 CountVoters(cstate.committed_config()) < replication_factor) {
-        // Add a server to the config if it is under-replicated.
-        //
-        // This is an idempotent operation due to a CAS enforced on the
-        // committed config's opid_index.
-        rpcs.emplace_back(new AsyncAddReplicaTask(
-            master_, tablet, cstate, RaftPeerPB::VOTER, &rng_));
       }
     }