Fix for SOLR-4982, creating cores with sysprops does not dereference them properly

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1500284 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Erick Erickson 2013-07-06 14:45:47 +00:00
parent 0f3e2dd245
commit 3b640360fd
7 changed files with 378 additions and 12 deletions

View File

@ -253,6 +253,12 @@ Bug Fixes
* SOLR-5000: ManagedIndexSchema doesn't persist uniqueKey tag after calling addFields
method. (Jun Ohtani, Steve Rowe)
* SOLR-4982: Creating a core while referencing system properties looks like it loses files
Actually, instanceDir, config, dataDir and schema are not dereferenced properly
when creating cores that reference sys vars (e.g. &dataDir=${dir}). In the dataDir
case in particular this leads to the index being put in a directory literally named
${dir} but on restart the sysvar will be properly dereferenced.
Optimizations
----------------------

View File

@ -46,7 +46,7 @@ public class CoreDescriptor {
public static final String CORE_TRANSIENT = "transient";
public static final String CORE_NODE_NAME = "coreNodeName";
static final String[] standardPropNames = {
public static final String[] standardPropNames = {
CORE_NAME,
CORE_CONFIG,
CORE_INSTDIR,
@ -65,7 +65,7 @@ public class CoreDescriptor {
// them individually.
private Properties coreProperties = new Properties();
//TODO: 5.0 remove this, this is solely a hack for persistence.
//TODO: 5.0 remove this, this is solely a hack for persistence. And perhaps creating cores in discovery mode?
private Properties createdProperties = new Properties();
private boolean loadedImplicit = false;

View File

@ -26,6 +26,7 @@ import java.io.Writer;
import java.lang.reflect.Constructor;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
@ -873,6 +874,18 @@ public final class SolrCore implements SolrInfoMBean {
propFile.getParentFile().mkdirs();
Properties props = new Properties();
props.put("name", cd.getName());
// This must be being created since there's no file here already. So write out all of the params we were
// created with. This _may_ overwrite the name above, but that's OK.
Collection<String> stds = new HashSet(Arrays.asList(CoreDescriptor.standardPropNames));
for (String prop : cd.getCreatedProperties().stringPropertyNames()) {
// Only preserve things that are legal, and let's just keep instDir right out of the persisted file even
// though it's part of the create properties on the URL.
if (! CoreDescriptor.CORE_INSTDIR.equals(prop) && stds.contains(prop)) {
props.put(prop, cd.getCreatedProperties().getProperty(prop));
}
}
if (cc.isZooKeeperAware()) {
String collection = cd.getCloudDescriptor().getCollectionName();
if (collection != null) {

View File

@ -57,6 +57,7 @@ import org.apache.solr.update.UpdateLog;
import org.apache.solr.update.processor.UpdateRequestProcessor;
import org.apache.solr.update.processor.UpdateRequestProcessorChain;
import org.apache.solr.util.NumberUtils;
import org.apache.solr.util.PropertiesUtil;
import org.apache.solr.util.RefCounted;
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
@ -414,6 +415,7 @@ public class CoreAdminHandler extends RequestHandlerBase {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"Core name is mandatory to CREATE a SolrCore");
}
CoreDescriptor dcore = null;
try {
@ -427,22 +429,30 @@ public class CoreAdminHandler extends RequestHandlerBase {
if (instanceDir == null) {
// instanceDir = coreContainer.getSolrHome() + "/" + name;
instanceDir = name; // bare name is already relative to solr home
} else {
instanceDir = PropertiesUtil.substituteProperty(instanceDir, null);
}
dcore = new CoreDescriptor(coreContainer, name, instanceDir);
// fillup optional parameters
String opts = params.get(CoreAdminParams.CONFIG);
if (opts != null)
if (opts != null) {
opts = PropertiesUtil.substituteProperty(opts, null);
dcore.setConfigName(opts);
}
opts = params.get(CoreAdminParams.SCHEMA);
if (opts != null)
if (opts != null) {
opts = PropertiesUtil.substituteProperty(opts, null);
dcore.setSchemaName(opts);
}
opts = params.get(CoreAdminParams.DATA_DIR);
if (opts != null)
if (opts != null) {
opts = PropertiesUtil.substituteProperty(opts, null);
dcore.setDataDir(opts);
}
opts = params.get(CoreAdminParams.ULOG_DIR);
if (opts != null)

View File

@ -0,0 +1,228 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.solr.handler.admin;
import org.apache.commons.io.FileUtils;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.core.CoreDescriptor;
import org.apache.solr.core.SolrCoreDiscoverer;
import org.apache.solr.response.SolrQueryResponse;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Properties;
public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
private static File solrHomeDirectory = null;
private static CoreAdminHandler admin = null;
// private static CoreContainer cc = null;
private static String coreNormal = "normal";
private static String coreSysProps = "sys_props";
@BeforeClass
public static void before() throws Exception {
useFactory(null); // I require FS-based indexes for this test.
solrHomeDirectory = new File(TEMP_DIR, "solrHome/" + CoreAdminCreateDiscoverTest.getClassName());
if (solrHomeDirectory.exists()) {
FileUtils.deleteDirectory(solrHomeDirectory);
}
assertTrue("Failed to mkdirs workDir", solrHomeDirectory.mkdirs());
setupNoCoreTest(solrHomeDirectory, null);
admin = new CoreAdminHandler(h.getCoreContainer());
}
@AfterClass
public static void after() throws Exception {
h.close();
if (solrHomeDirectory.exists()) {
FileUtils.deleteDirectory(solrHomeDirectory);
}
}
private static void setupCore(String coreName, boolean blivet) throws IOException {
File instDir = new File(solrHomeDirectory, coreName);
File subHome = new File(instDir, "conf");
assertTrue("Failed to make subdirectory ", subHome.mkdirs());
// Be sure we pick up sysvars when we create this
String srcDir = SolrTestCaseJ4.TEST_HOME() + "/collection1/conf";
FileUtils.copyFile(new File(srcDir, "schema-tiny.xml"), new File(subHome, "schema_ren.xml"));
FileUtils.copyFile(new File(srcDir, "solrconfig-minimal.xml"), new File(subHome, "solrconfig_ren.xml"));
FileUtils.copyFile(new File(srcDir, "solrconfig.snippet.randomindexconfig.xml"),
new File(subHome, "solrconfig.snippet.randomindexconfig.xml"));
}
@Test
public void testCreateSavesSysProps() throws Exception {
setupCore(coreSysProps, true);
// create a new core (using CoreAdminHandler) w/ properties
// Just to be sure its NOT written to the core.properties file
File workDir = new File(solrHomeDirectory, coreSysProps);
System.setProperty("INSTDIR_TEST", workDir.getAbsolutePath());
System.setProperty("CONFIG_TEST", "solrconfig_ren.xml");
System.setProperty("SCHEMA_TEST", "schema_ren.xml");
File dataDir = new File(workDir.getAbsolutePath(), "data_diff");
System.setProperty("DATA_TEST", "data_diff");
SolrQueryResponse resp = new SolrQueryResponse();
admin.handleRequestBody
(req(CoreAdminParams.ACTION,
CoreAdminParams.CoreAdminAction.CREATE.toString(),
CoreAdminParams.NAME, coreSysProps,
CoreAdminParams.INSTANCE_DIR, "${INSTDIR_TEST}",
CoreAdminParams.CONFIG, "${CONFIG_TEST}",
CoreAdminParams.SCHEMA, "${SCHEMA_TEST}",
CoreAdminParams.DATA_DIR, "${DATA_TEST}"),
resp);
assertNull("Exception on create", resp.getException());
// verify props are in persisted file
Properties props = new Properties();
File propFile = new File(solrHomeDirectory, coreSysProps + "/" + SolrCoreDiscoverer.CORE_PROP_FILE);
FileInputStream is = new FileInputStream(propFile);
try {
props.load(is);
} finally {
org.apache.commons.io.IOUtils.closeQuietly(is);
}
assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
props.getProperty(CoreAdminParams.NAME), coreSysProps);
assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
props.getProperty(CoreAdminParams.CONFIG), "${CONFIG_TEST}");
assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
props.getProperty(CoreAdminParams.SCHEMA), "${SCHEMA_TEST}");
assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
props.getProperty(CoreAdminParams.DATA_DIR), "${DATA_TEST}");
checkOnlyKnown(propFile);
// Now assert that certain values are properly dereferenced in the process of creating the core, see
// SOLR-4982. Really, we should be able to just verify that the index files exist.
// Should NOT be a datadir named ${DATA_TEST} (literal).
File badDir = new File(workDir, "${DATA_TEST}");
assertFalse("Should have substituted the sys var, found file " + badDir.getAbsolutePath(), badDir.exists());
// For the other 3 vars, we couldn't get past creating the core if dereferencing didn't work correctly.
// Should have segments in the directory pointed to by the ${DATA_TEST}.
File test = new File(dataDir, "index");
assertTrue("Should have found index dir at " + test.getAbsolutePath(), test.exists());
File gen = new File(test, "segments.gen");
assertTrue("Should be segments.gen in the dir at " + gen.getAbsolutePath(), gen.exists());
}
@Test
public void testCreateSavesRegProps() throws Exception {
setupCore(coreNormal, true);
// create a new core (using CoreAdminHandler) w/ properties
// Just to be sure its NOT written to the core.properties file
File workDir = new File(solrHomeDirectory, coreNormal);
File data = new File(workDir, "data");
SolrQueryResponse resp = new SolrQueryResponse();
admin.handleRequestBody
(req(CoreAdminParams.ACTION,
CoreAdminParams.CoreAdminAction.CREATE.toString(),
CoreAdminParams.NAME, coreNormal,
CoreAdminParams.INSTANCE_DIR, workDir.getAbsolutePath(),
CoreAdminParams.CONFIG, "solrconfig_ren.xml",
CoreAdminParams.SCHEMA, "schema_ren.xml",
CoreAdminParams.DATA_DIR, data.getAbsolutePath()),
resp);
assertNull("Exception on create", resp.getException());
// verify props are in persisted file
Properties props = new Properties();
File propFile = new File(solrHomeDirectory, coreNormal + "/" + SolrCoreDiscoverer.CORE_PROP_FILE);
FileInputStream is = new FileInputStream(propFile);
try {
props.load(is);
} finally {
org.apache.commons.io.IOUtils.closeQuietly(is);
}
assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
props.getProperty(CoreAdminParams.NAME), coreNormal);
assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
props.getProperty(CoreAdminParams.CONFIG), "solrconfig_ren.xml");
assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
props.getProperty(CoreAdminParams.SCHEMA), "schema_ren.xml");
assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
props.getProperty(CoreAdminParams.DATA_DIR), data.getAbsolutePath());
checkOnlyKnown(propFile);
// For the other 3 vars, we couldn't get past creating the core if dereferencing didn't work correctly.
// Should have segments in the directory pointed to by the ${DATA_TEST}.
File test = new File(data, "index");
assertTrue("Should have found index dir at " + test.getAbsolutePath(), test.exists());
File gen = new File(test, "segments.gen");
assertTrue("Should be segments.gen in the dir at " + gen.getAbsolutePath(), gen.exists());
}
// Insure that all the props we've preserved are ones that _should_ be in the properties file
private void checkOnlyKnown(File propFile) throws IOException {
Properties props = new Properties();
FileInputStream is = new FileInputStream(propFile);
try {
props.load(is);
} finally {
org.apache.commons.io.IOUtils.closeQuietly(is);
}
// Should never be preserving instanceDir in a core.properties file.
assertFalse("Should not be preserving instanceDir!", props.containsKey(CoreAdminParams.INSTANCE_DIR));
Collection<String> stds = new HashSet(Arrays.asList(CoreDescriptor.standardPropNames));
for (String key : props.stringPropertyNames()) {
assertTrue("Property '" + key + "' should NOT be preserved in the properties file", stds.contains(key));
}
}
}

View File

@ -17,9 +17,9 @@
package org.apache.solr.handler.admin;
import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrCore;
import org.apache.solr.handler.admin.CoreAdminHandler;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.util.NamedList;
@ -28,15 +28,14 @@ import org.apache.solr.SolrTestCaseJ4;
import java.util.Map;
import java.io.File;
import java.io.IOException;
import javax.xml.xpath.XPathExpressionException;
import org.apache.commons.io.FileUtils;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.xml.sax.SAXException;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
@ -44,7 +43,93 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
public static void beforeClass() throws Exception {
initCore("solrconfig.xml", "schema.xml");
}
@Rule
public TestRule solrTestRules = RuleChain.outerRule(new SystemPropertiesRestoreRule());
public String getCoreName() { return this.getClass().getName() + "_sys_vars"; }
@Test
public void testCreateWithSysVars() throws Exception {
useFactory(null); // I require FS-based indexes for this test.
final File workDir = new File(TEMP_DIR, getCoreName());
if (workDir.exists()) {
FileUtils.deleteDirectory(workDir);
}
assertTrue("Failed to mkdirs workDir", workDir.mkdirs());
String coreName = "with_sys_vars";
File instDir = new File(workDir, coreName);
File subHome = new File(instDir, "conf");
assertTrue("Failed to make subdirectory ", subHome.mkdirs());
// Be sure we pick up sysvars when we create this
String srcDir = SolrTestCaseJ4.TEST_HOME() + "/collection1/conf";
FileUtils.copyFile(new File(srcDir, "schema-tiny.xml"), new File(subHome, "schema_ren.xml"));
FileUtils.copyFile(new File(srcDir, "solrconfig-minimal.xml"), new File(subHome, "solrconfig_ren.xml"));
FileUtils.copyFile(new File(srcDir, "solrconfig.snippet.randomindexconfig.xml"),
new File(subHome, "solrconfig.snippet.randomindexconfig.xml"));
final CoreContainer cores = h.getCoreContainer();
cores.setPersistent(false); // we'll do this explicitly as needed
final CoreAdminHandler admin = new CoreAdminHandler(cores);
// create a new core (using CoreAdminHandler) w/ properties
System.setProperty("INSTDIR_TEST", instDir.getAbsolutePath());
System.setProperty("CONFIG_TEST", "solrconfig_ren.xml");
System.setProperty("SCHEMA_TEST", "schema_ren.xml");
File dataDir = new File(workDir.getAbsolutePath(), "data_diff");
System.setProperty("DATA_TEST", dataDir.getAbsolutePath());
SolrQueryResponse resp = new SolrQueryResponse();
admin.handleRequestBody
(req(CoreAdminParams.ACTION,
CoreAdminParams.CoreAdminAction.CREATE.toString(),
CoreAdminParams.NAME, getCoreName(),
CoreAdminParams.INSTANCE_DIR, "${INSTDIR_TEST}",
CoreAdminParams.CONFIG, "${CONFIG_TEST}",
CoreAdminParams.SCHEMA, "${SCHEMA_TEST}",
CoreAdminParams.DATA_DIR, "${DATA_TEST}"),
resp);
assertNull("Exception on create", resp.getException());
// verify props are in persisted file
final File xml = new File(workDir, "persist-solr.xml");
cores.persistFile(xml);
// First assert that these values are persisted.
assertXmlFile
(xml
,"/solr/cores/core[@name='" + getCoreName() + "' and @instanceDir='${INSTDIR_TEST}']"
,"/solr/cores/core[@name='" + getCoreName() + "' and @dataDir='${DATA_TEST}']"
,"/solr/cores/core[@name='" + getCoreName() + "' and @schema='${SCHEMA_TEST}']"
,"/solr/cores/core[@name='" + getCoreName() + "' and @config='${CONFIG_TEST}']"
);
// Now assert that certain values are properly dereferenced in the process of creating the core, see
// SOLR-4982.
// Should NOT be a datadir named ${DATA_TEST} (literal). This is the bug after all
File badDir = new File(instDir, "${DATA_TEST}");
assertFalse("Should have substituted the sys var, found file " + badDir.getAbsolutePath(), badDir.exists());
// For the other 3 vars, we couldn't get past creating the core fi dereferencing didn't work correctly.
// Should have segments in the directory pointed to by the ${DATA_TEST}.
File test = new File(dataDir, "index");
assertTrue("Should have found index dir at " + test.getAbsolutePath(), test.exists());
test = new File(test,"segments.gen");
assertTrue("Should have found segments.gen at " + test.getAbsolutePath(), test.exists());
// Cleanup
FileUtils.deleteDirectory(workDir);
}
@Test
public void testCoreAdminHandler() throws Exception {
final File workDir = new File(TEMP_DIR, this.getClass().getName());
@ -141,6 +226,8 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
// :TODO: because of SOLR-3665 we can't ask for status from all cores
}
// cleanup
FileUtils.deleteDirectory(workDir);
}
}

View File

@ -21,6 +21,7 @@ import com.carrotsearch.randomizedtesting.RandomizedContext;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
import org.apache.commons.io.FileUtils;
import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.util.QuickPatchThreadsFilter;
import org.apache.solr.client.solrj.util.ClientUtils;
@ -32,9 +33,11 @@ import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.XML;
import org.apache.solr.core.ConfigSolr;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrConfig;
import org.apache.solr.core.SolrCore;
import org.apache.solr.core.SolrResourceLoader;
import org.apache.solr.handler.JsonUpdateRequestHandler;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
@ -155,6 +158,25 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
}
}
/**
* Call this from @BeforeClass to set up the test harness and update handler with no cores.
*
* @param solrHome The solr home directory.
* @param xmlStr - the text of an XML file to use. If null, use the what's the absolute minimal file.
* @throws Exception Lost of file-type things can go wrong.
*/
public static void setupNoCoreTest(File solrHome, String xmlStr) throws Exception {
File tmpFile = new File(solrHome, ConfigSolr.SOLR_XML_FILE);
if (xmlStr == null) {
xmlStr = "<solr></solr>";
}
FileUtils.write(tmpFile, xmlStr, IOUtils.CHARSET_UTF_8.toString());
SolrResourceLoader loader = new SolrResourceLoader(solrHome.getAbsolutePath());
h = new TestHarness(loader, ConfigSolr.fromFile(loader, new File(solrHome, "solr.xml")));
lrf = h.getRequestFactory("standard", 0, 20, CommonParams.VERSION, "2.2");
}
@Override
public void setUp() throws Exception {