Executors.newCachedThreadPool() considered harmful boisvert - 24 Jun 2014

This is more of a public-service announcement blog-post...

That’s right, Executors.newCachedThreadPool() isn't a great choice for server code that's servicing multiple clients and concurrent requests.

Why? There are basically two (related) problems with it:

1) It's unbounded, which means that you're opening the door for anyone to cripple your JVM by simply injecting more work into the service (DoS attack). Threads consume a non-negligible amount of memory and also increase memory consumption based on their work-in-progress, so it's quite easy to topple a server this way (unless you have other circuit-breakers in place).

2) The unbounded problem is exacerbated by the fact that the Executor is fronted by a SynchronousQueue which means there's a direct handoff between the task-giver and the thread pool. Each new task will create a new thread if all existing threads are busy. This is generally a bad strategy for server code. When the CPU gets saturated, existing tasks take longer to finish. Yet more tasks are being submitted and more threads created, so tasks take longer and longer to complete. When the CPU is saturated, more threads is definitely not what the server needs.

Here are my recommendations:

Use a fixed-size thread pool (Executors.newFixedThreadPool) or a ThreadPoolExecutor with a set maximum number of threads;

Figure out how many threads you need to keep each CPU core saturated (but not too much). A good rule of thumb is to start around 10 threads per core for a workload that involves disk and network I/O, and see if it’s sufficient to keep all CPUs busy. Increase or decrease the number of maximum threads based on results obtained from load testing. Multiply this factor by Runtime.getRuntime.availableProcessors to obtain the total size of the thread pool.

I generally don't recommend queueing within a server's main request-processing loop. If you need queuing behavior, use a fixed-size ArrayBlockingQueue -- it's more efficient than LinkedBlockingQueue. Use only if you absolutely need it. A system with queuing is more difficult to understand and tune.

The default RejectedExecutionHandler is AbortPolicy, which throws a runtime RejectedExecutionException when the queue (if any) or the thread pool reach maximum capacity. This is not a bad default but I generally want to guarantee execution in spite of this, so I advise using the CallerRunsPolicy instead. The CallerRunsPolicy also helps to avoid deadlocks (many threads blocking on each other) in the case where tasks may have internal dependencies.

Always provide your own ThreadFactory so all your threads are named appropriately and have the daemon flag set. Your thread pool shouldn't keep the application alive. That's the responsibility of the application itself (i.e., main thread).

The result of all this should look similar to this (in Scala syntax):

object ThreadPoolHelpers { private def cpus = Runtime . getRuntime . availableProcessors def daemonThreadFactory ( name : String ) = new ThreadFactory { private val count = new AtomicInteger () override def newThread ( r : Runnable ) = { val thread = new Thread ( r ) thread . setName ( s "$name-${count.incrementAndGet}" ) thread . setDaemon ( true ) thread } } /** Core thread pool to be used for concurrent request-processing */ def newCoreThreadPool ( name : String ) = { // use direct handoff (SynchronousQueue) + CallerRunsPolicy to avoid deadlocks // since tasks may have internal dependencies. val pool = new ThreadPoolExecutor ( /* core size */ ( 5 * cpus ), /* max size */ ( 15 * cpus ), /* idle timeout */ 60 , TimeUnit . SECONDS , new SynchronousQueue [ Runnable ](), daemonThreadFactory ( name ) ) pool . setRejectedExecutionHandler ( new ThreadPoolExecutor . CallerRunsPolicy ) pool } }

About that ScheduledExecutorService ...

While we're at it, one last piece of advice is to separate request-processing threads from scheduled task execution. It’s often tempting to share a ScheduledExecutorService for both request-processing and scheduled tasks but I recommend using separate thread pools for two reasons:

1) separation allows you to balance the relative processing capacity of your request-processing workload vs scheduled tasks. In particular, if you use scheduled tasks to act as timeouts for incoming requests (e.g. if you are using Futures that get queued into your main thread pool), the separation can have a substantial impact on the timeliness of your timeouts.

2) ThreadPoolExecutor is more efficient for request-processing since it does not have to order incoming tasks with respect to other scheduled tasks. This is especially true if you are using a SynchronousQueue and requests are dispatched to threads in a straight-through fashion (no queuing).

I hope this advice is relevant to you and your server code. If not, please drop me a line!

Please enable JavaScript to view the comments powered by Disqus.

Disqus