Repository: helix Updated Branches: refs/heads/master 5f142f2ec -> 0147f9466
[HELIX-676] Fix the issue that the controller keep updating idealstates when there is no real diff. The causes of the problem are that: 1.A previous issue introduced into IntermediateStateCalcStage prevents ERROR/OFFLINE state replicas from being added to the intermediateState, given the controller thinks recovery rebalance is not necessary. This makes the processed stateMapping in pipeline always different from the cached IdeaStates. And then causes endless updating. 2.Another separate change in persistAssignmentStage is also related to this issue. When updating the map/list, we used putAll. This call will keep all existing items but only modify the intersect. Our assumption previously is allow customized items. However, when we investigate this issue, it would be error-prone to allow these customized items in the map/list fields. Helix won't be able to tell if one item is added by the controller or users. So we decide to use clear and set instead. This ensure the map/list fields are always uptodate. Project: http://git-wip-us.apache.org/repos/asf/helix/repo Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/9083950d Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/9083950d Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/9083950d Branch: refs/heads/master Commit: 9083950d9a0e5f5c377672fb5b21af13cdde0d9a Parents: 5f142f2 Author: Jiajun Wang <jjw...@linkedin.com> Authored: Mon Feb 12 11:45:18 2018 -0800 Committer: jiajunwang <ericwang1...@gmail.com> Committed: Thu Mar 8 17:24:28 2018 -0800 ---------------------------------------------------------------------- .../controller/stages/IntermediateStateCalcStage.java | 14 +++++++++++--- .../controller/stages/PersistAssignmentStage.java | 2 ++ 2 files changed, 13 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/helix/blob/9083950d/helix-core/src/main/java/org/apache/helix/controller/stages/IntermediateStateCalcStage.java ---------------------------------------------------------------------- diff --git a/helix-core/src/main/java/org/apache/helix/controller/stages/IntermediateStateCalcStage.java b/helix-core/src/main/java/org/apache/helix/controller/stages/IntermediateStateCalcStage.java index 047cc9b..785ed7b 100644 --- a/helix-core/src/main/java/org/apache/helix/controller/stages/IntermediateStateCalcStage.java +++ b/helix-core/src/main/java/org/apache/helix/controller/stages/IntermediateStateCalcStage.java @@ -220,6 +220,10 @@ public class IntermediateStateCalcStage extends AbstractBaseStage { RebalanceType rebalanceType = getRebalanceType(cache, bestPossibleMap, preferenceList, stateModelDef, currentStateMap, idealState); + + // TODO refine getRebalanceType to return more accurate rebalance types. + // So following logic doesn't need to check for more details. + boolean rebalanceNeeded = false; if (rebalanceType.equals(RebalanceType.RECOVERY_BALANCE)) { // Check if any error exist if (currentStateMap.values().contains(HelixDefinedState.ERROR.name())) { @@ -228,10 +232,14 @@ public class IntermediateStateCalcStage extends AbstractBaseStage { // Check if recovery is needed for this partition if (!currentStateMap.equals(bestPossibleMap)) { partitionsNeedRecovery.add(partition); - } - } else if (rebalanceType.equals(RebalanceType.LOAD_BALANCE)){ + rebalanceNeeded = true; + } // else, if currentState == bestPossibleState, no rebalance needed + } else if (rebalanceType.equals(RebalanceType.LOAD_BALANCE)) { partitionsNeedLoadbalance.add(partition); - } else { + rebalanceNeeded = true; + } + + if (!rebalanceNeeded) { // no rebalance needed. Map<String, String> intermediateMap = new HashMap<>(bestPossibleMap); intermediatePartitionStateMap.setState(partition, intermediateMap); http://git-wip-us.apache.org/repos/asf/helix/blob/9083950d/helix-core/src/main/java/org/apache/helix/controller/stages/PersistAssignmentStage.java ---------------------------------------------------------------------- diff --git a/helix-core/src/main/java/org/apache/helix/controller/stages/PersistAssignmentStage.java b/helix-core/src/main/java/org/apache/helix/controller/stages/PersistAssignmentStage.java index 047ded3..7281430 100644 --- a/helix-core/src/main/java/org/apache/helix/controller/stages/PersistAssignmentStage.java +++ b/helix-core/src/main/java/org/apache/helix/controller/stages/PersistAssignmentStage.java @@ -119,7 +119,9 @@ public class PersistAssignmentStage extends AbstractBaseStage { if (current != null) { // Overwrite MapFields and ListFields items with the same key. // Note that default merge will keep old values in the maps or lists unchanged, which is not desired. + current.getMapFields().clear(); current.getMapFields().putAll(idealState.getRecord().getMapFields()); + current.getListFields().clear(); current.getListFields().putAll(idealState.getRecord().getListFields()); } return current;
