Skip to content
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

Is there a way to rollback the whole sage using its effects? #58

Open
agleb opened this issue Dec 25, 2020 · 7 comments
Open

Is there a way to rollback the whole sage using its effects? #58

agleb opened this issue Dec 25, 2020 · 7 comments

Comments

@agleb
Copy link
Contributor

agleb commented Dec 25, 2020

In some specific cases I have to run sequences of sub-sages in one high-level sage.

run() allows to attach sages perfectly but only in forward direction. If error occurs somewhere in sub-sages I need to compensate everything happened before the error.

After the sage is completed, I'm left only with its effects and it would be nice to have some rollback(sage,effects) method to provide compensations mech for sub-sages.

Now I have to build nested sages with a last run() in each sage to run the subsequent sage, which seems an overkill for such a straightforward requirement.

@AndrewDryga
Copy link
Member

@agleb I'm sorry for giving so late reply. Can you please give here a few examples of sagas that are built like that? Without code, It's hard to suggest something specific. No need to share business logic, just Sagas composition and names (so that we can reason on them).

@agleb
Copy link
Contributor Author

agleb commented Jan 26, 2021

Yes, sure.

Let's say, we have

import Sage

def create_and_subscribe_user() do
new()
    |> run(:user, &create_user/2)
    |> run(:plans, &fetch_subscription_plans/2, &subscription_plans_circuit_breaker/3)
    |> run(:subscription, &create_subscription/2, &delete_subscription/3)
    |> run_async(:delivery, &schedule_delivery/2, &delete_delivery_from_schedule/3)
    |> run_async(:receipt, &send_email_receipt/2, &send_excuse_for_email_receipt/3)
    |> run(:update_user, &set_plan_for_a_user/2)
end

Then:

 {:ok, result, effects} =  create_and_subscribe_user() |> execute(attrs)

And then, if necessary:

 {:ok, result} = create_and_subscribe_user() |> rollback(effects)

I mean, is it possible to manually initiate rollback, using the effects of execution?

@AndrewDryga
Copy link
Member

AndrewDryga commented Jan 27, 2021

Right now that is not possible but I may have a suggestion. Do I understand it correctly that this saga is built like that because {:ok, result, effects} = create_and_subscribe_user() |> execute(attrs) is called within run/3 of another saga?

If that is the case, would it work if instead of executing sagas one in another one we will merge them and execute as a whole?

I'm thinking how we can improve Sage to fit your use case and see two options:

  1. Add a force_rollback(saga, effects)
  2. Add a merge(saga2, saga1) similarly to Ecto.Multi

@agleb
Copy link
Contributor Author

agleb commented Jan 29, 2021

In majority of cases - yes, it's about running sub-sagas, but there are cases, where a rollback is also necessary.

The product in my system has a lifecycle, controlled by a finite state machine. Some transitions between states do not require user/side-services interaction, so they happen in auto-forward mode. In my case, finite state machine decides which sagas to run, which in turn leads to building and running sagas, built of sagas.

In the use-case you're taling about it might be even more useful to have a run_sub_saga(saga) method and get nested effects and results.

Now I'm:

import Sage

def create_and_subscribe_user_body(saga) do
    saga
    |> run(:user, &create_user/2)
    # ...
    |> run(:update_user, &set_plan_for_a_user/2)
end

def complex_saga_body() do
   new()
   |> create_and_subscribe_user_body()
   |> other_saga_body()
   |> one_more_saga_body()
end

and then:

   complex_saga_body() |> execute(attrs)

which is in fact merge([create_and_subscribe_user_body(),saga2(),saga3()])

@AndrewDryga
Copy link
Member

@agleb so we did not have a rollback/3 before because to me it always felt like an anti-pattern: If there is some business logic that needs to abort Saga execution that means that it should be part of the Saga itself.

Is there a way to make that abort logic that lives in FSM also become the last run in a saga? Then the saga can return the new state for the FSM as a result of execution. And if that works for you that means we don't need a rollback :).

@agleb
Copy link
Contributor Author

agleb commented Feb 2, 2021

In our case, the FSM completely controls which transition handler (saga) to run and FSM can get a command to undo last step and it will try to, if this state is configured to allow an undo.

I'd disagree: an explicit, user-commanded and absolutely schema-vise valid "undo" transition seems not an antipattern. In contrary, duplicating the logic in a completely mirrored reverse transition and maintaining it along with a forward one - that is definitely an antipattern.

I think, having a method to correctly and reliably revert a revertable action (to roll-back a transaction) - is a good way to give an end-user a chance to painlessly fix mistakes and continue in comfort, while keeping the app's codebase clean of mirrored duplicates.

@AndrewDryga
Copy link
Member

AndrewDryga commented Feb 7, 2021

I think we have enough ideas for improvement and I'm planning to spend some time on Sage in near future, I think adding force_rollback/2 is a good idea. Probably we will raise if some stages were executed without compensation callback (eg. because Sage was run in an Ecto transaction).

If you don't want to wait, a PR would be welcome :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants