[asterixdb] branch master updated: [NO ISSUE] HTTP, halt improvements

[mb...@apache.org]

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

mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git


The following commit(s) were added to refs/heads/master by this push:
     new a603f49  [NO ISSUE] HTTP, halt improvements
     new c662b16  Merge branch 'gerrit/mad-hatter'
a603f49 is described below

commit a603f494d0c9fe53542df48d302f3722a6719364
Author: Michael Blow <mi...@couchbase.com>
AuthorDate: Thu Jun 4 11:26:47 2020 -0400

    [NO ISSUE] HTTP, halt improvements
    
    - don't propagate close to httpclient response stream, as it will block indefinitely
    - only wait for 60s to collect thread dump at halt; ensure we always halt once asked to
    
    Change-Id: I46de779e2f3ab0ff670ebec2966f364abcef0ecd
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/6623
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Reviewed-by: Till Westmann <ti...@apache.org>
---
 .../apache/hyracks/http/server/utils/HttpUtil.java | 19 +++++++++++++++---
 .../java/org/apache/hyracks/util/ExitUtil.java     | 23 +++++++++++++++++-----
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
index 77f6c6a..f9ada92 100644
--- a/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
+++ b/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/utils/HttpUtil.java
@@ -20,6 +20,7 @@ package org.apache.hyracks.http.server.utils;
 
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.io.Reader;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
@@ -194,13 +195,25 @@ public class HttpUtil {
     }
 
     public static void handleStreamInterruptibly(CloseableHttpResponse response,
-            ThrowingConsumer<InputStreamReader> streamProcessor, ExecutorService executor,
-            Supplier<String> taskDescription) throws IOException, InterruptedException, ExecutionException {
+            ThrowingConsumer<Reader> streamProcessor, ExecutorService executor, Supplier<String> taskDescription)
+            throws IOException, InterruptedException, ExecutionException {
         // we have to consume the stream in a separate thread, as it not stop on interrupt; we need to
         // instead close the connection to achieve the interrupt
         Future<Void> readFuture = executor.submit(() -> {
             InputStreamReader reader = new InputStreamReader(response.getEntity().getContent(), StandardCharsets.UTF_8);
-            streamProcessor.process(reader);
+            streamProcessor.process(new Reader() {
+                @Override
+                public int read(char[] cbuf, int off, int len) throws IOException {
+                    return reader.read(cbuf, off, len);
+                }
+
+                @Override
+                public void close() throws IOException {
+                    // this will block until the response is closed, which will cause hangs if the stream processor
+                    // tries to close the reader e.g. on processing failure
+                    LOGGER.debug("ignoring close on {}", reader);
+                }
+            });
             return null;
         });
         try {
diff --git a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/ExitUtil.java b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/ExitUtil.java
index 4991f86..abd9fda 100644
--- a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/ExitUtil.java
+++ b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/ExitUtil.java
@@ -20,6 +20,9 @@ package org.apache.hyracks.util;
 
 import java.lang.reflect.Field;
 import java.util.IdentityHashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 
@@ -63,6 +66,8 @@ public class ExitUtil {
     private static final ExitThread exitThread = new ExitThread();
     private static final ShutdownWatchdog watchdogThread = new ShutdownWatchdog();
     private static final MutableLong shutdownHaltDelay = new MutableLong(10 * 60 * 1000L); // 10 minutes default
+    private static final ExecutorService haltThreadDumpExecutor = Executors.newSingleThreadExecutor();
+    private static final long HALT_THREADDUMP_TIMEOUT_SECONDS = 60;
 
     static {
         watchdogThread.start();
@@ -97,11 +102,19 @@ public class ExitUtil {
     }
 
     public static synchronized void halt(int status, Level logLevel) {
-        LOGGER.log(logLevel, "JVM halting with status {}; thread dump at halt: {}", status,
-                ThreadDumpUtil.takeDumpString());
-        // try to give time for the log to be emitted...
-        LogManager.shutdown();
-        Runtime.getRuntime().halt(status);
+        try {
+            Future<?> future = haltThreadDumpExecutor.submit(() -> {
+                LOGGER.log(logLevel, "JVM halting with status {}; thread dump at halt: {}", status,
+                        ThreadDumpUtil.takeDumpString());
+                // try to give time for the log to be emitted...
+                LogManager.shutdown();
+            });
+            future.get(HALT_THREADDUMP_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+        } catch (Exception e) {
+            LOGGER.warn("exception logging thread dump on halt", e);
+        } finally {
+            Runtime.getRuntime().halt(status);
+        }
     }
 
     public static boolean registerShutdownHook(Thread shutdownHook) {