9

AFAIK submitting Callable/Runnable to ExecutorService is the way to go if I want to execute resource-heavy code in parallel. Hence my method structure:

public class ServiceClass {
    protected final ExecutorService executorService = Executors.newCachedThreadPool();

    public Future<Result> getResult(Object params) {
        if (params == null) {
            return null; // In situations like this the method should fail
        }
        // Do other fast pre-processing stuff
        return executorService.submit(new CallProcessResult(params));
    }

    private class CallProcessResult implements Callable<Result> {
        private Object params;
        public CallProcessResult(Object params) {
            this.params = params;
        }
        @Override
        public Result call() throws Exception {
            // Compute result for given params
            // Failure may happen here too!
            return result;
        }
    }
}
public class Result {
    ...
}

I have marked 2 spots in the code above in which failures can happen. The options available for error handling are quite different for those 2 cases.

Before submitting the task there can be issues like invalid parameters, some fast pre-processing code that may fail.

I see several ways to signify failure here:

  1. In case of invalid params supplied to getResult return null immediately. In this case I'll have to check if getResult returned null every time I call it.
  2. Throw checked exceptions instead of the above.
  3. Instantiate a Future<Result> that returns null on get() request. I would do that with Apache Commons ConcurrentUtils.constantFuture(null). In this case I would expect getResult to always return some non-null Future<Result>. I like this option more, because it is consistent with the second case.

During task execution I can expect serious errors like lack of memory, corrupted files, unavailable files etc.

  1. I suppose the better option in my case is to return null, because the result of the task is an object.
  2. Also, I could throw checked exceptions and handle them in ThreadPoolExecutor.afterExecute (as suggested by NiranjanBhat). See Handling exceptions from Java ExecutorService tasks

Which is the better practice (in both cases)?

Perhaps there is a different way to do this or a design pattern I should use?

Community
  • 1
  • 1
Hunternif
  • 2,370
  • 2
  • 17
  • 14

2 Answers2

13

I would suggest that for failure during task processing, you simply throw an appropriate exception. Don't add any special handling for this in the executor. What will happen is that it will be captured, and stored in the Future. When the Future's get method is called, it will throw an ExecutionException, which the caller of get can then unpack and handle. This is essentially how normal exception handling is transposed into the Callable/Future paradigm. This looks like this:

    Future<Result> futureResult = serviceClass.getResult("foo");

    try {
        Result result = futureResult.get();
        // do something with result
    }
    catch (ExecutionException ee) {
        Throwable e = ee.getCause();
        // do something with e
    }

Given that the caller of get has to have this handling of ExecutionExceptions, you could then take advantage of that to deal with failure during submission. To do this, you could construct a Future that is like Apache Commons's constantFuture, but which throws a given exception rather than returns a given value. I don't think there's anything like that in the JDK, but it's simple (if tedious) to write:

public class FailedFuture<T> implements Future<T> {
    private final Throwable exception;

    public FailedFuture(Throwable exception) {
        this.exception = exception;
    }

    @Override
    public T get() throws ExecutionException {
        throw new ExecutionException(exception);
    }

    @Override
    public T get(long timeout, TimeUnit unit) throws ExecutionException {
        return get();
    }

    @Override public boolean cancel(boolean mayInterruptIfRunning) { return false; }
    @Override public boolean isCancelled() { return false; }
    @Override public boolean isDone() { return true; }
}

This is somewhat dodgy - you're taking a failure during a synchronously-called method, and making it look like a failure during the asynchronously-called method. You're shifting the burden of handling the error from the code that actually caused it to some code that runs later. Still, it does mean you can have all the failure handling code in one place; that might be enough of an advantage to make this worthwhile.

Tom Anderson
  • 46,189
  • 17
  • 92
  • 133
  • Thank you very much for your suggestions, Tom. It makes sense to follow them, as handling `ExecutionException`s is required anyway. I actually didn't know that. I suppose, to avoid the tediousness of writing `FailedFuture`, I could defer _all_ code of the method to the `Callable`. – Hunternif Dec 18 '12 at 11:56
  • You could do that; it would certainly make things simpler. A downside would be that an executor thread has to pick up the task and run it purely to produce a failure indication that could have been produced immediately, which seems a little wasteful. – Tom Anderson Dec 18 '12 at 12:26
3

You can use afterExecute method. This is defined in the ThreadPoolExecutor, which you will need to override.
This method is called after the execution of each task is completed. You will get the task instance in this callback method. You can record the errors in some variable in your task and access it in this method.

NiranjanBhat
  • 1,812
  • 13
  • 17
  • What about the failures that happen _before_ I actually submit the task? – Hunternif Dec 18 '12 at 08:46
  • 1
    Can you give me example of these errors ? Before the submit is done, you are actually outside the ExecutorService and you have do it in a simple way like you have done above or you can move this validation within your Task code itself,so in that way everything happens at one place. – NiranjanBhat Dec 18 '12 at 08:53
  • I added examples and a couple more variants to the question. – Hunternif Dec 18 '12 at 09:11