You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by jx...@apache.org on 2018/07/12 20:48:31 UTC
[2/4] helix git commit: DelayedAutoRebalancer should calculate
assignment based on full partition list.
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 <jj...@linkedin.com>
Authored: Fri Jun 1 15:30:39 2018 -0700
Committer: jiajunwang <er...@gmail.com>
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();