First Request to Reactive SQL fail #14709
Comments
ghost
added
the
cescoffier
added
area/hibernate-reactive
|
/cc @Sanne @gavinking @DavideD |
|
I'm having a look |
|
Is there a test project somewhere that I can look at? |
|
not yet, but it is easy to reproduce. Only make two concurrent reactive request to database (postgresql 13 in my case), startup the application and execute a reactive service. |
That would be great and save me a lot of time |
|
ok, give me about 10 min |
|
Thanks |
|
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. |
I think so, but I prefer to check with the test case |
|
(I don't know for sure and will let @DavideD reprodice and confirm, I'm just suspecting this might be the case since I see RestEasy being used in the stacktrace) |
|
in repository example is even worse, since it gives error in all requests :( |
|
please note that just created a service called hello/first. It does not have initial concurrency. Now, if service "hello/first" is executed at first and then execute "hello" service, then no error is present |
|
Yes, this is the problem @Sanne was talking about. |
|
You say that the problem is resteasy? even if concurrent panache reactive code work fine with just before execute a call to non-concurrent panache code? |
|
This will work if you wrap everything in a transaction: |
|
Ok, i will use this way. |
Well it's not RESTEasy's fault, but a limitation of our current integration: it's expected to run all Hibernate Reactive code within a reactive context - which at this time means within the same Vert.x eventloop as the vert.x SQL client which it's using underneath. Failing to run it on the right thread will produce such obscure errors; we certainly need to fix this. N.B. if you run you reproducer as a test it will fail, as there are assertions which explicitly check for this code to be run on the right thread. One way we could fix this is to promote such assertions to hard requirements - haven't done so yet as we were exploring options, and trying to figure if there was a way to allow this without being that restrictive. |
|
Now, after calling a simple request to panache reactive, all subsequent concurrent request to database work fine! but anyway, thanks for helping me and giving me a solution to my issue |
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. |
|
Is this still an issue? |
|
this still fails for me in version 1.11.3 |
|
So, I'm a bit puzzled by the state of this issue. What is wrong and what can we fix? It looks like we're getting cryptic exceptions such as I was sorta believing that the issue was that we were invoking an operation on a worker thread which is forbidden, but I tried it in resteasy-reactive and got the same issue on an IO thread. So let's ask this very clearly @Sanne @DavideD @gavinking:
|
I don't think in this case the problem is the lack of a transaction, I think the issue get solved because when one calls
In Hibernate Reactive we check two things:
I don't know if this answer your question.
We probably need to have better error messages on the Hibernate Reactive side. But first we need to understand a bit better why this is happening I didn't have much time to investigate this issue further, but I should be able to get back to it soon |
|
Is this still a problem with the latest Quarkus? |
|
It's still a "problem". I can confirm that Sanne's solution works perfectly but it's not very obvious for beginners (like me ;-) and the error message is completely useless. |
Hi @geoand, yes this is still a problem. |
|
Thanks for the info folks |
|
Pfff, to get back to @Sanne's issue, which is: Without a transaction, if we obtain two I mean, correct me if I'm wrong, but this seems to be a problem entirely unrelated with Quarkus or Panache. 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. Now, this is true in the context of eager promises. But This really looks like a more fundamental question than this issue illustrates. |
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.
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...
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.
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 |
|
It's also not possible to use That's why the first solution proposed by @Sanne works. We briefly explain this in the Hibernate Reactive guide. Like @FroMage said, Hibernate Reactive internally uses the |
|
Please remember that if you first execute another simple method to establishes connection and then call the complex method with |
|
@DavideD do I understand you correctly that I should not use a pattern like this when using hibernate reactive? Even if the @GET
@ReactiveTransactional
fun range(
@QueryParam("startDate") startDate: LocalDate,
@QueryParam("endDate") endDate: LocalDate
): Uni<List<Entry>> {
return Multi.createFrom()
.items(startDate.datesUntil(endDate.plusDays(1)))
.onItem()
.transformToUniAndMerge { entryResource.list(userRef = "puschel", date = it) }
.collect()
.asList()
.onItem()
.transform { it.flatten() }
}
@GET
fun list(
@QueryParam("userRef") @DefaultValue("") userRef: String,
@QueryParam("page") @DefaultValue("0") pageIndex: Int = 0,
@QueryParam("size") @DefaultValue("100") pageSize: Int = 100,
@QueryParam("date") date: LocalDate?
): Uni<List<Entry>> {
val query = if (date == null) "userRef = :userRef" else "userRef = :userRef and date = :date"
val params = mutableMapOf<String, Any>("userRef" to userRef)
date?.let { params.put("date", it) }
return repository.find(query, params).page<Entry>(Page.of(pageIndex, pageSize)).list()
} |
|
I am facing the same problem where I encounter the |
|
@clahres That's correct, you shouldn't run code like that, the problem is that you are using @einarjohnson I would have to look at the code, but it usually happens that there a session used in parallel somewhere. |
|
@DavideD I have a test that reproduces this error. @QuarkusTest
public class TestPanache {
private static final org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(TestPanache.class);
@Inject
@Named("MySqlShipmentRepository")
ShipmentRepository shipmentRepository;
@Test
public void testPanacheMutinyFind_SessionError(){
List<Integer> ids = List.of(123,571,298109,1239,29,1,666,1337);
UniJoin.Builder<Shipment> builder = Uni.join().builder();
for(Integer id: ids){
builder.add(shipmentRepository.findById(1));
}
List<Shipment> shipments = builder.joinAll().andCollectFailures().await().atMost(Duration.ofSeconds(30L));
log.info("Results", shipments);
}
}However, if I wrap the uni building block inside a |
|
It doesn't seem correct. I don't think This means that running two or more findById, could potentially change the state of the session concurrently. I don't mean concurrently as in "different thread", I mean concurrently as it's happening in two different parallel execution streams. Probably, the transaction gives some guarantees about when the session gets opened and closed. That's why you don't see the error any more. |
|
In short, we are writing a lambda handlers that are processing a batch of messages either from Kafka or SQS. The idea is to iterate over all the elements in the batch and call the database similar to how this test is doing it. In the end we have to await on these uni's since we are running inside an AWS lambda. @DavideD, what is the "best practise" here? Should we not use My current implementation is roughly like this: Basically, it's fetching information from the database and calling DynamoDB to update a table with the results found. |
|
You need to run a query only after the previous one has completed: |
|
@DavideD , thank you very much for your feedback/help here, I will try this out. |
|
Hello, everyone! I have the same issue. There is my example (produces error "Detected use of the reactive Session from a different Thread", but with the same idea and more complex logic from more real application produces error "session is currently connecting to database"): Extra questions:
Logs |
|
I will need to check but I think the use of the emit on on the worker thread, causes the creation of multiple event loop thread. That's because the Hibernate Reactive only works on an event loop thread. The problem is that If you want to execute tasks concurrently (in the sense of parallel reactive streams) you need to create a new session each time. I think you can only do this with the Hibernate Reactive session factory. Something like this should work: @Inject
Mutiny.SessionFactory sf;
Uni<Void> dispatch() {
return Uni.join()
.all(Arrays.asList( createHandler(1), createHandler(2) ,createHandler(3) )
.andCollectFailures()
.replaceWithVoid();
}
Uni<Void> createHandler(int index) {
log.info("handle, {}", index);
return getMutinySessionFactory()
.openSession()
.chain( session -> session
.withTransaction( tx -> deleteAll( session, tx ) )
.eventually( session::close )
);
}
private Uni<Void> deleteAll(Mutiny.Session session, Mutiny.Transaction tx) {
try {
return session.createQuery( "..." ).executeUpdate().replaceWithVoid();
}
catch (Throwable t) {
// We need to make sure that any exception get caught so that it's
// always possible to close the session in case of error
return Uni.createFrom().failure( t );
}
}There is an issue with .eventually(...) not actually handling all possible scenarios, but it should be fixed in the next Mutiny release. |
|
@DavideD |
|
@DavideD, I have tried the code you suggested and the However, I am now seeing a very strange issue crop up now and then when this code runs. It looks as if the I can easily reproduce this locally by putting a breakpoint on the When I go to the MySQL database directly and update the shipments table and set some columns to a new value and then re-run A simple test/reproducer I have been using I have tried wrapping the That being said, I bumped into this discussion thread: #23166 When I add the This also seems to do the trick and I get the updated values logged out immediately This whole thing feels like some kind of caching issue and I have tried setting the |
|
You have already figured it out. 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. |
|
Great, it's good to know I am at least on the right track with this. :) @DavideD , do you know when your work might be released into the Quarkus framework? I would like to stay up-to-date and have the codebase as simple as possible if you guys are planning to make changes to the Hibernate/Panache API. |
|
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: #29761 |
|
I've just realized that if Uni<Void> createHandler(int index) {
log.info("handle, {}", index);
return getMutinySessionFactory()
.openSession()
.chain( session -> deleteAll( session ).eventually( session::close ) );
}
private Uni<Void> deleteAll(Mutiny.Session session) {
try {
return session.withTransaction( tx -> session.createQuery( "..." ).executeUpdate() ).replaceWithVoid();
}
catch (Throwable t) {
// We need to make sure that any exception get caught so that it's
// always possible to close the session in case of error
return Uni.createFrom().failure( t );
}
} |
|
Do we know if this problem still exists in the latest issues of Quarkus? |
|
As far as I understand the original problem was about using |
|
Thanks @DavideD |
Describe the bug
at first time that request an concurrency reactive sql client, it fail.
But the second time it work fine
Expected behavior
that also works fine the first time
Actual behavior
after the application startup and receives the first request to a method with reactive sql concurrent calls, the connection fail.
To Reproduce
In a Reactive SQL Cient Application with resteasy,reactive-pg-client,resteasy-mutiny extensions
The error is:
Environment (please complete the following information):
uname -aorver: Linux 5.3.18-lp152.60-default x86_64 x86_64 x86_64 GNU/Linuxjava -version: OpenJDK 64-Bit Server VM Corretto-11.0.10.9.1 (build 11.0.10+9-LTS, mixed mode)mvnw --versionorgradlew --version): 3.6.3The text was updated successfully, but these errors were encountered: