SOLR-14961 ZkMaintenanceUtils.clean doesn't remove zk nodes with same length

fixes #2042

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
 ---------------------
 

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 {

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());
   }
+
 }

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"