Inconsistent behaviour on transactional async method

[karbi]

Affects: 6.1.0+

Transactional method returning Future will sometimes be rollback, sometimes not, depending on time that Future is exceptionally completed. Here is example:

@Transactional
public CompletableFuture<Void> asyncMethod() {
    doSomeDatabaseModification();

    if (!isHealth()) {
        return CompletableFuture.failedFuture(new RuntimeException()); // case 1
    }

    final CompletableFuture<Void> result = new CompletableFuture<>();
    new Thread(() -> {
        try {
            Thread.sleep(10_000);
        } catch (InterruptedException e) {
            result.completeExceptionally(e); // case 2
            return;
        }
        result.complete(null);
    }).start();

    return result;
}

In case 1 transaction will be rolled back, in case 2 transaction will be committed, despite in both cases CompletableFuture is completed with error. From that reason method user cannot make any assumptions about transaction state after failure.

For example it could lead to unintended behaviour in method methodDependingOnRollback():

@Transactional
public void methodDependingOnRollback() {
    final CompletableFuture<Void> result = asyncMethod();
    while (!result.isDone()); // ugly but only on purpose of example
    // assuming that in case of asyncMethod() failure, database will not be modified
    saveSuccessInDatabase();
}

It also breaks code that was working in Spring 5, like methodWorkingInSpring5():

@Transactional
public void methodWorkingInSpring5() {
    asyncMethod().whenComplete((v, e) -> {
        if (e != null) {
            saveErrorReasonInDatabase(e);
        }
    });
}

Breaking change was introduced by issue #30018. Probably the same problem is with Vavr, but I'm not familiar with this library.

[jhoeller]

This is a semantically complicated matter and admittedly not obvious.

@Transactional on CompletableFuture-returning methods is only meant to apply to the immediately executing code within the original thread, not to any asynchronous operations triggered by it. Such scenarios that seem to have worked before did not have well-defined semantics - and if they worked with any transactional impact at all, they arguably only worked by accident. The asynchronous thread would not see the transaction-managed state, and the completion of the transaction happens at the end of the original method invocation, not at the end of the asynchronous operation.

The change in #30018 aims for a differerent scenario: The use of Future types as a return value for completed operations. There may have been asynchronous steps involved but further steps have followed in the original method before returning the completed Future handle. Such a method could also be designed to entirely run synchronously and to return a Future only for compliance with a given method signature (like in the case of @Async methods), indicating a rollback not through throwing an exception but rather through returning a Future handle that just wraps a result object or an exception.

As a consequence, this behaves as designed. I'm turning this into a documentation issue for adding that clarification to the reference docs.

[jhoeller]

P.S.: I realize that such CompletableFuture.whenComplete scenarios within a wider transaction would have worked for you before. However, this was never intended on our side, in particular not with the ambiguous meaning of the nested @Transactional there.

In order to make your scenario work with 6.1, you could remove the @Transactional declaration from asyncMethod and exclusively rely on the wider transaction demarcation in methodDependingOnRollback/methodWorkingInSpring5. The nested method call will still implicitly participate in your outer transaction - but without a @Transactional declaration of its own, it is not going to make rollback decisions of its own (neither for exceptions thrown nor for exceptions returned in a CompletableFuture).

Alternatively, you could change the nested declaration to @Transactional(noRollbackFor=Exception.class) and suppress a local rollback decision that way.