Why does this code force use of a singleton Transaction.Current to check for transactions?

[jeremy-holovacs-sp]

Library name and version

Azure Service Bus .NET library

Query/Question

https://github.com/Azure/azure-sdk-for-net/blob/0a82584b4dc7858ca6aab6e9cd0c5762c0367b5f/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageSender.cs#L547

We ran into this issue when we were running a SQL transaction via EF, while trying to send messages on the service bus, and we'd get an error like "The only supported IsolationLevel is Serializable" which makes no sense in this scenario.

It appears this code is tightly coupling a transaction scope to something completely unrelated via a singleton. Why is it doing that, and surely there can be some injectable means of doing the same thing?

Environment

all (this is a base code issue)

[jsquire]

Thank you for your feedback. Tagging and routing to the team member best able to assist.

[ghost]

Thank you for your feedback. This has been routed to the support team for assistance.

[navba-MSFT]

@jeremy-holovacs-sp Thanks for reaching out to us and reporting this issue. We are looking into this issue. In the meantime, could you please share the complete exception details with callstack (if any)?

[danielmarbach]

Chiming here with the perspective of me using several versions of the ServiceBus library. I have no insights into why the transaction scope was originally chosen. I think it would have been possible to design an explicit unit of work like concept like we have today with the batch instead of relying on ambient transaction.

I also understand that at the time this was introduced TransactionScope was the defacto standard of grouping different operations together into a single transaction by having the methods be automatically enlisted into the ambient transaction. Transaction.Current is not really a singleton. It is more an ambient either "thread-bound" or "async local bound" (when using the TransactionScopeAsyncFlowOptions) state that is "floated" around. This allows any operation that is surrounded by a scope to tap into the Transaction.Current and then enlist into it.

From the perspective of the SDK as far as I understand is to have a short live scope that is dedicated to wrap the receive operation and all the send operations. This scope is only available for the SDK operations and should never be used to enlist any other transaction on it. That's why the Serializable isolation level was chosen to make sure no other operation that taps into ambient transaction can enlist.

The design choice of using the TransactionScope makes this a bit hard to use because you might be tempted to simply span a scope, execute all your code and then finally send the messages all included in the scope while effectively any operations that might try to enlist that are not SDK operations need to have their own scope and the surrounding scope required for the SDK operations need to be suppressed for the lifetime of these other calls. That makes things very complicated to use because you end up with code like the following (pseudocode):

using var scopeForSdkOperations = new TransactionScope(...);
await receiver.ReceiveMessageAsync();
   using var suppressScope = new TransactionScope(Suppress);
       using var entityFrameworkScope = new TransactionScope(...);
       var efContext = new EfContext()...
await sender.SendMessageAsync();   
await sender.SendMessageAsync();

To avoid accidental leakage of the scope and the complex suppression "song and dance" I have opted in to explicitly "float" around a CommittableTransaction object and turn that into a scope only when needed. One additional benefit this has is that you can delay the instantiation of that CommittableTransaction to the last point, and then you are as well less likely to run into the default transaction timeout of 10 minutes and all the complex workarounds you need to do to set that value.

The code looks more less like the following:

                // somewhere feasible but as late as possible so that EF transaction
                var transaction = new CommittableTransaction(transactionOptions);

               // execute EF stuff with their own scopes

                // turn the committable transaction into a scope
                using var scope = transaction.ToScope();
                await sender.SendMessagesAsync(messageBatch, cancellationToken);
                //committable tx will not be committed because this scope is not the owner
                scope.Complete();

                // some time later commit the committable transaction

static class TransactionExtensions
    {
        public static TransactionScope ToScope(this Transaction transaction)
        {
            return transaction != null
                ? new TransactionScope(transaction, TransactionScopeAsyncFlowOption.Enabled)
                : new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled);
        }
    }

see https://github.com/Particular/NServiceBus.Transport.AzureServiceBus/blob/master/src/Transport/Sending/MessageDispatcher.cs#L188-L191 and https://github.com/Particular/NServiceBus.Transport.AzureServiceBus/blob/master/src/Transport/Sending/MessageDispatcher.cs#L120-L122 plus the higher up the chain commit https://github.com/Particular/NServiceBus.Transport.AzureServiceBus/blob/master/src/Transport/Receiving/MessagePump.cs#L296

But yeah this is some non-trivial gymnastics to comply with the rules of the transaction scope just to get cross entity transaction working and also making sure that any other send always has a suppression scope to not accidentally run into the exception mentioned here.

I believe it would be worthwhile improving this in the new Azure.Messaging.ServiceBus library too.

[danielmarbach]

@JoshLove-msft :point_up: some feedback for the Azure.Messaging.ServiceBus client

[navba-MSFT]

@jeremy-holovacs-sp Transaction.Current is a thread local static. It's set for the executing thread. It's not a singleton.

Service Bus uses System.Transactions for its own transactions, but it can't be enlisted into an ongoing distributed transaction that involves SQL.

When there is an existing transaction scope, you need to create a new TransactionScope around the Service Bus operations with the scope option RequiresNew of you want Service Bus use transactions or Suppress if not. Hope this answers.

[ghost]

Hi @jeremy-holovacs-sp. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

[ghost]

Hi @jeremy-holovacs-sp, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.