From e7f0294d85dbdc3d3499245c29bc269774c574e2 Mon Sep 17 00:00:00 2001 From: Michael Aleythe Date: Wed, 28 Oct 2020 14:46:53 +0100 Subject: [PATCH] SOLR-14961 ZkMaintenanceUtils.clean doesn't remove zk nodes with same length fixes #2042 --- solr/CHANGES.txt | 2 + .../solr/common/cloud/ZkMaintenanceUtils.java | 7 ++- .../solr/common/cloud/SolrZkClientTest.java | 1 + .../TestZkMaintenanceUtils.java | 45 ++++++++++++++++++- 4 files changed, 52 insertions(+), 3 deletions(-) rename solr/solrj/src/test/org/apache/solr/common/{util => cloud}/TestZkMaintenanceUtils.java (65%) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 8dc504afd78..361433f6d6d 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -173,6 +173,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 3125f28fad7..e3e61a44207 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 paths = new TreeSet<>(Comparator.comparingInt(String::length).reversed()); + ArrayList 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 34b8e0978d3..80257977c77 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 @@ -235,4 +235,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 d914382556e..3f6b6d7e9c0 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"