[INTERNAL] - Mark MinTopicLeaderPerBrokerGoal as soft goal

In our environment we've seen a case where RackAwareDistributionGoal and MinTopicLeaderPerBrokerGoal are conflicting. As both are hard goals the rebalance fails Lowering MinTopicLeaderPerBrokerGoal as a soft goal
adobe · Jan 3, 2024 · 8eec56d · 8eec56d
1 parent ef02cb6
commit 8eec56d
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 15 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -11,6 +11,8 @@ jobs:
       - checkout
       - restore_cache:
           key: dependency-cache-{{ checksum "build.gradle" }}
+      - run:
+          command: git tag 2.5.0-SNAPSHOT
       - run:
           command: ./gradlew --no-daemon clean javadoc
       - run:

diff --git a/...in/java/com/linkedin/kafka/cruisecontrol/analyzer/goals/MinTopicLeadersPerBrokerGoal.java b/...in/java/com/linkedin/kafka/cruisecontrol/analyzer/goals/MinTopicLeadersPerBrokerGoal.java
@@ -81,7 +81,7 @@ public ModelCompletenessRequirements clusterModelCompletenessRequirements() {
 
   @Override
   public boolean isHardGoal() {
-    return true;
+    return false;
   }
 
   /**
@@ -287,8 +287,7 @@ protected void updateGoalState(ClusterModel clusterModel, OptimizationOptions op
     finish();
   }
 
-  private void ensureBrokersAllHaveEnoughLeaderOfTopics(ClusterModel clusterModel, OptimizationOptions optimizationOptions)
-      throws OptimizationFailureException {
+  private void ensureBrokersAllHaveEnoughLeaderOfTopics(ClusterModel clusterModel, OptimizationOptions optimizationOptions) {
     if (_mustHaveTopicMinLeadersPerBroker.isEmpty()) {
       // Early termination to avoid some unnecessary computation
       return;
@@ -300,9 +299,9 @@ private void ensureBrokersAllHaveEnoughLeaderOfTopics(ClusterModel clusterModel,
       for (String mustHaveLeaderPerBrokerTopicName : _mustHaveTopicMinLeadersPerBroker.keySet()) {
         int leaderCount = broker.numLeadersFor(mustHaveLeaderPerBrokerTopicName);
         if (leaderCount < minTopicLeadersPerBroker(mustHaveLeaderPerBrokerTopicName)) {
-          throw new OptimizationFailureException(String.format("[%s] Broker %d has insufficient per-broker leaders for topic %s (required: %d "
-                                                               + "current: %d).", name(), broker.id(), mustHaveLeaderPerBrokerTopicName,
-                                                               minTopicLeadersPerBroker(mustHaveLeaderPerBrokerTopicName), leaderCount));
+          LOG.warn("[{}] Broker {} has insufficient per-broker leaders for topic {} (required: {} current: {}).",
+                  name(), broker.id(), mustHaveLeaderPerBrokerTopicName,
+                  minTopicLeadersPerBroker(mustHaveLeaderPerBrokerTopicName), leaderCount);
         }
       }
     }
@@ -338,7 +337,7 @@ private void maybeMoveLeaderOfTopicToBroker(String topicMustHaveLeaderPerBroker,
                                               Broker broker,
                                               ClusterModel clusterModel,
                                               Set<Goal> optimizedGoals,
-                                              OptimizationOptions optimizationOptions) throws OptimizationFailureException {
+                                              OptimizationOptions optimizationOptions) {
     int topicLeaderCountOnReceiverBroker = broker.numLeadersFor(topicMustHaveLeaderPerBroker);
     if (topicLeaderCountOnReceiverBroker >= minTopicLeadersPerBroker(topicMustHaveLeaderPerBroker)) {
       // This broker has enough leader replica(s) for the given topic
@@ -403,9 +402,10 @@ private void maybeMoveLeaderOfTopicToBroker(String topicMustHaveLeaderPerBroker,
         }
       }
     }
-    throw new OptimizationFailureException(String.format("[%s] Cannot make broker %d have at least %d leaders from topic %s.",
+    LOG.warn("{} Cannot make broker {} have at least {} leaders from topic {}. Leaders on broker: {}",
                                                          name(), broker.id(),
-                                                         minTopicLeadersPerBroker(topicMustHaveLeaderPerBroker), topicMustHaveLeaderPerBroker));
+                                                         minTopicLeadersPerBroker(topicMustHaveLeaderPerBroker),
+                                                         topicMustHaveLeaderPerBroker, topicLeaderCountOnReceiverBroker);
   }
 
   /**

diff --git a/.../src/test/java/com/linkedin/kafka/cruisecontrol/analyzer/ReplicationFactorChangeTest.java b/.../src/test/java/com/linkedin/kafka/cruisecontrol/analyzer/ReplicationFactorChangeTest.java
@@ -160,9 +160,8 @@ private static Class<? extends Throwable> expectedExceptionClass(short replicati
                                                                    Class<? extends Goal> goalClass,
                                                                    boolean smallCluster) {
     if ((replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == RackAwareGoal.class)
-        || (replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == ReplicaCapacityGoal.class && !smallCluster)
-        || (replicationFactor == SMALL_REPLICATION_FACTOR && goalClass == MinTopicLeadersPerBrokerGoal.class)
-        || (replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == MinTopicLeadersPerBrokerGoal.class && !smallCluster)) {
+            || (replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == ReplicaCapacityGoal.class && !smallCluster)
+    ) {
       return OptimizationFailureException.class;
     }
     return null;
@@ -192,9 +191,7 @@ private static boolean expectedToOptimize(short replicationFactor, Class<? exten
              || goalClass == LeaderBytesInDistributionGoal.class && !(replicationFactor != SMALL_REPLICATION_FACTOR && !smallCluster)
              || replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == DiskUsageDistributionGoal.class && !smallCluster
              || replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == NetworkInboundUsageDistributionGoal.class && !smallCluster
-             || replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == CpuUsageDistributionGoal.class
-             || replicationFactor == SMALL_REPLICATION_FACTOR && goalClass == MinTopicLeadersPerBrokerGoal.class
-             || replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == MinTopicLeadersPerBrokerGoal.class && !smallCluster);
+             || replicationFactor == LARGE_REPLICATION_FACTOR && goalClass == CpuUsageDistributionGoal.class);
   }
 
   @Test