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/06/01 15:50:17 UTC

lucene-solr:jira/solr-10745: SOLR-10745 Add nicer API for retrieving AutoScalingConfig and checking for existence of triggers of a certain type. Don't process markers for lost nodes that came back, or for added nodes that disappeared.

Repository: lucene-solr
Updated Branches:
  refs/heads/jira/solr-10745 b67de922e -> 302cfd992


SOLR-10745 Add nicer API for retrieving AutoScalingConfig and checking
for existence of triggers of a certain type. Don't process markers for
lost nodes that came back, or for added nodes that disappeared.


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

Branch: refs/heads/jira/solr-10745
Commit: 302cfd992e97f7470511367de1e78229ecf6aabb
Parents: b67de92
Author: Andrzej Bialecki <ab...@apache.org>
Authored: Thu Jun 1 17:48:42 2017 +0200
Committer: Andrzej Bialecki <ab...@apache.org>
Committed: Thu Jun 1 17:48:42 2017 +0200

----------------------------------------------------------------------
 .../org/apache/solr/cloud/ZkController.java     | 42 +++++++++++++----
 .../cloud/autoscaling/AutoScalingConfig.java    | 49 ++++++++++++++++++++
 .../cloud/autoscaling/NodeAddedTrigger.java     |  7 ++-
 .../solr/cloud/autoscaling/NodeLostTrigger.java |  7 ++-
 .../autoscaling/TriggerIntegrationTest.java     | 31 ++++++-------
 5 files changed, 107 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/302cfd99/solr/core/src/java/org/apache/solr/cloud/ZkController.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
index 37019ec..e3d9a64 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -49,6 +49,8 @@ import org.apache.commons.lang.StringUtils;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder;
 import org.apache.solr.client.solrj.request.CoreAdminRequest.WaitForState;
+import org.apache.solr.cloud.autoscaling.AutoScaling;
+import org.apache.solr.cloud.autoscaling.AutoScalingConfig;
 import org.apache.solr.cloud.overseer.OverseerAction;
 import org.apache.solr.cloud.overseer.SliceMutator;
 import org.apache.solr.common.SolrException;
@@ -558,6 +560,16 @@ public class ZkController {
   }
 
   /**
+   *
+   * @return current configuration from <code>autoscaling.json</code>. NOTE:
+   * this data is retrieved from ZK on each call.
+   */
+  public AutoScalingConfig getAutoScalingConfig() throws KeeperException, InterruptedException {
+    Map<String, Object> jsonMap = zkClient.getJson(ZkStateReader.SOLR_AUTOSCALING_CONF_PATH, true);
+    return new AutoScalingConfig(jsonMap);
+  }
+
+  /**
    * Returns config file data (in bytes)
    */
   public byte[] getConfigFileData(String zkConfigName, String fileName)
@@ -758,14 +770,24 @@ public class ZkController {
         i++;
       }
 
-      for (String n : oldNodes) {
-        String path = ZkStateReader.SOLR_AUTOSCALING_NODE_LOST_PATH + "/" + n;
-        try {
-          zkClient.create(path, null, CreateMode.PERSISTENT, true);
-        } catch (KeeperException.NodeExistsException e) {
-          // someone else already created this node - ignore
-        } catch (KeeperException | InterruptedException e1) {
-          log.warn("Unable to register nodeLost path for " + n, e1);
+      // retrieve current trigger config - if there are no nodeLost triggers
+      // then don't create markers
+      boolean createNodes = false;
+      try {
+        createNodes = getAutoScalingConfig().hasTriggerForEvents(AutoScaling.EventType.NODELOST);
+      } catch (KeeperException | InterruptedException e1) {
+        log.warn("Unable to read autoscaling.json", e1);
+      }
+      if (createNodes) {
+        for (String n : oldNodes) {
+          String path = ZkStateReader.SOLR_AUTOSCALING_NODE_LOST_PATH + "/" + n;
+          try {
+            zkClient.create(path, null, CreateMode.PERSISTENT, true);
+          } catch (KeeperException.NodeExistsException e) {
+            // someone else already created this node - ignore
+          } catch (KeeperException | InterruptedException e1) {
+            log.warn("Unable to register nodeLost path for " + n, e1);
+          }
         }
       }
     };
@@ -850,7 +872,9 @@ public class ZkController {
     log.info("Register node as live in ZooKeeper:" + nodePath);
     List<Op> ops = new ArrayList<>(2);
     ops.add(Op.create(nodePath, null, zkClient.getZkACLProvider().getACLsToAdd(nodePath), CreateMode.EPHEMERAL));
-    if (!zkClient.exists(nodeAddedPath, true)) {
+    // if there are nodeAdded triggers don't create nodeAdded markers
+    boolean createMarkerNode = getAutoScalingConfig().hasTriggerForEvents(AutoScaling.EventType.NODEADDED);
+    if (createMarkerNode && !zkClient.exists(nodeAddedPath, true)) {
       // use EPHEMERAL so that it disappears if this node goes down
       // and no other action is taken
       ops.add(Op.create(nodeAddedPath, null, zkClient.getZkACLProvider().getACLsToAdd(nodeAddedPath), CreateMode.EPHEMERAL));

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/302cfd99/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingConfig.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingConfig.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingConfig.java
index 2877cb9..fb812fe 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingConfig.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingConfig.java
@@ -1,3 +1,19 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package org.apache.solr.cloud.autoscaling;
 
 import java.util.Collections;
@@ -54,10 +70,17 @@ public class AutoScalingConfig {
     }
   }
 
+  /**
+   * Construct from a JSON map representation.
+   * @param jsonMap
+   */
   public AutoScalingConfig(Map<String, Object> jsonMap) {
     this.jsonMap = Utils.getDeepCopy(jsonMap, 10);
   }
 
+  /**
+   * Get {@link Policy} configuration.
+   */
   public Policy getPolicy() {
     if (policy == null) {
       policy = new Policy(jsonMap);
@@ -65,6 +88,9 @@ public class AutoScalingConfig {
     return policy;
   }
 
+  /**
+   * Get trigger configurations.
+   */
   public Map<String, TriggerConfig> getTriggerConfigs() {
     if (triggers == null) {
       Map<String, Object> trigMap = (Map<String, Object>)jsonMap.get("triggers");
@@ -80,6 +106,9 @@ public class AutoScalingConfig {
     return triggers;
   }
 
+  /**
+   * Get listener configurations.
+   */
   public Map<String, ListenerConfig> getListenerConfigs() {
     if (listeners == null) {
       Map<String, Object> map = (Map<String, Object>)jsonMap.get("listeners");
@@ -94,4 +123,24 @@ public class AutoScalingConfig {
     }
     return listeners;
   }
+
+  /**
+   * Check whether triggers for specific event type exist.
+   * @param types list of event types
+   * @return true if there's at least one trigger matching at least one event type,
+   * false otherwise,
+   */
+  public boolean hasTriggerForEvents(AutoScaling.EventType... types) {
+    if (types == null || types.length == 0) {
+      return false;
+    }
+    for (TriggerConfig config : getTriggerConfigs().values()) {
+      for (AutoScaling.EventType type : types) {
+        if (config.eventType.equals(type)) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/302cfd99/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 7a46fc7..4162ef2 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
@@ -103,8 +103,11 @@ public class NodeAddedTrigger extends TriggerBase {
     try {
       List<String> added = container.getZkController().getZkClient().getChildren(ZkStateReader.SOLR_AUTOSCALING_NODE_ADDED_PATH, null, true);
       added.forEach(n -> {
-        log.debug("Adding node from marker path: {}", n);
-        nodeNameVsTimeAdded.put(n, timeSource.getTime());
+        // don't add nodes that have since gone away
+        if (lastLiveNodes.contains(n)) {
+          log.debug("Adding node from marker path: {}", n);
+          nodeNameVsTimeAdded.put(n, timeSource.getTime());
+        }
         removeNodeAddedMarker(n);
       });
     } catch (KeeperException.NoNodeException e) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/302cfd99/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 2af4cc5..9db388d 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
@@ -102,8 +102,11 @@ public class NodeLostTrigger extends TriggerBase {
     try {
       List<String> lost = container.getZkController().getZkClient().getChildren(ZkStateReader.SOLR_AUTOSCALING_NODE_LOST_PATH, null, true);
       lost.forEach(n -> {
-        log.debug("Adding lost node from marker path: {}", n);
-        nodeNameVsTimeRemoved.put(n, timeSource.getTime());
+        // don't add nodes that have since came back
+        if (!lastLiveNodes.contains(n)) {
+          log.debug("Adding lost node from marker path: {}", n);
+          nodeNameVsTimeRemoved.put(n, timeSource.getTime());
+        }
         removeNodeLostMarker(n);
       });
     } catch (KeeperException.NoNodeException e) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/302cfd99/solr/core/src/test/org/apache/solr/cloud/autoscaling/TriggerIntegrationTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TriggerIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TriggerIntegrationTest.java
index ae3f72d..70f0fdc 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TriggerIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TriggerIntegrationTest.java
@@ -877,9 +877,9 @@ public class TriggerIntegrationTest extends SolrCloudTestCase {
     }
     assertEquals(1, listener.addedNodes.size());
     assertEquals(node.getNodeName(), listener.addedNodes.iterator().next());
-    // verify that a znode exists
+    // verify that a znode doesn't exist (no trigger)
     String pathAdded = ZkStateReader.SOLR_AUTOSCALING_NODE_ADDED_PATH + "/" + node.getNodeName();
-    assertTrue("Path " + pathAdded + " wasn't created", zkClient().exists(pathAdded, true));
+    assertFalse("Path " + pathAdded + " was created but there are no nodeAdded triggers", zkClient().exists(pathAdded, true));
     listener.reset();
     // stop overseer
     log.info("====== KILL OVERSEER 1");
@@ -898,17 +898,6 @@ public class TriggerIntegrationTest extends SolrCloudTestCase {
     assertFalse("Path " + pathLost + " exists", zkClient().exists(pathLost, true));
 
     listener.reset();
-    // create another node
-    log.info("====== ADD NODE 1");
-    JettySolrRunner node1 = cluster.startJettySolrRunner();
-    if (!listener.onChangeLatch.await(10, TimeUnit.SECONDS)) {
-      fail("onChange listener didn't execute on cluster change");
-    }
-    assertEquals(1, listener.addedNodes.size());
-    assertEquals(node1.getNodeName(), listener.addedNodes.iterator().next());
-    // verify that a znode exists
-    pathAdded = ZkStateReader.SOLR_AUTOSCALING_NODE_ADDED_PATH + "/" + node1.getNodeName();
-    assertTrue("Path " + pathAdded + " wasn't created", zkClient().exists(pathAdded, true));
 
     // set up triggers
     CloudSolrClient solrClient = cluster.getSolrClient();
@@ -949,10 +938,20 @@ public class TriggerIntegrationTest extends SolrCloudTestCase {
       }
     }
 
+    // create another node
+    log.info("====== ADD NODE 1");
+    JettySolrRunner node1 = cluster.startJettySolrRunner();
+    if (!listener.onChangeLatch.await(10, TimeUnit.SECONDS)) {
+      fail("onChange listener didn't execute on cluster change");
+    }
+    assertEquals(1, listener.addedNodes.size());
+    assertEquals(node1.getNodeName(), listener.addedNodes.iterator().next());
+    // verify that a znode exists
+    pathAdded = ZkStateReader.SOLR_AUTOSCALING_NODE_ADDED_PATH + "/" + node1.getNodeName();
+    assertTrue("Path " + pathAdded + " wasn't created", zkClient().exists(pathAdded, true));
+
     Thread.sleep(5000);
-    // old nodeAdded markers should be consumed now by nodeAdded trigger
-    // but it doesn't result in new events because all nodes have been added
-    // before we configured the trigger
+    // nodeAdded marker should be consumed now by nodeAdded trigger
     assertFalse("Path " + pathAdded + " should have been deleted", zkClient().exists(pathAdded, true));
 
     listener.reset();