diff --git a/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java b/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java index 679cb07ac2e..648eee8d3cc 100644 --- a/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/BackupCmd.java @@ -62,7 +62,6 @@ public class BackupCmd implements OverseerCollectionMessageHandler.Cmd { ShardHandler shardHandler = ocmh.shardHandlerFactory.getShardHandler(); String asyncId = message.getStr(ASYNC); String repo = message.getStr(CoreAdminParams.BACKUP_REPOSITORY); - String location = message.getStr(CoreAdminParams.BACKUP_LOCATION); Map requestMap = new HashMap<>(); Instant startTime = Instant.now(); @@ -72,7 +71,8 @@ public class BackupCmd implements OverseerCollectionMessageHandler.Cmd { BackupManager backupMgr = new BackupManager(repository, ocmh.zkStateReader, collectionName); // Backup location - URI backupPath = repository.createURI(location, backupName); + URI location = repository.createURI(message.getStr(CoreAdminParams.BACKUP_LOCATION)); + URI backupPath = repository.resolve(location, backupName); //Validating if the directory already exists. if (repository.exists(backupPath)) { @@ -94,7 +94,7 @@ public class BackupCmd implements OverseerCollectionMessageHandler.Cmd { params.set(CoreAdminParams.ACTION, CoreAdminParams.CoreAdminAction.BACKUPCORE.toString()); params.set(NAME, slice.getName()); params.set(CoreAdminParams.BACKUP_REPOSITORY, repo); - params.set(CoreAdminParams.BACKUP_LOCATION, backupPath.getPath()); // note: index dir will be here then the "snapshot." + slice name + params.set(CoreAdminParams.BACKUP_LOCATION, backupPath.toASCIIString()); // note: index dir will be here then the "snapshot." + slice name params.set(CORE_NAME_PROP, coreName); ocmh.sendShardRequest(replica.getNodeName(), params, shardHandler, asyncId, requestMap); diff --git a/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java b/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java index af2215c351a..63d56865700 100644 --- a/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/RestoreCmd.java @@ -79,13 +79,13 @@ public class RestoreCmd implements OverseerCollectionMessageHandler.Cmd { ShardHandler shardHandler = ocmh.shardHandlerFactory.getShardHandler(); String asyncId = message.getStr(ASYNC); String repo = message.getStr(CoreAdminParams.BACKUP_REPOSITORY); - String location = message.getStr(CoreAdminParams.BACKUP_LOCATION); Map requestMap = new HashMap<>(); CoreContainer cc = ocmh.overseer.getZkController().getCoreContainer(); BackupRepository repository = cc.newBackupRepository(Optional.ofNullable(repo)); - URI backupPath = repository.createURI(location, backupName); + URI location = repository.createURI(message.getStr(CoreAdminParams.BACKUP_LOCATION)); + URI backupPath = repository.resolve(location, backupName); ZkStateReader zkStateReader = ocmh.zkStateReader; BackupManager backupMgr = new BackupManager(repository, zkStateReader, restoreCollectionName); @@ -195,7 +195,7 @@ public class RestoreCmd implements OverseerCollectionMessageHandler.Cmd { ModifiableSolrParams params = new ModifiableSolrParams(); params.set(CoreAdminParams.ACTION, CoreAdminParams.CoreAdminAction.RESTORECORE.toString()); params.set(NAME, "snapshot." + slice.getName()); - params.set(CoreAdminParams.BACKUP_LOCATION, backupPath.getPath()); + params.set(CoreAdminParams.BACKUP_LOCATION, backupPath.toASCIIString()); params.set(CoreAdminParams.BACKUP_REPOSITORY, repo); ocmh.sliceCmd(clusterState, params, null, slice, shardHandler, asyncId, requestMap); diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java index 51227e86a77..e6505530e57 100644 --- a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java +++ b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java @@ -87,12 +87,12 @@ public class BackupManager { * @return the configuration parameters for the specified backup. * @throws IOException In case of errors. */ - public Properties readBackupProperties(String backupLoc, String backupId) throws IOException { + public Properties readBackupProperties(URI backupLoc, String backupId) throws IOException { Preconditions.checkNotNull(backupLoc); Preconditions.checkNotNull(backupId); // Backup location - URI backupPath = repository.createURI(backupLoc, backupId); + URI backupPath = repository.resolve(backupLoc, backupId); if (!repository.exists(backupPath)) { throw new SolrException(ErrorCode.SERVER_ERROR, "Couldn't restore since doesn't exist: " + backupPath); } @@ -113,8 +113,8 @@ public class BackupManager { * @param props The backup properties * @throws IOException in case of I/O error */ - public void writeBackupProperties(String backupLoc, String backupId, Properties props) throws IOException { - URI dest = repository.createURI(backupLoc, backupId, BACKUP_PROPS_FILE); + public void writeBackupProperties(URI backupLoc, String backupId, Properties props) throws IOException { + URI dest = repository.resolve(backupLoc, backupId, BACKUP_PROPS_FILE); try (Writer propsWriter = new OutputStreamWriter(repository.createOutput(dest), StandardCharsets.UTF_8)) { props.store(propsWriter, "Backup properties file"); } @@ -128,10 +128,10 @@ public class BackupManager { * @return the meta-data information for the backed-up collection. * @throws IOException in case of errors. */ - public DocCollection readCollectionState(String backupLoc, String backupId, String collectionName) throws IOException { + public DocCollection readCollectionState(URI backupLoc, String backupId, String collectionName) throws IOException { Preconditions.checkNotNull(collectionName); - URI zkStateDir = repository.createURI(backupLoc, backupId, ZK_STATE_DIR); + URI zkStateDir = repository.resolve(backupLoc, backupId, ZK_STATE_DIR); try (IndexInput is = repository.openInput(zkStateDir, COLLECTION_PROPS_FILE, IOContext.DEFAULT)) { byte[] arr = new byte[(int) is.length()]; // probably ok since the json file should be small. is.readBytes(arr, 0, (int) is.length()); @@ -149,9 +149,9 @@ public class BackupManager { * @param collectionState The collection meta-data to be stored. * @throws IOException in case of I/O errors. */ - public void writeCollectionState(String backupLoc, String backupId, String collectionName, + public void writeCollectionState(URI backupLoc, String backupId, String collectionName, DocCollection collectionState) throws IOException { - URI dest = repository.createURI(backupLoc, backupId, ZK_STATE_DIR, COLLECTION_PROPS_FILE); + URI dest = repository.resolve(backupLoc, backupId, ZK_STATE_DIR, COLLECTION_PROPS_FILE); try (OutputStream collectionStateOs = repository.createOutput(dest)) { collectionStateOs.write(Utils.toJSON(Collections.singletonMap(collectionName, collectionState))); } @@ -166,9 +166,9 @@ public class BackupManager { * @param targetConfigName The name of the config to be created. * @throws IOException in case of I/O errors. */ - public void uploadConfigDir(String backupLoc, String backupId, String sourceConfigName, String targetConfigName) + public void uploadConfigDir(URI backupLoc, String backupId, String sourceConfigName, String targetConfigName) throws IOException { - URI source = repository.createURI(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR, sourceConfigName); + URI source = repository.resolve(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR, sourceConfigName); String zkPath = ZkConfigManager.CONFIGS_ZKNODE + "/" + targetConfigName; uploadToZk(zkStateReader.getZkClient(), source, zkPath); } @@ -181,10 +181,10 @@ public class BackupManager { * @param configName The name of the config to be saved. * @throws IOException in case of I/O errors. */ - public void downloadConfigDir(String backupLoc, String backupId, String configName) throws IOException { - URI dest = repository.createURI(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR, configName); - repository.createDirectory(repository.createURI(backupLoc, backupId, ZK_STATE_DIR)); - repository.createDirectory(repository.createURI(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR)); + public void downloadConfigDir(URI backupLoc, String backupId, String configName) throws IOException { + URI dest = repository.resolve(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR, configName); + repository.createDirectory(repository.resolve(backupLoc, backupId, ZK_STATE_DIR)); + repository.createDirectory(repository.resolve(backupLoc, backupId, ZK_STATE_DIR, CONFIG_STATE_DIR)); repository.createDirectory(dest); downloadFromZK(zkStateReader.getZkClient(), ZkConfigManager.CONFIGS_ZKNODE + "/" + configName, dest); @@ -201,11 +201,11 @@ public class BackupManager { if (children.size() == 0) { log.info("Writing file {}", file); byte[] data = zkClient.getData(zkPath + "/" + file, null, null, true); - try (OutputStream os = repository.createOutput(repository.createURI(dir.getPath(), file))) { + try (OutputStream os = repository.createOutput(repository.resolve(dir, file))) { os.write(data); } } else { - downloadFromZK(zkClient, zkPath + "/" + file, repository.createURI(dir.getPath(), file)); + downloadFromZK(zkClient, zkPath + "/" + file, repository.resolve(dir, file)); } } } catch (KeeperException | InterruptedException e) { @@ -221,7 +221,7 @@ public class BackupManager { for (String file : repository.listAll(sourceDir)) { String zkNodePath = destZkPath + "/" + file; - URI path = repository.createURI(sourceDir.getPath(), file); + URI path = repository.resolve(sourceDir, file); PathType t = repository.getPathType(path); switch (t) { case FILE: { diff --git a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java b/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java index 8950ce7fa00..875be18559e 100644 --- a/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java +++ b/solr/core/src/java/org/apache/solr/core/backup/repository/BackupRepository.java @@ -57,13 +57,23 @@ public interface BackupRepository extends NamedListInitializedPlugin, Closeable T getConfigProperty(String name); /** - * This method creates a URI using the specified path components (as method arguments). + * This method returns the URI representation for the specified path. + * Note - the specified path could be a fully qualified URI OR a relative path for a file-system. * + * @param path The path specified by the user. + * @return the URI representation of the user supplied value + */ + URI createURI(String path); + + /** + * This method resolves a URI using the specified path components (as method arguments). + * + * @param baseUri The base URI to use for creating the path * @param pathComponents * The directory (or file-name) to be included in the URI. * @return A URI containing absolute path */ - URI createURI(String... pathComponents); + URI resolve(URI baseUri, String... pathComponents); /** * This method checks if the specified path exists in this repository. diff --git a/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java b/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java index bb148dea543..f12d9fdbb55 100644 --- a/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java +++ b/solr/core/src/java/org/apache/solr/core/backup/repository/HdfsBackupRepository.java @@ -20,6 +20,7 @@ package org.apache.solr.core.backup.repository; import java.io.IOException; import java.io.OutputStream; import java.net.URI; +import java.net.URISyntaxException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; @@ -88,11 +89,31 @@ public class HdfsBackupRepository implements BackupRepository { } @Override - public URI createURI(String... pathComponents) { - Path result = baseHdfsPath; - for (String p : pathComponents) { - result = new Path(result, p); + public URI createURI(String location) { + Preconditions.checkNotNull(location); + + URI result = null; + try { + result = new URI(location); + if (!result.isAbsolute()) { + result = resolve(this.baseHdfsPath.toUri(), location); + } + } catch (URISyntaxException ex) { + result = resolve(this.baseHdfsPath.toUri(), location); } + + return result; + } + + @Override + public URI resolve(URI baseUri, String... pathComponents) { + Preconditions.checkArgument(baseUri.isAbsolute()); + + Path result = new Path(baseUri); + for (String path : pathComponents) { + result = new Path(result, path); + } + return result.toUri(); } diff --git a/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java b/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java index 86c411059d8..4ac25588a69 100644 --- a/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java +++ b/solr/core/src/java/org/apache/solr/core/backup/repository/LocalFileSystemRepository.java @@ -20,19 +20,20 @@ package org.apache.solr.core.backup.repository; import java.io.IOException; import java.io.OutputStream; import java.net.URI; +import java.net.URISyntaxException; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; + import org.apache.lucene.store.Directory; import org.apache.lucene.store.FSDirectory; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.NoLockFactory; import org.apache.lucene.store.SimpleFSDirectory; -import org.apache.lucene.util.Constants; import org.apache.solr.common.util.NamedList; import org.apache.solr.core.DirectoryFactory; @@ -58,21 +59,28 @@ public class LocalFileSystemRepository implements BackupRepository { } @Override - public URI createURI(String... pathComponents) { - Preconditions.checkArgument(pathComponents.length > 0); + public URI createURI(String location) { + Preconditions.checkNotNull(location); - String basePath = Preconditions.checkNotNull(pathComponents[0]); - // Note the URI.getPath() invocation on Windows platform generates an invalid URI. - // Refer to http://stackoverflow.com/questions/9834776/java-nio-file-path-issue - // Since the caller may have used this method to generate the string representation - // for the pathComponents, we implement a work-around specifically for Windows platform - // to remove the leading '/' character. - if (Constants.WINDOWS) { - basePath = basePath.replaceFirst("^/(.:/)", "$1"); + URI result = null; + try { + result = new URI(location); + if (!result.isAbsolute()) { + result = Paths.get(location).toUri(); + } + } catch (URISyntaxException ex) { + result = Paths.get(location).toUri(); } - Path result = Paths.get(basePath); - for (int i = 1; i < pathComponents.length; i++) { + return result; + } + + @Override + public URI resolve(URI baseUri, String... pathComponents) { + Preconditions.checkArgument(pathComponents.length > 0); + + Path result = Paths.get(baseUri); + for (int i = 0; i < pathComponents.length; i++) { result = result.resolve(pathComponents[i]); } diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java index aee3b972d58..84e1ba2d9b3 100644 --- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java @@ -443,14 +443,15 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw location = core.getDataDir(); } + URI locationUri = repo.createURI(location); + //If name is not provided then look for the last unnamed( the ones with the snapshot.timestamp format) //snapshot folder since we allow snapshots to be taken without providing a name. Pick the latest timestamp. if (name == null) { - URI basePath = repo.createURI(location); - String[] filePaths = repo.listAll(basePath); + String[] filePaths = repo.listAll(locationUri); List dirs = new ArrayList<>(); for (String f : filePaths) { - OldBackupDirectory obd = new OldBackupDirectory(basePath, f); + OldBackupDirectory obd = new OldBackupDirectory(locationUri, f); if (obd.getTimestamp().isPresent()) { dirs.add(obd); } @@ -465,7 +466,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw name = "snapshot." + name; } - RestoreCore restoreCore = new RestoreCore(repo, core, location, name); + RestoreCore restoreCore = new RestoreCore(repo, core, locationUri, name); try { MDC.put("RestoreCore.core", core.getName()); MDC.put("RestoreCore.backupLocation", location); @@ -561,7 +562,8 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw } // small race here before the commit point is saved - SnapShooter snapShooter = new SnapShooter(repo, core, location, params.get(NAME), commitName); + URI locationUri = repo.createURI(location); + SnapShooter snapShooter = new SnapShooter(repo, core, locationUri, params.get(NAME), commitName); snapShooter.validateCreateSnapshot(); snapShooter.createSnapAsync(indexCommit, numberToKeep, (nl) -> snapShootDetails = nl); diff --git a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java b/solr/core/src/java/org/apache/solr/handler/RestoreCore.java index 6aef35c448a..62cb93fa078 100644 --- a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java +++ b/solr/core/src/java/org/apache/solr/handler/RestoreCore.java @@ -44,11 +44,11 @@ public class RestoreCore implements Callable { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private final String backupName; - private final String backupLocation; + private final URI backupLocation; private final SolrCore core; private final BackupRepository backupRepo; - public RestoreCore(BackupRepository backupRepo, SolrCore core, String location, String name) { + public RestoreCore(BackupRepository backupRepo, SolrCore core, URI location, String name) { this.backupRepo = backupRepo; this.core = core; this.backupLocation = location; @@ -62,7 +62,7 @@ public class RestoreCore implements Callable { public boolean doRestore() throws Exception { - URI backupPath = backupRepo.createURI(backupLocation, backupName); + URI backupPath = backupRepo.resolve(backupLocation, backupName); SimpleDateFormat dateFormat = new SimpleDateFormat(SnapShooter.DATE_FMT, Locale.ROOT); String restoreIndexName = "restore." + dateFormat.format(new Date()); String restoreIndexPath = core.getDataDir() + restoreIndexName; diff --git a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java b/solr/core/src/java/org/apache/solr/handler/SnapShooter.java index e12649dcd91..52f4889f9e9 100644 --- a/solr/core/src/java/org/apache/solr/handler/SnapShooter.java +++ b/solr/core/src/java/org/apache/solr/handler/SnapShooter.java @@ -19,6 +19,7 @@ package org.apache.solr.handler; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.net.URI; +import java.nio.file.Paths; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Collection; @@ -75,17 +76,17 @@ public class SnapShooter { } else { snapDirStr = core.getCoreDescriptor().getInstanceDir().resolve(location).normalize().toString(); } - initialize(new LocalFileSystemRepository(), core, snapDirStr, snapshotName, null); + initialize(new LocalFileSystemRepository(), core, Paths.get(snapDirStr).toUri(), snapshotName, null); } - public SnapShooter(BackupRepository backupRepo, SolrCore core, String location, String snapshotName, String commitName) { + public SnapShooter(BackupRepository backupRepo, SolrCore core, URI location, String snapshotName, String commitName) { initialize(backupRepo, core, location, snapshotName, commitName); } - private void initialize(BackupRepository backupRepo, SolrCore core, String location, String snapshotName, String commitName) { + private void initialize(BackupRepository backupRepo, SolrCore core, URI location, String snapshotName, String commitName) { this.solrCore = Preconditions.checkNotNull(core); this.backupRepo = Preconditions.checkNotNull(backupRepo); - this.baseSnapDirPath = backupRepo.createURI(Preconditions.checkNotNull(location)).normalize(); + this.baseSnapDirPath = location; this.snapshotName = snapshotName; if (snapshotName != null) { directoryName = "snapshot." + snapshotName; @@ -93,7 +94,7 @@ public class SnapShooter { SimpleDateFormat fmt = new SimpleDateFormat(DATE_FMT, Locale.ROOT); directoryName = "snapshot." + fmt.format(new Date()); } - this.snapshotDirPath = backupRepo.createURI(location, directoryName); + this.snapshotDirPath = backupRepo.resolve(location, directoryName); this.commitName = commitName; } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java index e4103c54e17..dfc7a6f4e9f 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java @@ -18,6 +18,7 @@ package org.apache.solr.handler.admin; import java.io.IOException; import java.lang.invoke.MethodHandles; +import java.net.URI; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; @@ -803,8 +804,9 @@ enum CoreAdminOperation implements CoreAdminOp { // parameter is not supplied, the latest index commit is backed-up. String commitName = params.get(CoreAdminParams.COMMIT_NAME); + URI locationUri = repository.createURI(location); try (SolrCore core = it.handler.coreContainer.getCore(cname)) { - SnapShooter snapShooter = new SnapShooter(repository, core, location, name, commitName); + SnapShooter snapShooter = new SnapShooter(repository, core, locationUri, name, commitName); // validateCreateSnapshot will create parent dirs instead of throw; that choice is dubious. // But we want to throw. One reason is that // this dir really should, in fact must, already exist here if triggered via a collection backup on a shared @@ -847,8 +849,9 @@ enum CoreAdminOperation implements CoreAdminOp { + " parameter or as a default repository property"); } + URI locationUri = repository.createURI(location); try (SolrCore core = it.handler.coreContainer.getCore(cname)) { - RestoreCore restoreCore = new RestoreCore(repository, core, location, name); + RestoreCore restoreCore = new RestoreCore(repository, core, locationUri, name); boolean success = restoreCore.doRestore(); if (!success) { throw new SolrException(ErrorCode.SERVER_ERROR, "Failed to restore core=" + core.getName()); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java b/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java index db6891349f5..da8e767db02 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java @@ -24,12 +24,20 @@ import org.junit.BeforeClass; * such file-system would be exposed via local file-system API. */ public class TestLocalFSCloudBackupRestore extends AbstractCloudBackupRestoreTestCase { + private static String backupLocation; @BeforeClass public static void setupClass() throws Exception { configureCluster(NUM_SHARDS)// nodes .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf")) .configure(); + + boolean whitespacesInPath = random().nextBoolean(); + if (whitespacesInPath) { + backupLocation = createTempDir("my backup").toFile().getAbsolutePath(); + } else { + backupLocation = createTempDir("mybackup").toFile().getAbsolutePath(); + } } @Override @@ -44,6 +52,6 @@ public class TestLocalFSCloudBackupRestore extends AbstractCloudBackupRestoreTes @Override public String getBackupLocation() { - return createTempDir().toFile().getAbsolutePath(); + return backupLocation; } }