-
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
Amazon Services: no current Vertx context found for an S3 call with reactive resteasy and panache reactive #34687
Comments
/cc @FroMage (panache), @loicmathieu (panache) |
cc @Sgitario as well |
I'm afraid that it would be too costly. Currently, we only inspect the contstant pool to detect the usage of an entity (and repository after #34671 is merged). Still, I'm trying to understand the problem. It seems that there's no current Vert.x context when the attempt to close the session... Maybe we should use the captured context instead. @cescoffier WDYT? |
I think all the s3 callbacks will be invoked on random thread (random in the unmanaged sense), which mean: no context. That could be the issue. Even with context propagation, it would not work as the CS must be decorated by context propagation. Is there a way to specify on which executor the completion stages from the AWS sdk are resolved? |
I think that we need someone from the |
@scrocquesel who created the issue is the maintainer of the Amazon Services extensions so we should be good :) |
@cescoffier I recently changed that so that it can now terminate on the Executor provided by The code looks like this right now @GET
public Uni<List<FileObject>> listFiles() {
ListObjectsRequest listRequest = ListObjectsRequest.builder()
.bucket(bucketName)
.build();
return Uni.createFrom().completionStage(() -> s3.listObjects(listRequest))
.onItem().transform(result -> toFileItems(result));
} I tried something like var e = ContextInternal.current().executor();
return Uni.createFrom().completionStage(() -> s3.listObjects(listRequest)).emitOn(e)
.onItem().transform(result -> toFileItems(result)); but it didn't work. |
Ok, this way it cannot go back to the context. We need to capture it and restore it. So, there are multiple ways to do this:
We can do the first approach for now. If someone tell me where to look I can have a look (next week unfortunately). |
I need to check one think about your second approach. I believe it's retrouve the netty executor, not the duplicated context one. So you execute on the root context, not on the duplicated context. |
With @GET
public Uni<List<FileObject>> listFiles() {
ListObjectsRequest listRequest = ListObjectsRequest.builder()
.bucket(bucketName)
.build();
System.out.println("capture isOnVertxThread:" + Context.isOnVertxThread());
System.out.println("capture isOnEventLoopThread:" + Context.isOnEventLoopThread());
System.out.println("capture isOnWorkerThread:" + Context.isOnWorkerThread());
var e = ContextInternal.current().executor();
return Uni.createFrom().completionStage(() -> s3.listObjects(listRequest)).emitOn(e)
.onItem().transform(result -> {
System.out.println("transform isOnVertxThread:" + Context.isOnVertxThread());
System.out.println("transform isOnEventLoopThread:" + Context.isOnEventLoopThread());
System.out.println("transform isOnWorkerThread:" + Context.isOnWorkerThread());
return toFileItems(result);
});
}
and without the
I also tried with |
I edit my previous comment to provide log trace with thread names |
Thread names are not enough, duplicated contexts are not threads. You cannot use You need to do |
Thanks, to compile I had to do Context context = Vertx.currentContext();
Executor e = command -> context.runOnContext(x -> command.run()); I guess the sample at quarkus/docs/src/main/asciidoc/duplicated-context.adoc Lines 96 to 103 in 26cb1bd
Is there an issue tracking the "next-to-be vertx utilities which will provide an executor doing ^" ? |
Trying to wrap the Executor is not working. By the time, the completion stage is executed, aws netty client has already moved to another thread and the public CompletableFuture<Void> execute() {
Promise<Channel> channelFuture = context.eventLoopGroup().next().newPromise();
executeFuture = createExecutionFuture(channelFuture);
acquireChannel(channelFuture);
channelFuture.addListener((GenericFutureListener) this::makeRequestListener);
return executeFuture;
} where |
You cannot use the netty executor or the EventLoopGroupBuildItem - because you will always have a root context, and not a duplicated context. Thus it breaks the propagation. You will have only one way or another to use Vert.x. The PR adding the next-to-be executor is this one: smallrye/smallrye-mutiny-vertx-bindings#733 |
try to do as below, capture the context and than restore it while emitting the completionStage
|
Hello Currently, we have
walkaround we are using
|
Losing the Vert.x context is problematic, because CP doesn't propagate it, and indeed, delegates to it. How can we fix this @cescoffier @mkouba? |
Unfortunately, there is no magic solution. You need to capture the context and restore it to emit the result produced by the completion stage of AWS. |
Well, there could be magic, because Mutiny uses CP, and if CP captured the current vert.x context, it could propagate it :) |
We have actually been using the workaround mentioned above of capturing & restoring the context via
We also tried using the suggested context-aware scheduler as follows: SqsAsyncClient.builder().asyncConfiguration(
b -> b.advancedOption(SdkAdvancedAsyncClientOption.FUTURE_COMPLETION_EXECUTOR,
ContextAwareScheduler.delegatingTo(Infrastructure.getDefaultWorkerPool())
.withGetOrCreateContext(vertx))); But that also only works if there is no parallelism, with other requests running in parallel we observe
Only "workaround" we found for now, if you want to call it that, is to set Any ideas how we could properly work around these issues? Edit: I can kinda work around the last exception I got with the ContextAwareScheduler using |
For completeness sake: we investigated the issues described above more thoroughly and it was actually related to the context being lost by the Quarkus Cache which we were using to reduce the number of calls to |
Is In the Quarkus AWS extension, I inject the executor provided by the Still, I am seeking a solution to capture the Vert.x context and restore it. One potential solution could involve having an AWS |
I'd say so |
From my tests, I think that this unfortunately doesn't work as expected, or at least I was unable to configure it correctly. The configuration I mentioned above with the Maybe someone who has a working configuration with |
|
Describe the bug
A user reported an issue using AWS extension and reactive panache at the same time. Trying to reproduce the issue with s3 quickstart and I'm stuck trying to make it work.
First, adding
quarkus-hibernate-reactive-panache
and calling a JAX-RS endpoint that do not use Panache entity or repository but make S3 calls fails, trying to autoclose an on demand Session.I see two issues:
Expected behavior
Request should return a 200 response. At least, it should not try to close a session. Looking at #34671, it seems that the
WithSessionOnDemand
annotation is added even if the Resource does not inject a Panache entity nor repository. The detection mechanism is a bit too wide and it detects static usage of entity/repo in others methods.Actual behavior
Request failed with
How to Reproduce?
With this repro : https://github.com/scrocquesel/quarkus-quickstarts/tree/s3_with_panache
amazon-s3-quickstart
(4. you can also try to upload a file)
Output of
uname -a
orver
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
orgradlew --version
)No response
Additional information
No response
The text was updated successfully, but these errors were encountered: