Hibernate ORM
  1. Hibernate ORM
  2. HHH-7689

Error executing batch should abort rest of batch for "cleanliness" sake

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.1.7
    • Fix Version/s: 4.3.0.Beta5
    • Component/s: core
    • Labels:
      None
    • Bug Testcase Reminder (view):

      Bug reports should generally be accompanied by a test case!

    • Last commented by a user?:
      true

      Description

      When actually performing the execution of the "batched" statement (whether from BatchingBatch or NonBatchingBatch) any exception should cause the rest of the batch to be aborted.


      Original report:

      I was going to log something similar to HHH-7688 but it's a bit different as that change only covers the NonBatchingBatch where in my case it was the BatchingBatch impl causing me the grief.

      The full details got lost in my notes and it also involved using a SavePoint extension so I wasn't sure if I was inducing some of my own pain at the time but I think there's something there.

      The complete details are a bit fuzzy but this is what I had recorded...

      The abortBatch() in the JdbcCoordinatorImpl is only called when it encounters a SQLException. However, when executing the batch Hibernate catches the original SQLException and rethrows a different exception which causes the abortBatch() not to be called.
      In the event that Hibernate got to the executeBatch() stage the worst that happens is that any registered batch listeners are not removed. Under a previous patch I ensured that the clearStatements() included a call to clear the batch.

      However, if it gets to the point where the execute hasn't actually been called yet, i.e. a validation exception occurs then the batch isn't actually cleared until the session controlling the transaction is closed. This isn't usually an issue since the exception is typically bubbled up the stack and everything is rolled back. However, in our case, we have some SavePoint functionality that required a workaround. I believe Spring would also encounter the same sort of issue.

      The statements like this from addToBatch() in BatchingBatch are what cause the abortBatch to get bypassed.

      catch ( SQLException e ) {
      			LOG.debugf( "SQLException escaped proxy", e );
      			throw sqlExceptionHelper().convert( e, "could not perform addBatch", currentStatementSql );
      		}
      

      Then in places that are wrapping the addToBatchCall, i.e. the insert method of AbstractEntityPersister you end up with...

      			catch ( SQLException e ) {
      				if ( useBatch ) {
      					session.getTransactionCoordinator().getJdbcCoordinator().abortBatch();
      				}
      				throw e;
      			}
      

      However, by this point it seems like the Exception has been rewrapped to a GenericJDBCException which doesn't extend from SQLException and blows out.

      In my case I ended up catching the exception in my extension and manually calling it but I think the Spring guys use the same concept (I think it's how their nested transaction functionality works)

        Issue Links

          Activity

          Hide
          Steve Ebersole added a comment -

          Ok, I can reproduce this using my original code by just persisting more entities than the batch size which causes the batch execution to happen on addToBatch rather than performBatch.

          As you seem to be aware, this is pretty much explicitly not supported. But you have worked up enough karma that I am trying to help you here

          An interesting option is to abort the batch from within the batch anytime there is a problem executing the batch. That is a simple localized change that actually fixes the test failure in this new BatchingBatchFailureTest test. I don't think there are any negative ramifications of that change. Running the test suite to verify.

          Show
          Steve Ebersole added a comment - Ok, I can reproduce this using my original code by just persisting more entities than the batch size which causes the batch execution to happen on addToBatch rather than performBatch. As you seem to be aware, this is pretty much explicitly not supported. But you have worked up enough karma that I am trying to help you here An interesting option is to abort the batch from within the batch anytime there is a problem executing the batch. That is a simple localized change that actually fixes the test failure in this new BatchingBatchFailureTest test. I don't think there are any negative ramifications of that change. Running the test suite to verify.
          Hide
          Steve Ebersole added a comment -

          This seems to work well.

          BTW, BeanValidation is disabled by default in the hibernate-core testsuite which is a recent change. Without that change there was a large chunk of Hibernate code (in memory nullability checking) that never got tested. Tests that explicitly test or use BeanValidation need to enable it by setting javax.persistence.validation.mode to a value other than NONE.

          Looking at applying something similar for HHH-7688 as well.

          Show
          Steve Ebersole added a comment - This seems to work well. BTW, BeanValidation is disabled by default in the hibernate-core testsuite which is a recent change. Without that change there was a large chunk of Hibernate code (in memory nullability checking) that never got tested. Tests that explicitly test or use BeanValidation need to enable it by setting javax.persistence.validation.mode to a value other than NONE . Looking at applying something similar for HHH-7688 as well.
          Hide
          Shawn Clowater added a comment -

          Thanks Steve, that was my gut of just having it fail on Exception rather than SQLException but I knew I was in uncharted territory with why I was hitting it in the first place. It just felt wrong to only have certain types of failure abort the batch.

          Show
          Shawn Clowater added a comment - Thanks Steve, that was my gut of just having it fail on Exception rather than SQLException but I knew I was in uncharted territory with why I was hitting it in the first place. It just felt wrong to only have certain types of failure abort the batch.
          Hide
          Steve Ebersole added a comment -

          Shawn, just pushed this work. Give it a go when you get a chance

          Show
          Steve Ebersole added a comment - Shawn, just pushed this work. Give it a go when you get a chance
          Hide
          Koen Janssens added a comment -

          Any chance of getting this fixed on on 4.2 ? We are using jboss EAP6.1.1 and that one can not work with hibernate 4.3 (it's still JPA 2.0)

          Show
          Koen Janssens added a comment - Any chance of getting this fixed on on 4.2 ? We are using jboss EAP6.1.1 and that one can not work with hibernate 4.3 (it's still JPA 2.0)

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - Not Specified
                Not Specified
                Logged:
                Time Spent - 3.3h
                3.3h

                  Development