search

Nebo15 / sage

A dependency-free tool to run distributed transactions in Elixir, inspired by Sagas pattern.
MIT License
912 stars 40 forks source link

Ecto.transaction/2 in case of async stages #69

Closed gaynetdinov closed 2 years ago

gaynetdinov commented 2 years ago

Hi!

Do I understand correctly that even if I call Sage.transaction in my saga, my SQL changes in run_async stages are not going to be rolled back due to the fact, that repo.transaction/2 and repo.rollback/1 are executed in the 'main' process while run_async step is executed in another process via Task.async? My understanding is that the task process would checkout new connection from ecto pool, run the queries and that's it. The transaction/rollback function calls are happening outside of the async process within different database connection, therefore the changes made in async stage are not going to be rolled back.

Thank you for your time!

AndrewDryga commented 2 years ago

Hello @gaynetdinov. That is a great catch. Do you have ideas on how we can solve that? I don't think there is a way in Ecto to reuse connection checked out for a process in it's child :(.

Maybe the simplest thing we can do is to add docs to run_async on transaction isolation when used with Ecto.Repo.transaction.

gaynetdinov commented 2 years ago

This is the most comprehensive explanation regarding this topic I found from @fishcakez https://elixirforum.com/t/using-ecto-to-run-a-long-running-multi-process-transaction/4365/18. Basically, if I understood it correctly, the only potential 'fix' is writing your own Ecto adapter similar to Sandbox adapter that would allow interactions with connections somehow.

So I think you're right, the only thing that is feasible to do is to put into docs the note that one should not expect that insert/update SQL queries in run_async stages are going to be rolled back in the case when saga fails.

AndrewDryga commented 2 years ago

@gaynetdinov I'll leave this open if somebody is willing to contribute a Sage.Ecto.SQLSandbox for async stages but realistically It's a thing that can be hard to maintain (I'm trying to keep sage as simple as possible so that it's less error-prone).

And I'll go and add docs about that. Thank you for findings this again ❤️.

AndrewDryga commented 2 years ago

Added a note in https://github.com/Nebo15/sage/commit/94b420a87802e8067617544add7c0d37a5a2222d, please take a look and tell if it's not clear enough.

gaynetdinov commented 2 years ago

It looks clear to me 👌 Thank you!