SOLR-3699: Fixed some Directory leaks when there were errors during SolrCore or SolrIndexWriter initialization

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1382187 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Chris M. Hostetter 2012-09-07 21:58:30 +00:00
parent bcf205c610
commit ee0bf2549b
10 changed files with 67 additions and 26 deletions

View File

@ -134,6 +134,10 @@ Bug Fixes
* SOLR-3795: Fixed LukeRequestHandler response to correctly return field name
strings in copyDests and copySources arrays (hossman)
* SOLR-3699: Fixed some Directory leaks when there were errors during SolrCore
or SolrIndexWriter initialization. (hossman)
Other Changes
----------------------

View File

@ -47,6 +47,9 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
int refCnt = 1;
public String path;
public boolean doneWithDir = false;
public String toString() {
return "CachedDir<<" + directory.toString() + ";refCount=" + refCnt + ";path=" + path + ";done=" + doneWithDir + ">>";
}
}
private static Logger log = LoggerFactory
@ -123,6 +126,9 @@ public abstract class CachingDirectoryFactory extends DirectoryFactory {
throw new IllegalArgumentException("Unknown directory: " + directory
+ " " + byDirectoryCache);
}
log.debug("Closing: {}", cacheValue);
cacheValue.refCnt--;
if (cacheValue.refCnt == 0 && cacheValue.doneWithDir) {
log.info("Closing directory:" + cacheValue.path);

View File

@ -906,6 +906,9 @@ public class CoreContainer
failure = e4;
throw e4;
} finally {
if (null != failure) {
log.error("Unable to create core: " + name, failure);
}
synchronized (coreInitFailures) {
// remove first so insertion order is updated and newest is last
coreInitFailures.remove(name);
@ -1066,6 +1069,9 @@ public class CoreContainer
failure = e4;
throw e4;
} finally {
if (null != failure) {
log.error("Unable to reload core: " + name, failure);
}
synchronized (coreInitFailures) {
// remove first so insertion order is updated and newest is last
coreInitFailures.remove(name);

View File

@ -445,7 +445,7 @@ public final class SolrCore implements SolrInfoMBean {
log.warn(logid+"Solr index directory '" + new File(indexDir) + "' doesn't exist."
+ " Creating new index...");
SolrIndexWriter writer = new SolrIndexWriter("SolrCore.initIndex", indexDir, getDirectoryFactory(), true, schema, solrConfig.indexConfig, solrDelPolicy, codec, false);
SolrIndexWriter writer = SolrIndexWriter.create("SolrCore.initIndex", indexDir, getDirectoryFactory(), true, schema, solrConfig.indexConfig, solrDelPolicy, codec, false);
writer.close();
}
@ -897,7 +897,15 @@ public final class SolrCore implements SolrInfoMBean {
}
try {
if (updateHandler != null) updateHandler.close();
if (null != updateHandler) {
updateHandler.close();
} else {
if (null != directoryFactory) {
// :HACK: normally we rely on updateHandler to do this,
// but what if updateHandler failed to init?
directoryFactory.close();
}
}
} catch (Throwable e) {
SolrException.log(log,e);
}

View File

@ -188,7 +188,7 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover
}
protected SolrIndexWriter createMainIndexWriter(SolrCore core, String name, boolean forceNewDirectory) throws IOException {
return new SolrIndexWriter(name, core.getNewIndexDir(),
return SolrIndexWriter.create(name, core.getNewIndexDir(),
core.getDirectoryFactory(), false, core.getSchema(),
core.getSolrConfig().indexConfig, core.getDeletionPolicy(), core.getCodec(), forceNewDirectory);
}

View File

@ -100,9 +100,9 @@ public class SolrIndexSplitter {
} else {
SolrCore core = searcher.getCore();
String path = paths.get(partitionNumber);
iw = new SolrIndexWriter("SplittingIndexWriter"+partitionNumber + " " + ranges.get(partitionNumber), path,
core.getDirectoryFactory(), true, core.getSchema(),
core.getSolrConfig().indexConfig, core.getDeletionPolicy(), core.getCodec(), true);
iw = SolrIndexWriter.create("SplittingIndexWriter"+partitionNumber + " " + ranges.get(partitionNumber), path,
core.getDirectoryFactory(), true, core.getSchema(),
core.getSolrConfig().indexConfig, core.getDeletionPolicy(), core.getCodec(), true);
}
try {

View File

@ -54,20 +54,38 @@ public class SolrIndexWriter extends IndexWriter {
String name;
private DirectoryFactory directoryFactory;
public SolrIndexWriter(String name, String path, DirectoryFactory directoryFactory, boolean create, IndexSchema schema, SolrIndexConfig config, IndexDeletionPolicy delPolicy, Codec codec, boolean forceNewDirectory) throws IOException {
super(
directoryFactory.get(path, config.lockType, forceNewDirectory),
config.toIndexWriterConfig(schema).
setOpenMode(create ? IndexWriterConfig.OpenMode.CREATE : IndexWriterConfig.OpenMode.APPEND).
setIndexDeletionPolicy(delPolicy).setCodec(codec).setInfoStream(toInfoStream(config))
);
public static SolrIndexWriter create(String name, String path, DirectoryFactory directoryFactory, boolean create, IndexSchema schema, SolrIndexConfig config, IndexDeletionPolicy delPolicy, Codec codec, boolean forceNewDirectory) throws IOException {
SolrIndexWriter w = null;
final Directory d = directoryFactory.get(path, config.lockType, forceNewDirectory);
try {
w = new SolrIndexWriter(name, path, d, create, schema,
config, delPolicy, codec, forceNewDirectory);
w.setDirectoryFactory(directoryFactory);
return w;
} finally {
if (null == w && null != d) {
directoryFactory.doneWithDirectory(d);
directoryFactory.release(d);
}
}
}
private SolrIndexWriter(String name, String path, Directory directory, boolean create, IndexSchema schema, SolrIndexConfig config, IndexDeletionPolicy delPolicy, Codec codec, boolean forceNewDirectory) throws IOException {
super(directory,
config.toIndexWriterConfig(schema).
setOpenMode(create ? IndexWriterConfig.OpenMode.CREATE : IndexWriterConfig.OpenMode.APPEND).
setIndexDeletionPolicy(delPolicy).setCodec(codec).setInfoStream(toInfoStream(config))
);
log.debug("Opened Writer " + name);
this.name = name;
this.directoryFactory = directoryFactory;
numOpens.incrementAndGet();
}
private void setDirectoryFactory(DirectoryFactory factory) {
this.directoryFactory = factory;
}
private static InfoStream toInfoStream(SolrIndexConfig config) throws IOException {
String infoStreamFile = config.infoStreamFile;
if (infoStreamFile != null) {

View File

@ -215,6 +215,7 @@ public class UpdateLog implements PluginInfoInitialized {
try {
versionInfo = new VersionInfo(this, 256);
} catch (SolrException e) {
log.error("Unable to use updateLog: " + e.getMessage(), e);
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
"Unable to use updateLog: " + e.getMessage(), e);
}

View File

@ -93,6 +93,7 @@ public class CoreContainerCoreInitFailuresTest extends SolrTestCaseJ4 {
// try to add a collection with a path that doesn't exist
final CoreDescriptor bogus = new CoreDescriptor(cc, "bogus", "bogus_path");
try {
ignoreException(Pattern.quote("bogus_path"));
cc.create(bogus);
fail("bogus inst dir failed to trigger exception from create");
} catch (Exception e) {
@ -123,12 +124,7 @@ public class CoreContainerCoreInitFailuresTest extends SolrTestCaseJ4 {
}
public void testFlowBadFromStart() throws Exception {
// TODO: even if we close all solr cores in the container, there is still a leaked dir?
// maybe from one that didnt load right?
// TODO: make SolrCore closeable since its has close()
System.setProperty("solr.directoryFactory", "org.apache.solr.core.SimpleFSDirectoryFactory");
// reused state
Map<String,Exception> failures = null;
Collection<String> cores = null;
@ -198,6 +194,7 @@ public class CoreContainerCoreInitFailuresTest extends SolrTestCaseJ4 {
// try to add a collection with a path that doesn't exist
final CoreDescriptor bogus = new CoreDescriptor(cc, "bogus", "bogus_path");
try {
ignoreException(Pattern.quote("bogus_path"));
cc.create(bogus);
fail("bogus inst dir failed to trigger exception from create");
} catch (Exception e) {
@ -254,6 +251,7 @@ public class CoreContainerCoreInitFailuresTest extends SolrTestCaseJ4 {
IOUtils.CHARSET_UTF_8.toString());
try {
ignoreException(Pattern.quote("SAX"));
cc.reload("col_bad");
fail("corrupt solrconfig.xml failed to trigger exception from reload");
} catch (SAXParseException e) {

View File

@ -29,11 +29,6 @@ public class TestBadConfig extends AbstractBadConfigTestBase {
public void testUpdateLogButNoVersionField() throws Exception {
// :TODO: neccessary until SOLR-3699 is fixed
System.setProperty("solr.directoryFactory",
"org.apache.solr.core.SimpleFSDirectoryFactory");
System.setProperty("enable.update.log", "true");
try {
assertConfigs("solrconfig.xml", "schema12.xml", "_version_");
@ -64,4 +59,9 @@ public class TestBadConfig extends AbstractBadConfigTestBase {
"schema.xml","currency.xml");
}
public void testBogusMergePolicy() throws Exception {
assertConfigs("bad-mp-solrconfig.xml", "schema-minimal.xml",
"DummyMergePolicy");
}
}