From d18d0859ebfc46a18fd9140b42fb95f1da96380e Mon Sep 17 00:00:00 2001
From: David Mollitor <david.mollitor@cloudera.com>
Date: Fri, 22 Mar 2019 10:29:27 +0000
Subject: [PATCH] HADOOP-16181. HadoopExecutors shutdown Cleanup.

Author:    David Mollitor <david.mollitor@cloudera.com>
---
 .../util/concurrent/HadoopExecutors.java      | 58 ++++++++++++-------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/concurrent/HadoopExecutors.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/concurrent/HadoopExecutors.java
index 7a04c304a36..7c09d93d115 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/concurrent/HadoopExecutors.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/concurrent/HadoopExecutors.java
@@ -92,33 +92,51 @@ public static ScheduledExecutorService newSingleThreadScheduledExecutor(
   }
 
   /**
-   * Helper routine to shutdown a executorService.
+   * Helper routine to shutdown a {@link ExecutorService}. Will wait up to a
+   * certain timeout for the ExecutorService to gracefully shutdown. If the
+   * ExecutorService did not shutdown and there are still tasks unfinished after
+   * the timeout period, the ExecutorService will be notified to forcibly shut
+   * down. Another timeout period will be waited before giving up. So, at most,
+   * a shutdown will be allowed to wait up to twice the timeout value before
+   * giving up.
    *
-   * @param executorService - executorService
-   * @param logger          - Logger
-   * @param timeout         - Timeout
-   * @param unit            - TimeUnits, generally seconds.
+   * @param executorService ExecutorService to shutdown
+   * @param logger Logger
+   * @param timeout the maximum time to wait
+   * @param unit the time unit of the timeout argument
    */
   public static void shutdown(ExecutorService executorService, Logger logger,
       long timeout, TimeUnit unit) {
-    try {
-      if (executorService != null) {
-        executorService.shutdown();
-        try {
-          if (!executorService.awaitTermination(timeout, unit)) {
-            executorService.shutdownNow();
-          }
 
-          if (!executorService.awaitTermination(timeout, unit)) {
-            logger.error("Unable to shutdown properly.");
-          }
-        } catch (InterruptedException e) {
-          logger.error("Error attempting to shutdown.", e);
-          executorService.shutdownNow();
-        }
+    if (executorService == null) {
+      return;
+    }
+
+    try {
+      executorService.shutdown();
+
+      logger.info(
+          "Gracefully shutting down executor service. Waiting max {} {}",
+          timeout, unit);
+      if (!executorService.awaitTermination(timeout, unit)) {
+        logger.info(
+            "Executor service has not shutdown yet. Forcing. "
+                + "Will wait up to an additional {} {} for shutdown",
+            timeout, unit);
+        executorService.shutdownNow();
       }
+      if (executorService.awaitTermination(timeout, unit)) {
+        logger.info("Succesfully shutdown executor service");
+      } else {
+        logger.error("Unable to shutdown executor service after timeout {} {}",
+            (2 * timeout), unit);
+      }
+    } catch (InterruptedException e) {
+      logger.error("Interrupted while attempting to shutdown", e);
+      executorService.shutdownNow();
     } catch (Exception e) {
-      logger.error("Error during shutdown: ", e);
+      logger.warn("Exception closing executor service {}", e.getMessage());
+      logger.debug("Exception closing executor service", e);
       throw e;
     }
   }