Skip to content
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

Produce EventLoopGroup build item #33239

Merged

Conversation

scrocquesel
Copy link
Contributor

Allow other extensions to reuse the EventLoopGroup that is effectively used without making assumptions on the presence of Vert.x extension.

As a non-CDI expert, I'm not sure if there's a better way to expose those suppliers. Consumers would implement a build step as follows:

@BuildStep(onlyIf = AmazonHttpClients.IsAmazonNettyHttpServicePresent.class)
@Record(ExecutionTime.RUNTIME_INIT)
void setupClient(EventLoopGroupBuildItem eventLoopGroupSupplier, ...) {
    produceSyntheticBean(..., recoder.createClientWithEventLoopGroup(eventLoopGroupSupplier.getMainEventLoopGroup());
}

The drawback is that supplier must return a singleton since it can be called multiple times.

@scrocquesel
Copy link
Contributor Author

@cescoffier could you take a look at this one?

@scrocquesel scrocquesel marked this pull request as ready for review May 16, 2023 14:29
@scrocquesel scrocquesel force-pushed the netty_produce_eventloop_builditem branch from 42a4bce to a896727 Compare May 16, 2023 14:32
@cescoffier
Copy link
Member

There is already a io.quarkus.netty.deployment.EventLoopSupplierBuildItem which does the same. Do I miss something?

@scrocquesel
Copy link
Contributor Author

There is already a io.quarkus.netty.deployment.EventLoopSupplierBuildItem which does the same. Do I miss something?

There is already a io.quarkus.netty.deployment.EventLoopSupplierBuildItem which does the same. Do I miss something?

@cescoffier If no io.quarkus.netty.deployment.EventLoopSupplierBuildItem is present, the netty extension actually create default ThreadPools which are not exposed. I think the buildStep cannot consume and produce io.quarkus.netty.deployment.EventLoopSupplierBuildItem at the same time.

@cescoffier
Copy link
Member

Ah, I see. This must be added in the javadoc, as it's a bit confusing to have two very similar build items.

@scrocquesel
Copy link
Contributor Author

Ah, I see. This must be added in the javadoc, as it's a bit confusing to have two very similar build items.

Already put some explanations on both build items. Would you mind help me rephrase to make it clearer ?

@scrocquesel scrocquesel force-pushed the netty_produce_eventloop_builditem branch from a896727 to db3f80b Compare May 25, 2023 21:41
@quarkus-bot

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Jun 6, 2023

@cescoffier friendly ping :)

@scrocquesel
Copy link
Contributor Author

@cescoffier friendly ping :) It would be great if it could be merged for 3.1. I'd like to refactor AWS extension to use the same event loop. I have made some commits to the AWS sdk and it is now possible to configure most of the used ThreadPool and EventLoop. I just need this PR to complete the work in AWS extension.

Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, the last few weeks got chaotic.

@cescoffier
Copy link
Member

Can you rebase to have another ci run?

@scrocquesel scrocquesel force-pushed the netty_produce_eventloop_builditem branch from db3f80b to f032a0b Compare June 19, 2023 18:36
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 20, 2023

Failing Jobs - Building f032a0b

Status Name Step Failures Logs Raw logs
✔️ Maven Tests - JDK 11
Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs
Native Tests - Windows - RESTEasy Jackson Setup GraalVM ⚠️ Check → Logs Raw logs

@cescoffier cescoffier merged commit 0ff8471 into quarkusio:main Jun 20, 2023
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 20, 2023
@scrocquesel scrocquesel deleted the netty_produce_eventloop_builditem branch June 30, 2023 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants