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

Mongo reactive client should use Netty transport #46248

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

franz1981
Copy link
Contributor

Fixes #46206

@franz1981
Copy link
Contributor Author

@cescoffier I have no idea how SSL/TLS can interact with it

This comment has been minimized.

@franz1981
Copy link
Contributor Author

@cescoffier @geoand do we want a special config to enable this?

@cescoffier
Copy link
Member

Yes, please add a flag. I would consider being enabled by default (even if it could be seen as a breaking change).

This comment has been minimized.

@franz1981
Copy link
Contributor Author

franz1981 commented Feb 13, 2025

Yes, please add a flag

we want a runtime proper config option?

@geoand
Copy link
Contributor

geoand commented Feb 13, 2025

we want a runtime proper config option?

Yes please

@franz1981
Copy link
Contributor Author

Ok I'm adding some test to the config and after speaking with @jponge I see that we always use the reactive driver so...I won't configure the Netty side with reactive only, but always

@franz1981
Copy link
Contributor Author

I am putting it in draft, since I see that:

  • non reactive uses mongo with blocking sockets
  • reactive uses mongo with async sockets

And I need to understand if I can safely make to always use Netty as a transport, even in the blocking case: I am not sure of it because will likely introduce an additional thread handoff from jboss worker pool to Netty

Any idea @jponge @geoand ?

@franz1981 franz1981 marked this pull request as draft February 13, 2025 14:13
@geoand
Copy link
Contributor

geoand commented Feb 13, 2025

I have no idea how the driver works internally

Copy link

github-actions bot commented Feb 13, 2025

🎊 PR Preview 22d8b35 has been successfully built and deployed to https://quarkus-pr-main-46248-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@jponge
Copy link
Member

jponge commented Feb 13, 2025

Just like @geoand (aka I'm posting this to let you know that I don't know)

@franz1981 franz1981 marked this pull request as ready for review February 13, 2025 14:53
@franz1981
Copy link
Contributor Author

Gotta add a test on the config @geoand but I have verified what it's written in the new config option - so I enabled Netty only in the reactive case.
I have to check what happen with tls still - since I don't see it as we test it (?)

NETTY,
/**
* With a reactive driver it uses an async transport backed by a driver-managed thread pool,
* while with a blocking driver it uses a blocking transport.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The non reactive one should just run inlined in the worker pool we provide - this by checking the mongo Java driver code + debugging it

This comment has been minimized.

@franz1981
Copy link
Contributor Author

franz1981 commented Feb 13, 2025

On https://www.mongodb.com/docs/drivers/java/sync/current/fundamentals/connection/tls/#customize-tls-ssl-configuration-through-the-netty-sslcontext seems that there's some work left to configure TLS/SSL or maybe it's just to switch to an alternative SSL (native) engine here - still checking

Copy link

quarkus-bot bot commented Feb 13, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit ee18cb3.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

You can consult the Develocity build scans.

Copy link

quarkus-bot bot commented Feb 13, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit ee18cb3.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@franz1981
Copy link
Contributor Author

It seems no need to do anything beyond what we already do (which is to apply SSL settings at client config level) - see https://stackoverflow.com/questions/51053679/how-to-enable-ssl-in-reactive-mongodb-client-in-spring-boot

@geoand geoand merged commit cda43af into quarkusio:main Feb 13, 2025
42 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.21 - main milestone Feb 13, 2025
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mongodb kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MongoClient should reduce the number of created threads
4 participants