r2dbc / r2dbc-mssql

R2DBC Driver for Microsoft SQL Server using TDS (Tabular Data Stream) Protocol
Apache License 2.0
178 stars 32 forks source link

Transaction Names should be truncated when too long. #264

Closed nhajratw closed 1 year ago

nhajratw commented 1 year ago

Bug Report

Current Behavior

When using r2dbc-mssql via spring-r2dbc with @Transactional, the name of the transaction is the FQ class name. This cannot be overridden:

https://docs.spring.io/spring-framework/docs/current/reference/html/data-access.html#transaction-declarative-attransactional-settings

Currently, you cannot have explicit control over the name of a transaction, where 'name' means the transaction name that appears in a transaction monitor, if applicable (for example, WebLogic’s transaction monitor), and in logging output. For declarative transactions, the transaction name is always the fully-qualified class name + . + the method name of the transactionally advised class. For example, if the handlePayment(..) method of the BusinessService class started a transaction, the name of the transaction would be: com.example.BusinessService.handlePayment.

r2dbc-mssql is correctly limiting the transaction name to 32 characters per https://learn.microsoft.com/en-us/sql/t-sql/language-elements/begin-transaction-transact-sql?view=sql-server-ver16#arguments

As such, we end up with this exception:

Exception in thread "DefaultDispatcher-worker-1" org.springframework.transaction.CannotCreateTransactionException: Could not open R2DBC Connection for transaction
    at ...
    at java.base/java.lang.Thread.run(Thread.java:833)
    Suppressed: kotlinx.coroutines.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@2800d129, Dispatchers.Default]
Caused by: java.lang.IllegalArgumentException: Transaction names must contain only characters and numbers and must not exceed 32 characters
    at io.r2dbc.mssql.util.Assert.isTrue(Assert.java:106)
    at io.r2dbc.mssql.MssqlConnection.lambda$beginTransaction$1(MssqlConnection.java:110)
    at io.r2dbc.mssql.MssqlConnection.lambda$useTransactionStatus$15(MssqlConnection.java:425)
    at reactor.core.publisher.FluxDefer.subscribe(FluxDefer.java:46)
    ... 59 more

Steps to reproduce

Any class like the following will reproduce the problem:

package com.company.name.here

@Service
class SomeServiceClass() {

    @Transactional
    suspend fun theMethod() {
        ...
    }
}

the transaction name here will be com.company.name.here.SomeServiceClass.theMethod which is > 32 chars an errors.

Possible Solution

Truncate the beginning of the name so that you end up with the last 32 characters (excluding punctuation): i.e. here.SomeServiceClass.theMethod

anassder commented 1 year ago

MSSQL truncates transaction's name automatically if it exceed 32 characters, so we can simply show warning instead of throwing exception.

nhajratw commented 1 year ago

agreed -- that could work as well. The reason i suggested truncating the beginning is that if you have two different methods:

com.company.name.here.SomeServiceClass.theMethod1
com.company.name.here.SomeServiceClass.theMethod2

Then truncating the beginning would give both transactions the same name of com.company.name.here.SomeServic, which would make it impossible to differentiate when looking at logs.

mp911de commented 1 year ago

Happy to include a truncation. We might not log this event but rather silently accept the fact that we're truncating the transaction name.

The issue originates from a default behavior where you (do/can) not specify the transaction name. Hence we can just remove the obstacle that causes transactions to fail. A natural consequence of truncating is information loss. If the data beforehand would be correct, then we would not run into that issue.