Hibernate Reactive with Panache : transaction rollback should trigger session re-creation

[FroMage]

trafficstars

Describe the bug

Turns out when a TX is rolled-back because of an error (or exception, I assume), we need to re-create the session, because otherwise it's polluted. Apparently HR does that, but the Session we get from ArC doesn't support it.

Test case:

    @Path("tx/rollback")
    public Uni<String> rollback() {
        return rollback1()
                .flatMap(v -> {
                    // run a rolled back transaction
                    return doTransaction();
                })
                .onItem().failWith(() -> new AssertionError("Should have failed transaction"))
                .onFailure().recoverWithItem(x -> {
                    assertTrue(x instanceof RuntimeException);
                    assertEquals(x.getMessage(), "this should rollback");
                    return null;
                })
                .flatMap(v -> rollback2())
                .map(v -> "OK");
    }

//    @ReactiveTransactional
    Uni<Void> rollback1(){
        return Panache.getSession().flatMap(session -> {
            return session.withTransaction(tx -> {
                return Person.deleteAll()
                        .flatMap(v -> Person.count())
                        .map(count -> {
                            // sanity check
                            assertEquals(0, count);
                            return null;
                        });
            });
        });
    }

//    @ReactiveTransactional
    Uni<Void> rollback2(){
        return Panache.getSession().flatMap(session -> {
            return session.withTransaction(tx -> {
                return Person.count()
                        .map(count -> {
                            // should have been rolled back
                            assertEquals(0, count);
                            return null;
                        });
            });
        });
    }
    
//    @ReactiveTransactional
    public Uni<Void> doTransaction(){
        return Panache.getSession().flatMap(session -> {
            return session.withTransaction(tx -> {
                Person p = new Person();
                return p.persist()
                        .flatMap(v -> Uni.createFrom().failure(new RuntimeException("this should rollback")));
            });
        });     
    }

@DavideD told me this, so I'll need to look into it.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot[bot]]

/cc @DavideD, @Sanne, @gavinking, @loicmathieu

[FroMage]

Related discussion: https://github.com/quarkusio/quarkus/discussions/23166

[DavideD]

There are two workaround for this:

1. Destroy the injected session object using arc:

   Arc.container().instance(Mutiny.Session.class).destroy()
   

   This way a new session will be recreated the next time. @mkouba Is this still a valid workaround? I think now we inject Uni<Mutiny.Session>. Should we do something different?

2. Use Hibernate Reactive without Panache:

   @Inject
   Mutiny.SessionFactory factory;
   
    public Uni<Void> create(Person person){
        return factory.withTransaction( session, tx ->  {
            // Now you have a new session and transaction and you can control them any way you want 
        });
    }
   

We should figure out a fix for this issue, because it causes that kind of errors that are hard to spot.

[mkouba]

@mkouba Is this still a valid workaround?

Yes, it is :-)

[danielbobbert]

Has this been fixed? I just tested with Quarkus 2.14.0.Final and the workaround doesn't seem to be necessary any longer.

[mkouba]

I can confirm that I'm not able to reproduce the issue with the original reproducer anymore. It seems to be fixed in 2.11 (not sure where... HR, Mutiny, hibernate-reactive-panache). Although one of my applications still fails without the workaround - I need to find out why.

Anyway, it seems that the test case in the description of this issue is not correct - as described in https://github.com/quarkusio/quarkus/discussions/23166 the error occured when a Mutiny.Session method did throw an exception, e.g. after an unsuccessful flush operation.

[DavideD]

I don't think we can close this issue, unless the session is actually recreated or replaced in case of error.

If that's not the case, even if it might work in some cases, in other it might look like it's working but actually doesn't. People should not rely on this.

[mkouba]

I don't think we can close this issue, unless the session is actually recreated or replaced in case of error.

+1

If that's not the case, even if it might work in some cases, in other it might look like it's working but actually doesn't. People should not rely on this.

So I found out that the other problem I mentioned above is different but also caused by the fact that the session remains open - the recovery fails because hibernate does attempt to auto flush the session before Mutiny.Query.getResultList() is executed.

So the workflow is:

1. Mutiny.Session.find(FooEntity.class, 1)
2. modify FooEntity
3. flush the session
4. transaction rollback because of a constraint violation
5. recovery outside the transaction -> Mutiny.Session.createQuery().getResultList() -> auto flush -> ConstraintViolationException again

It does work if I destroy the request scoped session and use a different one in the recovery.

[markusdlugi]

Any updates on this one? It's been open for quite a while and the suggested workaround is pretty ugly (and the Quarkus 3 workaround seems to be even uglier).

We've been lately seeing some issues in our applications with DB connections staying in idle in transaction state permanently , leading to the connection pool filling up over time until it's full and the application breaks down entirely. Not sure if this is something that could result from the issue mentioned in here.

[mkouba]

I'm not aware of any updates :shrug:.

You can use a util method to make the Quarkus 3 workaround a little bit less verbose:

Uni<T> recoverWithNewSession(Supplier<Uni<T>> action) {
        return Panache.getSession()
                .chain(s -> s.close())
                .chain(v -> Panache.withSession(() -> action.get()));
}

and then something like:

someUni.onFailure().recoverWithUni(t -> recoverWithNewSession(() -> doSomeRecovery(t)));

[markusdlugi]

Fair enough. But what about failures that happen outside the application code? For example, if I'm using @ReactiveTransactional / @WithTransaction at a REST endpoint and an exception happens on transaction commit, I won't be able to catch the exception in my application code, right? So I'm not sure if the workaround covers all cases.

[mkouba]

For example, if I'm using @ReactiveTransactional / @WithTransaction at a REST endpoint and an exception happens on transaction commit, I won't be able to catch the exception in my application code, right? So I'm not sure if the workaround covers all cases.

You can (A) use Panache.withTransaction() instead of @WithTransaction in the resource method, or (B) extract the business logic in another "service" bean/class and call this service/handle the fallback in the resource method.

[adampoplawski]

Hello Any updates on this one? I assume when using Panache we still need to manage session manually? Maybe there is some new approach planed in Panache2?

[FroMage]

In Panache 2 the session handling mechanism is still the same, although @gavinking has proposed a new way to inject sessions in https://github.com/quarkusio/quarkus/issues/47462, so this might be something we can fix at the same time as this is implemented