1 2 Previous Next 16 Replies Latest reply on Apr 7, 2010 7:42 AM by hernanbolido

    Question about envers and hibernate synchronization (when an error occurs inside AuditSync.beforeCompletion() method)

    hernanbolido

      Hi!

       

      I realized that if an error occurs persisting historical data o revisions (SQL errors), there is no excepcion propagated to the application. The transaction is rollbacked but there is no way to find out that an error happened.

       

      I've found 3 jira issues reporting this:

       

      http://opensource.atlassian.com/projects/hibernate/browse/HHH-4721 (envers)
      http://opensource.atlassian.com/projects/hibernate/browse/HHH-3543 (core)
      http://opensource.atlassian.com/projects/hibernate/browse/HHH-4784 (core)

       


      @Adam: Did you know/take a look at this issue?

       

      Any ideas?

       

      Thanks. Hernan.

        • 1. Re: Question about HHH-4721
          hernanbolido

          When hibernate executes the transaction synchronization mechanism, any exception throwed by any implementation of the Synchronization interface (like AuditSync) is discarded by hibernate. So the exception does not propagates.

          I don´t know if hibernate synchronization mechanism works like this and maybe envers has to use another mechanism... Or it´s a bug on hibernate and in that case envers is doing the right thing.

           

           

          Here is AudisSync.beforeCompletion() catch  and finally blocks

           

          ........

                 } catch  (RuntimeException e) {
                       // Rolling back the transaction in case of any exceptions
                       //noinspection finally
                       try {
                           if (session.getTransaction().isActive()) {
                               session.getTransaction().rollback();
                           }
                       } finally {
                           //noinspection ThrowFromFinallyBlock
                          throw e;
                       }
                   }

           

          And here is JDBCTransaction method (where the exception throwed is discard):

           

              private void notifyLocalSynchsBeforeTransactionCompletion() {
                  if (synchronizations!=null) {
                      for ( int i=0; i<synchronizations.size(); i++ ) {
                          Synchronization sync = (Synchronization) synchronizations.get(i);
                          try {
                              sync.beforeCompletion();
                          }
                          catch (Throwable t) {
                              log.error("exception calling user Synchronization", t);
                          }

                      }
                  }
              }

           

          What do you think?


          • 2. Re: Question about HHH-4721
            adamw

            I agree that maybe it would be nicer if the exception propagated (although I'm not sure on why such a design decision was made and if it was made with consciousness). However:

            1. in case of any exception, the transaction is rolledback in AuditSync, so no data will be written

            2. the exception is logged, so it doesn't disappear entirely

             

            So it's not really critical, is it?

             

            Adam

            • 3. Re: Question about HHH-4721
              hernanbolido

              Hi Adam! Thanks for your time.

               

              The problem I can see is that the application using hibernate and envers doesn´t realized about the rollbacked transaction (because there is no exception to catch). So, you can´t tell the user that the operation failed...

               

              Thanks. Hernán.

              • 4. Re: Question about HHH-4721
                adamw

                But isn't an exception thrown when the transaction is commited? (Which can't happen, as it's rolledback, so commit should throw an exception.)

                 

                Adam

                • 5. Re: Question about HHH-4721
                  hernanbolido

                  Hi Adam!

                   

                  That´'s the point... The trqnsaction commit executed after AuditSync doesn't throw any exception, so the execution follows the normal (transaction commit ok) behavior.

                   

                  Here is a patch with a simple test case for this. The revision table is dropped to force a sql error inside AuditSync.beforeCompletion().

                   

                  I hope it helps.

                   

                  Thanks! Hernán.

                  • 6. Re: Question about HHH-4721
                    adamw

                    Ah right, there is in fact a test for this: ExceptionListener, but it only checks that the data is not commited.

                    But the user isn't notified, you're right.

                     

                    I'll ask the other Hibernate guys why is this so and get back to you.

                     

                    Adam

                    • 7. Re: Question about HHH-4721
                      hernanbolido

                      Great!

                       

                      Maybe a new exception could be added to the catch block in JDBCTransaction.notifyLocalSynchsBeforeTransactionCompletion() method indicating that this exception must be throwed. Then, AuditSync doesn´t need to execute the rollback and let the up layer do that. It´s an idea... but i don´t know if there are other issues to care about. Hibernate team can clarify this.

                      Transaction and Synchronization interfaces from jta api don´t make any assumptions or put restrictions about what to do in case of errors inside synchornizers, so, it´s a implementation desicion.

                       

                       

                      Thanks for the feedback. I would like to help here if you need.

                       

                      Hernán.

                      • 8. Re: Question about HHH-4721
                        hernanbolido

                        Hi!

                         

                        Adam, did you get any feedback from hibernate team in relation to this issue?

                         

                         

                        Thanks. Hernán.

                        • 9. Re: Question about HHH-4721
                          adamw

                          Sorry, I'm out doing trainings these weeks, so I didn't have much time, only the evenings

                          I posted a question on the mailing list, I'll let you know if there are any answers, or you can of course watch yourself.

                           

                          Adam

                          • 10. Re: Question about HHH-4721
                            hernanbolido

                            Hi Adam! First of all, thanks for your time!

                             

                            I made that question because I can´t found anything related to this issue in the mailing list. Maybe I'm watching in the wrong place...

                             

                            Thanks again. Hernán.

                            • 11. Re: Question about HHH-4721
                              adamw

                              It's the hibernate-dev mailing list.

                               

                              Adam

                              • 12. Re: Question about HHH-4721
                                hernanbolido

                                Hello!

                                 

                                I think I'm looking in the correct place: http://lists.jboss.org/pipermail/hibernate-dev/

                                 

                                But I found one mail with you as the author (february and march) and it´s about the web site migration...

                                • 13. Re: Question about HHH-4721
                                  adamw

                                  Hmm, weird, maybe it takes some time for the messages to appear on the list. Anyway, I didn't receive any responses yet. I'll ask on IRC next week, as I'll be finished with the trainings then .

                                   

                                  Adam

                                  • 14. Re: Question about HHH-4721
                                    hernanbolido

                                    Hi Adam!

                                     

                                    I saw that the probem was fixed using BeforeTransactionCompletionProcess instead of Synchronization!

                                    That's great!

                                    Do you think this solution could be pushed to envers-hibernate3.3 branch?

                                     

                                    By the way, HHH-4721 must be closed too, isn't it?

                                    http://opensource.atlassian.com/projects/hibernate/browse/HHH-4721

                                     

                                    Many thanks for this fix!

                                     

                                    Hernán.

                                    1 2 Previous Next