Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: (Hibernate) Rollback to savepoint fails during transaction retry. #27274

Closed
volodymyr-shulga opened this issue Jul 9, 2018 · 14 comments
Closed
Assignees
Labels
A-tools-hibernate Issues that pertain to Hibernate integration. C-investigation Further steps needed to qualify. C-label will change. docs-todo O-community Originated from the community S-3 Medium-low impact: incurs increased costs for some users (incl lower avail, recoverable bad data) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@volodymyr-shulga
Copy link

volodymyr-shulga commented Jul 9, 2018

I'm working with single node cockroachdb configuration. Using Hibernate as on ORM. Implemented custom transaction manager according to https://github.com/cockroachdb/hibernate-savepoint-fix.

Cockroachdb fails to process concurrent inserts to multiple tables in one transaction with exception:

2018-07-04 08:44:03,0395 191f69c78b1b80c21ed7bb001da49719 ERROR org.hibernate.engine.jdbc.spi.SqlExceptionHelper - ERROR: restart transaction: HandledRetryableTxnError: TransactionAbortedError: txn aborted "sql txn" id=8e677091 key=/Table/112/1/362490898983419905/"FILTER_SET"/0 rw=true pri=0.16579172 iso=SERIALIZABLE stat=PENDING epo=0 ts=1530693821.068963662,1 orig=1530693815.911493801,0 max=1530693815.911493801,0 wto=false rop=false seq=7923

Transaction is aborted after this error and can't be rolled back to savepoint.

Caused by: javax.persistence.PersistenceException: org.hibernate.exception.GenericJDBCException: could not execute statement Caused by: org.hibernate.exception.GenericJDBCException: could not execute statement Caused by: org.postgresql.util.PSQLException: ERROR: Expected "ROLLBACK TO SAVEPOINT COCKROACH_RESTART": current transaction is aborted, commands ignored until end of transaction block at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2455) at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2155) at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:288) at org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:430) at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:356) at org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:168) at org.postgresql.jdbc.PgPreparedStatement.executeUpdate(PgPreparedStatement.java:135) 37 lines skipped for [org.hibernate]

Same inserts are successful when executed in one thread.
I'm attaching simplified version of my schema. Data is inserted into the frame and connected tables.

cockroach_schema.txt

Jira issue: CRDB-4961

@awoods187 awoods187 added the A-tools-hibernate Issues that pertain to Hibernate integration. label Jul 9, 2018
@awoods187
Copy link
Contributor

Hi @Vladimir-Shulga , we notice that your error message mentions cockroach_restart. This is only possible if you are running CockroachDB-specific changes in your application code: a native Hibernate app would never produce this error. We deduce your app code using Hibernate has already been equipped to handle these errors in CockroachDB. Hence:

  1. have you made these CockroachDB-specific changes yourself?
  2. if yes, how did you change the code? Perhaps we can help you fix it.
  3. if not, how do you explain there is CockroachDB-specific code in your app already? Are you perhaps using Hibernate via a 3rd party framework or plugin in your app? What would that software be? Perhaps we can contact the upstream software provider and help them fix it, or help you integrate with their changes

@volodymyr-shulga
Copy link
Author

Hi @awoods187,

"cockroach_restart" is name of savepoint that was created in my Java code.
I create savepoint like
session.createNativeQuery("SAVEPOINT cockroach_restart").executeUpdate();

if statements in scope of transaction executed successfully I do
session.createNativeQuery("RELEASE SAVEPOINT cockroach_restart").executeUpdate();

If specific exception with SqlState 40001 happens I do
session.createNativeQuery("ROLLBACK TO SAVEPOINT cockroach_restart").executeUpdate();

Exception I posted happens during rolling back transaction to manually created savepoint.

I haven't applied any changes to CocroachDB sources.

@knz
Copy link
Contributor

knz commented Jul 9, 2018

@Vladimir-Shulga you should create a specific transaction object (tx = session.beginTransaction();) and use the same transaction object tx to run SAVEPOINT cockroach_restart, the query, RELEASE then either ROLLBACK or commit the transaction object.

This needs to be done using the CockroachDBTransactionCoordinator provided in https://github.com/cockroachdb/hibernate-savepoint-fix.

If you use the session object directly hibernate will use separate transactions for each native query and the mechanism will not work.

@knz knz added the O-community Originated from the community label Jul 9, 2018
@knz knz changed the title [Hibernate] Rollback to savepoint fails during transaction retry. sql: (Hibernate) Rollback to savepoint fails during transaction retry. Jul 9, 2018
@knz knz added the docs-todo label Jul 9, 2018
@knz
Copy link
Contributor

knz commented Jul 9, 2018

(Arguably there is a pitfall in our docs here: we should be much more explicit in docs and the README of https://github.com/cockroachdb/hibernate-savepoint-fix that the code must use a single txn object and not the session directly.)

cc @BramGruneir @rmloveland

@volodymyr-shulga
Copy link
Author

@knz
Could you please clarify what do you mean by

"run SAVEPOINT cockroach_restart, the query, RELEASE then either ROLLBACK or commit the transaction object"?

I do have separate transaction object and do commit/rollback on it. How can I run queries like "ROLLBACK TO SAVEPOINT cockroach_restart" on transaction but not on session?
Here is code I use for transaction handling

`public class Txn {

private static final String SERIALIZATION_FAILURE = "40001";

/**
 * execTx runs fn inside a transaction and retries it as needed.
 * On non-retryable failures, the transaction is aborted and rolled
 * back; on success, the transaction is committed.
 * There are cases where the state of a transaction is inherently ambiguous: if
 * we err on RELEASE with a communication error it's unclear if the transaction
 * has been committed or not (similar to erroring on COMMIT in other databases).
 * In that case, we return AmbiguousCommitException.
 * There are cases when restarting a transaction fails: we err on ROLLBACK
 * to the SAVEPOINT. In that case, we return a TxnRestartException.
 * <p>
 * For more information about CockroachDB's transaction model see
 * https://cockroachlabs.com/docs/stable/transactions.html.
 * <p>
 * NOTE: the supplied fn closure should not have external side
 * effects beyond changes to the database.
 */
public static Object execTx(Session session, TxnFn<Object> fn) throws Throwable {
    Object ret;

    Transaction tx = session.beginTransaction();
    try {
        ret = execInTx(session, tx, fn);
        tx.commit();
    } catch (Throwable t) {
        tx.rollback();
        throw t;
    }

    return ret;
}

/**
 * execInTx runs fn inside tx which should already have begun.
 * *WARNING*: Do not execute any statements on the supplied tx before calling this function.
 * execInTx will only retry statements that are performed within the supplied
 * closure (fn). Any statements performed on the tx before execInTx is invoked will *not*
 * be re-run if the transaction needs to be retried.
 * <p>
 * fn is subject to the same restrictions as the fn passed to ExecuteTx.
 */
private static Object execInTx(Session session, Transaction tx, TxnFn<Object> fn) throws Throwable {
    Object ret;

    // Specify that we intend to retry this txn in case of CockroachDB retryable
    // errors.
    try {
        session.createNativeQuery("SAVEPOINT cockroach_restart").executeUpdate();
    } catch (Exception e) {
        log.error("Error while executing savepoint.", e);
        throw e;
    }

    while (true) {
        boolean released = false;

        try {
            Object result = fn.call();

            // RELEASE acts like COMMIT in CockroachDB. We use it since it gives us an
            // opportunity to react to retryable errors, whereas tx.commit() doesn't.
            released = true;
            log.info("transaction before savepoint release {}", tx.toString());
            session.createNativeQuery("RELEASE SAVEPOINT cockroach_restart").executeUpdate();

            ret = result;
            break;
        } catch (PersistenceException e) {
            // We got an error; let's see if it's a retryable one and, if so, restart. We look
            // for the standard PG errcode SerializationFailureError:40001.
            Throwable cause = e.getCause();
            final boolean serializationFailureException = cause instanceof SQLException
                    && ((SQLException) cause).getSQLState().equals(SERIALIZATION_FAILURE);
            log.info("Transaction status: {}", tx.getStatus());
            log.info("Transaction getRollbackonly: {}", tx.getRollbackOnly());
            if (!serializationFailureException) {
                log.info("------------------------------------------------------");
                log.info("Non serialization exception", e);
                log.info("Cause", cause);
                log.info("------------------------------------------------------");
            }
            if (serializationFailureException) {
                log.info("Serialization failure. Retrying transaction...");

                try {
                    session.createNativeQuery("ROLLBACK TO SAVEPOINT cockroach_restart").executeUpdate();
                } catch (Exception ex) {
                    throw new TxnRestartException(ex);
                }
            } else if (released) {
                throw new AmbiguousCommitException(e);
            } else {
                throw e;
            }
        } catch (Exception e) {
            log.info("++++++++++++++++++++++++++++++++++++++++++++++++++++++");
            log.info("Exception caught. Non hibernate", e);
            log.info("Transaction status: {}", tx.getStatus());
            log.info("Transaction getRollbackonly: {}", tx.getRollbackOnly());
            log.info("++++++++++++++++++++++++++++++++++++++++++++++++++++++");
            throw e;
        }
    }

    return ret;
}

}
`

@knz
Copy link
Contributor

knz commented Jul 12, 2018

Thanks for sharing this code with us. I'd need to ask for some colleague with more Java experience than I to look into it.

@knz
Copy link
Contributor

knz commented Jul 12, 2018

@Vladimir-Shulga I might take a guess as to what the problem is: at the very beginning you say "concurrent inserts". Are you trying to use multiple Java threads using the same Session object?

@volodymyr-shulga
Copy link
Author

@knz
I'm doing concurrent REST calls. In scope of rest call separate session with separate transaction used.
I did open this issue after discussing it with Bram in topic https://forum.cockroachlabs.com/t/transaction-rollback-to-savepoint-fails-rollback-to-savepoint-cockroach-restart/1804/4

@knz knz added the C-investigation Further steps needed to qualify. C-label will change. label Jul 21, 2018
@knz knz added this to the 2.1 milestone Jul 23, 2018
@knz knz modified the milestones: 2.1, 2.1.x Oct 15, 2018
@BramGruneir
Copy link
Member

So as of right now I think the best approach for this issue is twofold.

  1. Have someone from support look into this if @Vladimir-Shulga is still stuck here.
  2. @rmloveland is about to start work into adding more detailed descriptions on how to use hibernate with Cockroach correctly. See Transaction retry guidance for SQLAlchemy docs#3315

@lfmunoz
Copy link

lfmunoz commented Mar 13, 2019

any status updates, timeline?

@jordanlewis
Copy link
Member

jordanlewis commented May 9, 2019

@lfmunoz what is the problem that you are running into? More examples will help us prioritize.

@lfmunoz
Copy link

lfmunoz commented May 10, 2019

I got rollback to savepoints working with Spring Boot / JPA / Hibernate so this isn't a problem for me anymore. Having problems with liquid-base but that is a separate issue.

Special thanks to https://github.com/cockroachdb/hibernate-savepoint-fix and other code I found. See below for my implementation with Spring Annotation.

If anything I would appreciate feedback from cockroachdb people to make sure I am doing everything correctly in the code below.


    @CacheEvict(cacheNames = "domain.HeartBeat", allEntries = true)
    @CockroachSavePoint
    public HeartBeatDTO save(HeartBeatDTO heartBeatDTO) {
        log.debug("Request to save HeartBeat : {}", heartBeatDTO);
        HeartBeat heartBeat = heartBeatMapper.toEntity(heartBeatDTO);
        heartBeat = heartBeatRepository.save(heartBeat);
        HeartBeatDTO result = heartBeatMapper.toDto(heartBeat);
        return result;
    }
package eco.usp.hibernate;

import javax.annotation.PostConstruct;
import javax.persistence.EntityManagerFactory;
import javax.persistence.RollbackException;
import java.sql.SQLException;
import java.util.concurrent.Callable;
import java.util.function.Supplier;

import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.hibernate.Session;
import org.hibernate.SessionFactory;
import org.hibernate.Transaction;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import org.springframework.transaction.TransactionSystemException;

@Aspect
@Component
public class CockroachSavePointAspect {
    private static final Logger log = LoggerFactory.getLogger(CockroachSavePointAspect.class);

    @Autowired
    EntityManagerFactory entityManagerFactory;

    private SessionFactory sessionFactory;

    @PostConstruct
    public void init() {
        sessionFactory = entityManagerFactory.unwrap(SessionFactory.class);
    }

    /*
    @Around("@annotation(hibernate.CockroachSavePoint)")
    public Object cockroachSavePoint(ProceedingJoinPoint joinPoint) throws Throwable {
       return execTx(sessionFactory, () -> {
            try {
                return joinPoint.proceed();
            } catch (Throwable e) {
                throw new RuntimeException(e);
            }
        });
    }
     */

    @Around("@annotation(eco.usp.hibernate.CockroachSavePoint)")
    public Object cockroachSavePoint(ProceedingJoinPoint joinPoint) throws Throwable {
        return execTx(sessionFactory, new Callable<Object>() {
            public Object call() throws RuntimeException {
                try {
                    return joinPoint.proceed();
                } catch (Throwable e) {
                    throw new RuntimeException(e);
                }
            }
        });
    }

    public static Object execTx(SessionFactory sessionFactory, Callable<Object> fn) throws Throwable {
        Object ret;
        Session session = sessionFactory.openSession();
        Transaction tx = session.beginTransaction();
        try {
            ret = execInTx(session, tx, fn);
            //    ret = null;
            tx.commit();
        } catch (Throwable t) {
            tx.rollback();
            throw t;
        } finally {
            session.close();
        }
        return ret;
    }


    private static Object execInTx(Session session, Transaction tx, Callable<Object> fn) throws Throwable {
        Object ret;

        // Specify that we intend to retry this txn in case of CockroachDB retryable errors.
        try {
            session.createNativeQuery("SAVEPOINT cockroach_restart").executeUpdate();
        } catch (Exception e) {
            log.error("Error while executing savepoint.", e);
            throw e;
        }

        while (true) {
            boolean released = false;
            try {
                Object result = fn.call();
                // RELEASE acts like COMMIT in CockroachDB. We use it since it gives us an
                // opportunity to react to retryable errors, whereas tx.commit() doesn't.
                released = true;
                session.createNativeQuery("RELEASE SAVEPOINT cockroach_restart").executeUpdate();
                ret = result;
                break;
            } catch (RuntimeException e) {
                SQLException sqlEx = null;
                Throwable throwable = e.getCause();
                TransactionSystemException te = (TransactionSystemException) throwable;
                Throwable orig = te.getOriginalException();
                if (orig instanceof SQLException) {
                    sqlEx = (SQLException) orig;
                }
                if (orig instanceof RollbackException && orig.getCause() instanceof SQLException) {
                    sqlEx =  (SQLException) orig.getCause();
                }

                String sqlState = sqlEx.getSQLState();
                if (sqlState.equals("40001")) {
                    log.debug("Serialization failure. Retrying transaction...");
                    try {
                        session.createNativeQuery("ROLLBACK TO SAVEPOINT cockroach_restart").executeUpdate();
                    } catch (Exception ex) {
                        throw ex;
                    }
                }  else {
                    throw e;
                }
            } catch (Throwable e) {
                log.error("{}", e);
                throw e;
            }
        }
        return ret;
    }
}

@jordanlewis
Copy link
Member

@lfmunoz thanks for sharing, this is really useful.

@jordanlewis jordanlewis removed this from the 2.1.x milestone Oct 11, 2019
@rafiss rafiss added the S-3 Medium-low impact: incurs increased costs for some users (incl lower avail, recoverable bad data) label May 19, 2020
@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 12, 2021
@rafiss
Copy link
Collaborator

rafiss commented Nov 8, 2022

I'm closing this issue due to age.

@rafiss rafiss closed this as completed Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tools-hibernate Issues that pertain to Hibernate integration. C-investigation Further steps needed to qualify. C-label will change. docs-todo O-community Originated from the community S-3 Medium-low impact: incurs increased costs for some users (incl lower avail, recoverable bad data) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests

7 participants