You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2012/09/07 23:58:31 UTC
svn commit: r1382187 - in /lucene/dev/trunk/solr: ./
core/src/java/org/apache/solr/core/ core/src/java/org/apache/solr/update/
core/src/test/org/apache/solr/core/
Author: hossman
Date: Fri Sep 7 21:58:30 2012
New Revision: 1382187
URL: http://svn.apache.org/viewvc?rev=1382187&view=rev
Log:
SOLR-3699: Fixed some Directory leaks when there were errors during SolrCore or SolrIndexWriter initialization
Modified:
lucene/dev/trunk/solr/CHANGES.txt
lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateLog.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CoreContainerCoreInitFailuresTest.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestBadConfig.java
Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Fri Sep 7 21:58:30 2012
@@ -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
----------------------
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CachingDirectoryFactory.java Fri Sep 7 21:58:30 2012
@@ -47,6 +47,9 @@ public abstract class CachingDirectoryFa
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 CachingDirectoryFa
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);
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java Fri Sep 7 21:58:30 2012
@@ -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);
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java Fri Sep 7 21:58:30 2012
@@ -445,7 +445,7 @@ public final class SolrCore implements S
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 S
}
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);
}
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java Fri Sep 7 21:58:30 2012
@@ -188,7 +188,7 @@ public final class DefaultSolrCoreState
}
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);
}
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java Fri Sep 7 21:58:30 2012
@@ -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 {
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrIndexWriter.java Fri Sep 7 21:58:30 2012
@@ -54,20 +54,38 @@ public class SolrIndexWriter extends Ind
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) {
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateLog.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateLog.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateLog.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateLog.java Fri Sep 7 21:58:30 2012
@@ -215,6 +215,7 @@ public class UpdateLog implements Plugin
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);
}
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CoreContainerCoreInitFailuresTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CoreContainerCoreInitFailuresTest.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CoreContainerCoreInitFailuresTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/CoreContainerCoreInitFailuresTest.java Fri Sep 7 21:58:30 2012
@@ -93,6 +93,7 @@ public class CoreContainerCoreInitFailur
// 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 CoreContainerCoreInitFailur
}
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 CoreContainerCoreInitFailur
// 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 CoreContainerCoreInitFailur
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) {
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestBadConfig.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestBadConfig.java?rev=1382187&r1=1382186&r2=1382187&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestBadConfig.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestBadConfig.java Fri Sep 7 21:58:30 2012
@@ -29,11 +29,6 @@ public class TestBadConfig extends Abstr
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 Abstr
"schema.xml","currency.xml");
}
+ public void testBogusMergePolicy() throws Exception {
+ assertConfigs("bad-mp-solrconfig.xml", "schema-minimal.xml",
+ "DummyMergePolicy");
+ }
+
}