Do I need to shutdown ExecutorService that processes data on a consistent basis in java-CodePudding

I have a Spirngboot application that runs tasks in separate threads every 24 hours. Tasks take a little bit of time but after that the app becomes idle before the next batch triggers the api the next day. Demo code is as follows:

    private ExecutorService es = Executors.newFixedThreadPool(2);

    @PostMapping
    public void startTest(@RequestBody DummyModel dummy) throws InterruptedException {
        int i = 0;
        while(i<3) {
            es.execute(new ListProcessing(dummy));
            es.execute(new AnotherListProcessing(dummy));
            i  ;
        }
        // because methods are async this line is reached in an instant  before processing is done
    }

Now as you can see, I have no es.shutdown() after my while loop. In most articls and discussions, there seem to be an emphisis on how important a .shutdown() command is after you have completed your work. Adding it after my while loop would mean that that the next post request I do will result in error (which makes sense since .shutdown() states that it will not allow new tasks after the existing tasks are completed).

Now, I want to know is it really important to do .shutdown() here? My app will receive a post request once a day everyday, so ExecutorService will be used frequently. Are there downsides of not shuting down your Executor for a prolong period of time? And if I do really need to shut it down everytime ExecutorService was used, how can I do it so that app is ready to receive a new request the following day?

I was thinking of adding these lines after my while loop:

        es.shutdown();
        es = Executors.newFixedThreadPool(2);

It works, but that seems:

a) unnecessary (why shut it down and waste effort recreating it)

b) that, just looks & feels wrong. There has to be a better way.

Update

So it seems that you can either create a custom ThreadPoolExecutor (which for my simple usecase seems like an overkill) or you can use CachedThreadPool option of ExecutorService (thou it will attempt to use as many threads as there are available, so if you only need to use n number of them that option may not be for you).

Update 2 As explained by Thomas and Deinum, we can use customl defined executor. It does exactly what ExecutorService does clean up and also allows for quick & easy way to configure it. For anyone curious, here is how I implemented it:

    @Autowired
    private TaskExecutor taskExecutor;

    @PostMapping
    public void startTest(@RequestBody DummyModel dummy) throws InterruptedException {
        taskExecutor.execute(new ProcessList());
        taskExecutor.execute(new AnotherProcessList());
        taskExecutor.execute(new YetAnotherProcessList());
    }

where taskExecutor is a bean defined in my main class (or it can be defined in any class with @Configuration annotation). It is as follows:

    @Bean
    public TaskExecutor taskExecutor() {
        ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
        executor.setCorePoolSize(5); // min number of threads that are always there
        executor.setMaxPoolSize(10); // if threads are full and queue is full then additional threads will be created
        executor.setQueueCapacity(5); // the number of tasks to be placed in the queue (caution queues require memory. Larger queue = more memory)
        return executor;
    }

CodePudding user response：

Your proposed solution won't work reliably.

es.shutdown();
es = Executors.newFixedThreadPool(2);

This assumes that the method startTest is only ever to be invoked by 1 incoming request concurrently and that the next one that comes in is always after a executor has been shutdown and refreshed.

That solution would only work if you also create the ExecutorService inside the method for the scope of the method. However that is also problematic. If 100 requests come in you will create 200 concurrent threads, each thread takes up resources. So you effectivly created a potential resource leak (or at least an attack vector for your application).

General rule of thumb if you create the Executor yourself in the same scope then you should close it, if not leave it untouched. In your case you basically use a shared thread pool and should only do a shutdown on application stop. You could do that in an @PreDestroy method in your controller

@PreDestroy
public void cleanUp() {
  es.shutdown();
}

However instead of adding this to your controller you could also define the ExecutorService as a bean and configure a destroy method.

@Bean(destroyMethod="shutdown")
public ExecutorService taskExecutor() {
  return Executors.newFixedThreadPool(2);
}

You could now dependency inject the ExecutorService in your controller.

@RestController
public class YourController {

  private final ExecutorService es;

  public YourController(ExecutorService es) {
    this.es=es;
  }
}

Finally I suspect you are even better of using the Spring (Boot) provided TaskExecutor which taps into the Spring context lifecycle automatically. You can simply inject this into your controller instead of the ExecutorService.

@RestController
public class YourController {

  private final TaskExecutor executor;

  public YourController(TaskExecutor executor) {
    this.executor = executor;
  }
}

Spring Boot provides one by default which will be injected, you can control those by using the spring.task.execution.pool.* properties.

spring.task.execution.pool.max-size=10
spring.task.execution.pool.core-size=5
spring.task.execution.pool.queue-capacity=15

Or you could define a bean, this would override the default TaskExecutor as well.

@Bean
public ThreadPoolTaskExecutor taskExecutor() {
  ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
  executor.setCorePoolSize(5);
  executor.setMaxPoolSize(10);
  executor.setQueueCapacity(5);
  return executor;
}