-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Resteasy Reactive/Panache requests reactive Hibernate session from executor thread if request is large #31606
Comments
/cc @DavideD (hibernate-reactive), @FroMage (panache,resteasy-reactive), @Sanne (hibernate-reactive), @Sgitario (resteasy-reactive), @gavinking (hibernate-reactive), @geoand (resteasy-reactive), @loicmathieu (panache), @stuartwdouglas (resteasy-reactive) |
What's going on here is that because you are sending a large payload that needs to be built up in memory, Quarkus is automatically switching the handling of the request to a worker thread. You can tune this property by setting |
Let me check and see however if there is an easy way to fix the issue |
Thanks for looking into it. Just to make sure you're not doing any unnecessary work, have you read my analysis in the "Additional information" section? This suggests that the problem might be in Hibernate Reactive rather than RESTEasy Reactive. |
I have yes, thanks for that! There may be in an issue in HR, but there is definitely an issue on RR as well which I want to address :) |
I am pretty sure that #31613 fixes the whole problem, although I can't test the reproducer because currently Hibernate Reactive is temporarily disabled in the |
hi @TwoFX - what a great bugreport, thanks! So, yes Hibernate Reactive is currently disabled in The assertions in Hibernate Reactive are important; it can't work correctly when it's not run on the actual event-loop as that's were the responses from the database are delivered and we need to process them in well defined order; I'm surprised that Resteasy Reactive/Panache requests were allowed to occasionally fallback to a different model: this requirement is double-checked by Hibernate Reactive as it's otherwise tricky to debug, but similar event ordering problems could potentially affect other components too - just harder to notice. |
This was an oversight that should be fixed by the PR :) |
Ensure large payloads don't affect the invoking thread of a Resource Method
Describe the bug
I have a simple project which accepts some data via JSON through RESTEasy Reactive and saves it to the database using Hibernate Reactive with Panache. This works fine as long as the request object is small. However, if the request object is large (in my case, a few megabytes), then the request fails with
HR000068: This method should exclusively be invoked from a Vert.x EventLoop thread; currently running on thread 'executor-thread-0'
(see below for the full stack trace).Expected behavior
The request should always finish successfully, regardless of the size of the request object.
Actual behavior
The request fails and returns a 500 status code. The following logging output is generated:
How to Reproduce?
Reproducer: https://github.com/TwoFX/resteasy-reactive-large-request-bug
Steps to reproduce:
Run
./mvnw verify
. There are two tests, one for a request body of around 5000 bytes and one for a request body of around 5000000 bytes. The first test succeeds, and the second test fails.Output of
uname -a
orver
Linux markus-laptop 6.2.1-arch1-1 #1 SMP PREEMPT_DYNAMIC Sun, 26 Feb 2023 03:39:23 +0000 x86_64 GNU/Linux
Output of
java -version
GraalVM version (if different from Java)
No response
Quarkus version or git rev
3.0.0.Alpha4
Build tool (ie. output of
mvnw --version
orgradlew --version
)Additional information
I have verified that the problem is NOT present in Quarkus 2.16.4.Final.
I tried to test the current main, but unfortunately after changing the group id and the version the reproducer fails to build. Maven gives the following error:
I'm not sure what I'm doing wrong here. It would be very nice if someone could assist me with this, as this is currently preventing me from testing potential fixes.
I have digged into the bug a bit. The reason why we're on an executor thread in the first place is that the
InputHandler
does this if the request object is too large. Thus, for small request objects, we enter theUniResultHandler
on a Vertx event loop thread, while for large request objects, we enter it on an executor thread.Now, (but, as noted above, I haven't tested any of this), my guess is that the first commit with this problem is 47b10ae. Since then, the Uni subscription created by
Panache.withTransaction
uses a differentExecutor
. In Quarkus 2, it isAbstractJpaOperations::executeInVertxEventLoop
(see here). In Quarkus 3, it is theorg.hibernate.reactive.context.impl.VertxContext
(see here).Now, it turns out that
VertxContext::execute
doesn't actually always execute the runnable on a Vertx event loop thread, as you can see in theelse
branch. I don't understand any of this well enough to know whether this is intentional or not -- perhaps, the linerunnable.run()
(line 90) should actually becurrentContext.executeInContext( x -> runnable.run() )
. So it is actually possible that this isn't a problem with Quarkus, but with Hibernate Reactive. Again, as I mentioned, I haven't tried this potential fix because I can't get Quarkus to build properly locally.I would very much like to contribute a fix for this myself if possible, I only need some feedback if my analysis is correct and some assistance with the "POM is invalid" problem described above.
The text was updated successfully, but these errors were encountered: