From be10d9e501f791a709ff70ac90f89595bb574179 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 9 Nov 2015 17:50:19 +0000 Subject: [PATCH] SOLR-8260: Use nio2 API in core discovery git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1713490 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 2 + .../org/apache/solr/core/CoreContainer.java | 16 +- .../solr/core/CorePropertiesLocator.java | 139 +++++++++--------- .../apache/solr/core/TestCoreDiscovery.java | 22 ++- .../org/apache/solr/core/TestLazyCores.java | 23 +-- .../org/apache/solr/util/TestHarness.java | 30 ++-- 6 files changed, 116 insertions(+), 116 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 100967cc01b..a411b9a8e87 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -466,6 +466,8 @@ Other Changes * SOLR-8253: AbstractDistribZkTestBase can sometimes fail to shut down its ZKServer (Alan Woodward) +* SOLR-8260: Use NIO2 APIs in core discovery (Alan Woodward) + ================== 5.3.1 ================== Bug Fixes diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 9d20e1ed6a8..8e1bb56b38c 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -19,6 +19,7 @@ package org.apache.solr.core; import java.io.File; import java.io.IOException; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -33,7 +34,6 @@ import java.util.concurrent.Future; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; - import org.apache.solr.client.solrj.impl.HttpClientConfigurer; import org.apache.solr.client.solrj.impl.HttpClientUtil; import org.apache.solr.cloud.ZkController; @@ -55,8 +55,8 @@ import org.apache.solr.handler.component.ShardHandlerFactory; import org.apache.solr.logging.LogWatcher; import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.request.SolrRequestHandler; -import org.apache.solr.security.AuthorizationPlugin; import org.apache.solr.security.AuthenticationPlugin; +import org.apache.solr.security.AuthorizationPlugin; import org.apache.solr.security.HttpClientInterceptorPlugin; import org.apache.solr.security.PKIAuthenticationPlugin; import org.apache.solr.security.SecurityPluginHolder; @@ -69,7 +69,13 @@ import org.slf4j.LoggerFactory; import static com.google.common.base.Preconditions.checkNotNull; import static java.util.Collections.EMPTY_MAP; -import static org.apache.solr.common.params.CommonParams.*; +import static org.apache.solr.common.params.CommonParams.AUTHC_PATH; +import static org.apache.solr.common.params.CommonParams.AUTHZ_PATH; +import static org.apache.solr.common.params.CommonParams.COLLECTIONS_HANDLER_PATH; +import static org.apache.solr.common.params.CommonParams.CONFIGSETS_HANDLER_PATH; +import static org.apache.solr.common.params.CommonParams.CORES_HANDLER_PATH; +import static org.apache.solr.common.params.CommonParams.INFO_HANDLER_PATH; +import static org.apache.solr.common.params.CommonParams.ZK_PATH; import static org.apache.solr.security.AuthenticationPlugin.AUTHENTICATION_PLUGIN_PROP; @@ -198,11 +204,11 @@ public class CoreContainer { } public CoreContainer(NodeConfig config, Properties properties) { - this(config, properties, new CorePropertiesLocator(config.getCoreRootDirectory())); + this(config, properties, new CorePropertiesLocator(Paths.get(config.getCoreRootDirectory()))); } public CoreContainer(NodeConfig config, Properties properties, boolean asyncSolrCoreLoad) { - this(config, properties, new CorePropertiesLocator(config.getCoreRootDirectory()), asyncSolrCoreLoad); + this(config, properties, new CorePropertiesLocator(Paths.get(config.getCoreRootDirectory())), asyncSolrCoreLoad); } public CoreContainer(NodeConfig config, Properties properties, CoresLocator locator) { diff --git a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java index a25d99b8f00..d48b3c50701 100644 --- a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java +++ b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java @@ -17,24 +17,25 @@ package org.apache.solr.core; * limitations under the License. */ -import com.google.common.collect.Lists; - -import org.apache.solr.common.SolrException; -import org.apache.solr.common.util.IOUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStreamWriter; import java.io.Writer; import java.nio.charset.StandardCharsets; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; import java.util.List; import java.util.Properties; +import com.google.common.collect.Lists; +import org.apache.solr.common.SolrException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * Persists CoreDescriptors as properties files */ @@ -44,22 +45,22 @@ public class CorePropertiesLocator implements CoresLocator { private static final Logger logger = LoggerFactory.getLogger(CoresLocator.class); - private final File rootDirectory; + private final Path rootDirectory; - public CorePropertiesLocator(String coreDiscoveryRoot) { - this.rootDirectory = new File(coreDiscoveryRoot); - logger.info("Config-defined core root directory: {}", this.rootDirectory.getAbsolutePath()); + public CorePropertiesLocator(Path coreDiscoveryRoot) { + this.rootDirectory = coreDiscoveryRoot; + logger.info("Config-defined core root directory: {}", this.rootDirectory); } @Override public void create(CoreContainer cc, CoreDescriptor... coreDescriptors) { for (CoreDescriptor cd : coreDescriptors) { - File propFile = new File(new File(cd.getInstanceDir()), PROPERTIES_FILENAME); - if (propFile.exists()) + Path propertiesFile = this.rootDirectory.resolve(cd.getInstanceDir()).resolve(PROPERTIES_FILENAME); + if (Files.exists(propertiesFile)) throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Could not create a new core in " + cd.getInstanceDir() + "as another core is already defined there"); - writePropertiesFile(cd, propFile); + writePropertiesFile(cd, propertiesFile); } } @@ -70,24 +71,21 @@ public class CorePropertiesLocator implements CoresLocator { @Override public void persist(CoreContainer cc, CoreDescriptor... coreDescriptors) { for (CoreDescriptor cd : coreDescriptors) { - File propFile = new File(new File(cd.getInstanceDir()), PROPERTIES_FILENAME); + Path propFile = this.rootDirectory.resolve(cd.getInstanceDir()).resolve(PROPERTIES_FILENAME); writePropertiesFile(cd, propFile); } } - private void writePropertiesFile(CoreDescriptor cd, File propfile) { + private void writePropertiesFile(CoreDescriptor cd, Path propfile) { Properties p = buildCoreProperties(cd); - Writer os = null; try { - propfile.getParentFile().mkdirs(); - os = new OutputStreamWriter(new FileOutputStream(propfile), StandardCharsets.UTF_8); - p.store(os, "Written by CorePropertiesLocator"); + Files.createDirectories(propfile.getParent()); + try (Writer os = new OutputStreamWriter(Files.newOutputStream(propfile), StandardCharsets.UTF_8)) { + p.store(os, "Written by CorePropertiesLocator"); + } } catch (IOException e) { - logger.error("Couldn't persist core properties to {}: {}", propfile.getAbsolutePath(), e); - } - finally { - IOUtils.closeQuietly(os); + logger.error("Couldn't persist core properties to {}: {}", propfile, e.getMessage()); } } @@ -98,12 +96,12 @@ public class CorePropertiesLocator implements CoresLocator { } for (CoreDescriptor cd : coreDescriptors) { if (cd == null) continue; - File instanceDir = new File(cd.getInstanceDir()); - File propertiesFile = new File(instanceDir, PROPERTIES_FILENAME); - propertiesFile.renameTo(new File(instanceDir, PROPERTIES_FILENAME + ".unloaded")); - // This is a best-effort: the core.properties file may already have been - // deleted by the core unload, so we don't worry about checking if the - // rename has succeeded. + Path propfile = this.rootDirectory.resolve(cd.getInstanceDir()).resolve(PROPERTIES_FILENAME); + try { + Files.deleteIfExists(propfile); + } catch (IOException e) { + logger.warn("Couldn't delete core properties file {}: {}", propfile, e.getMessage()); + } } } @@ -118,56 +116,59 @@ public class CorePropertiesLocator implements CoresLocator { } @Override - public List discover(CoreContainer cc) { - logger.info("Looking for core definitions underneath {}", rootDirectory.getAbsolutePath()); - List cds = Lists.newArrayList(); - if (rootDirectory.canRead() == false) { - throw new RuntimeException("Solr home '" + rootDirectory.getAbsolutePath() + "' doesn't have read permissions"); + public List discover(final CoreContainer cc) { + logger.info("Looking for core definitions underneath {}", rootDirectory); + final List cds = Lists.newArrayList(); + try { + Files.walkFileTree(this.rootDirectory, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + if (file.getFileName().toString().equals(PROPERTIES_FILENAME)) { + CoreDescriptor cd = buildCoreDescriptor(file, cc); + logger.info("Found core {} in {}", cd.getName(), cd.getInstanceDir()); + cds.add(cd); + return FileVisitResult.SKIP_SIBLINGS; + } + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException { + // if we get an error on the root, then fail the whole thing + // otherwise, log a warning and continue to try and load other cores + if (file.equals(rootDirectory)) { + logger.error("Error reading core root directory {}: {}", file, exc); + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error reading core root directory"); + } + logger.warn("Error visiting {}: {}", file, exc); + return FileVisitResult.CONTINUE; + } + }); + } catch (IOException e) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Couldn't walk file tree under " + this.rootDirectory, e); } - discoverUnder(rootDirectory, cds, cc); logger.info("Found {} core definitions", cds.size()); return cds; } - private void discoverUnder(File root, List cds, CoreContainer cc) { - for (File child : root.listFiles()) { - if (child.canRead() == false) { - logger.warn("Cannot read directory or file during core discovery '" + child.getAbsolutePath() + "' during core discovery. Skipping"); - continue; - } - File propertiesFile = new File(child, PROPERTIES_FILENAME); - if (propertiesFile.exists()) { - CoreDescriptor cd = buildCoreDescriptor(propertiesFile, cc); - logger.info("Found core {} in {}", cd.getName(), cd.getInstanceDir()); - cds.add(cd); - continue; - } - if (child.isDirectory()) - discoverUnder(child, cds, cc); - } - } + protected CoreDescriptor buildCoreDescriptor(Path propertiesFile, CoreContainer cc) { - protected CoreDescriptor buildCoreDescriptor(File propertiesFile, CoreContainer cc) { - FileInputStream fis = null; - try { - File instanceDir = propertiesFile.getParentFile(); - Properties coreProperties = new Properties(); - fis = new FileInputStream(propertiesFile); + Path instanceDir = propertiesFile.getParent(); + Properties coreProperties = new Properties(); + try (InputStream fis = Files.newInputStream(propertiesFile)) { coreProperties.load(new InputStreamReader(fis, StandardCharsets.UTF_8)); String name = createName(coreProperties, instanceDir); - return new CoreDescriptor(cc, name, instanceDir.getAbsolutePath(), coreProperties); + return new CoreDescriptor(cc, name, instanceDir.toString(), coreProperties); } catch (IOException e) { - logger.error("Couldn't load core descriptor from {}:{}", propertiesFile.getAbsolutePath(), e.toString()); + logger.error("Couldn't load core descriptor from {}:{}", propertiesFile, e.toString()); return null; } - finally { - IOUtils.closeQuietly(fis); - } + } - protected static String createName(Properties p, File instanceDir) { - return p.getProperty(CoreDescriptor.CORE_NAME, instanceDir.getName()); + protected static String createName(Properties p, Path instanceDir) { + return p.getProperty(CoreDescriptor.CORE_NAME, instanceDir.getFileName().toString()); } protected Properties buildCoreProperties(CoreDescriptor cd) { diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java b/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java index 8a64eb87edc..31f82c26287 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java +++ b/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java @@ -17,14 +17,6 @@ package org.apache.solr.core; * limitations under the License. */ -import org.apache.commons.io.FileUtils; -import org.apache.lucene.util.IOUtils; -import org.apache.solr.SolrTestCaseJ4; -import org.apache.solr.common.SolrException; -import org.junit.After; -import org.junit.BeforeClass; -import org.junit.Test; - import java.io.File; import java.io.FileOutputStream; import java.io.OutputStreamWriter; @@ -33,6 +25,14 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Paths; import java.util.Properties; +import org.apache.commons.io.FileUtils; +import org.apache.lucene.util.IOUtils; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrException; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; + import static org.hamcrest.CoreMatchers.not; import static org.junit.internal.matchers.StringContains.containsString; @@ -429,11 +429,9 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 { CoreContainer cc = null; try { cc = init(); + fail("Should have thrown an exception here"); } catch (Exception ex) { - String eoe = ex.getMessage(); - - assertTrue("Should have had a runtime exception here", - 0 < ex.getMessage().indexOf("doesn't have read permissions")); + assertThat(ex.getMessage(), containsString("Error reading core root directory")); } finally { if (cc != null) { cc.shutdown(); diff --git a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java index 3ad6e4c2ad7..745f4bd4d0f 100644 --- a/solr/core/src/test/org/apache/solr/core/TestLazyCores.java +++ b/solr/core/src/test/org/apache/solr/core/TestLazyCores.java @@ -17,6 +17,17 @@ package org.apache.solr.core; * limitations under the License. */ +import java.io.File; +import java.io.IOException; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Pattern; + import com.google.common.collect.ImmutableList; import org.apache.commons.codec.Charsets; import org.apache.commons.io.FileUtils; @@ -35,16 +46,6 @@ import org.apache.solr.update.UpdateHandler; import org.apache.solr.util.ReadOnlyCoresLocator; import org.junit.Test; -import java.io.File; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.regex.Pattern; - public class TestLazyCores extends SolrTestCaseJ4 { private File solrHomeDirectory; @@ -567,7 +568,7 @@ public class TestLazyCores extends SolrTestCaseJ4 { NodeConfig config = SolrXmlConfig.fromFile(loader, solrXml); // OK this should succeed, but at the end we should have recorded a series of errors. - return createCoreContainer(config, new CorePropertiesLocator(config.getCoreRootDirectory())); + return createCoreContainer(config, new CorePropertiesLocator(Paths.get(config.getCoreRootDirectory()))); } // We want to see that the core "heals itself" if an un-corrupted file is written to the directory. diff --git a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java index b5b6d32126f..739039ee39e 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java +++ b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java @@ -17,22 +17,22 @@ package org.apache.solr.util; +import java.io.File; +import java.io.IOException; +import java.io.StringWriter; +import java.nio.file.Paths; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; + import com.google.common.collect.ImmutableList; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.NamedList.NamedListEntry; -import org.apache.solr.core.CloudConfig; -import org.apache.solr.core.CoreContainer; -import org.apache.solr.core.CoreDescriptor; -import org.apache.solr.core.CorePropertiesLocator; -import org.apache.solr.core.CoresLocator; -import org.apache.solr.core.NodeConfig; -import org.apache.solr.core.SolrConfig; -import org.apache.solr.core.SolrCore; -import org.apache.solr.core.SolrResourceLoader; -import org.apache.solr.core.SolrXmlConfig; +import org.apache.solr.core.*; import org.apache.solr.handler.UpdateRequestHandler; import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; @@ -45,14 +45,6 @@ import org.apache.solr.schema.IndexSchemaFactory; import org.apache.solr.servlet.DirectSolrConnection; import org.apache.solr.update.UpdateShardHandlerConfig; -import java.io.File; -import java.io.IOException; -import java.io.StringWriter; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Properties; - /** * This class provides a simple harness that may be useful when * writing testcases. @@ -162,7 +154,7 @@ public class TestHarness extends BaseTestHarness { } public TestHarness(NodeConfig nodeConfig) { - this(nodeConfig, new CorePropertiesLocator(nodeConfig.getCoreRootDirectory())); + this(nodeConfig, new CorePropertiesLocator(Paths.get(nodeConfig.getCoreRootDirectory()))); } /**