Java execute task with a number of retries and a t

Question

I'm trying to create a method that executes a given task in a maximum amount of time. If it fails to finish in that time, it should be retried a number of times before giving up. It should also wait a number of seconds between each try. Here's what I've come up with and I'd like some critiques on my approach. Is their a simpler way to do this using the ScheduledExecutorService or is my way of doing this suffice?

public static <T> T execute(Callable<T> task, int tries, int waitTimeSeconds, int timeout) 
    throws InterruptedException, TimeoutException, Exception {

    Exception lastThrown = null;
    for (int i = 0; i < tries; i++) {
        try {
            final Future<T> future = new FutureTask<T>(task);
            return future.get(timeout, TimeUnit.SECONDS);
        } catch (TimeoutException ex) {
            lastThrown = ex;
        } catch (ExecutionException ex) {
            lastThrown = (Exception) ex.getCause();
        }
        Thread.sleep(TimeUnit.SECONDS.toMillis(waitTimeSeconds));
    }
    if (lastThrown == null) {
        lastThrown = new TimeoutException("Reached max tries without being caused by some exception. " + task.getClass());
    }
    throw lastThrown;
}

Answer 1

I think, but it's my opinion, that if you are scheduling network related tasks, you should not retry but eventually run them in parallel. I describe this other approach later.

Regarding your code, you should pass the task to an executor, or the FutureTask to a thread. It will not spawn a thread or execute by itself. If you have an executor (see ExecutorService), you don't even need a FutureTask, you can simply schedule it and obtain a callable.

So, given that you have an ExecutorService, you can call :

Future<T> future = yourExecutor.submit(task);

Future.get(timeout) will wait for that timeout and eventually return with TimeoutException even if the task has never started at all, for example if the Executor is already busy doing other work and cannot find a free thread. So, you could end up trying 5 times and waiting for seconds without ever giving the task a chance to run. This may or may not be what you expect, but usually it is not. Maybe you should wait for it to start before giving it a timeout.

Also, you should explicitly cancel the Future even if it throws TimeoutException, otherwise it may keep running, since nor documentation nor code says it will stop when a get with timeout fails.

Even if you cancel it, unless the Callable has been "properly written", it could keep running for some time. Nothing you can do it about it in this part of code, just keep in mind that no thread can "really stop" what another thread is doing in Java, and for good reasons.

However I suppose your tasks will mostly be network related, so it should react correctly to a thread interruption.

I usually use a different strategy is situations like this:

1. I would write public static T execute(Callable task, int maxTries, int timeout), so the task, max number of tries (potentially 1), max total timeout ("I want an answer in max 10 seconds, no matter how many times you try, 10 seconds or nothing")
2. I start spawning the task, giving it to an executor, and then call future.get(timeout/tries)
3. If I receive a result, return it. If I receive an exception, will try again (see later)
4. If however i get a timeout, I DON'T cancel the future, instead I save it in a list.
5. I check if too much time has passed, or too many retries. In that case I cancel all the futures in the list and throw exception, return null, whatever
6. Otherwise, I cycle, schedule the task again (in parallel with the first one).
7. See point 2
8. If I have not received a result, I check the future(s) in the list, maybe one of the previous spawned task managed to do it.

Assuming your tasks can be executed more than once (as I suppose they are, otherwise no way to retry), for network stuff I found this solution to work better.

Suppose your network is actually very busy, you ask for a network connection, giving 20 retries 2 seconds each. Since your network is busy, none of the 20 retries manages to get the connection in 2 seconds. However, a single execution lasting 40 seconds may manage to connect and receive data. It's like a person pressing f5 compulsively on a page when the net is slow, it will not do any good, since every time the browser has to start from the beginning.

Instead, I keep the various futures running, the first one that manages to get the data will return a result and the others will be stopped. If the first one hangs, the second one will work, or the third one maybe.

Comparing with a browser, is like opening another tab and retrying to load the page there without closing the first one. If the net is slow, the second one will take some time, but not stopping the first one, which will eventually load properly. If instead the first tab was hung, the second one will load rapidly. Whichever loads first, we can close the other tab.

Answer 2

The thread on which your execute is called will block for so much time. Not sure if this is correct for you. Basically , for these types of tasks , ScheduledExecutorService is best.You can schedule a task and specify the timings. Take a look at ScheduledThreadPoolExecutor