You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2017/05/25 18:15:19 UTC

lucene-solr:jira/solr-10515: SOLR-10515 Improve TriggerBase.lastState caching so that it does not require predictable iterations from state and its objects.

Repository: lucene-solr
Updated Branches:
  refs/heads/jira/solr-10515 e4261d1cf -> aa4fa239d


SOLR-10515 Improve TriggerBase.lastState caching so that it does not require
predictable iterations from state and its objects.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/aa4fa239
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/aa4fa239
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/aa4fa239

Branch: refs/heads/jira/solr-10515
Commit: aa4fa239ddf241da990647702067cb5f75509204
Parents: e4261d1
Author: Andrzej Bialecki <ab...@apache.org>
Authored: Thu May 25 20:14:04 2017 +0200
Committer: Andrzej Bialecki <ab...@apache.org>
Committed: Thu May 25 20:14:04 2017 +0200

----------------------------------------------------------------------
 .../cloud/autoscaling/NodeAddedTrigger.java     | 12 ++---
 .../solr/cloud/autoscaling/NodeLostTrigger.java | 12 ++---
 .../solr/cloud/autoscaling/TriggerBase.java     | 19 ++++----
 .../java/org/apache/solr/common/util/Utils.java | 48 ++++++++++++++++----
 4 files changed, 58 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/aa4fa239/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java
index 58d4786..642ae56 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeAddedTrigger.java
@@ -29,8 +29,6 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.TreeMap;
-import java.util.TreeSet;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -62,7 +60,7 @@ public class NodeAddedTrigger extends TriggerBase {
 
   private Set<String> lastLiveNodes;
 
-  private Map<String, Long> nodeNameVsTimeAdded = new TreeMap<>();
+  private Map<String, Long> nodeNameVsTimeAdded = new HashMap<>();
 
   public NodeAddedTrigger(String name, Map<String, Object> properties,
                           CoreContainer container) {
@@ -82,7 +80,7 @@ public class NodeAddedTrigger extends TriggerBase {
     } else {
       actions = Collections.emptyList();
     }
-    lastLiveNodes = new TreeSet<>(container.getZkController().getZkStateReader().getClusterState().getLiveNodes());
+    lastLiveNodes = new HashSet<>(container.getZkController().getZkStateReader().getClusterState().getLiveNodes());
     log.debug("Initial livenodes: {}", lastLiveNodes);
     this.enabled = (boolean) properties.getOrDefault("enabled", true);
     this.waitForSecond = ((Long) properties.getOrDefault("waitFor", -1L)).intValue();
@@ -170,8 +168,8 @@ public class NodeAddedTrigger extends TriggerBase {
     if (old instanceof NodeAddedTrigger) {
       NodeAddedTrigger that = (NodeAddedTrigger) old;
       assert this.name.equals(that.name);
-      this.lastLiveNodes = new TreeSet<>(that.lastLiveNodes);
-      this.nodeNameVsTimeAdded = new TreeMap<>(that.nodeNameVsTimeAdded);
+      this.lastLiveNodes = new HashSet<>(that.lastLiveNodes);
+      this.nodeNameVsTimeAdded = new HashMap<>(that.nodeNameVsTimeAdded);
     } else  {
       throw new SolrException(SolrException.ErrorCode.INVALID_STATE,
           "Unable to restore state from an unknown type of trigger");
@@ -249,7 +247,7 @@ public class NodeAddedTrigger extends TriggerBase {
         }
       }
 
-      lastLiveNodes = new TreeSet(newLiveNodes);
+      lastLiveNodes = new HashSet(newLiveNodes);
     } catch (RuntimeException e) {
       log.error("Unexpected exception in NodeAddedTrigger", e);
     }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/aa4fa239/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeLostTrigger.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeLostTrigger.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeLostTrigger.java
index e89385d..76c59db 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeLostTrigger.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/NodeLostTrigger.java
@@ -29,8 +29,6 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.TreeMap;
-import java.util.TreeSet;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -62,7 +60,7 @@ public class NodeLostTrigger extends TriggerBase {
 
   private Set<String> lastLiveNodes;
 
-  private Map<String, Long> nodeNameVsTimeRemoved = new TreeMap<>();
+  private Map<String, Long> nodeNameVsTimeRemoved = new HashMap<>();
 
   public NodeLostTrigger(String name, Map<String, Object> properties,
                          CoreContainer container) {
@@ -82,7 +80,7 @@ public class NodeLostTrigger extends TriggerBase {
     } else {
       actions = Collections.emptyList();
     }
-    lastLiveNodes = new TreeSet<>(container.getZkController().getZkStateReader().getClusterState().getLiveNodes());
+    lastLiveNodes = new HashSet<>(container.getZkController().getZkStateReader().getClusterState().getLiveNodes());
     log.debug("Initial livenodes: {}", lastLiveNodes);
     this.enabled = (boolean) properties.getOrDefault("enabled", true);
     this.waitForSecond = ((Long) properties.getOrDefault("waitFor", -1L)).intValue();
@@ -169,8 +167,8 @@ public class NodeLostTrigger extends TriggerBase {
     if (old instanceof NodeLostTrigger) {
       NodeLostTrigger that = (NodeLostTrigger) old;
       assert this.name.equals(that.name);
-      this.lastLiveNodes = new TreeSet<>(that.lastLiveNodes);
-      this.nodeNameVsTimeRemoved = new TreeMap<>(that.nodeNameVsTimeRemoved);
+      this.lastLiveNodes = new HashSet<>(that.lastLiveNodes);
+      this.nodeNameVsTimeRemoved = new HashMap<>(that.nodeNameVsTimeRemoved);
     } else  {
       throw new SolrException(SolrException.ErrorCode.INVALID_STATE,
           "Unable to restore state from an unknown type of trigger");
@@ -245,7 +243,7 @@ public class NodeLostTrigger extends TriggerBase {
         }
       }
 
-      lastLiveNodes = new TreeSet<>(newLiveNodes);
+      lastLiveNodes = new HashSet<>(newLiveNodes);
     } catch (RuntimeException e) {
       log.error("Unexpected exception in NodeLostTrigger", e);
     }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/aa4fa239/solr/core/src/java/org/apache/solr/cloud/autoscaling/TriggerBase.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/TriggerBase.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/TriggerBase.java
index 517e051..ef9a3cf 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/TriggerBase.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/TriggerBase.java
@@ -38,7 +38,7 @@ public abstract class TriggerBase implements AutoScaling.Trigger {
   private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   protected SolrZkClient zkClient;
-  protected byte[] lastState;
+  protected Map<String,Object> lastState;
 
 
   protected TriggerBase(SolrZkClient zkClient) {
@@ -64,13 +64,12 @@ public abstract class TriggerBase implements AutoScaling.Trigger {
 
   @Override
   public void saveState() {
-    Map<String,Object> state = getState();
-    TreeMap<String, Object> map = new TreeMap<>(state);
-    byte[] data = Utils.toJSON(map);
-    // skip saving if identical
-    if (lastState != null && Arrays.equals(lastState, data)) {
+    Map<String,Object> state = Utils.getDeepCopy(getState(), 10, false, true);
+    if (lastState != null && lastState.equals(state)) {
+      // skip saving if identical
       return;
     }
+    byte[] data = Utils.toJSON(state);
     String path = ZkStateReader.SOLR_AUTOSCALING_TRIGGER_STATE_PATH + "/" + getName();
     try {
       if (zkClient.exists(path, true)) {
@@ -80,7 +79,7 @@ public abstract class TriggerBase implements AutoScaling.Trigger {
         // create
         zkClient.create(path, data, CreateMode.PERSISTENT, true);
       }
-      lastState = data;
+      lastState = state;
     } catch (KeeperException | InterruptedException e) {
       LOG.warn("Exception updating trigger state '" + path + "'", e);
     }
@@ -98,9 +97,11 @@ public abstract class TriggerBase implements AutoScaling.Trigger {
       LOG.warn("Exception getting trigger state '" + path + "'", e);
     }
     if (data != null) {
-      Map<String, Object> state = (Map<String, Object>) Utils.fromJSON(data);
+      Map<String, Object> state = (Map<String, Object>)Utils.fromJSON(data);
+      // make sure lastState is sorted
+      state = Utils.getDeepCopy(state, 10, false, true);;
       setState(state);
-      lastState = data;
+      lastState = state;
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/aa4fa239/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 7e92629..cf83dee 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -31,6 +31,8 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
+import java.util.TreeSet;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -53,33 +55,59 @@ public class Utils {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   
   public static Map getDeepCopy(Map map, int maxDepth) {
-    return getDeepCopy(map, maxDepth, true);
+    return getDeepCopy(map, maxDepth, true, false);
   }
 
   public static Map getDeepCopy(Map map, int maxDepth, boolean mutable) {
+    return getDeepCopy(map, maxDepth, mutable, false);
+  }
+
+  public static Map getDeepCopy(Map map, int maxDepth, boolean mutable, boolean sorted) {
     if(map == null) return null;
     if (maxDepth < 1) return map;
-    Map copy = new LinkedHashMap();
+    Map copy;
+    if (sorted) {
+      copy = new TreeMap();
+    } else {
+      copy = new LinkedHashMap();
+    }
     for (Object o : map.entrySet()) {
       Map.Entry e = (Map.Entry) o;
-      copy.put(e.getKey(), makeDeepCopy(e.getValue(),maxDepth, mutable));
+      copy.put(e.getKey(), makeDeepCopy(e.getValue(),maxDepth, mutable, sorted));
     }
     return mutable ? copy : Collections.unmodifiableMap(copy);
   }
 
-  private static Object makeDeepCopy(Object v, int maxDepth, boolean mutable) {
-    if (v instanceof MapWriter && maxDepth > 1) v = ((MapWriter) v).toMap(new LinkedHashMap<>());
-    else if (v instanceof IteratorWriter && maxDepth > 1) v = ((IteratorWriter) v).toList(new ArrayList<>());
+  private static Object makeDeepCopy(Object v, int maxDepth, boolean mutable, boolean sorted) {
+    if (v instanceof MapWriter && maxDepth > 1) {
+      v = ((MapWriter) v).toMap(new LinkedHashMap<>());
+    } else if (v instanceof IteratorWriter && maxDepth > 1) {
+      v = ((IteratorWriter) v).toList(new ArrayList<>());
+      if (sorted) {
+        Collections.sort((List)v);
+      }
+    }
 
-    if (v instanceof Map) v = getDeepCopy((Map) v, maxDepth - 1, mutable);
-    else if (v instanceof Collection) v = getDeepCopy((Collection) v, maxDepth - 1, mutable);
+    if (v instanceof Map) {
+      v = getDeepCopy((Map) v, maxDepth - 1, mutable, sorted);
+    } else if (v instanceof Collection) {
+      v = getDeepCopy((Collection) v, maxDepth - 1, mutable, sorted);
+    }
     return v;
   }
 
   public static Collection getDeepCopy(Collection c, int maxDepth, boolean mutable) {
+    return getDeepCopy(c, maxDepth, mutable, false);
+  }
+
+  public static Collection getDeepCopy(Collection c, int maxDepth, boolean mutable, boolean sorted) {
     if (c == null || maxDepth < 1) return c;
-    Collection result = c instanceof Set ? new HashSet() : new ArrayList();
-    for (Object o : c) result.add(makeDeepCopy(o, maxDepth, mutable));
+    Collection result = c instanceof Set ?
+        ( sorted? new TreeSet() : new HashSet()) : new ArrayList();
+    for (Object o : c) result.add(makeDeepCopy(o, maxDepth, mutable, sorted));
+    if (sorted && (result instanceof List)) {
+      Collections.sort((List)result);
+    }
     return mutable ? result : result instanceof Set ? unmodifiableSet((Set) result) : unmodifiableList((List) result);
   }