Skip to content

Commit

Permalink
Always close DIH EntityProcessor/DataSource
Browse files Browse the repository at this point in the history
Prior to this commit, the wrapup logic at the end of
DocBuilder.execute() closed out a series of DIH objects, but did so
in a way that an exception closing any of them resulted in the remainder
staying open.  This is especially problematic since Writer.close()
throws exceptions that DIH uses to determine the success/failure of the
run.

In practice this caused network errors sending DIH data to other Solr
nodes to result in leaked JDBC connections.

This commit changes DocBuilder's termination logic to handle exceptions
more gracefully, ensuring that errors closing a DIHWriter (for example)
don't prevent the closure of entity-processor and DataSource objects.

See also, SOLR-14677
  • Loading branch information
Jason Gerlowski committed Aug 14, 2020
1 parent 9a4fd35 commit b1fa2f1
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.solr.handler.dataimport;

import java.io.Closeable;
import java.util.Properties;

/**
Expand All @@ -35,7 +36,7 @@
*
* @since solr 1.3
*/
public abstract class DataSource<T> {
public abstract class DataSource<T> implements Closeable {

/**
* Initializes the DataSource with the <code>Context</code> and
Expand Down
28 changes: 22 additions & 6 deletions src/main/java/org/apache/solr/handler/dataimport/DocBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.util.IOUtils;
import org.apache.solr.core.SolrCore;
import org.apache.solr.handler.dataimport.config.ConfigNameConstants;
import org.apache.solr.handler.dataimport.config.DIHConfiguration;
Expand Down Expand Up @@ -269,24 +270,39 @@ public String toString() {
} catch(Exception e)
{
throw new RuntimeException(e);
} finally
{
if (writer != null) {
writer.close();
} finally {
// Cannot use IOUtils.closeQuietly since DIH relies on exceptions bubbling out of writer.close() to indicate
// success/failure of the run.
RuntimeException raisedDuringClose = null;
try {
if (writer != null) {
writer.close();
}
} catch (RuntimeException e) {
if (log.isWarnEnabled()) {
log.warn("Exception encountered while closing DIHWriter " + writer + "; temporarily suppressing to ensure other DocBuilder elements are closed", e); // logOk
}
raisedDuringClose = e;
}


if (epwList != null) {
closeEntityProcessorWrappers(epwList);
}
if(reqParams.isDebug()) {
reqParams.getDebugInfo().debugVerboseOutput = getDebugLogger().output;
}

if (raisedDuringClose != null) {
throw raisedDuringClose;
}
}
}
private void closeEntityProcessorWrappers(List<EntityProcessorWrapper> epwList) {
for(EntityProcessorWrapper epw : epwList) {
epw.close();
IOUtils.closeQuietly(epw);
if(epw.getDatasource()!=null) {
epw.getDatasource().close();
IOUtils.closeQuietly(epw.getDatasource());
}
closeEntityProcessorWrappers(epw.getChildren());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.solr.handler.dataimport;

import java.io.Closeable;
import java.util.Map;

/**
Expand All @@ -36,7 +37,7 @@
*
* @since solr 1.3
*/
public abstract class EntityProcessor {
public abstract class EntityProcessor implements Closeable {

/**
* This method is called when it starts processing an entity. When it comes
Expand Down

0 comments on commit b1fa2f1

Please sign in to comment.