From 2d5f162bb8cd35095a127d458180a6af660c6631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Sun, 26 Jul 2015 00:15:27 +0000 Subject: [PATCH] SOLR-7735: Look for solr.xml in Zookeeper by default git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1692673 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 6 +++ solr/bin/solr | 13 ++--- solr/bin/solr.cmd | 13 +++-- solr/bin/solr.in.cmd | 4 +- solr/bin/solr.in.sh | 4 +- .../solr/servlet/SolrDispatchFilter.java | 41 +++++++-------- .../solr/cloud/DistributedQueueTest.java | 5 -- .../apache/solr/cloud/SolrXmlInZkTest.java | 52 ++----------------- .../solr/cloud/MiniSolrCloudCluster.java | 2 - 9 files changed, 44 insertions(+), 96 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index b55cbf3e292..5f16ccc78e8 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -114,6 +114,9 @@ Upgrading from Solr 5.2 which can then be used to prepare for the distributed request. Users with custom ShardHandler implementations would need to modify their code to this effect. +* The system property "solr.solrxml.location" is not supported any more. Now, solr.xml is first + looked up in zookeeper, and if not found, fallback to SOLR_HOME. See SOLR-7735 for more info. + Detailed Change List ---------------------- @@ -355,6 +358,9 @@ Other Changes ERROR to WARN for zkcli.{sh,cmd} only. (Oliver Schrenk, Tim Potter, Uwe Schindler, shalin) +* SOLR-7735: Look for solr.xml in Zookeeper by default in SolrCloud mode. If not found, it will be loaded + from $SOLR_HOME/solr.xml as before. Sysprop solr.solrxml.location is now gone. (janhoy) + ================== 5.2.1 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release diff --git a/solr/bin/solr b/solr/bin/solr index edb92e1c5c6..17459265bb2 100755 --- a/solr/bin/solr +++ b/solr/bin/solr @@ -229,9 +229,9 @@ function print_usage() { echo " -s Sets the solr.solr.home system property; Solr will create core directories under" echo " this directory. This allows you to run multiple Solr instances on the same host" echo " while reusing the same server directory set using the -d parameter. If set, the" - echo " specified directory should contain a solr.xml file. The default value is server/solr." + echo " specified directory should contain a solr.xml file, unless solr.xml exists in ZooKeeper." echo " This parameter is ignored when running examples (-e), as the solr.solr.home depends" - echo " on which example is run." + echo " on which example is run. The default value is server/solr." echo "" echo " -e Name of the example to run; available examples:" echo " cloud: SolrCloud example" @@ -1216,10 +1216,6 @@ if [ ! -e "$SOLR_HOME" ]; then echo -e "\nSolr home directory $SOLR_HOME not found!\n" exit 1 fi -if [ ! -e "$SOLR_HOME/solr.xml" ]; then - echo -e "\nSolr home directory $SOLR_HOME must contain a solr.xml file!\n" - exit 1 -fi # backup the log files before starting if [ -f "$SOLR_LOGS_DIR/solr.log" ]; then @@ -1281,6 +1277,11 @@ if [ "$SOLR_MODE" == 'solrcloud' ]; then CLOUD_MODE_OPTS+=('-Dbootstrap_confdir=./solr/collection1/conf' '-Dcollection.configName=myconf' '-DnumShards=1') fi +else + if [ ! -e "$SOLR_HOME/solr.xml" ]; then + echo -e "\nSolr home directory $SOLR_HOME must contain a solr.xml file!\n" + exit 1 + fi fi # These are useful for attaching remote profilers like VisualVM/JConsole diff --git a/solr/bin/solr.cmd b/solr/bin/solr.cmd index e2a34bb38c0..04d27563ce1 100644 --- a/solr/bin/solr.cmd +++ b/solr/bin/solr.cmd @@ -188,9 +188,9 @@ goto done @echo -s dir Sets the solr.solr.home system property; Solr will create core directories under @echo this directory. This allows you to run multiple Solr instances on the same host @echo while reusing the same server directory set using the -d parameter. If set, the -@echo specified directory should contain a solr.xml file. The default value is example/solr. +@echo specified directory should contain a solr.xml file, unless solr.xml exists in ZooKeeper. @echo This parameter is ignored when running examples (-e), as the solr.solr.home depends -@echo on which example is run. +@echo on which example is run. The default value is server/solr. @echo. @echo -e example Name of the example to run; available examples: @echo cloud: SolrCloud example @@ -633,11 +633,6 @@ IF NOT EXIST "%SOLR_HOME%\" ( ) ) -IF NOT EXIST "%SOLR_HOME%\solr.xml" ( - set "SCRIPT_ERROR=Solr home directory %SOLR_HOME% must contain solr.xml!" - goto err -) - IF "%STOP_KEY%"=="" set STOP_KEY=solrrocks @REM This is quite hacky, but examples rely on a different log4j.properties @@ -807,6 +802,10 @@ IF "%SOLR_MODE%"=="solrcloud" ( IF EXIST "%SOLR_HOME%\collection1\core.properties" set "CLOUD_MODE_OPTS=!CLOUD_MODE_OPTS! -Dbootstrap_confdir=./solr/collection1/conf -Dcollection.configName=myconf -DnumShards=1" ) ELSE ( set CLOUD_MODE_OPTS= + IF NOT EXIST "%SOLR_HOME%\solr.xml" ( + set "SCRIPT_ERROR=Solr home directory %SOLR_HOME% must contain solr.xml!" + goto err + ) ) REM These are useful for attaching remove profilers like VisualVM/JConsole diff --git a/solr/bin/solr.in.cmd b/solr/bin/solr.in.cmd index 9bdcde814b2..e97e0b29687 100644 --- a/solr/bin/solr.in.cmd +++ b/solr/bin/solr.in.cmd @@ -73,8 +73,8 @@ REM set SOLR_OPTS=%SOLR_OPTS% -Dsolr.autoSoftCommit.maxTime=3000 REM set SOLR_OPTS=%SOLR_OPTS% -Dsolr.autoCommit.maxTime=60000 REM set SOLR_OPTS=%SOLR_OPTS% -Dsolr.clustering.enabled=true -REM Path to a directory where Solr creates index files, the specified directory -REM must contain a solr.xml; by default, Solr will use server/solr +REM Path to a directory for Solr to store cores and their data. By default, Solr will use server\solr +REM If solr.xml is not stored in ZooKeeper, this directory needs to contain solr.xml REM set SOLR_HOME= REM Sets the port Solr binds to, default is 8983 diff --git a/solr/bin/solr.in.sh b/solr/bin/solr.in.sh index 9169b2034a5..b55552fa9c4 100644 --- a/solr/bin/solr.in.sh +++ b/solr/bin/solr.in.sh @@ -79,8 +79,8 @@ ENABLE_REMOTE_JMX_OPTS="false" # If not set, the script will create PID files in $SOLR_TIP/bin #SOLR_PID_DIR= -# Path to a directory where Solr creates index files, the specified directory -# must contain a solr.xml; by default, Solr will use server/solr +# Path to a directory for Solr to store cores and their data. By default, Solr will use server/solr +# If solr.xml is not stored in ZooKeeper, this directory needs to contain solr.xml #SOLR_HOME= # Solr provides a default Log4J configuration properties file in server/resources 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 eaaf69772ec..098f898ed93 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -85,7 +85,7 @@ public class SolrDispatchFilter extends BaseSolrFilter { public static final String PROPERTIES_ATTRIBUTE = "solr.properties"; public static final String SOLRHOME_ATTRIBUTE = "solr.solr.home"; - + @Override public void init(FilterConfig config) throws ServletException { @@ -149,30 +149,25 @@ public class SolrDispatchFilter extends BaseSolrFilter { public static NodeConfig loadNodeConfig(String solrHome, Properties nodeProperties) { SolrResourceLoader loader = new SolrResourceLoader(solrHome, null, nodeProperties); - - String solrxmlLocation = System.getProperty("solr.solrxml.location", "solrhome"); - - if (solrxmlLocation == null || "solrhome".equalsIgnoreCase(solrxmlLocation)) - return SolrXmlConfig.fromSolrHome(loader, loader.getInstanceDir()); - - if ("zookeeper".equalsIgnoreCase(solrxmlLocation)) { - String zkHost = System.getProperty("zkHost"); - log.info("Trying to read solr.xml from {}", zkHost); - if (StringUtils.isEmpty(zkHost)) - throw new SolrException(ErrorCode.SERVER_ERROR, - "Could not load solr.xml from zookeeper: zkHost system property not set"); - try (SolrZkClient zkClient = new SolrZkClient(zkHost, 30000)) { - if (!zkClient.exists("/solr.xml", true)) - throw new SolrException(ErrorCode.SERVER_ERROR, "Could not load solr.xml from zookeeper: node not found"); - byte[] data = zkClient.getData("/solr.xml", null, null, true); - return SolrXmlConfig.fromInputStream(loader, new ByteArrayInputStream(data)); - } catch (Exception e) { - throw new SolrException(ErrorCode.SERVER_ERROR, "Could not load solr.xml from zookeeper", e); - } + if (!StringUtils.isEmpty(System.getProperty("solr.solrxml.location"))) { + log.warn("Solr property solr.solrxml.location is no longer supported. " + + "Will automatically load solr.xml from ZooKeeper if it exists"); } - throw new SolrException(ErrorCode.SERVER_ERROR, - "Bad solr.solrxml.location set: " + solrxmlLocation + " - should be 'solrhome' or 'zookeeper'"); + String zkHost = System.getProperty("zkHost"); + if (!StringUtils.isEmpty(zkHost)) { + try (SolrZkClient zkClient = new SolrZkClient(zkHost, 30000)) { + 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(loader, new ByteArrayInputStream(data)); + } + } catch (Exception e) { + throw new SolrException(ErrorCode.SERVER_ERROR, "Error occurred while loading solr.xml from zookeeper", e); + } + log.info("Loading solr.xml from SolrHome (not found in ZooKeeper)"); + } + return SolrXmlConfig.fromSolrHome(loader, loader.getInstanceDir()); } public CoreContainer getCores() { diff --git a/solr/core/src/test/org/apache/solr/cloud/DistributedQueueTest.java b/solr/core/src/test/org/apache/solr/cloud/DistributedQueueTest.java index 9f7d9e4a5fb..d2777e5d9f8 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DistributedQueueTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DistributedQueueTest.java @@ -34,11 +34,6 @@ public class DistributedQueueTest extends SolrTestCaseJ4 { protected ZkTestServer zkServer; protected SolrZkClient zkClient; - @Before - public void beforeClass() { - System.setProperty("solr.solrxml.location", "zookeeper"); - } - @Before @Override public void setUp() throws Exception { diff --git a/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java b/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java index 97c95295d45..786b9eaacc1 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java @@ -19,7 +19,6 @@ package org.apache.solr.cloud; import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule; import org.apache.commons.io.FileUtils; import org.apache.solr.SolrTestCaseJ4; -import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.core.NodeConfig; @@ -54,11 +53,6 @@ public class SolrXmlInZkTest extends SolrTestCaseJ4 { private SolrDispatchFilter solrDispatchFilter; - @Before - public void before() { - System.setProperty("solr.solrxml.location", "zookeeper"); - } - @After public void after() { if (solrDispatchFilter != null) { @@ -146,13 +140,11 @@ public class SolrXmlInZkTest extends SolrTestCaseJ4 { } @Test - public void testNotInZkAndShouldBe() throws Exception { + public void testNotInZkFallbackLocal() throws Exception { try { setUpZkAndDiskXml(false, true); - fail("Should have gotten an exception here!"); - } catch (InvocationTargetException ite) { - assertEquals("Should have an exception here, file not in ZK.", - "Could not load solr.xml from zookeeper", ite.getTargetException().getMessage()); + assertEquals("Should have gotten the default port", + cfg.getCloudConfig().getSolrHostPort(), 8983); } finally { closeZK(); } @@ -161,7 +153,6 @@ public class SolrXmlInZkTest extends SolrTestCaseJ4 { @Test public void testNotInZkOrOnDisk() throws Exception { try { - System.clearProperty("solr.solrxml.location"); System.setProperty("hostPort", "8787"); setUpZkAndDiskXml(false, false); // solr.xml not on disk either fail("Should have thrown an exception here"); @@ -176,7 +167,6 @@ public class SolrXmlInZkTest extends SolrTestCaseJ4 { @Test public void testOnDiskOnly() throws Exception { try { - System.clearProperty("solr.solrxml.location"); setUpZkAndDiskXml(false, true); assertEquals("Should have gotten the default port", cfg.getCloudConfig().getSolrHostPort(), 8983); } finally { @@ -184,42 +174,6 @@ public class SolrXmlInZkTest extends SolrTestCaseJ4 { } } - @Test - public void testBadSysProp() throws Exception { - try { - System.setProperty("solr.solrxml.location", "solrHomeDir"); - setUpZkAndDiskXml(false, true); - fail("Should have thrown exception in SolrXmlInZkTest.testBadSysProp"); - } catch (InvocationTargetException ite) { - assertEquals("Should have an exception in SolrXmlInZkTest.testBadSysProp, sysprop set to bogus value.", - ite.getTargetException().getMessage(), "Bad solr.solrxml.location set: solrHomeDir - should be 'solrhome' or 'zookeeper'"); - } finally { - closeZK(); - } - - } - - //SolrDispatchFilter.protected static ConfigSolr loadConfigSolr(SolrResourceLoader loader) { - @Test - public void testZkHostDiscovery() throws ClassNotFoundException, NoSuchMethodException, - IllegalAccessException, InstantiationException, InvocationTargetException { - - // Should see an error when zkHost is not defined but solr.solrxml.location is set to zookeeper. - System.clearProperty("zkHost"); - try { - Method method = SolrDispatchFilter.class.getDeclaredMethod("loadNodeConfig", String.class, Properties.class); - method.setAccessible(true); - if (solrDispatchFilter != null) solrDispatchFilter.destroy(); - solrDispatchFilter = new SolrDispatchFilter(); - method.invoke(solrDispatchFilter, "", new Properties()); - fail("Should have thrown an exception"); - } catch (InvocationTargetException ite) { - assertTrue("Should be catching a SolrException", ite.getTargetException() instanceof SolrException); - assertEquals("Caught Solr exception", ite.getTargetException().getMessage(), - "Could not load solr.xml from zookeeper: zkHost system property not set"); - } - } - // Just a random port, I'm not going to use it but just check that the Solr instance constructed from the XML // file in ZK overrides the default port. private final String XML_FOR_ZK = diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java index 064ad214eef..1183c39b36b 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java @@ -142,7 +142,6 @@ public class MiniSolrCloudCluster { } // tell solr to look in zookeeper for solr.xml - System.setProperty("solr.solrxml.location","zookeeper"); System.setProperty("zkHost", zkServer.getZkAddress()); List> startups = new ArrayList<>(numServers); @@ -366,7 +365,6 @@ public class MiniSolrCloudCluster { try { zkServer.shutdown(); } finally { - System.clearProperty("solr.solrxml.location"); System.clearProperty("zkHost"); } }