3

I'm creating a REST service using Spring with Jersey and I have a use case where for every request I get, I need to make several calls (N) to an upstream API.

I get one request, it has n items, for each item, I create a thread to call my dependency (REST) and process the response. At the end I collect all the responses together, maintaining the order, and return them as a single response to the client.

I am using Java 8's CompletableFuture and wanted to know if I was using the ExecutorService framework correctly.

@Component // automatic singleton by Spring
class A {
    private ExecutorService executorService = Executors.newCachedThreadPool();

    private RawResponse getRawResponse(Item item) {
        // make REST call
    }


    private Response processResponse(RawResponse rawResponse) {
        // process response
    }

    public List<Response> handleRequest(Request request) {
        List<CompletableFuture> futureResponses = new ArrayList<>();

        for(Item item : request.getItems()) {
            CompletableFuture<Response> futureResponse = CompletableFuture.supplyAsync(() -> getRawResponse(item), executorService)
            .thenApply(rawResponse -> processResponse(rawResponse))
            .handle((response, throwable) {
            if(throwable != null) { // log and return default response
            } else { return response;}});

            futureResponses.add(futureResponse);
        }

        List<Response> result = new ArrayList<>();

        for (CompletableFuture<Resource> futureResponse : futureResponses) {
                    try {
                        result.add(futureResponse.get());
                    } catch (Exception e) {

                        // log error
                    }
        }
        return result;
    }
}

The question I have now is, should I move the creation of the executorService right above:

List<CompletableFuture> futureResponses = new ArrayList<>();

and call shutdown on it right above:

return result;

because at this time, I am not really calling shutdown anywhere since the app will always run in it's docker container.

Is it costly to keep creating and discarding the pool, or is the current way going to leak memory? And I think calling the pool static as a private field var is redundant since the class is a spring bean anyways (singleton).

Any advice will be appreciated, also should I be using a cachedThreadPool? I am not sure how to approximate the number of threads I need.

Didier L
  • 18,905
  • 10
  • 61
  • 103
VSEWHGHP
  • 195
  • 2
  • 3
  • 12

2 Answers2

3

should I move the creation of the executorService right above?

No you don't, you have your ExecutorService the right place in your example code. Think it as a thread pool, you will not want to init a new thread pool and close it for each method call of handleRequest. Of course ExecutorService does more job than a thread pool, actually it'll manage a thread pool underneath, and provides life-circle management for async tasks.

I am not really calling shutdown anywhere since the app will always run in it's docker container.

In most of cases you init your ExecutorService when applications starts and shut it down when applications shuts down. So you can just leave it there, because it'll be closed when application exits, or you can add some kind of shutdown hooks if you need to do graceful shutdown.

Is it costly to keep creating and discarding the pool.

Kind of, we don't want to create and discard Thread very often, so we have thread pool, if you create/discard pool for each method call, what's the point to have a thread pool.

or is the current way going to leak memory?

No, as long as the task you submitted does not leak memory. The implementation of ExecutorService itself is good to use.

And I think calling the pool static as a private field var is redundant since the class is a spring bean anyways (singleton)

Yes, you're correct. You can also define ExecutorService as Spring Bean and inject it to service bean, if you want to do some customized init process.

should I be using a cachedThreadPool, I am not sure how to approximate the number of threads I need.

That's hard to say, you need to do some test to get the right number of threads for your application. But most of NIO or EventDriven framework has the twice number of available cores to be the number of threads by default.

shizhz
  • 11,715
  • 3
  • 39
  • 49
0

As you are using Spring, you might want to let it handle the asynchronous execution instead.

Just put @EnableAsync in one of your @Configuration classes to enable the @Async annotation on methods.

You would then change your getRawResponse() to

@Async
private CompletableFuture<RawResponse> getRawResponse(Item item) {
    // make REST call
    return CompletableFuture.completedFuture(rawResponse);
}

(you might need to put this method in a separate service to allow proper proxying, depending on how AOP is configured in your project)

and change your loop to simply

for(Item item : request.getItems()) {
    CompletableFuture<Response> futureResponse = getRawResponse(item)
        .thenApply(rawResponse -> processResponse(rawResponse))
        .handle((response, throwable) {
            if(throwable != null) { // log and return default response
            } else { return response;}
        });

    futureResponses.add(futureResponse);
}

As you can see, you do not need to care about the executor in your service anymore.

You can also customize your executor by declaring its Spring bean, for example:

@SpringBootApplication
@EnableAsync
public class Application extends AsyncConfigurerSupport {

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

    @Override
    public Executor getAsyncExecutor() {
        ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
        executor.setCorePoolSize(2);
        executor.setMaxPoolSize(2);
        executor.setQueueCapacity(500);
        executor.setThreadNamePrefix("GithubLookup-");
        executor.initialize();
        return executor;
    }
}

You can even configure several executors and select one by providing its name as parameter to the @Async annotation.

See also Getting Started: Creating Async Methods and The @Async annotation.

Didier L
  • 18,905
  • 10
  • 61
  • 103
  • I started using this framework but I had issues trying to set the context for the logger on the threads that got created. Nonetheless this is a valid suggestion, thanks – VSEWHGHP Apr 06 '17 at 04:50
  • @VSEWHGHP I suppose you are talking about MDC? There are solutions to copy the context when a task is submitted to an executor, like [How to use MDC with thread pools](http://stackoverflow.com/questions/6073019/how-to-use-mdc-with-thread-pools/). However this wouldn't be an issue specific to using Spring, you'd have to handle this in both cases. – Didier L Apr 06 '17 at 10:07