DelayedAutoRebalancer should calculate assignment based on full partition list.
An issue was report that DelayedAutoRebalancer will generate different assignment when user change their user-defined preference list. This is because for some full-auto rebalance strategy, the algorithm balance partition based on workload. As a result, config change in one partition will affect others. In this change, we changed the DelayedAutoRebalancer to calculate assignment based on full partition list first. Then apply user-defined list. This will fix the issue. Also updated related test to cover this scenario. RB=1327182 BUG=HELIX-1052 G=helix-reviewers A=lxia Project: http://git-wip-us.apache.org/repos/asf/helix/repo Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/c145c7c7 Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/c145c7c7 Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/c145c7c7 Branch: refs/heads/master Commit: c145c7c71b996f7a223d82683f1eeff8222f728c Parents: e2b1277 Author: Jiajun Wang <[email protected]> Authored: Fri Jun 1 15:30:39 2018 -0700 Committer: jiajunwang <[email protected]> Committed: Thu Jul 12 13:45:17 2018 -0700 ---------------------------------------------------------------------- .../rebalancer/DelayedAutoRebalancer.java | 5 +-- .../rebalancer/TestMixedModeAutoRebalance.java | 47 +++++++++++++++----- 2 files changed, 36 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/helix/blob/c145c7c7/helix-core/src/main/java/org/apache/helix/controller/rebalancer/DelayedAutoRebalancer.java ---------------------------------------------------------------------- diff --git a/helix-core/src/main/java/org/apache/helix/controller/rebalancer/DelayedAutoRebalancer.java b/helix-core/src/main/java/org/apache/helix/controller/rebalancer/DelayedAutoRebalancer.java index 02f96f6..7b38c31 100644 --- a/helix-core/src/main/java/org/apache/helix/controller/rebalancer/DelayedAutoRebalancer.java +++ b/helix-core/src/main/java/org/apache/helix/controller/rebalancer/DelayedAutoRebalancer.java @@ -148,12 +148,9 @@ public class DelayedAutoRebalancer extends AbstractRebalancer { Map<String, Map<String, String>> currentMapping = currentMapping(currentStateOutput, resourceName, allPartitions, stateCountMap); - - List<String> partitionsToAssign = new ArrayList<>(allPartitions); - partitionsToAssign.removeAll(userDefinedPreferenceList.keySet()); int maxPartition = currentIdealState.getMaxPartitionsPerInstance(); _rebalanceStrategy = - getRebalanceStrategy(currentIdealState.getRebalanceStrategy(), partitionsToAssign, resourceName, + getRebalanceStrategy(currentIdealState.getRebalanceStrategy(), allPartitions, resourceName, stateCountMap, maxPartition); // sort node lists to ensure consistent preferred assignments http://git-wip-us.apache.org/repos/asf/helix/blob/c145c7c7/helix-core/src/test/java/org/apache/helix/integration/rebalancer/TestMixedModeAutoRebalance.java ---------------------------------------------------------------------- diff --git a/helix-core/src/test/java/org/apache/helix/integration/rebalancer/TestMixedModeAutoRebalance.java b/helix-core/src/test/java/org/apache/helix/integration/rebalancer/TestMixedModeAutoRebalance.java index 77340af..156bad5 100644 --- a/helix-core/src/test/java/org/apache/helix/integration/rebalancer/TestMixedModeAutoRebalance.java +++ b/helix-core/src/test/java/org/apache/helix/integration/rebalancer/TestMixedModeAutoRebalance.java @@ -21,11 +21,14 @@ package org.apache.helix.integration.rebalancer; import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.helix.ConfigAccessor; import org.apache.helix.HelixDataAccessor; import org.apache.helix.NotificationContext; +import org.apache.helix.controller.rebalancer.strategy.CrushEdRebalanceStrategy; import org.apache.helix.controller.rebalancer.strategy.CrushRebalanceStrategy; import org.apache.helix.controller.rebalancer.util.RebalanceScheduler; import org.apache.helix.integration.common.ZkIntegrationTestBase; @@ -102,25 +105,27 @@ public class TestMixedModeAutoRebalance extends ZkIntegrationTestBase { @DataProvider(name = "stateModels") public static Object [][] stateModels() { - return new Object[][] { {BuiltInStateModelDefinitions.MasterSlave.name(), true}, - {BuiltInStateModelDefinitions.OnlineOffline.name(), true}, - {BuiltInStateModelDefinitions.LeaderStandby.name(), true}, - {BuiltInStateModelDefinitions.MasterSlave.name(), false}, - {BuiltInStateModelDefinitions.OnlineOffline.name(), false}, - {BuiltInStateModelDefinitions.LeaderStandby.name(), false}, + return new Object[][] { {BuiltInStateModelDefinitions.MasterSlave.name(), true, CrushRebalanceStrategy.class.getName()}, + {BuiltInStateModelDefinitions.OnlineOffline.name(), true, CrushRebalanceStrategy.class.getName()}, + {BuiltInStateModelDefinitions.LeaderStandby.name(), true, CrushRebalanceStrategy.class.getName()}, + {BuiltInStateModelDefinitions.MasterSlave.name(), false, CrushRebalanceStrategy.class.getName()}, + {BuiltInStateModelDefinitions.OnlineOffline.name(), false, CrushRebalanceStrategy.class.getName()}, + {BuiltInStateModelDefinitions.LeaderStandby.name(), false, CrushRebalanceStrategy.class.getName()}, + {BuiltInStateModelDefinitions.MasterSlave.name(), true, CrushEdRebalanceStrategy.class.getName()}, + {BuiltInStateModelDefinitions.OnlineOffline.name(), true, CrushEdRebalanceStrategy.class.getName()} }; } @Test(dataProvider = "stateModels") public void testUserDefinedPreferenceListsInFullAuto( - String stateModel, boolean delayEnabled) throws Exception { + String stateModel, boolean delayEnabled, String rebalanceStrateyName) throws Exception { String db = "Test-DB-" + stateModel; if (delayEnabled) { createResourceWithDelayedRebalance(CLUSTER_NAME, db, stateModel, _PARTITIONS, _replica, - _replica - 1, 200, CrushRebalanceStrategy.class.getName()); + _replica - 1, 200, rebalanceStrateyName); } else { createResourceWithDelayedRebalance(CLUSTER_NAME, db, stateModel, _PARTITIONS, _replica, - _replica, 0, CrushRebalanceStrategy.class.getName()); + _replica, 0, rebalanceStrateyName); } IdealState idealState = _gSetupTool.getClusterManagementTool().getResourceIdealState(CLUSTER_NAME, db); @@ -142,12 +147,18 @@ public class TestMixedModeAutoRebalance extends ZkIntegrationTestBase { //TODO: Trigger rebalancer, remove this once Helix controller is listening on resource config changes. RebalanceScheduler.invokeRebalance(_dataAccessor, db); + Assert.assertTrue(_clusterVerifier.verify(1000)); + verifyUserDefinedPreferenceLists(db, userDefinedPreferenceLists, userDefinedPartitions); while (userDefinedPartitions.size() > 0) { - Thread.sleep(100); - Assert.assertTrue(_clusterVerifier.verify()); - verifyUserDefinedPreferenceLists(db, userDefinedPreferenceLists, userDefinedPartitions); + IdealState originIS = _gSetupTool.getClusterManagementTool().getResourceIdealState(CLUSTER_NAME, db); + Set<String> nonUserDefinedPartitions = new HashSet<>(originIS.getPartitionSet()); + nonUserDefinedPartitions.removeAll(userDefinedPartitions); + removePartitionFromUserDefinedList(db, userDefinedPartitions); + Assert.assertTrue(_clusterVerifier.verify(1000)); + verifyUserDefinedPreferenceLists(db, userDefinedPreferenceLists, userDefinedPartitions); + verifyNonUserDefinedAssignment(db, originIS, nonUserDefinedPartitions); } } @@ -216,6 +227,18 @@ public class TestMixedModeAutoRebalance extends ZkIntegrationTestBase { } } + private void verifyNonUserDefinedAssignment(String db, IdealState originIS, Set<String> nonUserDefinedPartitions) + throws InterruptedException { + IdealState newIS = _gSetupTool.getClusterManagementTool().getResourceIdealState(CLUSTER_NAME, db); + Assert.assertEquals(originIS.getPartitionSet(), newIS.getPartitionSet()); + for (String p : newIS.getPartitionSet()) { + if (nonUserDefinedPartitions.contains(p)) { + // for non user defined partition, mapping should keep the same + Assert.assertEquals(newIS.getPreferenceList(p), originIS.getPreferenceList(p)); + } + } + } + private void removePartitionFromUserDefinedList(String db, List<String> userDefinedPartitions) { ResourceConfig resourceConfig = _configAccessor.getResourceConfig(CLUSTER_NAME, db); Map<String, List<String>> lists = resourceConfig.getPreferenceLists();
