You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by cp...@apache.org on 2017/02/24 21:20:00 UTC

[17/50] [abbrv] lucene-solr:jira/solr-6203: SOLR-9842: UpdateRequestProcessors have no way to guarantee the closing of resources used for a request.

SOLR-9842: UpdateRequestProcessors have no way to guarantee the closing of resources used for a request.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/f2f8154d
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/f2f8154d
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/f2f8154d

Branch: refs/heads/jira/solr-6203
Commit: f2f8154d0b6fe8d56974b008a0121d056dee3163
Parents: ee55bec
Author: markrmiller <ma...@apache.org>
Authored: Wed Feb 22 09:50:28 2017 -0500
Committer: markrmiller <ma...@apache.org>
Committed: Wed Feb 22 09:50:28 2017 -0500

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  3 +++
 .../solr/handler/dataimport/SolrWriter.java     |  6 +++++
 .../org/apache/solr/handler/BlobHandler.java    | 17 ++++++------
 .../solr/handler/ContentStreamHandlerBase.java  |  6 ++++-
 .../solr/search/function/FileFloatSource.java   | 14 +++++-----
 .../java/org/apache/solr/update/PeerSync.java   |  3 +++
 .../java/org/apache/solr/update/UpdateLog.java  |  3 +++
 .../DocExpirationUpdateProcessorFactory.java    |  6 ++++-
 .../processor/UpdateRequestProcessor.java       | 28 +++++++++++++++++++-
 .../org/apache/solr/search/TestSearchPerf.java  |  1 +
 .../processor/TolerantUpdateProcessorTest.java  |  7 +++--
 .../processor/UpdateProcessorTestBase.java      |  2 ++
 12 files changed, 77 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 20666fd..144a402 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -229,6 +229,9 @@ Other Changes
 * SOLR-10173: Make HttpShardHandlerFactory.getReplicaListTransformer more extensible.
   (Ramsey Haddad via Christine Poerschke)
 
+* SOLR-9842: UpdateRequestProcessors have no way to guarantee the closing of resources used for a request.
+  (Mark Miller)
+
 ==================  6.4.2 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java
----------------------------------------------------------------------
diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java
index 8428837..3964f3f 100644
--- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java
+++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.handler.dataimport;
 
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.UpdateParams;
 import org.apache.solr.request.SolrQueryRequest;
@@ -63,6 +64,11 @@ public class SolrWriter extends DIHWriterBase implements DIHWriter {
           "Unable to call finish() on UpdateRequestProcessor", e);
     } finally {
       deltaKeys = null;
+      try {
+        processor.close();
+      } catch (IOException e) {
+        SolrException.log(log, e);
+      }
     }
   }
   @Override

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
index f5b49ea..177af9e 100644
--- a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
@@ -231,14 +231,15 @@ public class BlobHandler extends RequestHandlerBase implements PluginInfoInitial
     SolrInputDocument solrDoc = new SolrInputDocument();
     for (Map.Entry<String, Object> e : doc.entrySet()) solrDoc.addField(e.getKey(), e.getValue());
     UpdateRequestProcessorChain processorChain = req.getCore().getUpdateProcessorChain(req.getParams());
-    UpdateRequestProcessor processor = processorChain.createProcessor(req, rsp);
-    AddUpdateCommand cmd = new AddUpdateCommand(req);
-    cmd.solrDoc = solrDoc;
-    log.info("Adding doc: " + doc);
-    processor.processAdd(cmd);
-    log.info("committing doc: " + doc);
-    processor.processCommit(new CommitUpdateCommand(req, false));
-    processor.finish();
+    try (UpdateRequestProcessor processor = processorChain.createProcessor(req, rsp)) {
+      AddUpdateCommand cmd = new AddUpdateCommand(req);
+      cmd.solrDoc = solrDoc;
+      log.info("Adding doc: " + doc);
+      processor.processAdd(cmd);
+      log.info("committing doc: " + doc);
+      processor.processCommit(new CommitUpdateCommand(req, false));
+      processor.finish();
+    }
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java b/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java
index 409b03f..1859f04 100644
--- a/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java
+++ b/solr/core/src/java/org/apache/solr/handler/ContentStreamHandlerBase.java
@@ -74,7 +74,11 @@ public abstract class ContentStreamHandlerBase extends RequestHandlerBase {
       }
     } finally {
       // finish the request
-      processor.finish();
+      try {
+        processor.finish();
+      } finally {
+        processor.close();
+      }
     }
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/java/org/apache/solr/search/function/FileFloatSource.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/function/FileFloatSource.java b/solr/core/src/java/org/apache/solr/search/function/FileFloatSource.java
index aa26ec8..58e9d2f 100644
--- a/solr/core/src/java/org/apache/solr/search/function/FileFloatSource.java
+++ b/solr/core/src/java/org/apache/solr/search/function/FileFloatSource.java
@@ -337,13 +337,15 @@ public class FileFloatSource extends ValueSource {
       FileFloatSource.resetCache();
       log.debug("readerCache has been reset.");
 
-      UpdateRequestProcessor processor =
-        req.getCore().getUpdateProcessingChain(null).createProcessor(req, rsp);
-      try{
+      UpdateRequestProcessor processor = req.getCore().getUpdateProcessingChain(null).createProcessor(req, rsp);
+      try {
         RequestHandlerUtils.handleCommit(req, processor, req.getParams(), true);
-      }
-      finally{
-        processor.finish();
+      } finally {
+        try {
+          processor.finish();
+        } finally {
+          processor.close();
+        }
       }
     }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/java/org/apache/solr/update/PeerSync.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/PeerSync.java b/solr/core/src/java/org/apache/solr/update/PeerSync.java
index 88900aa..ac07413 100644
--- a/solr/core/src/java/org/apache/solr/update/PeerSync.java
+++ b/solr/core/src/java/org/apache/solr/update/PeerSync.java
@@ -40,6 +40,7 @@ import org.apache.solr.cloud.ZkController;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.IOUtils;
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrInfoMBean;
@@ -865,6 +866,8 @@ public class PeerSync implements SolrMetricProducer {
         sreq.updateException = e;
         log.error(msg() + "Error applying updates from " + sreq.shards + " ,finish()", e);
         return false;
+      } finally {
+        IOUtils.closeQuietly(proc);
       }
     }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/java/org/apache/solr/update/UpdateLog.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/UpdateLog.java b/solr/core/src/java/org/apache/solr/update/UpdateLog.java
index aaa6b6a..16eff9c 100644
--- a/solr/core/src/java/org/apache/solr/update/UpdateLog.java
+++ b/solr/core/src/java/org/apache/solr/update/UpdateLog.java
@@ -53,6 +53,7 @@ import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
 import org.apache.solr.core.PluginInfo;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrInfoMBean;
@@ -1724,6 +1725,8 @@ public static final int VERSION_IDX = 1;
         } catch (IOException ex) {
           recoveryInfo.errors++;
           loglog.error("Replay exception: finish()", ex);
+        } finally {
+          IOUtils.closeQuietly(proc);
         }
 
       } finally {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/java/org/apache/solr/update/processor/DocExpirationUpdateProcessorFactory.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/processor/DocExpirationUpdateProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/DocExpirationUpdateProcessorFactory.java
index ffef1f3..332dba6 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/DocExpirationUpdateProcessorFactory.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/DocExpirationUpdateProcessorFactory.java
@@ -412,7 +412,11 @@ public final class DocExpirationUpdateProcessorFactory
             proc.processCommit(commit);
             
           } finally {
-            proc.finish();
+            try {
+              proc.finish();
+            } finally {
+              proc.close();
+            }
           }
 
           log.info("Finished periodic deletion of expired docs");

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessor.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessor.java
index 0a51d63..966299f 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessor.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/UpdateRequestProcessor.java
@@ -16,13 +16,18 @@
  */
 package org.apache.solr.update.processor;
 
+import java.io.Closeable;
 import java.io.IOException;
+import java.lang.invoke.MethodHandles;
 
+import org.apache.solr.common.SolrException;
 import org.apache.solr.update.AddUpdateCommand;
 import org.apache.solr.update.CommitUpdateCommand;
 import org.apache.solr.update.DeleteUpdateCommand;
 import org.apache.solr.update.MergeIndexesCommand;
 import org.apache.solr.update.RollbackUpdateCommand;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 
 /**
@@ -37,7 +42,9 @@ import org.apache.solr.update.RollbackUpdateCommand;
  * 
  * @since solr 1.3
  */
-public abstract class UpdateRequestProcessor {
+public abstract class UpdateRequestProcessor implements Closeable {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+  
   protected final UpdateRequestProcessor next;
 
   public UpdateRequestProcessor( UpdateRequestProcessor next) {
@@ -72,5 +79,24 @@ public abstract class UpdateRequestProcessor {
   public void finish() throws IOException {
     if (next != null) next.finish();    
   }
+  
+  @Override
+  public final void close() throws IOException {
+    UpdateRequestProcessor p = this;
+    while (p != null) {
+      try {
+        p.doClose();
+      } catch(Exception e) {
+        SolrException.log(log, "Exception closing processor", e);
+      }
+      p = p.next;
+    }
+  }
+
+  /**
+   * Override to implement resource release logic that *must* be called at the
+   * end of a request.
+   */
+  protected void doClose() {}
 }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java b/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java
index 8ad807c..a4b506b 100644
--- a/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java
+++ b/solr/core/src/test/org/apache/solr/search/TestSearchPerf.java
@@ -123,6 +123,7 @@ public class TestSearchPerf extends AbstractSolrTestCase {
       processor.processAdd(cmd);
     }
     processor.finish();
+    processor.close();
     req.close();
 
     assertU(commit());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java
index 7b077be..f4ab1fa 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java
@@ -33,6 +33,7 @@ import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.IOUtils;
 import org.apache.solr.common.util.SimpleOrderedMap;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.request.LocalSolrQueryRequest;
@@ -106,7 +107,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
    */
   public void testReflection() {
     for (Method method : TolerantUpdateProcessor.class.getMethods()) {
-      if (method.getDeclaringClass().equals(Object.class)) {
+      if (method.getDeclaringClass().equals(Object.class) || method.getName().equals("close")) {
         continue;
       }
       assertEquals("base class(es) has changed, TolerantUpdateProcessor needs updated to ensure it " +
@@ -427,8 +428,9 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
     }
     
     SolrQueryRequest req = new LocalSolrQueryRequest(core, requestParams);
+    UpdateRequestProcessor processor = null;
     try {
-      UpdateRequestProcessor processor = pc.createProcessor(req, rsp);
+      processor = pc.createProcessor(req, rsp);
       for(SolrInputDocument doc:docs) {
         AddUpdateCommand cmd = new AddUpdateCommand(req);
         cmd.solrDoc = doc;
@@ -437,6 +439,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
       processor.finish();
       
     } finally {
+      IOUtils.closeQuietly(processor);
       req.close();
     }
     return rsp;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f2f8154d/solr/core/src/test/org/apache/solr/update/processor/UpdateProcessorTestBase.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/processor/UpdateProcessorTestBase.java b/solr/core/src/test/org/apache/solr/update/processor/UpdateProcessorTestBase.java
index 24d818e..e069ee2 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/UpdateProcessorTestBase.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/UpdateProcessorTestBase.java
@@ -18,6 +18,7 @@ package org.apache.solr.update.processor;
 
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.IOUtils;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.SolrInputField;
@@ -130,6 +131,7 @@ public class UpdateProcessorTestBase extends SolrTestCaseJ4 {
     try {
       processor.finish();
     } finally {
+      IOUtils.closeQuietly(processor);
       req.close();
     }
   }