-
Notifications
You must be signed in to change notification settings - Fork 626
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
Message are not commited (loss) when using @TransactionalEventListener to send a message in a JPA Transaction #1309
Comments
I need to investigate, but most likely it's because it is called in the AFTER_COMMIT phase, so there is no transaction at this time. We certainly shouldn't be leaving the dangling bound resource, though; if there's really no transaction we should use a local transaction instead. This logic predated Your work around sounds correct.
Not sure what you mean by that, there is no "parent" RabbitMQ transaction, only JPA. I see you also asked this here https://stackoverflow.com/questions/66613863/message-are-not-commited-loss-when-using-transactionaleventlistener-to-send-a Please don't waste your time and ours asking the same question in multiple places. |
By parent transaction, I mean the actual transaction where I did initiate JPA transaction by using @transaction.
I asked in StackOverflow, but I have to ask it here also as I did not get any response/acknowledgment on that question. |
To be fair, you asked it there on a weekend; you should give us at least one working day to respond. The problem is, with this configuration, we are in a quasi-transactional state, while there is indeed a transaction in process, the synchronizations are already cleared because the transaction has already committed. Using |
I agree with you, and accept my apologies for being impatient. I might have some understanding gap here, If a transaction is already committed, Why we got So will we consider this kind of transaction as actual transaction active or will we ignore such transaction and do a local transaction commit ? |
The transaction is still "active", but we're in the after completion phase and the JPA TM (AbstractPlatformTM) clears the synchronizations before calling their private void triggerAfterCompletion(DefaultTransactionStatus status, int completionStatus) {
if (status.isNewSynchronization()) {
List<TransactionSynchronization> synchronizations = TransactionSynchronizationManager.getSynchronizations();
TransactionSynchronizationManager.clearSynchronization(); // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
if (!status.hasTransaction() || status.isNewTransaction()) {
if (status.isDebug()) {
logger.trace("Triggering afterCompletion synchronization");
}
// No transaction or new transaction for the current scope ->
// invoke the afterCompletion callbacks immediately
invokeAfterCompletion(synchronizations, completionStatus);
}
else if (!synchronizations.isEmpty()) {
// Existing transaction that we participate in, controlled outside
// of the scope of this Spring transaction manager -> try to register
// an afterCompletion callback with the existing (JTA) transaction.
registerAfterCompletionWithExistingTransaction(status.getTransaction(), synchronizations);
}
}
} I need to think through a solution, but BEFORE_COMMIT should work for you. |
Hi Gary, (I am in same team of Kuldeep) By following statement : Are you suggesting that the @TransactionalEventListener we are using is an older pattern. Does this mean we should be using something else? Thanks |
No; I am not suggesting that, I am suggesting that nobody has hit this problem before. This capability was added to Spring in 2015; the Rabbit transaction logic has been around since 2010. That just tells me that nobody has used this feature (with AFTER_COMMIT) before you. |
I tried BEFORE_COMMIT and it worked as expected. Now we have two workarounds for now.
I checked how BEFORE_COMMIT Works and found that
In processCommit calls, triggerBeforeCommit, and that will trigger the ActivityEvent (BEFORE_COMMIT) and send the message. And I also understand Propagation.REQUIRES_NEW will start a fresh new transaction. @garyrussell, So what will you suggest in our case? BEFORE_COMMIT or Propagation.REQUIRES_NEW? |
The problem is BEFORE_COMMIT will send the message even if the JPA transaction commit() fails afterwards, so the better work around is However, I wouldn't start a new JPA transaction, use a |
I checked it, I deleted the table, at first After sending the message, now In Short, the message will not be committed in this case as well because of DB exception/rollback. And I agree with the point that the new JPA transaction is not good. But in our actual framework, we use DB Operation + Send Message So I am not sure about |
In that case, that is the better solution. Starting a new transaction for the record send will be an independent transaction that will commit whether or not the JPA transaction commits. To answer your last point... But the rabbit transaction is simply synchronized with the JPA transaction; there is no point in starting a new JPA transaction that does nothing. Using a RabbitTransactionManager (just for that one But, it seems BEFORE_COMMIT is the best work around until I fix the problem (should be fixed by Wednesday). |
Actually, I don't think it's a work-around, it's the solution (using I can fix the |
Resolves spring-projects#1309 If the `RabbitTemplate` was called from a `@TransactionalEventListener`, with phase `AFTER_COMMIT`, a transaction is "active", but synchronizations are already cleared. It is too late to synchronize this transaction. We end up with an orphaned `ResourceHolder` with pending transaction commits. Don't bind the resource holder if synchronization is not active. However, the proper solution, if users want to synchronize the rabbit transaction with the global transaction, is to use the `BEFORE_COMMIT` phase. See the discussion on the Github issue for more information. **cherry-pick to 2.2.x**
Resolves spring-projects#1309 If the `RabbitTemplate` was called from a `@TransactionalEventListener`, with phase `AFTER_COMMIT`, a transaction is "active", but synchronizations are already cleared. It is too late to synchronize this transaction. We end up with an orphaned `ResourceHolder` with pending transaction commits. Don't bind the resource holder if synchronization is not active. However, the proper solution, if users want to synchronize the rabbit transaction with the global transaction, is to use the `BEFORE_COMMIT` phase. See the discussion on the Github issue for more information. **cherry-pick to 2.2.x**
Resolves spring-projects#1309 If the `RabbitTemplate` was called from a `@TransactionalEventListener`, with phase `AFTER_COMMIT`, a transaction is "active", but synchronizations are already cleared. It is too late to synchronize this transaction. We end up with an orphaned `ResourceHolder` with pending transaction commits. Don't bind the resource holder if synchronization is not active. However, the proper solution, if users want to synchronize the rabbit transaction with the global transaction, is to use the `BEFORE_COMMIT` phase. See the discussion on the Github issue for more information. **cherry-pick to 2.2.x**
Resolves spring-projects#1309 If the `RabbitTemplate` was called from a `@TransactionalEventListener`, with phase `AFTER_COMMIT`, a transaction is "active", but synchronizations are already cleared. It is too late to synchronize this transaction. We end up with an orphaned `ResourceHolder` with pending transaction commits. Don't bind the resource holder if synchronization is not active. However, the proper solution, if users want to synchronize the rabbit transaction with the global transaction, is to use the `BEFORE_COMMIT` phase. See the discussion on the Github issue for more information. **cherry-pick to 2.2.x**
Resolves spring-projects#1309 If the `RabbitTemplate` was called from a `@TransactionalEventListener`, with phase `AFTER_COMMIT`, a transaction is "active", but synchronizations are already cleared. It is too late to synchronize this transaction. We end up with an orphaned `ResourceHolder` with pending transaction commits. Don't bind the resource holder if synchronization is not active. However, the proper solution, if users want to synchronize the rabbit transaction with the global transaction, is to use the `BEFORE_COMMIT` phase. See the discussion on the Github issue for more information. **cherry-pick to 2.2.x**
I Agree with this, and your fix will ensure no orphan resource will be added due to But we came up with another solution :
So now we are not going to use this new |
Resolves #1309 If the `RabbitTemplate` was called from a `@TransactionalEventListener`, with phase `AFTER_COMMIT`, a transaction is "active", but synchronizations are already cleared. It is too late to synchronize this transaction. We end up with an orphaned `ResourceHolder` with pending transaction commits. Don't bind the resource holder if synchronization is not active. However, the proper solution, if users want to synchronize the rabbit transaction with the global transaction, is to use the `BEFORE_COMMIT` phase. See the discussion on the Github issue for more information. **cherry-pick to 2.2.x** * Fix since version. * Reset physical close required flag left over from another test. * Capture test results for all modules.
Resolves #1309 If the `RabbitTemplate` was called from a `@TransactionalEventListener`, with phase `AFTER_COMMIT`, a transaction is "active", but synchronizations are already cleared. It is too late to synchronize this transaction. We end up with an orphaned `ResourceHolder` with pending transaction commits. Don't bind the resource holder if synchronization is not active. However, the proper solution, if users want to synchronize the rabbit transaction with the global transaction, is to use the `BEFORE_COMMIT` phase. See the discussion on the Github issue for more information. **cherry-pick to 2.2.x** * Fix since version. * Reset physical close required flag left over from another test. * Capture test results for all modules. # Conflicts: # .github/workflows/pr-build-workflow.yml # spring-rabbit/src/test/java/org/springframework/amqp/rabbit/connection/ClientRecoveryCompatibilityTests.java
Thanks for the fix @garyrussell and @artembilan, If it is possible, can you help me to get it merged on v2.2.1? |
@kuldeepkalassts It's already available in v2.2.16. https://github.com/spring-projects/spring-amqp/releases/tag/v2.2.16.RELEASE |
Hi Gary, |
@iav20 It doesn't work that way. When a bug is found in, say, 2.2.1, it is fixed in the next patch version, 2.2.2. The current 2.2.x version is 2.2.16.RELEASE.
That is not possible. You have to upgrade to 2.2.16. 2.2.1 was released well over a year ago; you must keep your versions up-to-date to get the latest fixes and updates. Using old software versions is generally not a good idea. Installing patch releases is much different to upgrading to, say, 2.3.x, when you would need to do much more testing. Changes in the 2.2.x line are generally just bug fixes. |
Hi Gary , So we need to take spring-core.2.2.16 jar and spring-rabbit.2.2.16 jar version for this change ? |
spring-amqp 2.2.16.RELEASE; spring-rabbit 2.2.16.RELEASE; spring-core (and all spring framework dependencies) 5.2.13.RELEASE. When using maven/gradle, It's best to just import spring-rabbit and let it bring in all the correct dependency versions. |
Hi Gary, We are currently using spring family of jars (version 5.2.1) throughout our codebase. Upgrading all spring family of jars is again a major effort |
I am not aware of any issues you might have but, once again, you should really keep up-to-date on versions, upgrading spring jars within the 5.2.x line should not cause you any issues, they only contain bug fixes; the latest 5.2.x is 5.2.13 and that is the version we test spring-rabbit 2.2.16 against. |
Background of the code:
In order to replicate a production scenario, I have created a dummy app that will basically save something in DB in a transaction, and in the same method, it publishEvent and publishEvent send a message to rabbitMQ.
Classes and usages
Transaction Starts from this method.:
This method saves the record in DB and also triggers publishEvent
This is ActivityEvent
And this is TransactionalEventListener for the above Event.
This is kRabbitTemplate is a bean config like this :
Problem Definition
When I am saving a record and sending a message on rabbitMQ using the above code flow, My messages are not delivered on the server means they lost.
What I understand about the transaction in AMQP is :
What I found after investigating the reason for message loss in my above code.
In my code, after resource bind, it is not able to registerSynchronization because
TransactionSynchronizationManager.isSynchronizationActive()==false
. and since it fails to registerSynchronization, spring commit did not happen for the rabbitMQ message asAbstractPlatformTransactionManager.triggerAfterCompletion
calls RabbitMQ's commit for each synchronization.What problem I faced because of the above issue.
Possible Root Cause of
TransactionSynchronizationManager.isSynchronizationActive()==false
status.isNewSynchronization()
evaluatedtrue
after DB opertation (this usually not happens if I call convertAndSend without ApplicationEvent).What I Did to overcome on this issue
I simply added
@Transactional(propagation = Propagation.REQUIRES_NEW)
along with on@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
in onActivitySave method and it worked as a new transaction was started.What I need to know
status.isNewSynchronization
in triggerAfterCompletion method when using ApplicationEvent?TransactionSynchronizationManager.isActualTransactionActive()==true
in Listner class?Please help me with this, it is a hot production issue, and I am not very sure about the fix I have done.
The text was updated successfully, but these errors were encountered: