Multithreading best practices : constraining tasks newFixedThreadPool

Question

I want to launch a lot of tasks to run on a database of +-42Mio records. I want to run this in batches of 5000 records/time (results in 850 tasks). I also want to limit the number of threads (to 16) java starts to do this for me and I am using the current code to accomplish this task:

 ExecutorService executorService = Executors.newFixedThreadPool(16);
 for (int j = 1; j < 900 + 1; j++) {
     int start = (j - 1) * 5000;
     int stop = (j) * 5000- 1;
     FetcherRunner runner = new FetcherRunner(routes, start, stop);
     executorService.submit(runner);

     Thread t = new Thread(runner);
     threadsList.add(t);
     t.start();
 }

Is this the correct way to do this? Particularly as I have the impression that java just fires away all tasks ...(FetcherRunner implements runnable)

Answer 1

The first part using ExecutorService looks good:

...
FetcherRunner runner = new FetcherRunner(routes, start, stop);
executorService.submit(runner);

The part with Thread should not be there, I am assuming you have it there just to show how you had it before?

Update: Yes, you don't require the code after executorService.submit(runner), that is going to end up spawning a huge number of threads. If your objective is to wait for all submitted tasks to complete after the loop, then you can get a reference to Future when submitting tasks and wait on the Future, something like this:

ExecutorService executorService = Executors.newFixedThreadPool(16);
List<Future<Result>> futures = ..;
 for (int j = 1; j < 900+ 1; j++) {
 int start = (j - 1) * 5000;
 int stop = (j) * 5000- 1;
 FetcherRunner runner = new FetcherRunner(routes, start, stop);
 futures.add(executorService.submit(runner));

}
for (Future<Result> future:futures){
    future.get(); //Do something with the results..
}

Answer 2

Is this the correct way of working?

The first part is correct. But you shouldn't be creating and starting new Thread objects. When you submit the Runnable, the ExecutorService puts it on its queue, and then runs it when a worker thread becomes available.

.... I use the threadlist to detect when all my threads are finished so I can continue processing results.

Well if you do what you are currently doing, you are running each task twice. Worse still, the swarm of manually created threads will all try to run in parallel.

A simple way to make sure that all of the tasks have completed is to call awaitTermination(...) on the ExecutorService. (An orderly shutdown of the executor service will have the same effect ... if you don't intend to use it again.)

The other approach is to create a Future for each FetcherRunner's results, and attempt to get the result after all of the tasks have been submitted. That has the advantage that you can start processing early results before later ones have been produced. (However, if you don't need to ... or can't ... do that, using Futures won't achieve anything.)