You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2020/07/17 20:23:28 UTC
[lucene-solr] branch branch_8x updated: SOLR-14653: Replace usages
of org.apache.hadoop.fs.Path that are used to construct Zookeeper paths
This is an automated email from the ASF dual-hosted git repository.
erick 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 121db63 SOLR-14653: Replace usages of org.apache.hadoop.fs.Path that are used to construct Zookeeper paths
121db63 is described below
commit 121db63ae99051a22db955e07c7de9c185742de6
Author: Erick Erickson <Er...@gmail.com>
AuthorDate: Fri Jul 17 16:18:21 2020 -0400
SOLR-14653: Replace usages of org.apache.hadoop.fs.Path that are used to construct Zookeeper paths
---
.../solr/cloud/ShardLeaderElectionContextBase.java | 13 ++---
.../solr/handler/admin/ConfigSetsHandler.java | 3 +-
.../collections/TestLocalFSCloudBackupRestore.java | 3 +-
.../solr/common/cloud/ZkMaintenanceUtils.java | 31 +++++++++---
.../solr/common/util/TestZkMaintenanceUtils.java | 55 ++++++++++++++++++++++
5 files changed, 87 insertions(+), 18 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContextBase.java b/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContextBase.java
index c97e4cf..500776f 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContextBase.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContextBase.java
@@ -22,14 +22,13 @@ import java.lang.invoke.MethodHandles;
import java.util.List;
import java.util.ArrayList;
-// DO NOT SUBSTITUTE java nio Path here, see SOLR-13939
-import org.apache.hadoop.fs.Path;
import org.apache.solr.cloud.overseer.OverseerAction;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.cloud.ZkCmdExecutor;
+import org.apache.solr.common.cloud.ZkMaintenanceUtils;
import org.apache.solr.common.cloud.ZkNodeProps;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.util.RetryUtil;
@@ -72,8 +71,7 @@ class ShardLeaderElectionContextBase extends ElectionContext {
this.shardId = shardId;
this.collection = collection;
- // Fails on Windows if you use nio Paths.get here
- String parent = new Path(leaderPath).getParent().toString();
+ String parent = ZkMaintenanceUtils.getZkParent(leaderPath);
ZkCmdExecutor zcmd = new ZkCmdExecutor(30000);
// only if /collections/{collection} exists already do we succeed in creating this path
log.info("make sure parent is created {}", parent);
@@ -101,8 +99,8 @@ class ShardLeaderElectionContextBase extends ElectionContext {
// version whenever a leader registers.
log.debug("Removing leader registration node on cancel: {} {}", leaderPath, leaderZkNodeParentVersion);
List<Op> ops = new ArrayList<>(2);
- // Fails on Windows if you use nio Paths.get here
- ops.add(Op.check(new Path(leaderPath).getParent().toString(), leaderZkNodeParentVersion));
+ String parent = ZkMaintenanceUtils.getZkParent(leaderPath);
+ ops.add(Op.check(parent, leaderZkNodeParentVersion));
ops.add(Op.delete(leaderPath, -1));
zkClient.multi(ops, true);
} catch (InterruptedException e) {
@@ -122,8 +120,7 @@ class ShardLeaderElectionContextBase extends ElectionContext {
throws KeeperException, InterruptedException, IOException {
// register as leader - if an ephemeral is already there, wait to see if it goes away
- // Fails on Windows if you use nio Paths.get here
- String parent = new Path(leaderPath).getParent().toString();
+ String parent = ZkMaintenanceUtils.getZkParent(leaderPath);
try {
RetryUtil.retryOnThrowable(NodeExistsException.class, 60000, 5000, () -> {
synchronized (lock) {
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java
index 566a5d8..728bf9f 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java
@@ -29,7 +29,6 @@ import java.util.zip.ZipInputStream;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
-import org.apache.hadoop.fs.Path;
import org.apache.solr.api.Api;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.cloud.OverseerSolrResponse;
@@ -153,7 +152,7 @@ public class ConfigSetsHandler extends RequestHandlerBase implements PermissionN
}
SolrZkClient zkClient = coreContainer.getZkController().getZkClient();
- String configPathInZk = ZkConfigManager.CONFIGS_ZKNODE + Path.SEPARATOR + configSetName;
+ String configPathInZk = ZkConfigManager.CONFIGS_ZKNODE + "/" + configSetName;
if (zkClient.exists(configPathInZk, true)) {
throw new SolrException(ErrorCode.BAD_REQUEST,
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestLocalFSCloudBackupRestore.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestLocalFSCloudBackupRestore.java
index d35b072..b74080a 100644
--- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestLocalFSCloudBackupRestore.java
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestLocalFSCloudBackupRestore.java
@@ -20,7 +20,6 @@ import java.io.IOException;
import java.io.OutputStream;
import java.net.URI;
-import org.apache.hadoop.fs.Path;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexInput;
@@ -63,7 +62,7 @@ public class TestLocalFSCloudBackupRestore extends AbstractCloudBackupRestoreTes
.addConfig("confFaulty", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
.withSolrXml(solrXml)
.configure();
- cluster.getZkClient().delete(ZkConfigManager.CONFIGS_ZKNODE + Path.SEPARATOR + "confFaulty" + Path.SEPARATOR + "solrconfig.xml", -1, true);
+ cluster.getZkClient().delete(ZkConfigManager.CONFIGS_ZKNODE + "/" + "confFaulty" + "/" + "solrconfig.xml", -1, true);
boolean whitespacesInPath = random().nextBoolean();
if (whitespacesInPath) {
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 915547e..3125f28 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
@@ -33,6 +33,7 @@ import java.util.TreeSet;
import java.util.function.Predicate;
import java.util.regex.Pattern;
+import org.apache.commons.lang.StringUtils;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.data.Stat;
@@ -48,6 +49,7 @@ public class ZkMaintenanceUtils {
private static final String ZKNODE_DATA_FILE = "zknode.data";
private ZkMaintenanceUtils() {} // don't let it be instantiated, all methods are static.
+
/**
* Lists a ZNode child and (optionally) the znodes of all the children. No data is dumped.
*
@@ -69,7 +71,7 @@ public class ZkMaintenanceUtils {
}
StringBuilder sb = new StringBuilder();
-
+
if (recurse == false) {
for (String node : zkClient.getChildren(root, null, true)) {
if (node.equals("zookeeper") == false) {
@@ -78,7 +80,7 @@ public class ZkMaintenanceUtils {
}
return sb.toString();
}
-
+
traverseZkTree(zkClient, root, VISIT_ORDER.VISIT_PRE, znode -> {
if (znode.startsWith("/zookeeper")) return; // can't do anything with this node!
int iPos = znode.lastIndexOf("/");
@@ -105,8 +107,8 @@ public class ZkMaintenanceUtils {
* @throws InterruptedException Thread interrupted
*/
public static void zkTransfer(SolrZkClient zkClient, String src, Boolean srcIsZk,
- String dst, Boolean dstIsZk,
- Boolean recurse) throws SolrServerException, KeeperException, InterruptedException, IOException {
+ String dst, Boolean dstIsZk,
+ Boolean recurse) throws SolrServerException, KeeperException, InterruptedException, IOException {
if (srcIsZk == false && dstIsZk == false) {
throw new SolrServerException("One or both of source or destination must specify ZK nodes.");
@@ -274,7 +276,7 @@ public class ZkMaintenanceUtils {
}
}
}
-
+
public static void uploadToZK(SolrZkClient zkClient, final Path fromPath, final String zkPath,
final Pattern filenameExclusions) throws IOException {
@@ -284,7 +286,7 @@ public class ZkMaintenanceUtils {
}
final Path rootPath = Paths.get(path);
-
+
if (!Files.exists(rootPath))
throw new IOException("Path " + rootPath + " does not exist");
@@ -418,7 +420,24 @@ public class ZkMaintenanceUtils {
}
}
+ // Get the parent path. This is really just the string before the last slash (/)
+ // Will return empty string if there are no slashes.
+ // Will return empty string if the path is just "/"
+ // Will return empty string if the path is just ""
+ public static String getZkParent(String path) {
+ // Remove trailing slash if present.
+ if (StringUtils.endsWith(path, "/")) {
+ path = StringUtils.substringBeforeLast(path, "/");
+ }
+ if (StringUtils.contains(path, "/") == false) {
+ return "";
+ }
+ return (StringUtils.substringBeforeLast(path, "/"));
+ }
+
// Take into account Windows file separators when making a Znode's name.
+ // Used particularly when uploading configsets since the path we're copying
+ // up may be a file path.
public static String createZkNodeName(String zkRoot, Path root, Path file) {
String relativePath = root.relativize(file).toString();
// Windows shenanigans
diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestZkMaintenanceUtils.java b/solr/solrj/src/test/org/apache/solr/common/util/TestZkMaintenanceUtils.java
new file mode 100644
index 0000000..94e0465
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/common/util/TestZkMaintenanceUtils.java
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.util;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.cloud.ZkMaintenanceUtils;
+import org.junit.Test;
+
+public class TestZkMaintenanceUtils extends SolrTestCaseJ4 {
+ @Test
+ public void testPaths() {
+ assertEquals("Unexpected path construction"
+ , ""
+ , ZkMaintenanceUtils.getZkParent(null));
+
+ assertEquals("Unexpected path construction"
+ , "this/is/a"
+ , ZkMaintenanceUtils.getZkParent("this/is/a/path"));
+
+ assertEquals("Unexpected path construction"
+ , "/root"
+ , ZkMaintenanceUtils.getZkParent("/root/path/"));
+
+ assertEquals("Unexpected path construction"
+ , ""
+ , ZkMaintenanceUtils.getZkParent("/"));
+
+ assertEquals("Unexpected path construction"
+ , ""
+ , ZkMaintenanceUtils.getZkParent(""));
+
+ assertEquals("Unexpected path construction"
+ , ""
+ , ZkMaintenanceUtils.getZkParent("noslashesinstring"));
+
+ assertEquals("Unexpected path construction"
+ , ""
+ , ZkMaintenanceUtils.getZkParent("/leadingslashonly"));
+
+ }
+}