You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by zz...@apache.org on 2015/01/28 02:53:01 UTC

helix git commit: [HELIX-563] Throw more meaningful exceptions when AutoRebalanceStrategy#computePartitionAssignment inputs are invalid, [HELIX-564] StateModelFactory is not removing empty map when a resource is dropped, rb=30324

Repository: helix
Updated Branches:
  refs/heads/helix-0.6.x d5687fa41 -> 03d25dd5b


[HELIX-563] Throw more meaningful exceptions when AutoRebalanceStrategy#computePartitionAssignment inputs are invalid, [HELIX-564] StateModelFactory is not removing empty map when a resource is dropped, rb=30324


Project: http://git-wip-us.apache.org/repos/asf/helix/repo
Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/03d25dd5
Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/03d25dd5
Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/03d25dd5

Branch: refs/heads/helix-0.6.x
Commit: 03d25dd5bdbd30df5d5c7b9807ccacc1462266fd
Parents: d5687fa
Author: zzhang <zz...@apache.org>
Authored: Tue Jan 27 17:52:48 2015 -0800
Committer: zzhang <zz...@apache.org>
Committed: Tue Jan 27 17:52:48 2015 -0800

----------------------------------------------------------------------
 .../strategy/AutoRebalanceStrategy.java         |  7 +++++++
 .../statemachine/StateModelFactory.java         | 21 ++++++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/helix/blob/03d25dd5/helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java b/helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java
index 6578af0..11b5b0d 100644
--- a/helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java
+++ b/helix-core/src/main/java/org/apache/helix/controller/strategy/AutoRebalanceStrategy.java
@@ -436,6 +436,13 @@ public class AutoRebalanceStrategy {
         // check if its in one of the preferred position
         for (int replicaId = 0; replicaId < count; replicaId++) {
           Replica replica = new Replica(partition, replicaId);
+          if (!_preferredAssignment.containsKey(replica)) {
+
+            logger.info("partitions: " + _partitions);
+            logger.info("currentMapping.keySet: " + currentMapping.keySet());
+            throw new IllegalArgumentException("partition: " + replica + " is in currentMapping but not in partitions");
+          }
+
           if (_preferredAssignment.get(replica).id != node.id
               && !_existingPreferredAssignment.containsKey(replica)
               && !existingNonPreferredAssignment.containsKey(replica)) {

http://git-wip-us.apache.org/repos/asf/helix/blob/03d25dd5/helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java b/helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java
index 3a6c6d5..b775016 100644
--- a/helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java
+++ b/helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java
@@ -56,8 +56,12 @@ public abstract class StateModelFactory<T extends StateModel> {
    */
   public T createAndAddStateModel(String resourceName, String partitionKey) {
     T stateModel = createNewStateModel(resourceName, partitionKey);
-    _stateModelMap.putIfAbsent(resourceName, new ConcurrentHashMap<String, T>());
-    _stateModelMap.get(resourceName).put(partitionKey, stateModel);
+    synchronized(_stateModelMap) {
+      if (!_stateModelMap.containsKey(resourceName)) {
+        _stateModelMap.put(resourceName, new ConcurrentHashMap<String, T>());
+      }
+      _stateModelMap.get(resourceName).put(partitionKey, stateModel);
+    }
     return stateModel;
   }
 
@@ -79,8 +83,17 @@ public abstract class StateModelFactory<T extends StateModel> {
    * @return state model removed or null if not exist
    */
   public T removeStateModel(String resourceName, String partitionKey) {
-    Map<String, T> map = _stateModelMap.get(resourceName);
-    return map == null? null : map.remove(partitionKey);
+    T stateModel = null;
+    synchronized(_stateModelMap) {
+      Map<String, T> map = _stateModelMap.get(resourceName);
+      if (map != null) {
+        stateModel = map.remove(partitionKey);
+        if (map.isEmpty()) {
+          _stateModelMap.remove(resourceName);
+        }
+      }
+    }
+    return stateModel;
   }
 
   /**