(kudu) branch master updated: auto_leader_rebalancer: best-effort leader step-down

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits 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 cb747ff0c auto_leader_rebalancer: best-effort leader step-down
cb747ff0c is described below

commit cb747ff0ca0209e3fa98be4167c67edd474acd67
Author: Gabriella Lotz <lo...@gmail.com>
AuthorDate: Thu Jul 16 14:50:56 2026 +0200

    auto_leader_rebalancer: best-effort leader step-down
    
    When rebalancing leaders for a table, RunLeaderRebalanceForTable
    resolved the current leader's address and sent a LeaderStepDown RPC
    with RETURN_NOT_OK. A single transient failure (an RPC timeout, the
    target briefly unavailable, or an election in flight) aborted the whole
    rebalancing pass and skipped every remaining tablet and table in that
    round.
    
    A leader transfer is best effort. The next round recomputes the plan
    and retries, so a single failed transfer should warn and move on to the
    next tablet rather than fail the pass. RunLoop already only logs a
    warning for the overall call, so this just stops one bad RPC from
    starving the rest of the round.
    
    Seen under heavy load, where one failed LeaderStepDown would otherwise
    end a round early and leave the leaders unbalanced for longer.
    
    Change-Id: I0fb2c7a8bf750520d34d9aba62274dbe7433d80d
    Reviewed-on: http://gerrit.cloudera.org:8080/24594
    Tested-by: Alexey Serbin <al...@apache.org>
    Reviewed-by: Alexey Serbin <al...@apache.org>
---
 src/kudu/master/auto_leader_rebalancer-test.cc | 43 ++++++++++++++++++++++++++
 src/kudu/master/auto_leader_rebalancer.cc      | 32 +++++++++++++++++--
 2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/src/kudu/master/auto_leader_rebalancer-test.cc b/src/kudu/master/auto_leader_rebalancer-test.cc
index cfa4262cf..a2848e26e 100644
--- a/src/kudu/master/auto_leader_rebalancer-test.cc
+++ b/src/kudu/master/auto_leader_rebalancer-test.cc
@@ -87,6 +87,7 @@ DECLARE_uint32(auto_rebalancing_interval_seconds);
 DECLARE_uint32(auto_rebalancing_max_moves_per_server);
 DECLARE_uint32(auto_rebalancing_wait_for_replica_moves_seconds);
 DECLARE_uint32(leader_rebalancing_max_moves_per_round);
+DECLARE_bool(auto_leader_rebalancing_fail_moves_for_test);
 
 namespace kudu {
 namespace master {
@@ -117,11 +118,19 @@ class LeaderRebalancerTest : public KuduTest {
     if (cluster_) {
       cluster_->Shutdown();
     }
+    FLAGS_auto_leader_rebalancing_fail_moves_for_test = false;
     KuduTest::TearDown();
   }
 
   std::string table_name() { return workload_->table_name(); }
 
+  // Exposes the task's private test counter (the fixture is a friend of the
+  // task, individual TEST_F bodies are not).
+  int MovesScheduledThisRoundForTest() {
+    return cluster_->mini_master()->master()->catalog_manager()
+        ->auto_leader_rebalancer()->moves_scheduled_this_round_for_test_;
+  }
+
   Status RunLeaderRebalanceForTable(
       const scoped_refptr<TableInfo>& table_info,
       const vector<string>& tserver_uuids,
@@ -516,6 +525,40 @@ TEST_F(LeaderRebalancerTest, FunctionalTestForNotDivided) {
   }
 }
 
+// A single failed leader step-down RPC must not abort the whole rebalancing
+// pass. With every step-down forced to fail, the rebalancer should still
+// schedule the moves and return OK (before it was best-effort it returned the
+// RPC error and skipped the rest of the round).
+TEST_F(LeaderRebalancerTest, TestStepDownFailureDoesNotAbortPass) {
+  const int kNumTServers = 3;
+  const int kNumTablets = 6;
+  cluster_opts_.num_tablet_servers = kNumTServers;
+  ASSERT_OK(CreateAndStartCluster());
+  CreateWorkloadTable(kNumTablets, /*num_replicas*/ 3);
+
+  // Pile every leader onto ts0 so the rebalancer has transfers to schedule.
+  const string ts0_uuid = cluster_->mini_tablet_server(0)->uuid();
+  ASSERT_OK(MakeLeaderDistribution({kNumTablets, 0, 0}, table_name()));
+  std::map<string, int32_t> leader_map;
+  ASSERT_EVENTUALLY([&] {
+    ASSERT_OK(GetLeaderDistribution(&leader_map, table_name()));
+    // Over the per-tserver target, so moves will be scheduled.
+    ASSERT_GT(leader_map[ts0_uuid], kNumTablets / kNumTServers);
+  });
+
+  master::AutoLeaderRebalancerTask* leader_rebalancer =
+      cluster_->mini_master()->master()->catalog_manager()->auto_leader_rebalancer();
+
+  // Force every step-down RPC to fail. The pass must treat each failure as
+  // best-effort and still return OK instead of aborting the round.
+  FLAGS_auto_leader_rebalancing_fail_moves_for_test = true;
+  ASSERT_OK(leader_rebalancer->RunLeaderRebalancer());
+
+  // The failure path was actually exercised: moves were scheduled (and all
+  // failed), yet the pass still succeeded.
+  ASSERT_GT(MovesScheduledThisRoundForTest(), 0);
+}
+
 // Create a cluster, and create a table,
 // whether tablets is balanced, which decided by creating table process.
 // Bring up another tserver, the table is not balanced and leaders is also
diff --git a/src/kudu/master/auto_leader_rebalancer.cc b/src/kudu/master/auto_leader_rebalancer.cc
index b0224714c..52b51e096 100644
--- a/src/kudu/master/auto_leader_rebalancer.cc
+++ b/src/kudu/master/auto_leader_rebalancer.cc
@@ -96,6 +96,13 @@ DEFINE_bool(leader_rebalancing_ignore_soft_deleted_tables, false,
 TAG_FLAG(leader_rebalancing_ignore_soft_deleted_tables, advanced);
 TAG_FLAG(leader_rebalancing_ignore_soft_deleted_tables, runtime);
 
+DEFINE_bool(auto_leader_rebalancing_fail_moves_for_test, false,
+            "Force every leader step-down RPC issued by the per-table pass of "
+            "the leader rebalancer (RunLeaderRebalanceForTable) to fail. This "
+            "is only used for testing.");
+TAG_FLAG(auto_leader_rebalancing_fail_moves_for_test, unsafe);
+TAG_FLAG(auto_leader_rebalancing_fail_moves_for_test, hidden);
+
 DECLARE_bool(auto_leader_rebalancing_enabled);
 
 namespace kudu {
@@ -431,16 +438,35 @@ Status AutoLeaderRebalancerTask::RunLeaderRebalanceForTable(
     }
 
     vector<Sockaddr> resolved;
-    RETURN_NOT_OK(host_port->ResolveAddresses(&resolved));
+    if (Status s = host_port->ResolveAddresses(&resolved); !s.ok()) {
+      WARN_NOT_OK(s, Substitute("leader transfer for tablet $0: could not resolve $1",
+                                task.first, host_port->ToString()));
+      continue;
+    }
     ConsensusServiceProxy proxy(messenger_, resolved[0], host_port->host());
-    RETURN_NOT_OK(proxy.LeaderStepDown(request, &response, &rpc));
-    leader_transfer_count++;
+    // A single leader transfer is best effort. A transient failure (an RPC
+    // timeout, the target briefly unavailable, or an election in flight) should
+    // not abort the whole rebalancing pass and skip the remaining tablets and
+    // tables; the next round recomputes and retries.
+    Status s = PREDICT_FALSE(FLAGS_auto_leader_rebalancing_fail_moves_for_test)
+        ? Status::ServiceUnavailable("TEST: forced leader step-down failure")
+        : proxy.LeaderStepDown(request, &response, &rpc);
+    if (!s.ok()) {
+      WARN_NOT_OK(s, Substitute("leader transfer for tablet $0 from $1 to $2 failed",
+                                task.first, leader_uuid, task.second.second));
+      continue;
+    }
     if (!response.has_error()) {
+      leader_transfer_count++;
       VLOG(1) << Substitute("leader transfer table: $0, tablet_id: $1, from: $2 to: $3",
                             table_data.name(),
                             task.first,
                             leader_uuid,
                             task.second.second);
+    } else {
+      LOG(WARNING) << Substitute(
+          "leader transfer for tablet $0 (from $1 to $2) failed: $3",
+          task.first, leader_uuid, task.second.second, response.error().ShortDebugString());
     }
   }
   // @TODO(duyuqi)