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:38 UTC

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

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;