Updated README.md with more precise information #347
Conversation
| The `SaleRepository` handles recording the sale in the customer's account, the `StockReductionEvent` goes off to our _warehouse_ service, and the `IncomeEvent` goes to our financial records service (let's ignore the potential flaws in the domain modelling for now). | ||
|
|
||
| There's a big problem here: the `@Transactional` annotation is a lie (no, [really](https://lmgtfy.com/?q=dont+use+distributed+transactions)). It only really wraps the `SaleRepository` call, but not the two event postings. This means that we could end up sending the two events and fail to actually commit the sale. Our system is now inconsistent. | ||
| The problem is that spring `@Transactional` annotation here is bounded to a current thread, and [it is wrapping the annotated method execution by hte means of AOP Proxies](https://docs.spring.io/spring-framework/docs/4.2.x/spring-framework-reference/html/transaction.html#tx-decl-explained) (honestly, the exact behavior depends on propagation level of transaction, but most of the times the propagation level is either `REQUIRED` or `REQUIRES_NEW`, where, in the absence of the already opened transaction for current thread will lead to the result of opening the transaction for the method execution). That is, before the start of the method, spring `TransactionInterceptor` will begin the transaction an delegate the exectuion to the `createWidget` method. |
There was a problem hiding this comment.
Thanks for this @mikhail2048!
This isn't strictly accurate; one does not need to use the Spring AOP interceptor; one can use an AspectJ implementation if one prefers. And I have seen implementations which aren't thread-bound, but are bound to an active JTA transaction to try and achieve distributed transactions (which doesn't really work IMO).
However, you're right in that the way you describe is the most common, and it makes me think that "less is more" here: we can just say that the transaction won't include async code.
There was a problem hiding this comment.
That's perfectly true, I agree. I guess I just made an assumption that the reader use regular spring AOP (which is the most common case in my experience).
Maybe I should make a note here about thread-bounded transaction manager, you agree?
There was a problem hiding this comment.
I don't think it matters really: the important thing is not whether the transaction is thread-bound or not. The problem is that we can't reliably perform transactions across more than one application.
Maybe instead of
the
StockReductionEventgoes off to our warehouse service, and theIncomeEventgoes to our financial records service
We should use the word "application":
the
StockReductionEventgoes off to our warehouse application via a REST API, and theIncomeEventgoes to our financial records service via a REST API
Then all we need to say is:
There's a big problem here: the
@Transactionalannotation can only (reliably) affect transactions in a single database instance and a single application. The code run in the warehouse application and financial records application won't be included in that transaction. This means that we could end up committing the changes in warehouse and financial records, then fail to actually commit the sale. Our overall system is now inconsistent.
I have slightly adjusted the doc, because there are some errors in the current README file regarding work with transactions in spring. The message that the author was trying to deliver make sense, it is correct, but just to not leave the spring knowledgeable readers confused I have decided to change the doc a little bit ;)