[lucene-solr] branch branch_8x updated: SOLR-14677: Always close DIH EntityProcessor/DataSource (#1741)

[ge...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gerlowskija pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 8bc6688  SOLR-14677: Always close DIH EntityProcessor/DataSource (#1741)
8bc6688 is described below

commit 8bc6688e6a846451f7b94052290588c70e4dfd99
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Thu Aug 13 21:21:31 2020 -0400

    SOLR-14677: Always close DIH EntityProcessor/DataSource (#1741)
    
    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.
---
 solr/CHANGES.txt                                   |  2 ++
 .../apache/solr/handler/dataimport/DataSource.java |  3 ++-
 .../apache/solr/handler/dataimport/DocBuilder.java | 30 +++++++++++++++++-----
 .../solr/handler/dataimport/EntityProcessor.java   |  3 ++-
 4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 72280e7..ec89f64c 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -72,6 +72,8 @@ Bug Fixes
 
 * SOLR-14751: Zookeeper Admin screen not working for old ZK versions (janhoy)
 
+* SOLR-14677: Improve DIH termination logic to close all DataSources, EntityProcessors (Jason Gerlowski)
+
 Other Changes
 ---------------------
 
diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataSource.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataSource.java
index e217ddd..aeded27 100644
--- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataSource.java
+++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataSource.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.handler.dataimport;
 
+import java.io.Closeable;
 import java.util.Properties;
 
 /**
@@ -35,7 +36,7 @@ import java.util.Properties;
  *
  * @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
diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java
index 72bcc03..df6f9ad 100644
--- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java
+++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java
@@ -18,6 +18,7 @@ package org.apache.solr.handler.dataimport;
 
 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;
@@ -272,24 +273,39 @@ public class DocBuilder {
     } 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();
-      if(epw.getDatasource()!=null) {
-        epw.getDatasource().close();
+      IOUtils.closeQuietly(epw);
+
+      if(epw.getDatasource() != null) {
+        IOUtils.closeQuietly(epw.getDatasource());
       }
       closeEntityProcessorWrappers(epw.getChildren());
     }
diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/EntityProcessor.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/EntityProcessor.java
index 8cfbed9..7ded623 100644
--- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/EntityProcessor.java
+++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/EntityProcessor.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.handler.dataimport;
 
+import java.io.Closeable;
 import java.util.Map;
 
 /**
@@ -36,7 +37,7 @@ import java.util.Map;
  *
  * @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