Refactor BackgroundExecutor add submit() methods returning Future (#2121)

- Adds submit() methods that return Future
- Refactor internals to use DaemonScheduleThreadPool
- Delete the now unused DaemonExecutorService
- Tidy internals using wrapMDC() methods

ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBackgroundExecutor.java

@@ -1,12 +1,13 @@
 package io.ebeaninternal.server.core;
 
 import io.ebeaninternal.api.SpiBackgroundExecutor;
-import io.ebeaninternal.server.lib.DaemonExecutorService;
 import io.ebeaninternal.server.lib.DaemonScheduleThreadPool;
 import org.slf4j.MDC;
 
 import java.util.Map;
 import java.util.concurrent.Callable;
+import java.util.concurrent.Future;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
@@ -15,98 +16,94 @@ import java.util.concurrent.TimeUnit;
  */
 public class DefaultBackgroundExecutor implements SpiBackgroundExecutor {
 
-  private final DaemonScheduleThreadPool schedulePool;
-
-  private final DaemonExecutorService pool;
+  private final ScheduledExecutorService executor;
 
   /**
    * Construct the default implementation of BackgroundExecutor.
    */
   public DefaultBackgroundExecutor(int schedulePoolSize, int shutdownWaitSeconds, String namePrefix) {
-    this.pool = new DaemonExecutorService(shutdownWaitSeconds, namePrefix);
-    this.schedulePool = new DaemonScheduleThreadPool(schedulePoolSize, shutdownWaitSeconds, namePrefix + "-periodic-");
+    this.executor = new DaemonScheduleThreadPool(schedulePoolSize, shutdownWaitSeconds, namePrefix);
+  }
+
+  /**
+   * Wrap the task with MDC context if defined.
+   */
+  <T> Callable<T> wrapMDC(Callable<T> task) {
+    final Map<String, String> map = MDC.getCopyOfContextMap();
+    if (map == null) {
+      return task;
+    } else {
+      return () -> {
+        MDC.setContextMap(map);
+        try {
+          return task.call();
+        } finally {
+          MDC.clear();
+        }
+      };
+    }
+  }
+
+  /**
+   * Wrap the task with MDC context if defined.
+   */
+  Runnable wrapMDC(Runnable task) {
+    final Map<String, String> map = MDC.getCopyOfContextMap();
+    if (map == null) {
+      return task;
+    } else {
+      return () -> {
+        MDC.setContextMap(map);
+        try {
+          task.run();
+        } finally {
+          MDC.clear();
+        }
+      };
+    }
+  }
+
+  @Override
+  public <T> Future<T> submit(Callable<T> task) {
+    return executor.submit(wrapMDC(task));
   }
 
   /**
    * Execute a Runnable using a background thread.
    */
   @Override
-  public void execute(Runnable r) {
-    final Map<String, String> map = MDC.getCopyOfContextMap();
-    if (map == null) {
-      pool.execute(r);
-    } else {
-      pool.execute(() -> {
-        MDC.setContextMap(map);
-        try {
-          r.run();
-        } finally {
-          MDC.clear();
-        }
-      });
-    }
+  public Future<?> submit(Runnable task) {
+    return executor.submit(wrapMDC(task));
   }
 
   @Override
-  public void executePeriodically(Runnable r, long delay, TimeUnit unit) {
-    executePeriodically(r, delay, delay, unit);
+  public void execute(Runnable task) {
+    submit(task);
   }
 
   @Override
-  public void executePeriodically(Runnable r, long initialDelay, long delay, TimeUnit unit) {
-    final Map<String, String> map = MDC.getCopyOfContextMap();
-    if (map == null) {
-      schedulePool.scheduleWithFixedDelay(r, initialDelay, delay, unit);
-    } else {
-      schedulePool.scheduleWithFixedDelay(() -> {
-        MDC.setContextMap(map);
-        try {
-          r.run();
-        } finally {
-          MDC.clear();
-        }
-      }, initialDelay, delay, unit);
-    }
+  public void executePeriodically(Runnable task, long delay, TimeUnit unit) {
+    executePeriodically(task, delay, delay, unit);
   }
 
   @Override
-  public ScheduledFuture<?> schedule(Runnable r, long delay, TimeUnit unit) {
-    final Map<String, String> map = MDC.getCopyOfContextMap();
-    if (map == null) {
-      return schedulePool.schedule(r, delay, unit);
-    } else {
-      return schedulePool.schedule(() -> {
-        MDC.setContextMap(map);
-        try {
-          r.run();
-        } finally {
-          MDC.clear();
-        }
-      }, delay, unit);
-    }
+  public void executePeriodically(Runnable task, long initialDelay, long delay, TimeUnit unit) {
+    executor.scheduleWithFixedDelay(wrapMDC(task), initialDelay, delay, unit);
   }
 
   @Override
-  public <V> ScheduledFuture<V> schedule(Callable<V> c, long delay, TimeUnit unit) {
-    final Map<String, String> map = MDC.getCopyOfContextMap();
-    if (map == null) {
-      return schedulePool.schedule(c, delay, unit);
-    } else {
-      return schedulePool.schedule(() -> {
-        MDC.setContextMap(map);
-        try {
-          return c.call();
-        } finally {
-          MDC.clear();
-        }
-      }, delay, unit);
-    }
+  public ScheduledFuture<?> schedule(Runnable task, long delay, TimeUnit unit) {
+    return executor.schedule(wrapMDC(task), delay, unit);
+  }
+
+  @Override
+  public <V> ScheduledFuture<V> schedule(Callable<V> task, long delay, TimeUnit unit) {
+    return executor.schedule(wrapMDC(task), delay, unit);
   }
 
   @Override
   public void shutdown() {
-    pool.shutdown();
-    schedulePool.shutdown();
+    executor.shutdown();
   }
 
 }

ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java

@@ -451,16 +451,20 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer {
     backgroundExecutor.shutdown();
     // shutdown DataSource (if its an Ebean one)
     transactionManager.shutdown(shutdownDataSource, deregisterDriver);
+    dumpMetrics();
     shutdown = true;
     if (shutdownDataSource) {
       config.setDataSource(null);
     }
   }
 
-  private void shutdownPlugins() {
+  private void dumpMetrics() {
     if (config.isDumpMetricsOnShutdown()) {
       new DumpMetrics(this, config.getDumpMetricsOptions()).dump();
     }
+  }
+
+  private void shutdownPlugins() {
     for (Plugin plugin : serverPlugins) {
       try {
         plugin.shutdown();

ebean-core/src/main/java/io/ebeaninternal/server/core/bootup/BootupClasses.java

@@ -223,12 +223,9 @@ public class BootupClasses implements ClassFilter {
    */
   private <T> T create(Class<T> cls, boolean logOnException) {
     try {
-      // instantiate via found class
-      Constructor<T> constructor = cls.getConstructor();
-      return constructor.newInstance();
-
+      return cls.getConstructor().newInstance();
     } catch (NoSuchMethodException e) {
-      logger.debug("Ignore/expected - no default constructor", e);
+      logger.debug("Ignore/expected - no default constructor: " +e.getMessage());
       return null;
 
     } catch (Exception e) {

ebean-core/src/main/java/io/ebeaninternal/server/lib/DaemonExecutorService.java

@@ -1,78 +0,0 @@
-package io.ebeaninternal.server.lib;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.locks.ReentrantLock;
-
-/**
- * A "CachedThreadPool" based on Daemon threads.
- * <p>
- * The Threads are created as needed and once idle live for 60 seconds.
- */
-public final class DaemonExecutorService {
-
-  private static final Logger logger = LoggerFactory.getLogger(DaemonExecutorService.class);
-
-  private final ReentrantLock lock = new ReentrantLock(false);
-
-  private final String namePrefix;
-
-  private final int shutdownWaitSeconds;
-
-  private final ExecutorService service;
-
-  /**
-   * Construct the DaemonThreadPool.
-   *
-   * @param shutdownWaitSeconds the time in seconds allowed for the pool to shutdown nicely. After
-   *                            this the pool is forced to shutdown.
-   */
-  public DaemonExecutorService(int shutdownWaitSeconds, String namePrefix) {
-    this.service = Executors.newCachedThreadPool(new DaemonThreadFactory(namePrefix));
-    this.shutdownWaitSeconds = shutdownWaitSeconds;
-    this.namePrefix = namePrefix;
-  }
-
-  /**
-   * Execute the Runnable.
-   */
-  public void execute(Runnable runnable) {
-    service.execute(runnable);
-  }
-
-  /**
-   * Shutdown this thread pool nicely if possible.
-   * <p>
-   * This will wait a maximum of 20 seconds before terminating any threads still
-   * working.
-   * </p>
-   */
-  public void shutdown() {
-    lock.lock();
-    try {
-      if (service.isShutdown()) {
-        logger.debug("DaemonExecutorService[{}] already shut down", namePrefix);
-        return;
-      }
-      try {
-        logger.debug("DaemonExecutorService[{}] shutting down...", namePrefix);
-        service.shutdown();
-        if (!service.awaitTermination(shutdownWaitSeconds, TimeUnit.SECONDS)) {
-          logger.info("DaemonExecutorService[{}] shut down timeout exceeded. Terminating running threads.", namePrefix);
-          service.shutdownNow();
-        }
-
-      } catch (Exception e) {
-        logger.error("Error during shutdown of DaemonThreadPool[" + namePrefix + "]", e);
-        e.printStackTrace();
-      }
-    } finally {
-      lock.unlock();
-    }
-  }
-
-}

ebean-core/src/main/java/io/ebeaninternal/server/lib/DaemonScheduleThreadPool.java

@@ -9,15 +9,12 @@ import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * Daemon based ScheduleThreadPool.
- * <p>
- * Uses Daemon threads and hooks into shutdown event.
- * </p>
  */
 public final class DaemonScheduleThreadPool extends ScheduledThreadPoolExecutor {
 
   private static final Logger logger = LoggerFactory.getLogger(DaemonScheduleThreadPool.class);
 
-  private final ReentrantLock lock = new ReentrantLock(false);
+  private final ReentrantLock lock = new ReentrantLock();
 
   private final String namePrefix;
 
@@ -35,26 +32,25 @@ public final class DaemonScheduleThreadPool extends ScheduledThreadPoolExecutor
   /**
    * Shutdown this thread pool nicely if possible.
    * <p>
-   * This will wait a maximum of 20 seconds before terminating any threads still
-   * working.
-   * </p>
+   * This will wait a maximum of shutdownWaitSeconds seconds before
+   * terminating any threads still working.
    */
   @Override
   public void shutdown() {
     lock.lock();
     try {
       if (super.isShutdown()) {
-        logger.debug("DaemonScheduleThreadPool {} already shut down", namePrefix);
+        logger.debug("Already shutdown {}", namePrefix);
         return;
       }
       try {
-        logger.debug("DaemonScheduleThreadPool {} shutting down...", namePrefix);
+        logger.trace("Shutting down {} ...", namePrefix);
         super.shutdown();
         if (!super.awaitTermination(shutdownWaitSeconds, TimeUnit.SECONDS)) {
-          logger.info("DaemonScheduleThreadPool shut down timeout exceeded. Terminating running threads.");
+          logger.info("Shutdown wait timeout exceeded. Terminating running threads for {}", namePrefix);
           super.shutdownNow();
         }
-
+        logger.debug("Shutdown complete for {}", namePrefix);
       } catch (Exception e) {
         logger.error("Error during shutdown of " + namePrefix, e);
         e.printStackTrace();

ebean-core/src/main/java/io/ebeaninternal/server/lib/DaemonThreadFactory.java

@@ -1,6 +1,5 @@
 package io.ebeaninternal.server.lib;
 
-
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -9,42 +8,28 @@ import java.util.concurrent.atomic.AtomicInteger;
  * <p>
  * Daemon threads do not stop a JVM stopping. If an application only has Daemon
  * threads left it will shutdown.
- * </p>
  * <p>
  * In using Daemon threads you need to either not care about being interrupted
  * on shutdown or register with the JVM shutdown hook to perform a nice shutdown
  * of the daemon threads etc.
- * </p>
- *
- * @author rbygrave
  */
 public class DaemonThreadFactory implements ThreadFactory {
 
-  private static final AtomicInteger poolNumber = new AtomicInteger(1);
-
-  private final ThreadGroup group;
-
   private final AtomicInteger threadNumber = new AtomicInteger(1);
 
   private final String namePrefix;
 
   public DaemonThreadFactory(String namePrefix) {
-    SecurityManager s = System.getSecurityManager();
-    this.group = (s != null) ? s.getThreadGroup() : Thread.currentThread().getThreadGroup();
-    this.namePrefix = namePrefix != null ? namePrefix : "pool-" + poolNumber.getAndIncrement() + "-thread-";
+    this.namePrefix = namePrefix;
   }
 
   @Override
   public Thread newThread(Runnable r) {
-
-    Thread t = new Thread(group, r, namePrefix + threadNumber.getAndIncrement(), 0);
-
+    Thread t = new Thread(null, r, namePrefix + threadNumber.getAndIncrement(), 0);
     t.setDaemon(true);
-
     if (t.getPriority() != Thread.NORM_PRIORITY) {
       t.setPriority(Thread.NORM_PRIORITY);
     }
-
     return t;
   }
 }