You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ge...@apache.org on 2019/01/03 19:50:37 UTC

[1/3] lucene-solr:branch_7_6: SOLR-13045: Harden TestSimPolicyCloud

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_7_6 e1d5761f7 -> 34d82ed03


SOLR-13045: Harden TestSimPolicyCloud

This commit fixes a race condition in SimClusterStateProvider, fixing
several fails in TestSimPolicyCloud.


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

Branch: refs/heads/branch_7_6
Commit: 7a0b0590a0db5def3886ac85b80f2aee4fae85bc
Parents: e1d5761
Author: Jason Gerlowski <ge...@apache.org>
Authored: Fri Dec 7 11:07:42 2018 -0500
Committer: Jason Gerlowski <ge...@apache.org>
Committed: Thu Jan 3 10:52:20 2019 -0500

----------------------------------------------------------------------
 .../cloud/autoscaling/sim/SimCloudManager.java  |   2 +-
 .../sim/SimClusterStateProvider.java            | 139 ++++++++++---------
 .../autoscaling/sim/TestSimExtremeIndexing.java |   1 +
 .../autoscaling/sim/TestSimPolicyCloud.java     |   2 +-
 4 files changed, 75 insertions(+), 69 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7a0b0590/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java
index 9a6b73f..b6d0e09 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java
@@ -838,7 +838,7 @@ public class SimCloudManager implements SolrCloudManager {
           results.add("success", "");
           break;
         case ADDROLE:
-          nodeStateProvider.simAddNodeValue(req.getParams().get("node"), "nodeRole", req.getParams().get("role"));
+          nodeStateProvider.simSetNodeValue(req.getParams().get("node"), "nodeRole", req.getParams().get("role"));
           break;
         case CREATESHARD:
           try {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7a0b0590/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
index a7471eb..40d5cad 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
@@ -691,79 +691,84 @@ public class SimClusterStateProvider implements ClusterStateProvider {
 
   private void simRunLeaderElection(String collection, Slice s, boolean saveState) throws Exception {
     AtomicBoolean stateChanged = new AtomicBoolean(Boolean.FALSE);
-    Replica leader = s.getLeader();
-    if (leader == null || !liveNodes.contains(leader.getNodeName())) {
-      log.trace("Running leader election for {} / {}", collection, s.getName());
-      if (s.getReplicas().isEmpty()) { // no replicas - punt
-        log.trace("-- no replicas in {} / {}", collection, s.getName());
-        return;
-      }
-      ActionThrottle lt = getThrottle(collection, s.getName());
-      synchronized (lt) {
-        // collect all active and live
-        List<ReplicaInfo> active = new ArrayList<>();
-        AtomicBoolean alreadyHasLeader = new AtomicBoolean(false);
-        s.getReplicas().forEach(r -> {
-          // find our ReplicaInfo for this replica
-          ReplicaInfo ri = getReplicaInfo(r);
-          if (ri == null) {
-            throw new IllegalStateException("-- could not find ReplicaInfo for replica " + r);
-          }
-          synchronized (ri) {
-            if (r.isActive(liveNodes.get())) {
-              if (ri.getVariables().get(ZkStateReader.LEADER_PROP) != null) {
-                log.trace("-- found existing leader {} / {}: {}, {}", collection, s.getName(), ri, r);
-                alreadyHasLeader.set(true);
-                return;
-              } else {
-                active.add(ri);
-              }
-            } else { // if it's on a node that is not live mark it down
-              log.trace("-- replica not active on live nodes: {}, {}", liveNodes.get(), r);
-              if (!liveNodes.contains(r.getNodeName())) {
-                ri.getVariables().put(ZkStateReader.STATE_PROP, Replica.State.DOWN.toString());
-                ri.getVariables().remove(ZkStateReader.LEADER_PROP);
-                stateChanged.set(true);
+    lock.lockInterruptibly();
+    try {
+      Replica leader = s.getLeader();
+      if (leader == null || !liveNodes.contains(leader.getNodeName())) {
+        log.trace("Running leader election for {} / {}", collection, s.getName());
+        if (s.getReplicas().isEmpty()) { // no replicas - punt
+          log.trace("-- no replicas in {} / {}", collection, s.getName());
+          return;
+        }
+        ActionThrottle lt = getThrottle(collection, s.getName());
+        synchronized (lt) {
+          // collect all active and live
+          List<ReplicaInfo> active = new ArrayList<>();
+          AtomicBoolean alreadyHasLeader = new AtomicBoolean(false);
+          s.getReplicas().forEach(r -> {
+            // find our ReplicaInfo for this replica
+            ReplicaInfo ri = getReplicaInfo(r);
+            if (ri == null) {
+              throw new IllegalStateException("-- could not find ReplicaInfo for replica " + r);
+            }
+            synchronized (ri) {
+              if (r.isActive(liveNodes.get())) {
+                if (ri.getVariables().get(ZkStateReader.LEADER_PROP) != null) {
+                  log.trace("-- found existing leader {} / {}: {}, {}", collection, s.getName(), ri, r);
+                  alreadyHasLeader.set(true);
+                  return;
+                } else {
+                  active.add(ri);
+                }
+              } else { // if it's on a node that is not live mark it down
+                log.trace("-- replica not active on live nodes: {}, {}", liveNodes.get(), r);
+                if (!liveNodes.contains(r.getNodeName())) {
+                  ri.getVariables().put(ZkStateReader.STATE_PROP, Replica.State.DOWN.toString());
+                  ri.getVariables().remove(ZkStateReader.LEADER_PROP);
+                  stateChanged.set(true);
+                }
               }
             }
+          });
+          if (alreadyHasLeader.get()) {
+            log.trace("-- already has leader {} / {}: {}", collection, s.getName(), s);
+            return;
           }
-        });
-        if (alreadyHasLeader.get()) {
-          log.trace("-- already has leader {} / {}: {}", collection, s.getName(), s);
-          return;
-        }
-        if (active.isEmpty()) {
-          log.warn("Can't find any active replicas for {} / {}: {}", collection, s.getName(), s);
-          log.debug("-- liveNodes: {}", liveNodes.get());
-          return;
-        }
-        // pick first active one
-        ReplicaInfo ri = null;
-        for (ReplicaInfo a : active) {
-          if (!a.getType().equals(Replica.Type.PULL)) {
-            ri = a;
-            break;
+          if (active.isEmpty()) {
+            log.warn("Can't find any active replicas for {} / {}: {}", collection, s.getName(), s);
+            log.debug("-- liveNodes: {}", liveNodes.get());
+            return;
           }
+          // pick first active one
+          ReplicaInfo ri = null;
+          for (ReplicaInfo a : active) {
+            if (!a.getType().equals(Replica.Type.PULL)) {
+              ri = a;
+              break;
+            }
+          }
+          if (ri == null) {
+            log.warn("-- can't find any suitable replica type for {} / {}: {}", collection, s.getName(), s);
+            return;
+          }
+          // now mark the leader election throttle
+          lt.minimumWaitBetweenActions();
+          lt.markAttemptingAction();
+          synchronized (ri) {
+            ri.getVariables().put(ZkStateReader.LEADER_PROP, "true");
+          }
+          log.debug("-- elected new leader for {} / {} (currentVersion={}): {}", collection,
+              s.getName(), clusterStateVersion, ri);
+          stateChanged.set(true);
         }
-        if (ri == null) {
-          log.warn("-- can't find any suitable replica type for {} / {}: {}", collection, s.getName(), s);
-          return;
-        }
-        // now mark the leader election throttle
-        lt.minimumWaitBetweenActions();
-        lt.markAttemptingAction();
-        synchronized (ri) {
-          ri.getVariables().put(ZkStateReader.LEADER_PROP, "true");
-        }
-        log.debug("-- elected new leader for {} / {} (currentVersion={}): {}", collection,
-            s.getName(), clusterStateVersion, ri);
-        stateChanged.set(true);
+      } else {
+        log.trace("-- already has leader for {} / {}", collection, s.getName());
       }
-    } else {
-      log.trace("-- already has leader for {} / {}", collection, s.getName());
-    }
-    if (stateChanged.get() || saveState) {
-      collectionsStatesRef.set(null);
+    } finally {
+      if (stateChanged.get() || saveState) {
+        collectionsStatesRef.set(null);
+      }
+      lock.unlock();
     }
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7a0b0590/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExtremeIndexing.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExtremeIndexing.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExtremeIndexing.java
index aea7a5f..c5690d9 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExtremeIndexing.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExtremeIndexing.java
@@ -21,6 +21,7 @@ import java.util.Iterator;
 import java.util.Locale;
 
 import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite;
+
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7a0b0590/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java
index b9abc93..c2519d7 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java
@@ -292,7 +292,7 @@ public class TestSimPolicyCloud extends SimSolrCloudTestCase {
     assertEquals(3, coll.getSlice("s3").getReplicas().size());
     coll.forEachReplica(verifyReplicas);
   }
-  @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 14-Oct-2018
+  
   public void testCreateCollectionAddShardUsingPolicy() throws Exception {
     SolrClient solrClient = cluster.simGetSolrClient();
     String nodeId = cluster.getSimClusterStateProvider().simGetRandomNode();


[2/3] lucene-solr:branch_7_6: SOLR-13045: Allow SimDistribStateManager to create top-level data nodes

Posted by ge...@apache.org.
SOLR-13045: Allow SimDistribStateManager to create top-level data nodes

While working on a related issue in SimDistribStateManager, I noticed
that `createData()` only worked successfully on nodes nested more than
one level under root.  (i.e. `createData("/foo", someData, mode)` would
fail, while the same with "/foo/bar" wouldn't).  This was due to an edge
case in SimDistribStateManager's path building logic.  This commit fixes
this issue.


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

Branch: refs/heads/branch_7_6
Commit: dcc09411a09d0addf972ade9da3db01c7f510232
Parents: 7a0b059
Author: Jason Gerlowski <ge...@apache.org>
Authored: Thu Dec 20 11:06:02 2018 -0500
Committer: Jason Gerlowski <ge...@apache.org>
Committed: Thu Jan 3 10:58:19 2019 -0500

----------------------------------------------------------------------
 .../autoscaling/sim/SimDistribStateManager.java | 35 ++++++++++++--------
 .../sim/TestSimDistribStateManager.java         |  7 ++++
 2 files changed, 28 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/dcc09411/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
index 2b8940a..1ca832e 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
@@ -446,27 +446,34 @@ public class SimDistribStateManager implements DistribStateManager {
     }
     // check if parent exists
     String relPath = path.charAt(0) == '/' ? path.substring(1) : path;
-    if (relPath.length() > 0) { // non-root path - check if parent exists
-      String[] elements = relPath.split("/");
-      StringBuilder sb = new StringBuilder();
+    if (relPath.length() == 0) { //Trying to create root-node, return null.
+      // TODO should trying to create a root node throw an exception since its always init'd in the ctor?
+      return null;
+    }
+
+    String[] elements = relPath.split("/");
+    StringBuilder parentStringBuilder = new StringBuilder();
+    Node parentNode = null;
+    if (elements.length == 1) { // Direct descendant of '/'.
+      parentNode = getRoot();
+    } else { // Indirect descendant of '/', lookup parent node
       for (int i = 0; i < elements.length - 1; i++) {
-        sb.append('/');
-        sb.append(elements[i]);
+        parentStringBuilder.append('/');
+        parentStringBuilder.append(elements[i]);
       }
-      if (!hasData(sb.toString())) {
-        throw new NoSuchElementException(sb.toString());
+      if (!hasData(parentStringBuilder.toString())) {
+        throw new NoSuchElementException(parentStringBuilder.toString());
       }
+      parentNode = traverse(parentStringBuilder.toString(), false, mode);
     }
     Node n = null;
     multiLock.lock();
     try {
-      n = traverse(path, true, mode);
-    } finally {
-      multiLock.unlock();
-    }
-    try {
-      n.setData(data, -1);
-      return n.path;
+      String nodeName = elements[elements.length-1];
+      Node childNode = createNode(parentNode, mode, parentStringBuilder.append("/"), nodeName, false);
+      childNode.setData(data, -1);
+      parentNode.setChild(childNode.name, childNode);
+      return childNode.path;
     } catch (BadVersionException e) {
       // not happening
       return null;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/dcc09411/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java
index b161a15..1f80e23 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java
@@ -18,6 +18,7 @@
 package org.apache.solr.cloud.autoscaling.sim;
 
 import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
@@ -262,6 +263,12 @@ public class TestSimDistribStateManager extends SolrTestCaseJ4 {
     assertEquals(0, kids.size());
   }
 
+  @Test
+  public void testCanCreateNodesWithDataAtTopLevel() throws Exception {
+    final String path = stateManager.createData("/topLevelNodeWithData", new String("helloworld").getBytes(StandardCharsets.UTF_8), CreateMode.PERSISTENT);
+    assertEquals("/topLevelNodeWithData", path);
+  }
+
   static class OnceWatcher implements Watcher {
     CountDownLatch triggered = new CountDownLatch(1);
     WatchedEvent event;


[3/3] lucene-solr:branch_7_6: SOLR-13045: Sim node versioning should start at 0

Posted by ge...@apache.org.
SOLR-13045: Sim node versioning should start at 0

Prior to this commit, new ZK nodes being simulated by the sim framework
were started with a version of -1.  This causes problems, since -1 is
also coincidentally the flag value used to ignore optimistic concurrency
locking and force overwrite values.


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

Branch: refs/heads/branch_7_6
Commit: 34d82ed033cccd8120431b73e93554b85b24a278
Parents: dcc0941
Author: Jason Gerlowski <ge...@apache.org>
Authored: Thu Dec 20 14:20:50 2018 -0500
Committer: Jason Gerlowski <ge...@apache.org>
Committed: Thu Jan 3 11:00:31 2019 -0500

----------------------------------------------------------------------
 .../autoscaling/OverseerTriggerThread.java      |  2 +-
 .../sim/SimClusterStateProvider.java            |  6 +--
 .../autoscaling/sim/SimDistribStateManager.java | 48 ++++++++++++--------
 .../sim/TestSimDistribStateManager.java         | 29 ++++++++++++
 .../cloud/autoscaling/AutoScalingConfig.java    |  2 +-
 5 files changed, 64 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34d82ed0/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java
index 052b4c4..bddc28b 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java
@@ -72,7 +72,7 @@ public class OverseerTriggerThread implements Runnable, SolrCloseable {
   /*
   Following variables are only accessed or modified when updateLock is held
    */
-  private int znodeVersion = -1;
+  private int znodeVersion = 0;
 
   private Map<String, AutoScaling.Trigger> activeTriggers = new HashMap<>();
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34d82ed0/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
index 40d5cad..b8897ea 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
@@ -624,10 +624,10 @@ public class SimClusterStateProvider implements ClusterStateProvider {
     byte[] data = Utils.toJSON(state);
     try {
       VersionedData oldData = stateManager.getData(ZkStateReader.CLUSTER_STATE);
-      int version = oldData != null ? oldData.getVersion() : -1;
-      Assert.assertEquals(clusterStateVersion, version + 1);
+      int version = oldData != null ? oldData.getVersion() : 0;
+      Assert.assertEquals(clusterStateVersion, version);
       stateManager.setData(ZkStateReader.CLUSTER_STATE, data, version);
-      log.debug("** saved cluster state version " + (version + 1));
+      log.debug("** saved cluster state version " + (version));
       clusterStateVersion++;
     } catch (Exception e) {
       throw new IOException(e);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34d82ed0/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
index 1ca832e..0d8b3ba 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimDistribStateManager.java
@@ -72,7 +72,7 @@ public class SimDistribStateManager implements DistribStateManager {
 
   public static final class Node {
     ReentrantLock dataLock = new ReentrantLock();
-    private int version = -1;
+    private int version = 0;
     private int seq = 0;
     private final CreateMode mode;
     private final String clientId;
@@ -90,7 +90,11 @@ public class SimDistribStateManager implements DistribStateManager {
       this.path = path;
       this.mode = mode;
       this.clientId = clientId;
+    }
 
+    Node(Node parent, String name, String path, byte[] data, CreateMode mode, String clientId) {
+      this(parent, name, path, mode, clientId);
+      this.data = data;
     }
 
     public void clear() {
@@ -311,17 +315,7 @@ public class SimDistribStateManager implements DistribStateManager {
       n = parentNode.children != null ? parentNode.children.get(currentName) : null;
       if (n == null) {
         if (create) {
-          if ((parentNode.mode == CreateMode.EPHEMERAL || parentNode.mode == CreateMode.EPHEMERAL_SEQUENTIAL) &&
-              (mode == CreateMode.EPHEMERAL || mode == CreateMode.EPHEMERAL_SEQUENTIAL)) {
-            throw new IOException("NoChildrenEphemerals for " + parentNode.path);
-          }
-          if (CreateMode.PERSISTENT_SEQUENTIAL == mode || CreateMode.EPHEMERAL_SEQUENTIAL == mode) {
-            currentName = currentName + String.format(Locale.ROOT, "%010d", parentNode.seq);
-            parentNode.seq++;
-          }
-          currentPath.append(currentName);
-          n = new Node(parentNode, currentName, currentPath.toString(), mode, id);
-          parentNode.setChild(currentName, n);
+          n = createNode(parentNode, mode, currentPath, currentName,null, true);
         } else {
           break;
         }
@@ -333,6 +327,26 @@ public class SimDistribStateManager implements DistribStateManager {
     return n;
   }
 
+  private Node createNode(Node parentNode, CreateMode mode, StringBuilder fullChildPath, String baseChildName, byte[] data, boolean attachToParent) throws IOException {
+    String nodeName = baseChildName;
+    if ((parentNode.mode == CreateMode.EPHEMERAL || parentNode.mode == CreateMode.EPHEMERAL_SEQUENTIAL) &&
+        (mode == CreateMode.EPHEMERAL || mode == CreateMode.EPHEMERAL_SEQUENTIAL)) {
+      throw new IOException("NoChildrenEphemerals for " + parentNode.path);
+    }
+    if (CreateMode.PERSISTENT_SEQUENTIAL == mode || CreateMode.EPHEMERAL_SEQUENTIAL == mode) {
+      nodeName = nodeName + String.format(Locale.ROOT, "%010d", parentNode.seq);
+      parentNode.seq++;
+    }
+
+    fullChildPath.append(nodeName);
+    Node child = new Node(parentNode, nodeName, fullChildPath.toString(), data, mode, id);
+
+    if (attachToParent) {
+      parentNode.setChild(nodeName, child);
+    }
+    return child;
+  }
+
   @Override
   public void close() throws IOException {
     multiLock.lock();
@@ -470,13 +484,11 @@ public class SimDistribStateManager implements DistribStateManager {
     multiLock.lock();
     try {
       String nodeName = elements[elements.length-1];
-      Node childNode = createNode(parentNode, mode, parentStringBuilder.append("/"), nodeName, false);
-      childNode.setData(data, -1);
+      Node childNode = createNode(parentNode, mode, parentStringBuilder.append("/"), nodeName, data,false);
       parentNode.setChild(childNode.name, childNode);
       return childNode.path;
-    } catch (BadVersionException e) {
-      // not happening
-      return null;
+    } finally {
+      multiLock.unlock();
     }
   }
 
@@ -570,7 +582,7 @@ public class SimDistribStateManager implements DistribStateManager {
   @Override
   public AutoScalingConfig getAutoScalingConfig(Watcher watcher) throws InterruptedException, IOException {
     Map<String, Object> map = new HashMap<>();
-    int version = -1;
+    int version = 0;
     try {
       VersionedData data = getData(ZkStateReader.SOLR_AUTOSCALING_CONF_PATH, watcher);
       if (data != null && data.getData() != null && data.getData().length > 0) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34d82ed0/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java
index 1f80e23..731d6e8 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimDistribStateManager.java
@@ -342,6 +342,35 @@ public class TestSimDistribStateManager extends SolrTestCaseJ4 {
   }
 
   @Test
+  public void testNewlyCreatedPathsStartWithVersionZero() throws Exception {
+    stateManager.makePath("/createdWithoutData");
+    VersionedData vd = stateManager.getData("/createdWithoutData", null);
+    assertEquals(0, vd.getVersion());
+
+    stateManager.createData("/createdWithData", new String("helloworld").getBytes(StandardCharsets.UTF_8), CreateMode.PERSISTENT);
+    vd = stateManager.getData("/createdWithData");
+    assertEquals(0, vd.getVersion());
+  }
+
+  @Test
+  public void testModifiedDataNodesGetUpdatedVersion() throws Exception {
+    stateManager.createData("/createdWithData", new String("foo").getBytes(StandardCharsets.UTF_8), CreateMode.PERSISTENT);
+    VersionedData vd = stateManager.getData("/createdWithData");
+    assertEquals(0, vd.getVersion());
+
+    stateManager.setData("/createdWithData", new String("bar").getBytes(StandardCharsets.UTF_8), 0);
+    vd = stateManager.getData("/createdWithData");
+    assertEquals(1, vd.getVersion());
+  }
+
+  // This is a little counterintuitive, so probably worth its own testcase so we don't break it accidentally.
+  @Test
+  public void testHasDataReturnsTrueForExistingButEmptyNodes() throws Exception {
+    stateManager.makePath("/nodeWithoutData");
+    assertTrue(stateManager.hasData("/nodeWithoutData"));
+  }
+
+  @Test
   public void testMulti() throws Exception {
 
   }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/34d82ed0/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AutoScalingConfig.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AutoScalingConfig.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AutoScalingConfig.java
index ccd02eb..335312e 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AutoScalingConfig.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AutoScalingConfig.java
@@ -311,7 +311,7 @@ public class AutoScalingConfig implements MapWriter {
    */
   public AutoScalingConfig(Map<String, Object> jsonMap) {
     this.jsonMap = jsonMap;
-    int version = -1;
+    int version = 0;
     if (jsonMap.containsKey(AutoScalingParams.ZK_VERSION)) {
       try {
         version = (Integer)jsonMap.get(AutoScalingParams.ZK_VERSION);