cshannon commented on code in PR #2585:
URL: https://github.com/apache/activemq/pull/2585#discussion_r4082540733
##########
activemq-broker/src/main/java/org/apache/activemq/transaction/Transaction.java:
##########
@@ -134,24 +134,59 @@ protected void fireBeforeCommit() throws Exception {
}
protected void fireAfterCommit() throws Exception {
+ // The transaction outcome is already durable when afterCommit runs,
so a
+ // failing synchronization must not prevent the remaining ones from
+ // executing (e.g. the destination synchronization that maintains the
+ // destination statistics) - run them all and rethrow the first
failure.
+ Throwable firstException = null;
synchronized(synchronizations) {
for (Iterator<Synchronization> iter = synchronizations.iterator();
iter.hasNext();) {
Synchronization s = iter.next();
- s.afterCommit();
+ try {
+ s.afterCommit();
+ } catch (Throwable t) {
Review Comment:
I'm not sure even catching Exception and continuing is ideal but there may
not really be a choice. I'm a little concerned there could be some exceptions
that happen that we would want to abort the chain, but I'd have to think it
through. It's probably fine because the transaction is already
durable/committed at that point so it won't be rolled back either way. We would
def not want to execute if the transaction was not going to finish but this is
already post-commit as pointed out in the comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
