NServiceBus.Persistence.Sql icon indicating copy to clipboard operation
NServiceBus.Persistence.Sql copied to clipboard

Unexpected Serializable isolation level when using the outbox with `UseTransactionScope` while ReadCommitted is the default everywhere else

Open pardahlman opened this issue 3 years ago • 7 comments

We are migrating from using TransactionScope via the UnitOfWork feature to using the outbox in TransactionScope mode. With the default configuration, the IsolationLevel of the TransactionScope is changed from ReadCommitted to Serializable. At first I thought this was unintentional (something like falling back to a default value on an enum), but when I looked at the code I saw that it is explicit and even had the default value specified in the summary comment.

For TransactionScope on the transport the default value is ReadCommited according to these docs. The same is true for the ADO Transaction for the outbox according to the code. The same is also true for UnitOfWork (code). Why use a different isolation level for the outbox created TransactionScope?

pardahlman avatar May 17 '21 10:05 pardahlman

@pardahlman I'm not exactly sure why that is. I've asked my colleagues for information.

Are you in any way affected by the unexpected default?

ramonsmits avatar May 25 '21 15:05 ramonsmits

There is a simple workaround: set an explicit isolation level when configuring the outbox with TransactionScope:

var outboxSettings = endpointConfiguration.EnableOutbox();
outboxSettings.UseTransactionScope(IsolationLevel.ReadCommitted);

However, it was not trivial to pin down the reason for the unexpected behavior in the application to this default value. That's why I raised the issue here: I think it would make sense to default to ReadCommitted for the outbox in TransactionScope mode to prevent other developers to spend time troubleshooting the same thing in the future 👍

pardahlman avatar May 26 '21 08:05 pardahlman

@pardahlman thank you for sharing the context. I don't think there is any concrete reason why we when for Serializable apart from the fact that the TransactionScope has that as default. That said we should get aligned with the rest of the infrastructure to (as you indicated) prevent unexpected behavior.

I'll mark this issue as a candidate for the next enhancement release. Thank you!

tmasternak avatar May 26 '21 08:05 tmasternak

@pardahlman Are you getting an exception/error when using the default API? How did you notice the unexpected behavior? Could you provide the error details that could help us with providing assistance to our users?

ramonsmits avatar May 26 '21 13:05 ramonsmits

@ramonsmits we started to notice deadlocks in calls made to the SQL-server for certain handlers. It was not deterministic failure, sometimes a message would be processed in the problematic handler without any errors. Here's an example StackTrace form our logs

Microsoft.Data.SqlClient.SqlException (0x80131904): Transaction (Process ID 243) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
   at Microsoft.Data.SqlClient.SqlCommand.InternalExecuteNonQuery(TaskCompletionSource`1 completion, Boolean sendToPipe, Int32 timeout, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String methodName)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteNonQuery()

pardahlman avatar May 26 '21 13:05 pardahlman

@pardahlman thanks for that information.

ramonsmits avatar May 26 '21 16:05 ramonsmits

I think it would be best to drop the UseTransactionScope() with defaults to make the isolation level explicit.

ramonsmits avatar Jun 01 '21 07:06 ramonsmits