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

Lack of ambient transactions + Outbox/Inbox pattern #298

Open
andrewlock opened this issue Aug 29, 2019 · 11 comments
Open

Lack of ambient transactions + Outbox/Inbox pattern #298

andrewlock opened this issue Aug 29, 2019 · 11 comments

Comments

@andrewlock
Copy link

Hey guys,

I'm aware that you're adamant that there is no support for ambient System.Transaction scopes (and never will be), for the reasons @damianh describes in this issue.

The problem is that we want to enforce an outbox/inbox pattern for MassTransit/NServiceBus messages, which requires everything be in the same transaction (pretty much exactly the reason ambient transactions were added to NES by the sound of it). Obviously the design of SSS causes a problem with that.

Is there any possibility that you'd enable ambient transactions, even if in a completely unsupported way, e.g. through a config switch? I realise that you don't want to support those use cases, but it completely blocks some (as far as I understand it) best practices/patterns. Even if it's an unsupported edge case, it would allow us to use SSS as-is, instead of having to look elsewhere or forking.

Thanks for the help!

@thefringeninja
Copy link
Member

Can you be more specific? I'll admit it's been forever since I did anything with System.Transactions, but from what I remember, since we do not use TransactionScopeOption.Suppress you are free to wrap it up in your own TransactionScope. That is to say we will never support it but also not prevent it.

@andrewlock
Copy link
Author

Ah, System.Transaction was maybe a red herring (can't remember last time I used it either!) We're using Postgres and are creating an NpgsqlTransaction at the start of the handler processing, and calling commit/rollback at the end of the handler (which will also persist inbox/outbox messages).

SqlStreamStore has its own transaction handling, in the various methods like AppendToStreamInternal, for example:

using(var transaction = connection.BeginTransaction())

These are on a separate connection, in a separate transaction, so it would be possible for the SSS commit to succeed, but the overall handler transaction to fail for some unrelated reason.

mausch added a commit to elevate/SQLStreamStore that referenced this issue Oct 9, 2019
@maldworth
Copy link

I like this allowing an external transaction, would this also mean it's probably going to have PostgresStreamStore lifetime be scoped .AddScoped<PostgresStreamStore>(). Because my transaction is likely to be within the lifetime of a HTTP Request, or a Message Consumption (MassTransit/NServiceBus). And I am possibly doing some other DB Calls, all within the same transaction.

This also brings me to another problem I have. If I'm doing other DB calls, unrelated to EventStore, but I want it to be in the same transaction. This line

using(var connection = await OpenConnection(cancellationToken))

causes problem because it's disposing the connection when done. I'd like to adopt the idea that Dapper uses. It uses the connection it was given, if already open, or trys to open the connection if it was not. But the calling party is responsible for disposing the connection, not dapper. I'd be willing to put in a PR for this change as well, if the maintainers have an appetite for this.

@yreynhout
Copy link
Member

yreynhout commented Nov 1, 2019

What benefit are you getting from SSS at this stage though? I imagine the concept of an idempotent stream producer and a subscriber that gives you position management. The changes suggested will only work when people are comfortable putting their regular tables and SSS event tables in the same database (unless one were to entertain cross database txns). I'll let @damianh chime in, but my gut reaction is that this is not a use case SSS is optimized / designed for. Outbox pattern applied with SSS is usually when SSS's streams are the single source of truth (i.e. there are no other tables involved) and the only record of state changes. Obviously, @mausch 's solution (elevate@60b96f5) solves this particular problem - however, it also breaks the IStreamStore abstraction in the process. To be fair, it isn't a problem as a consumer if you only depend on PostgresStreamStore.

@mausch
Copy link
Contributor

mausch commented Nov 1, 2019

Unfortunately my spike isn't nearly good enough - it works on concrete types and the SSS code still disposes the transaction at the end of the operation.

@yreynhout
Copy link
Member

I imagine you'd want to derive from NpgsqlTransaction, decorating the original NpgsqlTransaction (the one returned from the connection or an ambient one), delegating all the work to it, but override dispose in such a way that disposal does not happen when used inside SSS code.

@mausch
Copy link
Contributor

mausch commented Nov 1, 2019

In principle yes, that was the general idea. My goal was to make the change as small as possible so that you might accept it in a PR. However NpgsqlTransaction is sealed :-)

@MarcusChamberlain
Copy link

Is there any followup on this? I'm using NServicebus with Outbox, but noticing that even though NSB is supposed to be wrapping SSS in a transaction scope, it's not actually rolling back DB changes in the event of an exception. I'm guessing it's related to the dispose being called in SSS?

In response to @yreynhout above

Outbox pattern applied with SSS is usually when SSS's streams are the single source of truth (i.e. there are no other tables involved) and the only record of state changes.

If SSS is the wrong stage to be applying SSS + Message Consumption Outbox transaction scopes, are there any recommendations on where this responsibility is usually handled? I imagine there must be many people using NServicebus/MassTransit Outbox with SSS, but there doesn't seem to be any way of actually getting the transaction scopes to play nicely together.

@maldworth
Copy link

@yreynhout The benefit is the service bus event outbox is now tied to the same transaction as the SSS write, without having to use a DTC.

@gaevoy
Copy link

gaevoy commented Mar 19, 2021

I experimented a bit to took NEventStore and SQLStreamStore (https://github.com/gaevoy/EventSourcingDoor/blob/main/EventSourcingDoor.SqlStreamStore/SqlStreamStoreOutbox.cs) in the role of transactional outbox and it turned out working well https://gaevoy.com/2021/03/18/audit-log-via-transactional-outbox.html

In my case I wanted audit log which is implemented via transactional outbox.

Let me know what issues do you see.

@alexeyzimarev
Copy link

The benefit is the service bus event outbox is now tied to the same transaction as the SSS write, without having to use a DTC.

It should not be used like this. SSS is producing events. SSS subscriptions consume events. Produce an event from a subscription, like with any other event store.

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

8 participants