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

@@ -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

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) {

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) {
+  public CorePropertiesLocator(Path coreDiscoveryRoot) {
-    this.rootDirectory = new File(coreDiscoveryRoot);
+    this.rootDirectory = coreDiscoveryRoot;
-    logger.info("Config-defined core root directory: {}", this.rootDirectory.getAbsolutePath());
+    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);
+      Path propertiesFile = this.rootDirectory.resolve(cd.getInstanceDir()).resolve(PROPERTIES_FILENAME);
-      if (propFile.exists())
+      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();
+      Files.createDirectories(propfile.getParent());
-      os = new OutputStreamWriter(new FileOutputStream(propfile), StandardCharsets.UTF_8);
+      try (Writer os = new OutputStreamWriter(Files.newOutputStream(propfile), StandardCharsets.UTF_8)) {
-      p.store(os, "Written by CorePropertiesLocator");
+        p.store(os, "Written by CorePropertiesLocator");
+      }
     }
     catch (IOException e) {
-      logger.error("Couldn't persist core properties to {}: {}", propfile.getAbsolutePath(), e);
+      logger.error("Couldn't persist core properties to {}: {}", propfile, e.getMessage());
-    }
-    finally {
-      IOUtils.closeQuietly(os);
     }
   }
 
@@ -98,12 +96,12 @@ public class CorePropertiesLocator implements CoresLocator {
     }
     for (CoreDescriptor cd : coreDescriptors) {
       if (cd == null) continue;
-      File instanceDir = new File(cd.getInstanceDir());
+      Path propfile = this.rootDirectory.resolve(cd.getInstanceDir()).resolve(PROPERTIES_FILENAME);
-      File propertiesFile = new File(instanceDir, PROPERTIES_FILENAME);
+      try {
-      propertiesFile.renameTo(new File(instanceDir, PROPERTIES_FILENAME + ".unloaded"));
+        Files.deleteIfExists(propfile);
-      // This is a best-effort: the core.properties file may already have been
+      } catch (IOException e) {
-      // deleted by the core unload, so we don't worry about checking if the
+        logger.warn("Couldn't delete core properties file {}: {}", propfile, e.getMessage());
-      // rename has succeeded.
+      }
     }
   }
 
@@ -118,56 +116,59 @@ public class CorePropertiesLocator implements CoresLocator {
   }
 
   @Override
-  public List<CoreDescriptor> discover(CoreContainer cc) {
+  public List<CoreDescriptor> discover(final CoreContainer cc) {
-    logger.info("Looking for core definitions underneath {}", rootDirectory.getAbsolutePath());
+    logger.info("Looking for core definitions underneath {}", rootDirectory);
-    List<CoreDescriptor> cds = Lists.newArrayList();
+    final List<CoreDescriptor> cds = Lists.newArrayList();
-    if (rootDirectory.canRead() == false) {
+    try {
-      throw new RuntimeException("Solr home '" + rootDirectory.getAbsolutePath() + "' doesn't have read permissions");
+      Files.walkFileTree(this.rootDirectory, new SimpleFileVisitor<Path>() {
+        @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<CoreDescriptor> cds, CoreContainer cc) {
+  protected CoreDescriptor buildCoreDescriptor(Path propertiesFile, 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(File propertiesFile, CoreContainer cc) {
+    Path instanceDir = propertiesFile.getParent();
-    FileInputStream fis = null;
+    Properties coreProperties = new Properties();
-    try {
+    try (InputStream fis = Files.newInputStream(propertiesFile)) {
-      File instanceDir = propertiesFile.getParentFile();
-      Properties coreProperties = new Properties();
-      fis = new FileInputStream(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) {
+  protected static String createName(Properties p, Path instanceDir) {
-    return p.getProperty(CoreDescriptor.CORE_NAME, instanceDir.getName());
+    return p.getProperty(CoreDescriptor.CORE_NAME, instanceDir.getFileName().toString());
   }
 
   protected Properties buildCoreProperties(CoreDescriptor cd) {

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();
+      assertThat(ex.getMessage(), containsString("Error reading core root directory"));
-
-      assertTrue("Should have had a runtime exception here",
-          0 < ex.getMessage().indexOf("doesn't have read permissions"));
     } finally {
       if (cc != null) {
         cc.shutdown();

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.

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.*;
-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.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())));
   }
 
   /**