[lucene-solr] branch branch_8x updated: SOLR-12120: Do not fail the main request if synchronous auditing fails, log ERROR Document that sub classes should call super.close() or a new waitForQueueToDrain() before closing itself

[ja...@apache.org]

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

janhoy 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 77a4604  SOLR-12120: Do not fail the main request if synchronous auditing fails, log ERROR Document that sub classes should call super.close() or a new waitForQueueToDrain() before closing itself
77a4604 is described below

commit 77a4604c39e192edd785d30c905c4b604b67a4e2
Author: Jan Høydahl <ja...@apache.org>
AuthorDate: Tue Apr 9 14:25:52 2019 +0200

    SOLR-12120: Do not fail the main request if synchronous auditing fails, log ERROR
    Document that sub classes should call super.close() or a new waitForQueueToDrain() before closing itself
    
    (cherry picked from commit 3e628b562cb57349503e8ccdfe4909aedcbe78b2)
---
 .../apache/solr/security/AuditLoggerPlugin.java    | 27 +++++++++++++++++-----
 .../solr/security/MultiDestinationAuditLogger.java |  2 +-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/security/AuditLoggerPlugin.java b/solr/core/src/java/org/apache/solr/security/AuditLoggerPlugin.java
index 36affa1..80fe11e 100644
--- a/solr/core/src/java/org/apache/solr/security/AuditLoggerPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/AuditLoggerPlugin.java
@@ -150,7 +150,7 @@ public abstract class AuditLoggerPlugin implements Closeable, Runnable, SolrInfo
         audit(event);
       } catch(Exception e) {
         numErrors.mark();
-        throw e;
+        log.error("Exception when attempting to audit log", e);
       } finally {
         totalTime.inc(timer.stop());
       }
@@ -209,7 +209,7 @@ public abstract class AuditLoggerPlugin implements Closeable, Runnable, SolrInfo
         log.warn("Interrupted while waiting for next audit log event");
         Thread.currentThread().interrupt();
       } catch (Exception ex) {
-        log.warn("Exception when attempting to audit log asynchronously", ex);
+        log.error("Exception when attempting to audit log asynchronously", ex);
         numErrors.mark();
       }
     }
@@ -309,20 +309,35 @@ public abstract class AuditLoggerPlugin implements Closeable, Runnable, SolrInfo
     }
   }
 
+  /**
+   * Waits 30s for async queue to drain, then closes executor threads.
+   * Subclasses should either call <code>super.close()</code> or {@link #waitForQueueToDrain(int)}
+   * <b>before</b> shutting itself down to make sure they can complete logging events in the queue. 
+   */
   @Override
   public void close() throws IOException {
     if (async && executorService != null) {
+      waitForQueueToDrain(30);
+      closed = true;
+      log.info("Shutting down async Auditlogger background thread(s)");
+      executorService.shutdownNow();
+    }
+  }
+
+  /**
+   * Blocks until the async event queue is drained
+   * @param timeoutSeconds number of seconds to wait for queue to drain
+   */
+  protected void waitForQueueToDrain(int timeoutSeconds) {
+    if (async && executorService != null) {
       int timeSlept = 0;
-      while (!queue.isEmpty() && timeSlept < 30) {
+      while (!queue.isEmpty() && timeSlept < timeoutSeconds) {
         try {
           log.info("Async auditlogger queue still has {} elements, sleeping to let it drain...", queue.size());
           Thread.sleep(1000);
           timeSlept ++;
         } catch (InterruptedException ignored) {}
       }
-      closed = true;
-      log.info("Shutting down async Auditlogger background thread(s)");
-      executorService.shutdownNow();
     }
   }
 
diff --git a/solr/core/src/java/org/apache/solr/security/MultiDestinationAuditLogger.java b/solr/core/src/java/org/apache/solr/security/MultiDestinationAuditLogger.java
index 9aaae5c..7ad6b5b 100644
--- a/solr/core/src/java/org/apache/solr/security/MultiDestinationAuditLogger.java
+++ b/solr/core/src/java/org/apache/solr/security/MultiDestinationAuditLogger.java
@@ -126,6 +126,7 @@ public class MultiDestinationAuditLogger extends AuditLoggerPlugin implements Re
 
   @Override
   public void close() throws IOException {
+    super.close(); // Waiting for queue to drain before shutting down the loggers
     plugins.forEach(p -> {
       try {
         p.close();
@@ -133,6 +134,5 @@ public class MultiDestinationAuditLogger extends AuditLoggerPlugin implements Re
         log.error("Exception trying to close {}", p.getName());
       }
     });
-    super.close();
   }
 }