You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by md...@apache.org on 2020/11/02 23:08:30 UTC

[lucene-solr] branch branch_8x updated: SOLR-14961 ZkMaintenanceUtils.clean doesn't remove zk nodes with same length

This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new d2c88e3  SOLR-14961 ZkMaintenanceUtils.clean doesn't remove zk nodes with same length
d2c88e3 is described below

commit d2c88e3a912bb3882f01e95de5901298a753f915
Author: Michael Aleythe <mi...@sternwald.com>
AuthorDate: Wed Oct 28 14:46:53 2020 +0100

    SOLR-14961 ZkMaintenanceUtils.clean doesn't remove zk nodes with same length
    
    fixes #2042
---
 solr/CHANGES.txt                                   |  2 +
 .../solr/common/cloud/ZkMaintenanceUtils.java      |  7 +++-
 .../apache/solr/common/cloud/SolrZkClientTest.java |  1 +
 .../{util => cloud}/TestZkMaintenanceUtils.java    | 45 +++++++++++++++++++++-
 4 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3c66082..2767434 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -34,6 +34,8 @@ Bug Fixes
 * SOLR-14969: Prevent creating multiple cores with the same name which leads to instabilities (race condition)
   (Erick Erickson, Andreas Hubold)
 
+* SOLR-14961: Fix for deleting zookeeper nodes with same path length. Only the first zk-node was removed. (Michael Aleythe via Mike Drob)
+
 Other Changes
 ---------------------
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
index 3125f28..e3e61a4 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
@@ -26,10 +26,10 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
+import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Locale;
-import java.util.TreeSet;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
 
@@ -260,12 +260,15 @@ public class ZkMaintenanceUtils {
       return;
     }
 
-    TreeSet<String> paths = new TreeSet<>(Comparator.comparingInt(String::length).reversed());
+    ArrayList<String> paths = new ArrayList<>();
 
     traverseZkTree(zkClient, path, VISIT_ORDER.VISIT_POST, znode -> {
       if (!znode.equals("/") && filter.test(znode)) paths.add(znode);
     });
 
+    // sort the list in descending order to ensure that child entries are deleted first
+    paths.sort(Comparator.comparingInt(String::length).reversed());
+
     for (String subpath : paths) {
       if (!subpath.equals("/")) {
         try {
diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java b/solr/solrj/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java
index 01d412e..536c888 100644
--- a/solr/solrj/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java
@@ -236,4 +236,5 @@ public class SolrZkClientTest extends SolrCloudTestCase {
     SolrZkClient.checkInterrupted(new InterruptedException());
     assertTrue(Thread.currentThread().isInterrupted());
   }
+
 }
diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestZkMaintenanceUtils.java b/solr/solrj/src/test/org/apache/solr/common/cloud/TestZkMaintenanceUtils.java
similarity index 65%
rename from solr/solrj/src/test/org/apache/solr/common/util/TestZkMaintenanceUtils.java
rename to solr/solrj/src/test/org/apache/solr/common/cloud/TestZkMaintenanceUtils.java
index d914382..3f6b6d7 100644
--- a/solr/solrj/src/test/org/apache/solr/common/util/TestZkMaintenanceUtils.java
+++ b/solr/solrj/src/test/org/apache/solr/common/cloud/TestZkMaintenanceUtils.java
@@ -14,9 +14,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.solr.common.util;
+package org.apache.solr.common.cloud;
 
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -24,9 +26,11 @@ import java.util.List;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.cloud.ZkTestServer;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkMaintenanceUtils;
+import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -57,6 +61,45 @@ public class TestZkMaintenanceUtils extends SolrTestCaseJ4 {
     }
   }
 
+  /**
+   * This test reproduces the issue of trying to delete zk-nodes that have the same length. (SOLR-14961).
+   *
+   * @throws InterruptedException when having trouble creating test nodes
+   * @throws KeeperException error when talking to zookeeper
+   * @throws SolrServerException when having trouble connecting to solr
+   * @throws UnsupportedEncodingException when getBytes() uses unknown encoding
+   *
+   */
+  @Test
+  public void testClean() throws KeeperException, InterruptedException, SolrServerException, UnsupportedEncodingException {
+    try(SolrZkClient zkClient = new SolrZkClient(zkServer.getZkHost(), 10000)){
+      /* PREPARE */
+      String path = "/myPath/isTheBest";
+      String data1 = "myStringData1";
+      String data2 = "myStringData2";
+      String longData = "myLongStringData";
+      // create zk nodes that have the same path length
+      zkClient.create("/myPath", null, CreateMode.PERSISTENT, true);
+      zkClient.create(path, null, CreateMode.PERSISTENT, true);
+      zkClient.create(path +"/file1.txt", data1.getBytes(StandardCharsets.UTF_8), CreateMode.PERSISTENT, true);
+      zkClient.create(path +"/nothing.txt", null, CreateMode.PERSISTENT, true);
+      zkClient.create(path +"/file2.txt", data2.getBytes(StandardCharsets.UTF_8), CreateMode.PERSISTENT, true);
+      zkClient.create(path +"/some_longer_file2.txt", longData.getBytes(StandardCharsets.UTF_8), CreateMode.PERSISTENT, true);
+
+      /* RUN */
+      // delete all nodes that contain "file"
+      ZkMaintenanceUtils.clean(zkClient,path, node -> node.contains("file"));
+
+      /* CHECK */
+      String listZnode = zkClient.listZnode(path, false);
+      // list of node must not contain file1, file2 or some_longer_file2 because they where deleted
+      assertFalse(listZnode.contains("file1"));
+      assertFalse(listZnode.contains("file2"));
+      assertFalse(listZnode.contains("some_longer_file2"));
+      assertTrue(listZnode.contains("nothing"));
+    }
+  }
+
   @Test
   public void testPaths() {
     assertEquals("Unexpected path construction"