Collect events from transaction and pass them to final callback

[AndrewDryga]

We should educate developers to never send events from within the transaction.

A this is a very common case I've dealt with. Usually, we spawn a lot of events and it's a very bad practice to when code that spawns them is used within a database transaction - we don't have a way to cancel them but the transaction can be rolled back. But you never know all the details and you never know who would use your code within a transaction in one of the new business processes.

Sometimes sending events before a transaction is committed creates a race condition. When there is a code that listens for events and reads the value from the database it would crash, because the value can be not there yet.

Also, sometimes the high-level code just knows better what events it should send and when, so we don't want to fire them from within a context at all. A good example from practice - when a user is signed up with a pre-defined plan (by a voucher code), we don't want to broadcast subscription.created event right there, because it would come before the user.created event.

Speaking in terms of code, this is bad:

defmodule MyApp.Domain.Users.UseCase do
  def sign_up(attrs) do
    MyApp.Domain.Repo.transaction(fn ->
      # ... create an account ...
      MyApp.broadcast(MyApp.PubSub, "account.created", account: account)
      account
    end)
  end
end

Good:

defmodule MyApp.Domain.Users.UseCase do
  def sign_up(attrs) do
    with {:ok, {account, events}} <- create_account(attrs) do
      for {:event, kind, metadata} <- events do
        MyApp.broadcast(MyApp.PubSub, kind, metadata)
      end

      {:ok, account}
    end
  end

  defp create_account(attrs) do
    MyApp.Domain.Repo.transaction(fn ->
      # ... create an account ...
      {account, [{:event, "account.created", account: account}]}
    end)
  end
end

To handle those scenarios we can threat events as effects, return them as the third element of :ok tuple for compensations, accumulate (with scoping them by stage name to don't add a second place where developer should keep track on the names) and only fire when the transaction is succeeded from final callback.

Actionable items:

• Make sure events are sent only AFTER transaction is committed;
• Leverage persistence layer so that those events should be set at least once (if Sage fails between commit and events dispatching we need to retry later stage).

[tlvenn]

Hi @AndrewDryga,

It's definitely an improvement but if you publish events after a transaction is committed, you run the risk of your program crashing after the commit, but before the events are broadcasted.

I have seen a article on staged jobs which aims to solve a similar issue but for background jobs.

https://brandur.org/job-drain

I wonder if a similar approach shouldn't be use to deal with events as well.

[AndrewDryga]

@tlvenn that's a nice idea, however, I'm not sure that it's doable right now because we don't know if there would be a transaction and we don't have a database to stage events.

Edge case where Sage transaction is committed and we crashed right after that can be covered with #9. So we would still have a log of execution and list of effects we need to spin off, so we would be able to spin off them after a node is recovered. But guarantee there would be that events are sent "at least once".

[tlvenn]

Hi @AndrewDryga,

If there is no transaction, the staged event would be picked up by the event enqueuer right away and if there is a transaction, only when that transaction is committed the events become visible to the enqueuer. So basically the approach works in both cases.

Now I am not saying this should be implemented by Sage at all, maybe that's where the confusion is coming from. I am merely suggesting that maybe Sage should not try to deal with this or maybe it should and it would over different tradeoffs over a staged job drain system.

[tlvenn]

Btw I kinda feel it's a bit misleading to say the following in the doc regarding finally/2:

Sage does it's best to make sure the final callback is executed even if there is a program bug in the code. This guarantee simplifies integration with job processing queues, you can read more about it at GenTask Readme.

What you are saying regarding events here is spot on but the same applies to background jobs. The final callbacks created with finally/2 are executed within the transaction which mean that the background job could be triggered before the transaction has been committed and if the transaction fails, there is nothing you can do about that background job you already have scheduled.

The way finally/2 documentation is articulated kinda give the false impression that the callbacks are executed after the transaction complete imho.

[tlvenn]

And btw, amazing work with Sage, thanks a lot for this library @AndrewDryga ! (and for confex as well)

[AndrewDryga]

@tlvenn that's a good catch, I already added TODO for myself that it should be actually executed after the transaction is committed (if there is one). Right now it's called within the transaction before it is committed.

[tlvenn]

Ha that would be great @AndrewDryga !