From d5a7ca79f3ac88d4de54c013eb6b29f72e52c907 Mon Sep 17 00:00:00 2001 From: Varun Thacker Date: Thu, 4 Aug 2016 22:13:05 +0530 Subject: [PATCH] SOLR-9242: fix windows path issue + load live cluster properties. This closes #62 --- .../repository/LocalFileSystemRepository.java | 15 ++++++++++++++- .../solr/handler/admin/CollectionsHandler.java | 8 ++------ .../solr/cloud/TestLocalFSCloudBackupRestore.java | 2 -- .../apache/solr/common/cloud/ZkStateReader.java | 8 -------- 4 files changed, 16 insertions(+), 17 deletions(-) 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 4eb77908c5a..86c411059d8 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 @@ -32,6 +32,7 @@ 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; @@ -59,10 +60,22 @@ public class LocalFileSystemRepository implements BackupRepository { @Override public URI createURI(String... pathComponents) { Preconditions.checkArgument(pathComponents.length > 0); - Path result = Paths.get(pathComponents[0]); + + 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"); + } + + Path result = Paths.get(basePath); for (int i = 1; i < pathComponents.length; i++) { result = result.resolve(pathComponents[i]); } + return result.toUri(); } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java index cb7279080ea..3ba5cb71222 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java @@ -714,10 +714,8 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission String location = repository.getBackupLocation(req.getParams().get(CoreAdminParams.BACKUP_LOCATION)); if (location == null) { //Refresh the cluster property file to make sure the value set for location is the latest - h.coreContainer.getZkController().getZkStateReader().forceUpdateClusterProperties(); - // Check if the location is specified in the cluster property. - location = h.coreContainer.getZkController().getZkStateReader().getClusterProperty(CoreAdminParams.BACKUP_LOCATION, null); + location = new ClusterProperties(h.coreContainer.getZkController().getZkClient()).getClusterProperty(CoreAdminParams.BACKUP_LOCATION, null); if (location == null) { throw new SolrException(ErrorCode.BAD_REQUEST, "'location' is not specified as a query" + " parameter or as a default repository property or as a cluster property."); @@ -755,10 +753,8 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission String location = repository.getBackupLocation(req.getParams().get(CoreAdminParams.BACKUP_LOCATION)); if (location == null) { //Refresh the cluster property file to make sure the value set for location is the latest - h.coreContainer.getZkController().getZkStateReader().forceUpdateClusterProperties(); - // Check if the location is specified in the cluster property. - location = h.coreContainer.getZkController().getZkStateReader().getClusterProperty("location", null); + location = new ClusterProperties(h.coreContainer.getZkController().getZkClient()).getClusterProperty("location", null); if (location == null) { throw new SolrException(ErrorCode.BAD_REQUEST, "'location' is not specified as a query" + " parameter or as a default repository property or as a cluster property."); 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 c6f6a04bfed..db6891349f5 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestLocalFSCloudBackupRestore.java @@ -16,7 +16,6 @@ */ package org.apache.solr.cloud; -import org.apache.lucene.util.Constants; import org.junit.BeforeClass; /** @@ -28,7 +27,6 @@ public class TestLocalFSCloudBackupRestore extends AbstractCloudBackupRestoreTes @BeforeClass public static void setupClass() throws Exception { - assumeFalse("Backup/Restore is currently buggy on Windows. Tracking the fix on SOLR-9242", Constants.WINDOWS); configureCluster(NUM_SHARDS)// nodes .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf")) .configure(); diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java index 9928346c22b..9df4a76aa8e 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java @@ -866,14 +866,6 @@ public class ZkStateReader implements Closeable { loadClusterProperties(); }; - /** - * We should try keeping this to a minimum. Only in scenarios where the value being read is a user facing property - * should we force update to make sure we are reading the latest value. - */ - public void forceUpdateClusterProperties() { - loadClusterProperties(); - } - @SuppressWarnings("unchecked") private void loadClusterProperties() { try {