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 2017/03/30 04:14:40 UTC
lucene-solr:master: SOLR-10387: zkTransfer normalizes destination
path incorrectly if source is a windows directory
Repository: lucene-solr
Updated Branches:
refs/heads/master efdb04d06 -> edcdc3052
SOLR-10387: zkTransfer normalizes destination path incorrectly if source is a windows directory
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/edcdc305
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/edcdc305
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/edcdc305
Branch: refs/heads/master
Commit: edcdc3052ba95840593ace32d6d9a7a6e4ebe7ea
Parents: efdb04d
Author: Erick Erickson <er...@apache.org>
Authored: Wed Mar 29 21:13:40 2017 -0700
Committer: Erick Erickson <er...@apache.org>
Committed: Wed Mar 29 21:13:40 2017 -0700
----------------------------------------------------------------------
solr/CHANGES.txt | 3 ++
.../apache/solr/cloud/SolrCLIZkUtilsTest.java | 13 ++++---
.../solr/common/cloud/ZkMaintenanceUtils.java | 41 +++++++++++---------
3 files changed, 32 insertions(+), 25 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/edcdc305/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3692bd5..840de48 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -148,6 +148,9 @@ Bug Fixes
* SOLR-10369: bin\solr.cmd delete and healthcheck now works again; fixed continuation chars ^ (Luis Goes via janhoy)
+* SOLR-10387: zkTransfer normalizes destination path incorrectly if source is a windows directory
+ (gopikannan venugopalsamy, Erick Erickson)
+
Other Changes
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/edcdc305/solr/core/src/test/org/apache/solr/cloud/SolrCLIZkUtilsTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/SolrCLIZkUtilsTest.java b/solr/core/src/test/org/apache/solr/cloud/SolrCLIZkUtilsTest.java
index 35ba1d4..776075e 100644
--- a/solr/core/src/test/org/apache/solr/cloud/SolrCLIZkUtilsTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/SolrCLIZkUtilsTest.java
@@ -18,6 +18,7 @@
package org.apache.solr.cloud;
import java.io.ByteArrayOutputStream;
+import java.io.File;
import java.io.IOException;
import java.io.PrintStream;
import java.nio.charset.Charset;
@@ -229,9 +230,9 @@ public class SolrCLIZkUtilsTest extends SolrCloudTestCase {
// NOTE: really can't test copying to '.' because the test framework doesn't allow altering the source tree
// and at least IntelliJ's CWD is in the source tree.
- // copy to local ending in '/'
+ // copy to local ending in separator
//src and cp3 and cp4 are valid
- String localSlash = tmp.normalize() + "/cpToLocal/";
+ String localSlash = tmp.normalize() + File.separator +"cpToLocal" + File.separator;
args = new String[]{
"-src", "zk:/cp3/schema.xml",
"-dst", localSlash,
@@ -246,7 +247,7 @@ public class SolrCLIZkUtilsTest extends SolrCloudTestCase {
// copy to ZK ending in '/'.
//src and cp3 are valid
args = new String[]{
- "-src", "file:" + srcPathCheck.normalize().toAbsolutePath().toString() + "/solrconfig.xml",
+ "-src", "file:" + srcPathCheck.normalize().toAbsolutePath().toString() + File.separator + "solrconfig.xml",
"-dst", "zk:/powerup/",
"-recurse", "false",
"-zkHost", zkAddr,
@@ -259,7 +260,7 @@ public class SolrCLIZkUtilsTest extends SolrCloudTestCase {
// copy individual file up
//src and cp3 are valid
args = new String[]{
- "-src", "file:" + srcPathCheck.normalize().toAbsolutePath().toString() + "/solrconfig.xml",
+ "-src", "file:" + srcPathCheck.normalize().toAbsolutePath().toString() + File.separator + "solrconfig.xml",
"-dst", "zk:/copyUpFile.xml",
"-recurse", "false",
"-zkHost", zkAddr,
@@ -272,7 +273,7 @@ public class SolrCLIZkUtilsTest extends SolrCloudTestCase {
// copy individual file down
//src and cp3 are valid
- String localNamed = tmp.normalize().toString() + "/localnamed/renamed.txt";
+ String localNamed = tmp.normalize().toString() + File.separator + "localnamed" + File.separator + "renamed.txt";
args = new String[]{
"-src", "zk:/cp4/solrconfig.xml",
"-dst", "file:" + localNamed,
@@ -404,7 +405,7 @@ public class SolrCLIZkUtilsTest extends SolrCloudTestCase {
// Files are in mv2
// Now fail if we specify "file:". Everything should still be in /mv2
args = new String[]{
- "-src", "file:/mv2",
+ "-src", "file:" + File.separator + "mv2",
"-dst", "/mv3",
"-zkHost", zkAddr,
};
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/edcdc305/solr/solrj/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
----------------------------------------------------------------------
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 f569ae3..dfdc8cf 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
@@ -119,33 +119,36 @@ public class ZkMaintenanceUtils {
throw new SolrServerException("Local path " + Paths.get(src).toAbsolutePath() + " is a directory and recurse is false");
}
}
- if (srcIsZk == false && dstIsZk == false) {
- throw new SolrServerException("At least one of the source and dest parameters must be prefixed with 'zk:' ");
- }
+
if (dstIsZk && dst.length() == 0) {
dst = "/"; // for consistency, one can copy from zk: and send to zk:/
}
- dst = normalizeDest(src, dst);
+ dst = normalizeDest(src, dst, srcIsZk, dstIsZk);
+ // ZK -> ZK copy.
if (srcIsZk && dstIsZk) {
traverseZkTree(zkClient, src, VISIT_ORDER.VISIT_PRE, new ZkCopier(zkClient, src, dst));
return;
}
+
+ //local -> ZK copy
if (dstIsZk) {
uploadToZK(zkClient, Paths.get(src), dst, null);
return;
}
- // Copying individual files from ZK requires special handling since downloadFromZK assumes it's a directory.
+ // Copying individual files from ZK requires special handling since downloadFromZK assumes the node has children.
// This is kind of a weak test for the notion of "directory" on Zookeeper.
+ // ZK -> local copy where ZK is a parent node
if (zkClient.getChildren(src, null, true).size() > 0) {
downloadFromZK(zkClient, src, Paths.get(dst));
return;
}
+ // Single file ZK -> local copy where ZK is a leaf node
if (Files.isDirectory(Paths.get(dst))) {
- if (dst.endsWith("/") == false) dst += "/";
- dst = normalizeDest(src, dst);
+ if (dst.endsWith(File.separator) == false) dst += File.separator;
+ dst = normalizeDest(src, dst, srcIsZk, dstIsZk);
}
byte[] data = zkClient.getData(src, null, null, true);
Path filename = Paths.get(dst);
@@ -154,31 +157,32 @@ public class ZkMaintenanceUtils {
Files.write(filename, data);
}
-
- private static String normalizeDest(String srcName, String dstName) {
+ // If the dest ends with a separator, it's a directory or non-leaf znode, so return the
+ // last element of the src to appended to the dstName.
+ private static String normalizeDest(String srcName, String dstName, boolean srcIsZk, boolean dstIsZk) {
// Special handling for "."
if (dstName.equals(".")) {
return Paths.get(".").normalize().toAbsolutePath().toString();
}
- // Pull the last element of the src path and add it to the dst if the src does NOT end in a slash
- // If the source ends in a slash, do not append the last segment to the dest
-
- if (dstName.endsWith("/")) { // Dest is a directory.
- int pos = srcName.lastIndexOf("/");
+ String dstSeparator = (dstIsZk) ? "/" : File.separator;
+ String srcSeparator = (srcIsZk) ? "/" : File.separator;
+
+ if (dstName.endsWith(dstSeparator)) { // Dest is a directory or non-leaf znode, append last element of the src path.
+ int pos = srcName.lastIndexOf(srcSeparator);
if (pos < 0) {
dstName += srcName;
} else {
dstName += srcName.substring(pos + 1);
}
}
-
+
log.info("copying from '{}' to '{}'", srcName, dstName);
return dstName;
}
public static void moveZnode(SolrZkClient zkClient, String src, String dst) throws SolrServerException, KeeperException, InterruptedException {
- String destName = normalizeDest(src, dst);
+ String destName = normalizeDest(src, dst, true, true);
// Special handling if the source has no children, i.e. copying just a single file.
if (zkClient.getChildren(src, null, true).size() == 0) {
@@ -384,12 +388,11 @@ public class ZkMaintenanceUtils {
}
}
- // Take into account Windows file separaters when making a Znode's name.
+ // Take into account Windows file separators when making a Znode's name.
public static String createZkNodeName(String zkRoot, Path root, Path file) {
String relativePath = root.relativize(file).toString();
// Windows shenanigans
- String separator = root.getFileSystem().getSeparator();
- if ("\\".equals(separator))
+ if ("\\".equals(File.separator))
relativePath = relativePath.replaceAll("\\\\", "/");
// It's possible that the relative path and file are the same, in which case
// adding the bare slash is A Bad Idea unless it's a non-leaf data node