From e0eff149b56b25048191a0a9d891e4d70f74b2b3 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Fri, 15 Jul 2016 20:48:41 +1200 Subject: [PATCH] #775 - Change DefaultBackgroundExecutor to use JDK cached thread pool (rather than configure min/max size etc) --- .../com/avaje/ebean/config/ServerConfig.java | 47 +--------- .../core/DefaultBackgroundExecutor.java | 24 ++---- .../server/core/DefaultContainer.java | 6 +- .../server/lib/DaemonExecutorService.java | 72 ++++++++++++++++ .../server/lib/DaemonScheduleThreadPool.java | 20 ----- .../server/lib/DaemonThreadPool.java | 86 ------------------- .../avaje/ebean/config/ServerConfigTest.java | 4 + .../core/DefaultBackgroundExecutorTest.java | 54 ++++++++++++ .../lib/sql/TestDataSourceMaxWithEntity.java | 2 +- 9 files changed, 141 insertions(+), 174 deletions(-) create mode 100644 src/main/java/com/avaje/ebeaninternal/server/lib/DaemonExecutorService.java delete mode 100644 src/main/java/com/avaje/ebeaninternal/server/lib/DaemonThreadPool.java create mode 100644 src/test/java/com/avaje/ebeaninternal/server/core/DefaultBackgroundExecutorTest.java diff --git a/src/main/java/com/avaje/ebean/config/ServerConfig.java b/src/main/java/com/avaje/ebean/config/ServerConfig.java index 217f70034..47ee62914 100644 --- a/src/main/java/com/avaje/ebean/config/ServerConfig.java +++ b/src/main/java/com/avaje/ebean/config/ServerConfig.java @@ -384,9 +384,6 @@ public class ServerConfig { // configuration for the background executor service (thread pool) private int backgroundExecutorSchedulePoolSize = 1; - private int backgroundExecutorCorePoolSize = 1; - private int backgroundExecutorMaxPoolSize = 8; - private int backgroundExecutorIdleSecs = 60; private int backgroundExecutorShutdownSecs = 30; // defaults for the L2 bean caching @@ -1070,48 +1067,6 @@ public class ServerConfig { this.backgroundExecutorSchedulePoolSize = backgroundExecutorSchedulePoolSize; } - /** - * Return the Background executor core pool size. - */ - public int getBackgroundExecutorCorePoolSize() { - return backgroundExecutorCorePoolSize; - } - - /** - * Set the Background executor core pool size. - */ - public void setBackgroundExecutorCorePoolSize(int backgroundExecutorCorePoolSize) { - this.backgroundExecutorCorePoolSize = backgroundExecutorCorePoolSize; - } - - /** - * Return the Background executor max pool size. - */ - public int getBackgroundExecutorMaxPoolSize() { - return backgroundExecutorMaxPoolSize; - } - - /** - * Set the Background executor max pool size. - */ - public void setBackgroundExecutorMaxPoolSize(int backgroundExecutorMaxPoolSize) { - this.backgroundExecutorMaxPoolSize = backgroundExecutorMaxPoolSize; - } - - /** - * Return the Background executor idle seconds. - */ - public int getBackgroundExecutorIdleSecs() { - return backgroundExecutorIdleSecs; - } - - /** - * Set the Background executor idle seconds. - */ - public void setBackgroundExecutorIdleSecs(int backgroundExecutorIdleSecs) { - this.backgroundExecutorIdleSecs = backgroundExecutorIdleSecs; - } - /** * Return the Background executor shutdown seconds. This is the time allowed for the pool to shutdown nicely * before it is forced shutdown. @@ -2399,6 +2354,8 @@ public class ServerConfig { autoCommitMode = p.getBoolean("autoCommitMode", autoCommitMode); useJtaTransactionManager = p.getBoolean("useJtaTransactionManager", useJtaTransactionManager); + backgroundExecutorSchedulePoolSize = p.getInt("backgroundExecutorSchedulePoolSize", backgroundExecutorSchedulePoolSize); + backgroundExecutorShutdownSecs = p.getInt("backgroundExecutorShutdownSecs", backgroundExecutorShutdownSecs); disableClasspathSearch = p.getBoolean("disableClasspathSearch", disableClasspathSearch); currentUserProvider = createInstance(p, CurrentUserProvider.class, "currentUserProvider", currentUserProvider); databasePlatform = createInstance(p, DatabasePlatform.class, "databasePlatform", databasePlatform); diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultBackgroundExecutor.java b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultBackgroundExecutor.java index 81ba2af0b..e71bc42ef 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultBackgroundExecutor.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultBackgroundExecutor.java @@ -1,35 +1,25 @@ package com.avaje.ebeaninternal.server.core; -import java.util.concurrent.TimeUnit; - import com.avaje.ebeaninternal.api.SpiBackgroundExecutor; +import com.avaje.ebeaninternal.server.lib.DaemonExecutorService; import com.avaje.ebeaninternal.server.lib.DaemonScheduleThreadPool; -import com.avaje.ebeaninternal.server.lib.DaemonThreadPool; + +import java.util.concurrent.TimeUnit; /** * The default implementation of the BackgroundExecutor. */ public class DefaultBackgroundExecutor implements SpiBackgroundExecutor { - private final DaemonThreadPool pool; - private final DaemonScheduleThreadPool schedulePool; + private final DaemonExecutorService pool; + /** * Construct the default implementation of BackgroundExecutor. - * - * @param corePoolSize - * the core size of the thread pool. - * @param maximumPoolSize - * the maximum pool size before jobs are queued - * @param keepAliveSecs - * the time in seconds idle threads are keep alive - * @param shutdownWaitSeconds - * the time in seconds allowed for the pool to shutdown nicely. - * After this the pool is forced to shutdown. */ - public DefaultBackgroundExecutor(int schedulePoolSize, int corePoolSize, int maximumPoolSize, long keepAliveSecs,int shutdownWaitSeconds, String namePrefix) { - this.pool = new DaemonThreadPool(corePoolSize, maximumPoolSize, keepAliveSecs, shutdownWaitSeconds, namePrefix); + public DefaultBackgroundExecutor(int schedulePoolSize, int shutdownWaitSeconds, String namePrefix) { + this.pool = new DaemonExecutorService(shutdownWaitSeconds, namePrefix); this.schedulePool = new DaemonScheduleThreadPool(schedulePoolSize, shutdownWaitSeconds, namePrefix+"-periodic-"); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultContainer.java b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultContainer.java index 8ff82fcd9..210fde590 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultContainer.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultContainer.java @@ -78,14 +78,10 @@ public class DefaultContainer implements SpiContainer { private SpiBackgroundExecutor createBackgroundExecutor(ServerConfig serverConfig) { String namePrefix = "ebean-" + serverConfig.getName(); - int schedulePoolSize = serverConfig.getBackgroundExecutorSchedulePoolSize(); - int corePoolSize = serverConfig.getBackgroundExecutorCorePoolSize(); - int maxPoolSize = serverConfig.getBackgroundExecutorMaxPoolSize(); - int idleSecs = serverConfig.getBackgroundExecutorIdleSecs(); int shutdownSecs = serverConfig.getBackgroundExecutorShutdownSecs(); - return new DefaultBackgroundExecutor(schedulePoolSize, corePoolSize, maxPoolSize, idleSecs, shutdownSecs, namePrefix); + return new DefaultBackgroundExecutor(schedulePoolSize, shutdownSecs, namePrefix); } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonExecutorService.java b/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonExecutorService.java new file mode 100644 index 000000000..bb92f84c3 --- /dev/null +++ b/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonExecutorService.java @@ -0,0 +1,72 @@ +package com.avaje.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; + +/** + * A "CachedThreadPool" based on Daemon threads. + *

+ * 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 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. + *

+ * This will wait a maximum of 20 seconds before terminating any threads still + * working. + *

+ */ + public void shutdown() { + synchronized (this) { + 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(); + } + } + } + +} diff --git a/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonScheduleThreadPool.java b/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonScheduleThreadPool.java index 8362defd4..f420e59d2 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonScheduleThreadPool.java +++ b/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonScheduleThreadPool.java @@ -11,8 +11,6 @@ import org.slf4j.LoggerFactory; *

* Uses Daemon threads and hooks into shutdown event. *

- * - * @author rbygrave */ public final class DaemonScheduleThreadPool extends ScheduledThreadPoolExecutor { @@ -26,19 +24,11 @@ public final class DaemonScheduleThreadPool extends ScheduledThreadPoolExecutor * Construct the DaemonScheduleThreadPool. */ public DaemonScheduleThreadPool(int coreSize, int shutdownWaitSeconds, String namePrefix) { - super(coreSize, new DaemonThreadFactory(namePrefix)); this.namePrefix = namePrefix; this.shutdownWaitSeconds = shutdownWaitSeconds; } - /** - * Register a shutdown hook with the JVM Runtime. - */ - public void registerShutdownHook() { - Runtime.getRuntime().addShutdownHook(new ShutdownHook()); - } - /** * Shutdown this thread pool nicely if possible. *

@@ -66,14 +56,4 @@ public final class DaemonScheduleThreadPool extends ScheduledThreadPoolExecutor } } } - - /** - * Fired by the JVM Runtime shutdown. - */ - private class ShutdownHook extends Thread { - @Override - public void run() { - shutdown(); - } - } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonThreadPool.java b/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonThreadPool.java deleted file mode 100644 index c10a1251a..000000000 --- a/src/main/java/com/avaje/ebeaninternal/server/lib/DaemonThreadPool.java +++ /dev/null @@ -1,86 +0,0 @@ -package com.avaje.ebeaninternal.server.lib; - -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * The Thread Pool based on Daemon threads. - * - * @author rbygrave - */ -public final class DaemonThreadPool extends ThreadPoolExecutor { - - private static final Logger logger = LoggerFactory.getLogger(DaemonThreadPool.class); - - private final String namePrefix; - - private final int shutdownWaitSeconds; - - /** - * Construct the DaemonThreadPool. - * - * @param coreSize - * the core size of the thread pool. - * @param keepAliveSecs - * the time in seconds idle threads are keep alive - * @param shutdownWaitSeconds - * the time in seconds allowed for the pool to shutdown nicely. After - * this the pool is forced to shutdown. - */ - public DaemonThreadPool(int coreSize, int maximumPoolSize, long keepAliveSecs, int shutdownWaitSeconds, String namePrefix) { - - super(coreSize, maximumPoolSize, keepAliveSecs, TimeUnit.SECONDS, new LinkedBlockingQueue(), new DaemonThreadFactory(namePrefix)); - allowCoreThreadTimeOut(true); - this.shutdownWaitSeconds = shutdownWaitSeconds; - this.namePrefix = namePrefix; - } - - /** - * Register a shutdown hook with the JVM Runtime. - */ - public void registerShutdownHook() { - Runtime.getRuntime().addShutdownHook(new ShutdownHook()); - } - - /** - * Shutdown this thread pool nicely if possible. - *

- * This will wait a maximum of 20 seconds before terminating any threads still - * working. - *

- */ - public void shutdown() { - synchronized (this) { - if (super.isShutdown()) { - logger.debug("DaemonThreadPool[" + namePrefix + "] already shut down"); - return; - } - try { - logger.debug("DaemonThreadPool[" + namePrefix + "] shutting down..."); - super.shutdown(); - if (!super.awaitTermination(shutdownWaitSeconds, TimeUnit.SECONDS)) { - logger.info("DaemonThreadPool[" + namePrefix+ "] shut down timeout exceeded. Terminating running threads."); - super.shutdownNow(); - } - - } catch (Exception e) { - logger.error("Error during shutdown of DaemonThreadPool[" + namePrefix + "]", e); - e.printStackTrace(); - } - } - } - - /** - * Fired by the JVM Runtime shutdown. - */ - private class ShutdownHook extends Thread { - @Override - public void run() { - shutdown(); - } - } -} diff --git a/src/test/java/com/avaje/ebean/config/ServerConfigTest.java b/src/test/java/com/avaje/ebean/config/ServerConfigTest.java index 6274e5a7e..202caca0f 100644 --- a/src/test/java/com/avaje/ebean/config/ServerConfigTest.java +++ b/src/test/java/com/avaje/ebean/config/ServerConfigTest.java @@ -32,6 +32,8 @@ public class ServerConfigTest { props.setProperty("dbuuid","binary"); props.setProperty("jdbcFetchSizeFindEach", "42"); props.setProperty("jdbcFetchSizeFindList", "43"); + props.setProperty("backgroundExecutorShutdownSecs", "98"); + props.setProperty("backgroundExecutorSchedulePoolSize", "4"); serverConfig.loadFromProperties(props); @@ -40,6 +42,8 @@ public class ServerConfigTest { assertEquals(ServerConfig.DbUuid.BINARY, serverConfig.getDbUuid()); assertEquals(42, serverConfig.getJdbcFetchSizeFindEach()); assertEquals(43, serverConfig.getJdbcFetchSizeFindList()); + assertEquals(4, serverConfig.getBackgroundExecutorSchedulePoolSize()); + assertEquals(98, serverConfig.getBackgroundExecutorShutdownSecs()); serverConfig.setPersistBatch(PersistBatch.NONE); serverConfig.setPersistBatchOnCascade(PersistBatch.NONE); diff --git a/src/test/java/com/avaje/ebeaninternal/server/core/DefaultBackgroundExecutorTest.java b/src/test/java/com/avaje/ebeaninternal/server/core/DefaultBackgroundExecutorTest.java new file mode 100644 index 000000000..1a6569208 --- /dev/null +++ b/src/test/java/com/avaje/ebeaninternal/server/core/DefaultBackgroundExecutorTest.java @@ -0,0 +1,54 @@ +package com.avaje.ebeaninternal.server.core; + +import org.junit.Ignore; +import org.junit.Test; + +public class DefaultBackgroundExecutorTest { + + @Test @Ignore + public void shutdown_when_running_expect_waitAndNiceShutdown() throws Exception { + + DefaultBackgroundExecutor es = new DefaultBackgroundExecutor(1, 20, "test"); + + es.execute(new RunFor(3000,"a")); + es.execute(new RunFor(3000,"b")); + es.execute(new RunFor(3000,"c")); + + es.shutdown(); + } + + @Test @Ignore + public void shutdown_when_rougeRunnable_expect_InterruptedException() throws Exception { + + DefaultBackgroundExecutor es = new DefaultBackgroundExecutor(1, 10, "test"); + + es.execute(new RunFor(300000,"a")); + es.execute(new RunFor(3000,"b")); + es.execute(new RunFor(3000,"c")); + + es.shutdown(); + } + + + class RunFor implements Runnable { + + final long wait; + final String id; + + RunFor(long wait, String id) { + this.wait = wait; + this.id = id; + } + + @Override + public void run() { + try { + System.out.println("start " + id); + Thread.sleep(wait); + System.out.println("done " + id); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + } +} \ No newline at end of file diff --git a/src/test/java/com/avaje/ebeaninternal/server/lib/sql/TestDataSourceMaxWithEntity.java b/src/test/java/com/avaje/ebeaninternal/server/lib/sql/TestDataSourceMaxWithEntity.java index 5ea6bdcfa..8abda7611 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/lib/sql/TestDataSourceMaxWithEntity.java +++ b/src/test/java/com/avaje/ebeaninternal/server/lib/sql/TestDataSourceMaxWithEntity.java @@ -22,7 +22,7 @@ public class TestDataSourceMaxWithEntity extends BaseTestCase { EbeanServer server = Ebean.getServer(null); - DefaultBackgroundExecutor bg = new DefaultBackgroundExecutor(1, 1, 2, 180, 30, "testDs"); + DefaultBackgroundExecutor bg = new DefaultBackgroundExecutor(1, 30, "testDs"); try { for (int i = 0; i < 12; i++) {