quarkus: First Request to Reactive SQL fail

This will work if you wrap everything in a transaction:

   public Uni<PageResponse<Entity>> getPage() {
        return Panache.withTransaction(() -> {
            var query = this.findAll();
            var list = query.page( Page.of( 1, 10 ) ).list();
            var total = query.count();
            return Uni.combine().all().unis(list, total).asTuple().onItem().transform((tuple) ->
                PageResponse.<Entity>newBuilder()
                    .setElements(tuple.getItem1())
                    .setTotalCount(tuple.getItem2())
                    .build() );
        });
    }

For sure.

I am of the opinion (as I’ve stated in other places like the discussions about the reactive rest client), that we should just disallow stuff that needs a Vert.x thread when we are on a worker thread. By allowing it there is little to no gain and too many issues like this could arise.

But back to what I posted, as I mentioned, in the case where the correct thread is actually being used (due to RESTEeasy Reactive), then you still have a problem when combining Uni, which Panache.withTransaction solves only because it obtains the connection to start the transaction.

It’s also not possible to use CompletableFuture.allOf( ) or Uni.combine().all() with Hibernate Reactive. That’s because the order of the operations using the same session is not guaranteed. Maybe it works in this case but it’s something one should not rely on.

That’s why the first solution proposed by @Sanne works. I would also expect the second example to work. I need to check what’s going on with it.

We briefly explain this in the Hibernate Reactive guide.

Like @FroMage said, Hibernate Reactive internally uses the CompletionStage API, but the Mutiny layer on top of it should create a “lazy layer” that it’s only executed at subscription time.

This seems indeed working fine now. Thanks all!

This looks like the same issue as others have reported: not using a transaction and/or not delegating to the right thread.

IMO it’s Panache Reactive which should not allow this usage and throw a better exception.

The best way to keep track of the changes is to follow the labels area/panache or area/hibernate-reactive.

A PR with a big set of changes has already been sent: https://github.com/quarkusio/quarkus/pull/29761 But it shouldn’t cause too many changes to your code.

You have already figured it out. There is only one session, the first time you load the value it gets cached in the session and it will stay there until you destroy the session or clear it.

Keep in mind that a session is a lightweight object that should represent a logical single unit of work.

I’ll admit that in Quarkus, at the moment, it’s hard to figure out when a session is created or destroyed. We are working on it.

@DavideD Yea, thanks. I’ve also tried to use the withTransaction method from Mutiny.SessionFactory to acquire an auto-closable session object. And it works. But it’s sad because I can’t use panache to work with DB in that case.

@DavideD , thank you very much for your feedback/help here, I will try this out.

You need to run a query only after the previous one has completed:

    @ReactiveTransactional
    public Uni<List<Shipment>> callDatabase() {
        List<Integer> ids = List.of(123,571,298109,1239,29,1,666,1337);
        List<Shipment> shipments = new ArrayList<>();
        Uni<Void> loop = Uni.createFrom().voidItem();
        for(Integer id: ids){
            loop = loop.call( () -> shipmentRepository
                            .findById(id)
                            .invoke(shipments::add));
        }
        return loop.replaceWith(shipments) ;
    }

@clahres That’s correct, you shouldn’t run code like that, the problem is that you are using transformToUniAndMerge and sharing the session. Basically, you are running queries in parallel using the same session. This might work in some situations but it’s not something one should rely on. I think transformToUniAndConcatenate is the correct one to use (but I need to doublecheck that it actually runs the queries one after the other).

@einarjohnson I would have to look at the code, but it usually happens that there a session used in parallel somewhere.

Pfff, to get back to @Sanne’s issue, which is:

Without a transaction, if we obtain two Uni executing an operation, and execute them without chaining them, something gets broken in HR.

This is most likely relating with the ordering of events. When using the Transaction to wrap all operations, the beginning of the transaction enforces opening of a connection and at that point HR will enforce that such connection-opening work and all subsequent operations are executed within the same vertx. context.

I mean, correct me if I’m wrong, but this seems to be a problem entirely unrelated with Quarkus or Panache.

It relates to Quarkus as it should ensure different tasks are not re-ordered or dispatched to multiple vertx. context, multiple verticles, other threads, etc…

In any case, to get back to the drawing board, I recall that Julien Viet and @cescoffier told me that with things like Netty’s async operations, even if you don’t chain the returned promises, they should still all be executed in the sequence that you obtained/invoked them in. Because Netty keeps a queue underneath, so invocation order is always preserved.

Sure. But Quarkus doesn’t seem to honour the premise: there are multiple parallel threads executing multiple vertx contexts. Need to ensure all end user tasks are dispatched consistently and respecting the expected order.

Now, this is true in the context of eager promises. But Uni are lazy, and only execute on subscription, which means it’s the subscription order that should order the operations. OTOH, I’m pretty sure that HR uses CompletionStage underneath, which are eager, so I’m not sure about the order semantics of HR calls.

It’s been a long time since I made the above “spot the difference” comment so TBH I don’t remember it all, but I think the gist was that one of the code examples triggers use of executeInVertxEventLoop in such a way to throw off the sequential ordering guarantees one would expect.

I’ve had another look and yes it’s failing still - but it depends on the effective order of execution of the tasks being passed onto Uni.combine().all().unis(list, total), so I guess this explains why I might have seen it pass.

Having had a better look now, I suspect the failure is now triggered by different reasons than before: those multiple operations are being scheduled w/o a running Transaction. You can see that if you annotate the getPage() method with @ReactiveTransactional it all works correctly.

But I’ll leave this open for now: at very least we need to provide a better error message, and I’m wondering if we should make Transactions mandatory or if that’s going too far.

It certainly seems necessary to ensure the connection is initialized before using a combine().

I was talking to @FroMage about some other stuff and he suggested this was a problem that we would need to solve before doing anything else, so I volunteered to have a look to provide a fresh pair of eyes.

Disclaimer: I don’t know much about HR, so my conclusions might be entirely mistaken.

First of all, I used the reproducer, but switched to RESTEasy Reactive in order to ensure that no threading issues exist during the request handling. Using that, I was consistently able to reproduce:

2021-05-11 13:23:21,961 ERROR [org.jbo.res.rea.com.cor.AbstractResteasyReactiveContext] (vert.x-eventloop-thread-35) Request failed: java.lang.IllegalStateException: session is currently connecting to database

when hitting http://localhost:8080/hello. This was the case for both the supplied version of Quarkus and Quarkus from main.

Following a debugging session, I believe that the problem is simply that the operations performed by the application (Repository#getPage) are happening concurrently (due to the use of Uni.combine()) which leads to the fact that the first operation has not finished obtaining and setting the connection when the second operation requests a connection - this leads to the error we see (which AFAICT, is not bogus).

The reason the problem goes away when the operations of Repository#getPage are wrapped in Panache.withTransaction is because by the time the actual operations try to obtain that connection, the connection has already been set by virtue of the transaction being started (which of course entails that the connection was obtained)

this still fails for me in version 1.11.3

I do not know how initialization connections works in panache reactive, but from the behavior and the error “Session/EntityManager is closed” all this looks like initialization connections trouble in hibernete reactive. Isn’t it?

The code is fine but a single Session is not designed to be threadsafe; a consequence of running this on the wrong thread is that both your original thread and the Vert.x thread make state changes, leading likely to illegal state (and complex to diagnose issues).

So we need to ensure all operations are being scheduled on the right thread.

This looks like the same issue as others have reported: not using a transaction and/or not delegating to the right thread.

I think so, but I prefer to check with the test case