SOLR-14289 Skip ZkChroot check when not necessary (#1298)

solr/core/src/java/org/apache/solr/core/NodeConfig.java

@@ -80,6 +80,10 @@ public class NodeConfig {
 
   private final PluginInfo tracerConfig;
 
+  // Track if this config was loaded from zookeeper so that we can skip validating the zookeeper connection later
+  // If it becomes necessary to track multiple potential sources in the future, replace this with an Enum
+  private final boolean fromZookeeper;
+
   private NodeConfig(String nodeName, Path coreRootDirectory, Path solrDataHome, Integer booleanQueryMaxClauseCount,
                      Path configSetBaseDirectory, String sharedLibDirectory,
                      PluginInfo shardHandlerFactoryConfig, UpdateShardHandlerConfig updateShardHandlerConfig,
@@ -89,7 +93,8 @@ public class NodeConfig {
                      int transientCacheSize, boolean useSchemaCache, String managementPath,
                      Path solrHome, SolrResourceLoader loader,
                      Properties solrProperties, PluginInfo[] backupRepositoryPlugins,
-                     MetricsConfig metricsConfig, PluginInfo transientCacheConfig, PluginInfo tracerConfig) {
+                     MetricsConfig metricsConfig, PluginInfo transientCacheConfig, PluginInfo tracerConfig,
+                     boolean fromZookeeper) {
     this.nodeName = nodeName;
     this.coreRootDirectory = coreRootDirectory;
     this.solrDataHome = solrDataHome;
@@ -117,6 +122,7 @@ public class NodeConfig {
     this.metricsConfig = metricsConfig;
     this.transientCacheConfig = transientCacheConfig;
     this.tracerConfig = tracerConfig;
+    this.fromZookeeper = fromZookeeper;
 
     if (this.cloudConfig != null && this.getCoreLoadThreadCount(false) < 2) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
@@ -248,6 +254,10 @@ public class NodeConfig {
     return tracerConfig;
   }
 
+  public boolean isFromZookeeper() {
+    return fromZookeeper;
+  }
+
   public static class NodeConfigBuilder {
 
     private SolrResourceLoader loader;
@@ -277,6 +287,7 @@ public class NodeConfig {
     private MetricsConfig metricsConfig;
     private PluginInfo transientCacheConfig;
     private PluginInfo tracerConfig;
+    private boolean fromZookeeper = false;
 
     private final Path solrHome;
     private final String nodeName;
@@ -439,6 +450,11 @@ public class NodeConfig {
       return this;
     }
 
+    public NodeConfigBuilder setFromZookeeper(boolean fromZookeeper) {
+      this.fromZookeeper = fromZookeeper;
+      return this;
+    }
+
     public NodeConfig build() {
       // if some things weren't set then set them now.  Simple primitives are set on the field declaration
       if (loader == null) {
@@ -449,7 +465,7 @@ public class NodeConfig {
                             updateShardHandlerConfig, coreAdminHandlerClass, collectionsAdminHandlerClass, healthCheckHandlerClass, infoHandlerClass, configSetsHandlerClass,
                             logWatcherConfig, cloudConfig, coreLoadThreads, replayUpdatesThreads, transientCacheSize, useSchemaCache, managementPath,
                             solrHome, loader, solrProperties,
-                            backupRepositoryPlugins, metricsConfig, transientCacheConfig, tracerConfig);
+                            backupRepositoryPlugins, metricsConfig, transientCacheConfig, tracerConfig, fromZookeeper);
     }
 
     public NodeConfigBuilder setSolrResourceLoader(SolrResourceLoader resourceLoader) {

solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java

@@ -67,7 +67,7 @@ public class SolrXmlConfig {
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  public static NodeConfig fromConfig(Path solrHome, XmlConfigFile config) {
+  public static NodeConfig fromConfig(Path solrHome, XmlConfigFile config, boolean fromZookeeper) {
 
     checkForIllegalConfig(config);
 
@@ -109,6 +109,7 @@ public class SolrXmlConfig {
       configBuilder.setCloudConfig(cloudConfig);
     configBuilder.setBackupRepositoryPlugins(getBackupRepositoryPluginInfos(config));
     configBuilder.setMetricsConfig(getMetricsConfig(config));
+    configBuilder.setFromZookeeper(fromZookeeper);
     return fillSolrSection(configBuilder, entries);
   }
 
@@ -140,6 +141,10 @@ public class SolrXmlConfig {
   }
 
   public static NodeConfig fromInputStream(Path solrHome, InputStream is, Properties substituteProps) {
+    return fromInputStream(solrHome, is, substituteProps, false);
+  }
+
+  public static NodeConfig fromInputStream(Path solrHome, InputStream is, Properties substituteProps, boolean fromZookeeper) {
     SolrResourceLoader loader = new SolrResourceLoader(solrHome);
     if (substituteProps == null) {
       substituteProps = new Properties();
@@ -148,7 +153,7 @@ public class SolrXmlConfig {
       byte[] buf = IOUtils.toByteArray(is);
       try (ByteArrayInputStream dup = new ByteArrayInputStream(buf)) {
         XmlConfigFile config = new XmlConfigFile(loader, null, new InputSource(dup), null, substituteProps);
-        return fromConfig(solrHome, config);
+        return fromConfig(solrHome, config, fromZookeeper);
       }
     } catch (SolrException exc) {
       throw exc;

solr/core/src/java/org/apache/solr/core/ZkContainer.java

@@ -61,9 +61,6 @@ public class ZkContainer {
   }
 
   public void initZooKeeper(final CoreContainer cc, CloudConfig config) {
-
-    ZkController zkController = null;
-
     String zkRun = System.getProperty("zkRun");
 
     if (zkRun != null && config == null)
@@ -108,12 +105,14 @@ public class ZkContainer {
         }
         String confDir = System.getProperty("bootstrap_confdir");
         boolean boostrapConf = Boolean.getBoolean("bootstrap_conf");  
-        
+
-        if(!ZkController.checkChrootPath(zookeeperHost, (confDir!=null) || boostrapConf || zkRunOnly)) {
+        // We may have already loaded NodeConfig from zookeeper with same connect string, so no need to recheck chroot
+        boolean alreadyUsedChroot = cc.getConfig().isFromZookeeper() && zookeeperHost.equals(System.getProperty("zkHost"));
+        if(!alreadyUsedChroot && !ZkController.checkChrootPath(zookeeperHost, (confDir!=null) || boostrapConf || zkRunOnly)) {
           throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR,
               "A chroot was specified in ZkHost but the znode doesn't exist. " + zookeeperHost);
         }
-        zkController = new ZkController(cc, zookeeperHost, zkClientConnectTimeout, config,
+        ZkController zkController = new ZkController(cc, zookeeperHost, zkClientConnectTimeout, config,
             new CurrentCoreDescriptorProvider() {
 
               @Override
@@ -145,12 +144,11 @@ public class ZkContainer {
           configManager.uploadConfigDir(configPath, confName);
         }
 
-
-        
         if(boostrapConf) {
           ZkController.bootstrapConf(zkController.getZkClient(), cc);
         }
-        
+
+        this.zkController = zkController;
       } catch (InterruptedException e) {
         // Restore the interrupted status
         Thread.currentThread().interrupt();
@@ -166,10 +164,7 @@ public class ZkContainer {
         throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR,
             "", e);
       }
-
-
     }
-    this.zkController = zkController;
   }
   
   private String stripChroot(String zkRun) {

solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java

@@ -283,7 +283,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
         if (zkClient.exists("/solr.xml", true)) {
           log.info("solr.xml found in ZooKeeper. Loading...");
           byte[] data = zkClient.getData("/solr.xml", null, null, true);
-          return SolrXmlConfig.fromInputStream(solrHome, new ByteArrayInputStream(data), nodeProperties);
+          return SolrXmlConfig.fromInputStream(solrHome, new ByteArrayInputStream(data), nodeProperties, true);
         }
       } catch (Exception e) {
         throw new SolrException(ErrorCode.SERVER_ERROR, "Error occurred while loading solr.xml from zookeeper", e);

solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java

@@ -88,7 +88,6 @@ public class TestZkChroot extends SolrTestCaseJ4 {
       assertTrue(zkClient2.exists(chroot + "/clusterstate.json", true));
       assertFalse(zkClient2.exists("/clusterstate.json", true));
     } finally {
-      if (cores != null) cores.shutdown();
       if (zkClient != null) zkClient.close();
       if (zkClient2 != null) zkClient2.close();
     }
@@ -101,8 +100,7 @@ public class TestZkChroot extends SolrTestCaseJ4 {
     System.setProperty("bootstrap_conf", "false");
     System.setProperty("zkHost", zkServer.getZkHost() + chroot);
 
-    try(SolrZkClient zkClient = new SolrZkClient(zkServer.getZkHost(),
+    try (SolrZkClient zkClient = new SolrZkClient(zkServer.getZkHost(), AbstractZkTestCase.TIMEOUT)) {
-        AbstractZkTestCase.TIMEOUT)) {
       expectThrows(ZooKeeperException.class,
           "did not get a top level exception when more then 4 updates failed",
           () -> {
@@ -112,8 +110,6 @@ public class TestZkChroot extends SolrTestCaseJ4 {
       });
       assertFalse("Path shouldn't have been created",
           zkClient.exists(chroot, true));// check the path was not created
-    } finally {
-      if (cores != null) cores.shutdown();
     }
   }
   
@@ -126,11 +122,8 @@ public class TestZkChroot extends SolrTestCaseJ4 {
     System.setProperty("bootstrap_confdir", home + "/collection1/conf");
     System.setProperty("collection.configName", configName);
     System.setProperty("zkHost", zkServer.getZkHost() + chroot);
-    SolrZkClient zkClient = null;
+
-    
+    try (SolrZkClient zkClient = new SolrZkClient(zkServer.getZkHost(), AbstractZkTestCase.TIMEOUT)) {
-    try {
-      zkClient = new SolrZkClient(zkServer.getZkHost(),
-          AbstractZkTestCase.TIMEOUT);
       assertFalse("Path '" + chroot + "' should not exist before the test",
           zkClient.exists(chroot, true));
       cores = CoreContainer.createAndLoad(home);
@@ -138,9 +131,6 @@ public class TestZkChroot extends SolrTestCaseJ4 {
           "solrconfig.xml should have been uploaded to zk to the correct config directory",
           zkClient.exists(chroot + ZkConfigManager.CONFIGS_ZKNODE + "/"
               + configName + "/solrconfig.xml", true));
-    } finally {
-      if (cores != null) cores.shutdown();
-      if (zkClient != null) zkClient.close();
     }
   }
   
@@ -150,20 +140,14 @@ public class TestZkChroot extends SolrTestCaseJ4 {
     
     System.setProperty("bootstrap_conf", "true");
     System.setProperty("zkHost", zkServer.getZkHost() + chroot);
-    SolrZkClient zkClient = null;
+
-    
+    try (SolrZkClient zkClient = new SolrZkClient(zkServer.getZkHost(), AbstractZkTestCase.TIMEOUT)) {
-    try {
-      zkClient = new SolrZkClient(zkServer.getZkHost(),
-          AbstractZkTestCase.TIMEOUT);
       zkClient.makePath("/foo/bar4", true);
       assertTrue(zkClient.exists(chroot, true));
       assertFalse(zkClient.exists(chroot + "/clusterstate.json", true));
       
       cores = CoreContainer.createAndLoad(home);
       assertTrue(zkClient.exists(chroot + "/clusterstate.json", true));
-    } finally {
-      if (cores != null) cores.shutdown();
-      if (zkClient != null) zkClient.close();
     }
   }
 }