10

I have multiple cases when I have to deal retrial for DB and networking operations. Everywhere I do it I have the following type of code:

    for (int iteration = 1; ; iteration++) {
        try {
            data = doSomethingUseful(data);

            break;
        } catch (SomeException | AndAnotherException e) {
            if (iteration == helper.getNumberOfRetries()) {
                throw e;
            } else {
                errorReporter.reportError("Got following error for data = {}. Continue trying after delay...", data, e);
                utilities.defaultDelayForIteration(iteration);
                handleSpecificCase(data);
            }
        }
    }

The issue is that this code pattern is copy-pasted all over my classes. Which is really bad. I can't figure out how to get rid of this for-break-catch copy-paste pattern, since I usually get different exception to handle, I want to log data I failed on (usually also different ways).

Is there a good way to avoid this copy-paste in Java 7?

Edit: I do use guice for dependency injection. I do have checked exceptions. There could be multiple variables instead of just one data and they are all of different type.

Edit2: AOP approach looks as the most promising for me.

tshepang
  • 12,111
  • 21
  • 91
  • 136
Artem
  • 7,275
  • 15
  • 57
  • 97
  • Why are the exceptions often different, if this is all DB access code? Can you give a couple of examples? – DNA Mar 02 '12 at 20:38
  • I just edited the question, but reasoning is that there are DB and networking and even for just DB depending on operation it will fail with different exception. Also doSomethingUseful() might throw my own exceptions which I want to handle the same way. – Artem Mar 02 '12 at 20:42
  • 1
    are the exceptions checked or unchecked? – meriton Mar 02 '12 at 20:45
  • There are checked and unchecked exceptions – Artem Mar 02 '12 at 21:35
  • there is a similar question: http://stackoverflow.com/questions/13239972/how-do-you-implement-a-re-try-catch i took the accepted answer. – dermoritz Nov 06 '13 at 09:31
  • @dermoritz the accepted answer is what I was trying to avoid. Since it works perfectly if you only need it once but produces a copy-paste solution if you need retry in a lot of places. Thanks for the link anyway! – Artem Nov 07 '13 at 06:52

6 Answers6

5

Off-hand, I can think of two different approaches:

If the differences in exception handling can be expressed declaratively, you might use AOP to weave the exception handling code around your methods. Then, your business code could look like:

@Retry(times = 3, loglevel = LogLevel.INFO)
List<User> getActiveUsers() throws DatabaseException {
    // talk to the database
}

The advantage is that it is really easy to add retry behaviour to a method, the disadvantage is the complexity of weaving the advice (which you only have to implement once. If you are using a dependency injection library, chances are it will offer method interception support).

The other approach is to use the command pattern:

abstract class Retrieable<I,O> {
    private final LogLevel logLevel;

    protected Retrieable(LogLevel loglevel) {
        this.logLevel = loglevel;
    }

    protected abstract O call(I input);

    // subclasses may override to perform custom logic.
    protected void handle(RuntimeException e) {
        // log the exception. 
    }

    public O execute(I input) {
        for (int iteration = 1; ; iteration++) {
            try {
                return call(input);
            } catch (RuntimeException e) {
                if (iteration == helper.getNumberOfRetries()) {
                    throw e;
                } else {
                    handle();
                    utilities.defaultDelayForIteration(iteration);
                }
            }
        }
    }
}

The problem with the command pattern are the method arguments. You are restricted to a single parameter, and the generics are rather unwieldly for the caller. In addition, it won't work with checked exceptions. On the plus side, no fancy AOP stuff :-)

meriton
  • 68,356
  • 14
  • 108
  • 175
3

I have implemented the RetryLogic class below which provides reusable retry logic and supports parameters because the code to be retried is in a delegate passed in.

/**
 * Generic retry logic. Delegate must throw the specified exception type to trigger the retry logic.
 */
public class RetryLogic<T>
{
    public static interface Delegate<T>
    {
        T call() throws Exception;
    }

    private int maxAttempts;
    private int retryWaitSeconds;
    @SuppressWarnings("rawtypes")
    private Class retryExceptionType;

    public RetryLogic(int maxAttempts, int retryWaitSeconds, @SuppressWarnings("rawtypes") Class retryExceptionType)
    {
        this.maxAttempts = maxAttempts;
        this.retryWaitSeconds = retryWaitSeconds;
        this.retryExceptionType = retryExceptionType;
    }

    public T getResult(Delegate<T> caller) throws Exception {
        T result = null;
        int remainingAttempts = maxAttempts;
        do {
            try {
                result = caller.call();
            } catch (Exception e){
                if (e.getClass().equals(retryExceptionType))
                {
                    if (--remainingAttempts == 0)
                    {
                        throw new Exception("Retries exausted.");
                    }
                    else
                    {
                        try {
    Thread.sleep((1000*retryWaitSeconds));
                        } catch (InterruptedException ie) {
                        }
                    }
                }
                else
                {
                    throw e;
                }
            }
        } while  (result == null && remainingAttempts > 0);
        return result;
    }
}

Below is a use example. The code to be retried is within the call method.

private MyResultType getDataWithRetry(final String parameter) throws Exception {
    return new RetryLogic<MyResultType>(5, 15, Exception.class).getResult(new RetryLogic.Delegate<MyResultType> () {
        public MyResultType call() throws Exception {
            return  dataLayer.getData(parameter);
        }});
}

In case you want to retry only when a specific type of exception occurs (and fail on all other types of exceptions) the RetryLogic class supports an exception class parameter.

Marquez
  • 5,891
  • 3
  • 31
  • 40
2

Make your doSomething implement an interface, e.g., Runable and create a method containing your code above with doSomething replaced with interface.run(data)

user454322
  • 7,300
  • 5
  • 41
  • 52
stryba
  • 1,979
  • 13
  • 19
  • I think this is the right idea, but needs extending to handle the logging (probably straightforward) and exceptions (perhaps less easy) – DNA Mar 02 '12 at 20:37
  • I agree with DNA. The approach is right, but it does not take into account different exceptions and custom logging. Also type for data is always different. – Artem Mar 02 '12 at 20:39
  • Logging can be delegated to the interface as well, so could the actual exception distinction. I know the the devil is in the details. – stryba Mar 02 '12 at 20:42
  • Oh, I want to do different stuff on exceptions every time. – Artem Mar 02 '12 at 20:44
  • Then delegate that specific actions to your interface as well. I don't see a problem there. – stryba Mar 02 '12 at 20:47
  • Wait, if you want to do different stuff on exceptions every time, how is this copy-paste? – Tom Anderson Mar 02 '12 at 21:14
  • My guess is he doesn't always wants to throw e but always likes to check the iteration count – stryba Mar 02 '12 at 21:17
1

take a look at: this retry utility

this method should work for most use cases:

public static <T> T executeWithRetry(final Callable<T> what, final int nrImmediateRetries,
            final int nrTotalRetries, final int retryWaitMillis, final int timeoutMillis)

you can eassily implement an aspect using this utility to do this with even less code.

Zong
  • 6,160
  • 5
  • 32
  • 46
user2179737
  • 493
  • 3
  • 6
0

One thing I would like to add. Most exceptions (99.999%) mean there is something very wrong with your code or environment that needs an admins attention. If your code can't connect to the database it's probably a misconfigured environment there is little point to retrying it just to find out it didn't work the 3rd, 4th, or 5th time either. If you're throwing an exception because the person didn't give a valid credit card number, retrying isn't going to magically fill in a credit card number.

The only situations that are remotely worth retrying is when a system is tremendously strained and things are timing out, but in this situation retry logic is probably going to cause more strain than less (3x for 3 retries on every transaction). But this is what systems do to back down demand (see the apollo lander mission story). When a system is asked to do more than it can it starts dropping jobs and timeouts are the signal the system is strained (or poorly written). You'd be in a far better situation if you just increased the capacity of your system (add more ram, bigger servers, more servers, better algorithms, scale it!).

The other situation would be if you're using optimistic locking and you can somehow recover and auto merge two versions of an object. While I have seen this before I'd caution this approach, but it could be done for simple objects that can be merged without conflicts 100% of the time.

Most exceptions logic should be catch at the appropriate level (very important), make sure your system is in a good consistent state (ie rollback transactions, close files, etc), log it, inform user it didn't work.

But I'll humor this idea and try to give a good framework (well because it's fun like crossword puzzle fun).

// client code - what you write a lot
public class SomeDao {
    public SomeReturn saveObject( final SomeObject obj ) throws RetryException {
        Retry<SomeReturn> retry = new Retry<SomeReturn>() {
            public SomeReturn execute() throws Exception {
               try {
                  // doSomething
                  return someReturn;
               } catch( SomeExpectedBadExceptionNotWorthRetrying ex ) {
                  throw new NoRetryException( ex ); // optional exception block
               }
            }
        }
        return retry.run();
    }
}

// framework - what you write once
public abstract class Retry<T> {

    public static final int MAX_RETRIES = 3;

    private int tries = 0;

    public T execute() throws Exception;

    public T run() throws RetryException {
        try {
           return execute();
        } catch( NoRetryException ex ) {
           throw ex;
        } catch( Exception ex ) {
           tries++;
           if( MAX_RETRIES == tries ) {
              throw new RetryException("Maximum retries exceeded", ex );
           } else {
              return run();
           }
        }
    }
}
chubbsondubs
  • 37,646
  • 24
  • 106
  • 138
  • I agree with you about the right place to catch exceptions. I agree this is THE most important part. However I was talking about copy-pasting "try-catch-catch" all over your code for each framework method you write :) – Artem Jun 11 '14 at 16:24
  • Err. Not sure what you mean. In the the method I showed above there is no required try-catch logic that would be copied around. In the client code the try-catch I included is to show you that you could have some exceptions you want to bypass retry logic for that would be special to that call. That try-catch I included is purely optional. Special feature that the other frameworks didn't add. – chubbsondubs Jun 13 '14 at 18:53
0

Extending the approach discusssed already, how about something like this (no IDE on this netbook, so regard this as pseudocode...)

// generics left as an exercise for the reader...
public Object doWithRetry(Retryable r){
for (int iteration = 1; ; iteration++) {
    try {
        return r.doSomethingUseful(data);
    } catch (Exception e) {
        if (r.isRetryException(e)) {
           if(r.tooManyRetries(i){
            throw e;
           }
        } else {
           r.handleOtherException(e);
        }
    }
}
DNA
  • 42,007
  • 12
  • 107
  • 146