JDBI transaction - should rollback be allowed to throw exceptions?

[Serban Tatu]

Consider the following case:

1. write some transactional SQL: jdbi.inTransaction(new TransactionCallback() { ... run some jdbc code ... })
   
2. while execution is in the middle of the transactional callback, restart the DB server (e.g. sudo mysql restart)
3. the SQL code will throw a communication failure exception (which is a SQLRecoverableException kind)
4. JDBI (LocalTransactionHandler.inTransaction()) will trap an UnableToExecuteStatementException whose cause is the comm failure exception and will try to rollback the transaction
5. The rollback will fail. This time the driver will not toss a SQLRecoverableException. Instead it will get severe - MySQLNonTransientConnectionException. 
6. The client now gets a CallbackFailedException with a cause of TransactionException with a cause of MySQLNonTransientConnectionException

The outcome is that the client code can no longer reason that this is a recoverable exception and retry. 

My question is - should a rollback handle mask the original exception that caused the issue? We already know that our transaction did not commit properly due to comm failure. I think the client should be given the opportunity to catch the SQLRecoverableException (or scan the stack trace for it) and retry for a while if it can - after all the server is going to come back.

Thank you,

Serban

[Brian McCallister]

Yes, this is very reasonable, but I am not sure how to make it work in
2.X given the mess of exceptions we have there.

One option might be to just add isRecoverable() to the jdbi exception
type, which proxies on down causes chain to answer the question. Not
beautiful, but won't break compat.

I don't care about supporting 1.5 anymore, so this is fine form that POV too.

-Brian

> --
> You received this message because you are subscribed to the Google Groups
> "jDBI" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to jdbi+uns...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Serban Tatu]

I am fine with having isRecoverable() on a DBI exception.

But if the rollback turns an otherwise recoverable exception into a non-recoverable one, isRecoverable() will still return false and I still can't retry.

Currently I solved my problem by overriding LocalTransactionHandler and providing my own tryRollback(handle)  method. I would have preferred to not have to copy code from LocalTransactionHandler into my own method though. 

 

/**

 * Overrides the JDBI LocalTransactionHandler so that rollback methods called in response to other exceptions are not

 * allowed to throw their own exceptions and thus mask the original exception that causes the rollback.

 */

public class RollbackSafeTransactionHandler extends LocalTransactionHandler 

{

    protected final Logger log = LoggerFactory.getLogger(getClass());

    @Override

    public <ReturnType> ReturnType inTransaction(Handle handle, TransactionCallback<ReturnType> callback) 

    {

        final AtomicBoolean failed = new AtomicBoolean(false);

        TransactionStatus status = new TransactionStatus() {

            @Override

            public void setRollbackOnly() {

                failed.set(true);

            }

        };

        final ReturnType returnValue;

        try {

            handle.begin();

            returnValue = callback.inTransaction(handle, status);

            if (!failed.get()) {

                handle.commit();

            }

        } catch (RuntimeException e) {

            tryRollback(handle); // instead of handle.rollback()

            throw e;

        } catch (Exception e) {

            tryRollback(handle);

            throw new TransactionFailedException("Transaction failed do to exception being thrown " +

                    "from within the callback. See cause " +

                    "for the original exception.", e);

        }

        if (failed.get()) {

            tryRollback(handle);

            throw new TransactionFailedException("Transaction failed due to transaction status being set " +

                    "to rollback only.");

        } else {

            return returnValue;

        }

    }

    protected void tryRollback(Handle handle) 

    {

        try {

            handle.rollback();

        } catch (Exception re) {

            log.error("could not roll back transaction: {}", re.toString(), re);

        }

    }

}

Thanks,

Serban

[Steven Schlansker]

One thought, in the case where a transaction exits abnormally (due to an exception), any exceptions thrown in rollback are attached to the already-existing exception via addSuppressed. That gives access to both exceptions but allows naive code to “just do the right thing”.