helix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject [1/3] helix git commit: [HELIX-676] Fix the issue that the controller keep updating idealstates when there is no real diff.
Date Tue, 13 Mar 2018 22:50:20 GMT
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 <jjwang@linkedin.com>
Authored: Mon Feb 12 11:45:18 2018 -0800
Committer: jiajunwang <ericwang1985@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;


Mime
View raw message