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

Improve the jOOQ transaction experience #24049

Open
lukaseder opened this issue Nov 5, 2020 · 19 comments
Open

Improve the jOOQ transaction experience #24049

lukaseder opened this issue Nov 5, 2020 · 19 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@lukaseder
Copy link
Contributor

When using Spring Boot with jOOQ, a common source of confusion is how to use transactions. There are mainly three options:

  • Ignore jOOQ's transaction APIs, and do everything Spring style (using @Transactional annotations or programmatically, using a TransactionManager
  • Ignore Spring's transaction APIs and use jOOQ's APIs only
  • Mix the two APIs

The first case doesn't lead to any integration problems, but the second and third ones do. For example, the org.springframework.boot.autoconfigure.jooq.SpringTransactionProvider does not work with jOOQ's asynchronous transactions as can be seen in this issue here: jOOQ/jOOQ#10850 (helpful MCVE here: https://github.com/anushreegupta44/jOOQ-mcve). The workaround is to manually remove the SpringTransactionProvider again from the jOOQ Configuration, but users can reasonably expect this to work out of the box, especially when there are no Spring transactions involved.

The SpringTransactionProvider was originally implemented as a bridge to allow for mixing Spring transactions (e.g. based on @Transactional annotations), and then in some methods, use jOOQ's transaction APIs, which delegate to Spring, but it's probably not implemented correctly for all cases, nor am I sure whether it should always be added to Spring Boot's jOOQ auto-configuration, because when people want to use jOOQ's transaction APIs, it's not necessary.

I'm not sure what the best approach here is to improve the out-of-the-box integration experience for all three of the above cases... My intutition tells me that the SpringTransactionProvider should get an instance of the org.jooq.impl.DefaultTransactionProvider and delegate to it for all three methods (begin, commit, rollback) when TransactionSynchronizationManager.isActualTransactionActive() is false, but that may not be sufficient for asynchronous transactions.

Thoughts?

@wilkinsona
Copy link
Member

wilkinsona commented Nov 5, 2020

My first thoughts are that I'd need to learn more about jOOQ's transaction support to form an opinion. @michael-simons as someone who knows Spring and jOOQ pretty well, your thoughts on this would be more than welcome if you have the time.

@michael-simons
Copy link
Contributor

I can only answer from Neo4j's perspective:
Neo4j-OGM added something like you have on top to integrate driver transaction with Spring Transaction and it is a major pain. Something fails all the time.

We decided to provide both imperative and reactive, native Spring Transaction managers, that opt all in.
If people don't like it, they can bail out and use our driver transaction (imperative, async or reactive).

I'm advocating internally for exact that approach.

@lukaseder
Copy link
Contributor Author

If people don't like it, they can bail out and use our driver transaction (imperative, async or reactive).

How does bailing out work in your case? Is that something that has to be done explicitly via extra configuration? Or is it automatic, once they use your own native transaction APIs?

@michael-simons
Copy link
Contributor

michael-simons commented Nov 5, 2020 via email

@lukaseder
Copy link
Contributor Author

Oh interesting, so you start a new one despite there possibly already being one from the container. I kinda tend to think that whoever's first, wins, and the other "party" needs to make sure their transaction is properly nested, unless a new one is requested explicitly... That way, users can nest A in B or B in A, irrespective of what library/framework created A and/or B

@michael-simons
Copy link
Contributor

michael-simons commented Nov 6, 2020 via email

@lukaseder
Copy link
Contributor Author

Sure, there's always a way to bypass things.

But the Spring Boot jOOQ starter configures jOOQ with a managed DataSource and a SpringTransactionProvider, which doesn't work correctly for some cases. This isn't about calling JDBC API on unmanaged data sources. This is about calling jOOQ API which can be reasonably expected to operate on the managed data source. Something that is supposed to work out of the box currently isn't working, so combining Spring Boot and jOOQ (in this case) leads to more setup effort rather than less setup effort.

My question here is whether jOOQ needs to be fixed, or the SpringTransactionProvider, or something entirely different. I tend to think the SpringTransactionProvider is not up to date, but before I look into providing a PR, I wanted to collect a few opinions.

@wilkinsona
Copy link
Member

wilkinsona commented Nov 6, 2020

My intutition tells me that the SpringTransactionProvider should get an instance of the org.jooq.impl.DefaultTransactionProvider and delegate to it for all three methods (begin, commit, rollback) when TransactionSynchronizationManager.isActualTransactionActive() is false, but that may not be sufficient for asynchronous transactions.

Would this not make this significantly more complicated? Assuming I've understood things correctly, won't it result in either Spring's PlatformTransactionManager or jOOQ managing the current transaction? The current intent of SpringTransactionProvider is to ensure that Spring's PlatformTransactionManager is solely responsible for transaction management.

As far as I can tell, the crux of the problem is jOOQ's async transaction support and the lack of good mapping to Spring's imperative transaction support which is ThreadLocal-based. The MCVE doesn't seem to gain anything from adapting jOOQ's transactions to Spring's PlatformTransactionManager and is, in fact, hurt by it.

Rather than trying to allow users to mix and match the two APIs (which will be complex and may not even be possible), I wonder if we'd be better providing a configuration property that allows uses to specify whether they want to use Spring's transaction management or jOOQ's transaction management. Basically make it easier to opt out of (or into if we flip the default) SpringTransactionProvider being configured.

@lukaseder
Copy link
Contributor Author

Would this not make this significantly more complicated?

Yes, absolutely, for us maintainers. But not necessarily for the users.

Assuming I've understood things correctly, won't it result in either Spring's PlatformTransactionManager or jOOQ managing the current transaction?

Depending on who starts things, Spring or jOOQ would manage the current transaction, and the "other side" would have to pick it up.

Looking at it more closely, jOOQ would actually not start the current transaction itself, but the SpringTransactionProvider SPI implementation would start a "jOOQ aware Spring transaction", if that makes sense.

The current intent of SpringTransactionProvider is to ensure that Spring's PlatformTransactionManager is solely responsible for transaction management.

That, and that jOOQ's sync transaction APIs can make use of it, as clients of Spring's PlatformTransactionManager

As far as I can tell, the crux of the problem is jOOQ's async transaction support and the lack of good mapping to Spring's imperative transaction support which is ThreadLocal-based.

Well, if there were any plans of offering "imperative" (or rather "programmatic") transaction support that is not ThreadLocal based in Spring, those could be used by SpringTransactionProvider, too. But I don't know Spring well enough to know if this is already possible, or will be possible in the near future. It would be my preferred solution.

The MCVE doesn't seem to gain anything from adapting jOOQ's transactions to Spring's PlatformTransactionManager and is, in fact, hurt by it.

Yes

Rather than trying to allow users to mix and match the two APIs (which will be complex and may not even be possible)

My recommendation is to generally not mix things, but people will inevitably try it.

Of course, we could also change course here, and disallow mixing things per documentation and implementation, but that would probably break quite a few applications out there, at least in the sync case. A possibility would be to log a warning, about mixing not being recommended.

In the async case, jOOQ could just check the TransactionProvider class name and log a warning or throw an UnsupportedOperationException, if it is SpringTransactionProvider, with some explanations.

I wonder if we'd be better providing a configuration property that allows uses to specify whether they want to use Spring's transaction management or jOOQ's transaction management. Basically make it easier to opt out of (or into if we flip the default) SpringTransactionProvider being configured.

That would be great in addition to what I'm suggesting (and the above thrown exception). Personally, I wouldn't flip the default. If someone uses jOOQ with Spring (Boot), they're very likely to want to use Spring for transaction management (mostly via @Transactional annotations), or they already use Spring and want to start using jOOQ. I think that happens more often than someone already using jOOQ and starting to want to use Spring.

But it will still mean that people will first have to understand all of this, and make an informed decision, possibly when evaluating things for the first time. Until that decision, things may simply seem not to work. If users are not jOOQ and/or Spring (Boot) power users, they might already be deep down in some rabbit hole, finding it hard to recover from the "mistake" of mixing the two APIs, at least, that's how I perceive it from my support case experience.

@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Dec 1, 2020
@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Dec 9, 2020
@philwebb philwebb added this to the 2.5.x milestone Dec 9, 2020
@wilkinsona
Copy link
Member

wilkinsona commented Dec 9, 2020

Considering the resources that we have available and the varying costs of some different approaches to fixing this, we think that our best option is to add a configuration property that allows the auto-configuration of SpringTransactionProvider to be disabled. We'll hopefully do that in Boot 2.5.

@lukaseder
Copy link
Contributor Author

Makes sense. On our side, we could use reflection to detect whether the SpringTransactionProvider is configured and async transactions are executed using jOOQ API, and improve our error messages accordingly, pointing at the options, and future properties.

@philwebb philwebb modified the milestones: 2.5.x, 2.x Mar 19, 2021
@steinard
Copy link

steinard commented Mar 24, 2021

Unfortunately we have this use-case in one app. Parts of the SpringBoot app has been implemented with JPA, and we are thinking of moving away from that. Meanwhile I have written a helper class similar to SpringTransactionProvider which taps into the PlatformTransactionManager, it is currently working and I haven't come across any errors yet, but then I haven't tested extensively either. Should we abandon this approach?

    //

    import org.jooq.ConnectionProvider;
    import org.jooq.TransactionContext;

    import org.jooq.TransactionProvider;
    import org.jooq.impl.ThreadLocalTransactionProvider;
    import org.jooq.tools.JooqLogger;

    import org.springframework.transaction.PlatformTransactionManager;
    import org.springframework.transaction.TransactionStatus;
    import org.springframework.transaction.support.DefaultTransactionDefinition;
    import static org.springframework.transaction.TransactionDefinition.ISOLATION_DEFAULT;
    import static org.springframework.transaction.TransactionDefinition.PROPAGATION_NESTED;


    public class JooqPlatformTransactionProvider extends ThreadLocalTransactionProvider implements TransactionProvider {

        private static final JooqLogger LOGGER = JooqLogger.getLogger(JooqPlatformTransactionProvider.class);
        private PlatformTransactionManager txMgr;
        private int propagationBehavior;
        private int isolationLevel;

        public JooqPlatformTransactionProvider(ConnectionProvider provider, PlatformTransactionManager txMgr) {
            super(provider);
            this.txMgr = txMgr;
            this.propagationBehavior = PROPAGATION_NESTED;
            this.isolationLevel = ISOLATION_DEFAULT;
        }

        public JooqPlatformTransactionProvider(ConnectionProvider provider, PlatformTransactionManager txMgr, int propagationBehavior, int isolationLevel) {
            super(provider);
            this.txMgr = txMgr;
            this.propagationBehavior = propagationBehavior;
            this.isolationLevel = isolationLevel;
       }

        @Override
        public void begin(TransactionContext ctx) {
            LOGGER.debug("Begin transaction");

            // This TransactionProvider behaves like jOOQ's DefaultTransactionProvider,
            // which supports nested transactions using Savepoints
            DefaultTransactionDefinition def = new DefaultTransactionDefinition(propagationBehavior);
            if (isolationLevel != ISOLATION_DEFAULT) {
                def.setIsolationLevel(isolationLevel);
            }
            TransactionStatus tx = txMgr.getTransaction(def);
            ctx.transaction(new SpringTransaction(tx));
        }

        @Override
        public void commit(TransactionContext ctx) {
            LOGGER.debug("commit transaction");
            txMgr.commit(((SpringTransaction) ctx.transaction()).tx);
        }

        @Override
        public void rollback(TransactionContext ctx) {
            LOGGER.error("rollback transaction");
            txMgr.rollback(((SpringTransaction) ctx.transaction()).tx);
        }

        public JooqPlatformTransactionProvider derive(ConnectionProvider provider, int propagationBehavior, int isolationLevel) {
            return new JooqPlatformTransactionProvider(provider, txMgr, propagationBehavior, isolationLevel);
        }
    }

The services using JOOQ will not be proxied and we will not use the Transaction attribute (one less aop-proxy). The transactions are controlled by delegates instead:

try (var uow = unitOfWorkFactory.create()) {
            uow.doWork(() -> result.set(UserDataCmdUtil.save(uow, user)));
        }

In the JOOQ configuration we are tapping into the PlatformTransactionManager like this:

//

    @Bean
    public org.jooq.Configuration jooqConfig(DatabaseProperties props,
                                             ConnectionProvider connectionProvider,
                                             PlatformTransactionManager transactionManager
        ) {

            SQLDialect sqlDialect = SQLDialect.ORACLE11G;
            if (props.sqlDialect == SqlDialect.H2) sqlDialect = SQLDialect.H2;
            if (props.sqlDialect == SqlDialect.PostgreSql) sqlDialect = SQLDialect.POSTGRES;
            return new DefaultConfiguration() 
                    .derive(connectionProvider)
                    .derive(new JooqPlatformTransactionProvider(connectionProvider, transactionManager))
                    .derive(sqlDialect);
    }

Is this a way to go?

Thanks,
Steinar

@sbuettner
Copy link

sbuettner commented Feb 2, 2022

The same applies for reactive transaction management. Our testing shows that mixing spring and JOOQ leads to transactions not being rolled back:

Config:

@Bean
fun dslContext(connectionFactory: ConnectionFactory): DSLContext =
    DSL.using(
        DefaultConfiguration()
            .set(TransactionAwareConnectionFactoryProxy(connectionFactory))
            .set(SQLDialect.POSTGRES)
    ).dsl()

Example code:

transactionalOperator.transactional(
    Mono.from(dslContext.insertInto(Tables.MY_TABLE)
        .set(MyRecord().apply {id = UUID.randomUUID()})
     ).map { throw RuntimeException("Rollback!") }
)

The code leads to a successfully inserted record despite the RuntimeException. It seems like the connections provided by the connectionFactory are not attached to the same transaction despite running inside the same transactionalOperator.transactional scope.

@wilkinsona Please let me know if we should rather create an issue on the spring framework side.

@michael-simons
Copy link
Contributor

The transactional operators are unrelated to the PlatformTransactionManager. There's a completely parallel stack to it.

@sbuettner
Copy link

@michael-simons Sure but my commet is about transactions and JOOQ in general and that the situation is also unclear in the reactive world.

@wilkinsona
Copy link
Member

Boot doesn't yet do anything with jOOQ in a reactive (I assume you're using R2DBC) context and, especially given that you're configuring things yourself, I can't see a connection with Boot. From what you've shared thus far, the only connection to Spring in general seems to be your @Bean definition. If you're looking for some help with jOOQ, reactive, and transactions, Stack Overflow would be a better place to ask.

@sbuettner
Copy link

Yep only using the preconfigured io.r2dbc.spi.ConnectionFactory and ReactiveTransactionManager from boot. The assumption was that it already provides everything needed. Thanks for the feedback though.

@fcabouat
Copy link

Hi,

Isn't what @sbuettner reported a bug ? I'll gladly open another issue or a Stackoverflow if I'm mistaken, but it seems like a weird behavior with Jooq usage of connectionFactory proxies ?

I'm able to have my db request successfully participate in a current Tx if using a connection directly opened from a TransactionAwareConnectionFactoryProxy, but it doesn't seem to work if passing the same factory to settings ? Am I missing something here ?

class CommandLineSpringApp(
    @Autowired private val transactionalOperator: TransactionalOperator,
    @Autowired private val connectionFactory: ConnectionFactory
) : CommandLineRunner {

    override fun run(vararg args: String?) {
        runBlocking {
            getTxIdFromDBTransactional()
            getTxIdFromDBTransactional()
        }
    }

    private suspend fun getTxIdFromDBTransactional() =
        transactionalOperator.executeAndAwait {
            println("With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: " + getTxIdJooqDirectConnection())
            println("With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: " + getTxIdJooqDirectConnection())
            println("With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: " + getTxIdJooqDirectConnection())
            println("It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: " + getTxIdJooqConnectionFactory())
            println("It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: " + getTxIdJooqConnectionFactory())
            println("It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: " + getTxIdJooqConnectionFactory())
        }

    private suspend fun getTxIdJooqDirectConnection(): Int {
        val reactiveConn = TransactionAwareConnectionFactoryProxy(connectionFactory).create().awaitSingle()

        return DSL.using(
            reactiveConn
        ).dsl()
            .select(DSL.function("txid_current", SQLDataType.INTEGER))
            .fetchOneAwait()
            .value1()
    }

    private suspend fun getTxIdJooqConnectionFactory(): Int =
        DSL.using(
            DefaultConfiguration()
                .set(TransactionAwareConnectionFactoryProxy(connectionFactory))
                .set(SQLDialect.POSTGRES)
        ).dsl()
            .select(DSL.function("txid_current", SQLDataType.INTEGER))
            .fetchOneAwait()
            .value1()

    suspend fun <T : Record> ResultQuery<T>.fetchOneAwait(): T =
        Mono.from(this).awaitSingle()

Output :

With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: 1560
With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: 1560
With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: 1560
It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: 1561
It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: 1562
It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: 1563
With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: 1564
With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: 1564
With direct connection from TransactionAwareConnectionFactoryProxy = all calls happen in same tx: 1564
It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: 1565
It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: 1566
It doesn't seem to be when we use TransactionAwareConnectionFactoryProxy: 1567

Regards,

@wilkinsona
Copy link
Member

wilkinsona commented Mar 15, 2022

It may well be a bug – I don't know enough about jOOQ's support for R2DBC to comment with any degree of certainty – but I would be surprised if it has anything to do with Spring Boot. As I said above, Boot doesn't yet do anything with jOOQ in a reactive context so its involvement is minimal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

9 participants