Skip to content

Updated README.md with more precise information #347

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.

Sign up for GitHub

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

Open
mipo256 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Updated README.md with more precise information #347

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ public SaleId createWidget(Sale sale) {

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.
Copy link
Copy Markdown
Member

@badgerwithagun badgerwithagun Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Author

@mipo256 mipo256 Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

@badgerwithagun badgerwithagun Dec 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 StockReductionEvent goes off to our warehouse service, and the IncomeEvent goes to our financial records service

We should use the word "application":

the StockReductionEvent goes off to our warehouse application via a REST API, and the IncomeEvent goes to our financial records service via a REST API

Then all we need to say is:

There's a big problem here: the @Transactional annotation 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.


So, first,we call `saleRepository#save()` method to save entity to a database. As we understood, calling `saleRepository#save()` will not trigger the `COMMIT` statemnt, becasue we are still in the middle of method execution. Then, we will publish the message to a message broker. Here the problem comes - after publication of message to some abstract message broker, lets say it is Kafka, we cannot some sort of unpublish it - it is already gone, so, if the microservice will for some reason fail or get restarted in between (for isntance you are running a K8S cluster, where the pods get recreate/restarterd all the time), lets say after publication of the messsage, but before transaction commit. In this case we ended up in the incosistent state - we have published message to a message broker, but did not persist it into the database, becuase we did not commi tthe transaction.

Now, we understood the problem, lets try to fix it.

### Attempt 2 - Use Idempotency

Expand Down