Merge pull request #155 from kkoz/thread-pools

Run PixelDataThread Application Events in SYSTEM Thread Pool

src/main/java/ome/services/pixeldata/PixelDataThread.java

@@ -30,6 +30,7 @@
 import ome.services.util.ExecutionThread;
 import ome.services.util.Executor;
 import ome.services.util.ReadOnlyStatus;
+import ome.services.util.Executor.Priority;
 import ome.system.EventContext;
 import ome.system.Principal;
 import ome.system.ServiceFactory;
@@ -345,8 +346,12 @@ public void onApplicationEvent(final MissingPyramidMessage mpm) {
         if (null == ec.getCurrentUserId()) {
             throw new InternalException("No user! Must be wrapped by call to Executor?");
         }
-
-        Future<EventLog> future = this.executor.submit(cd.getContext(),
+        /* Because other tasks depend on these completing, we submit these at
+         * SYSTEM priority so that a new thread is always created to handle them
+         * and we prevent the possibility of deadlock
+         */
+        Future<EventLog> future = this.executor.submit(Priority.SYSTEM,
+                cd.getContext(),
                 new Callable<EventLog>(){
             public EventLog call() throws Exception {
                 return makeEvent(ec, mpm);

src/main/java/ome/services/scheduler/ThreadPool.java

@@ -47,6 +47,12 @@ public class ThreadPool extends ThreadPoolExecutor {
      */
     private final long backgroundTimeout;
 
+    /**
+     * Default constructor. Unlike the argument constructor, it effectively
+     * has no queue for tasks and will always create a new thread to
+     * accommodate new tasks.
+     * Background tasks are limited to 10.
+     */
     public ThreadPool() {
         // Values from Executors.newCachedThreadPool
         super(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS,
@@ -56,6 +62,28 @@ public ThreadPool() {
 
     }
 
+    /**
+     * This constructor creates a thread pool with an unbounded queue.
+     * This means that {@code minThreads} will set the number of core threads,
+     * which is the maximum number of threads active at one time when an
+     * unbounded queue is used. This also means that {@code maxThreads} does
+     * nothing, since threads are created beyond the core pool size only when the
+     * queue is full. Additionally, by default core threads never time out,
+     * so {@code msTimeout} also does nothing.
+     * See the {@link ThreadPoolExecutor} docs for more information.
+     * @param minThreads Sets the core pool size which is also the MAX pool size
+     * @param maxThreads This does NOTHING
+     * @param msTimeout This does NOTHING
+     * @param backgroundThreads Parameter name is a bit misleading. It is
+     * the maximum number of background tasks that can be submitted
+     * (queued or running) at once. The background threads come from the same
+     * pool, which is limited to {@code minThreads} in size, so if
+     * {@code minThreads} is lower, that will control the maximum number
+     * of threads capable of running background tasks.
+     * @param backgroundTimeout If more than {@code backgroundThreads}
+     * tasks are queued or processing, this is how long a task will wait
+     * to be submitted before being dropped
+     */
     public ThreadPool(int minThreads, int maxThreads, long msTimeout,
             int backgroundThreads, long backgroundTimeout) {
         super(minThreads, maxThreads, msTimeout, TimeUnit.MILLISECONDS,

src/main/resources/omero-server.properties

@@ -293,21 +293,30 @@ omero.sessions.max_user_time_to_live=0
 ## for internal server threads.
 #############################################
 
-# Number of threads that will be kept waiting
-# at all times.
+# Maximum and minimum number of threads that can
+# simultaneously run at the "USER" and "BACKGROUND"
+# priority level. Internal system threads may still run.
+# Note when setting this that these threads do not
+# time out.
 omero.threads.min_threads=5
 
+# This setting does nothing.
+# See https://github.com/ome/omero-server/issues/154
+# And https://github.com/ome/omero-server/pull/155
 omero.threads.idle_timeout=5000
+
 omero.threads.cancel_timeout=5000
 
-# Maximum number of threads that can simultaneously
-# run at the "USER" priority level. Internal system
-# threads may still run.
+# This setting does nothing.
+# See https://github.com/ome/omero-server/issues/154
+# And https://github.com/ome/omero-server/pull/155
 omero.threads.max_threads=50
 
-# Number of threads from the max_threads pool that can
+# Number of threads from the min_threads pool that can
 # be used at any given time for background tasks like
-# import.
+# import. Note that if this value is less than min_threads,
+# min_threads will limit the number of background
+# tasks which can run simultaneously.
 omero.threads.background_threads=10
 
 # Number of milliseconds to wait for a slot in the