From 74721fa4c653b1bb909e4934f22da891350a4d46 Mon Sep 17 00:00:00 2001 From: Mike Date: Fri, 13 Mar 2020 11:56:17 -0700 Subject: [PATCH] SOLR-14289 Skip ZkChroot check when not necessary (#1298) --- .../java/org/apache/solr/core/NodeConfig.java | 20 ++++++++++++-- .../org/apache/solr/core/SolrXmlConfig.java | 9 +++++-- .../org/apache/solr/core/ZkContainer.java | 19 +++++--------- .../solr/servlet/SolrDispatchFilter.java | 2 +- .../org/apache/solr/cloud/TestZkChroot.java | 26 ++++--------------- 5 files changed, 38 insertions(+), 38 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/NodeConfig.java b/solr/core/src/java/org/apache/solr/core/NodeConfig.java index 4afca972548..01aaf3512ab 100644 --- a/solr/core/src/java/org/apache/solr/core/NodeConfig.java +++ b/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) { diff --git a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java index df4b488c997..e61836f5ceb 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java +++ b/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; diff --git a/solr/core/src/java/org/apache/solr/core/ZkContainer.java b/solr/core/src/java/org/apache/solr/core/ZkContainer.java index 417a84f47e0..d4665a0a05e 100644 --- a/solr/core/src/java/org/apache/solr/core/ZkContainer.java +++ b/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) { diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index d744484f5ca..8123f478695 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/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); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java b/solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java index edf596f8aba..134e332b833 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java +++ b/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(), - AbstractZkTestCase.TIMEOUT)) { + try (SolrZkClient zkClient = new SolrZkClient(zkServer.getZkHost(), 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 { - zkClient = new SolrZkClient(zkServer.getZkHost(), - AbstractZkTestCase.TIMEOUT); + + try (SolrZkClient 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 { - zkClient = new SolrZkClient(zkServer.getZkHost(), - AbstractZkTestCase.TIMEOUT); + + try (SolrZkClient 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(); } } }