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

Expose the shared attribute in the reactive data source configuration #22292

Merged
merged 1 commit into from
Jan 18, 2022

Conversation

cescoffier
Copy link
Member

This attribute was introduced in Vert.x 4.2.2.

@quarkus-bot
Copy link

quarkus-bot bot commented Dec 16, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building d9de188

Status Name Step Failures Logs Raw logs
Initial JDK 11 Build Build Failures Logs Raw logs

Failures

⚙️ Initial JDK 11 Build #

- Failing: extensions/reactive-db2-client/runtime 
! Skipped: devtools/bom-descriptor-json docs extensions/reactive-db2-client/deployment and 3 more

📦 extensions/reactive-db2-client/runtime

Failed to execute goal net.revelc.code.formatter:formatter-maven-plugin:2.17.1:validate (default) on project quarkus-reactive-db2-client: File '/home/runner/work/quarkus/quarkus/extensions/reactive-db2-client/runtime/src/main/java/io/quarkus/reactive/db2/client/runtime/DB2PoolRecorder.java' has not been previously formatted. Please format file and commit before running validation!

@geoand
Copy link
Contributor

geoand commented Dec 16, 2021

This needs a formatting change

@cescoffier cescoffier force-pushed the reactive-data-source-shared branch from d9de188 to 30ce7e5 Compare December 16, 2021 17:31
@cescoffier cescoffier requested a review from Sanne January 7, 2022 10:27
@gsmet gsmet force-pushed the reactive-data-source-shared branch from 30ce7e5 to caa01df Compare January 12, 2022 14:19
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I rebased and adjusted a small thing in the javadoc. Otherwise, it looks good to go!

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 12, 2022
@Sanne
Copy link
Member

Sanne commented Jan 12, 2022

I can't say I understand how to use this... is there a better explanation in the vertx docs?

I found this one but I'm not sure how this translates to Quarkus users as it seems very specific to vert.x:
-https://vertx.io/docs/vertx-pg-client/java/#_pool_sharing

@gsmet
Copy link
Member

gsmet commented Jan 18, 2022

@cescoffier could you have a look at @Sanne 's question?

@cescoffier
Copy link
Member Author

@Sanne it allows sharing the connection pool. It can be useful when you do not use Hibernate Reactive :-).
Now, it's a pretty advanced use case.

@cescoffier cescoffier merged commit 50233fc into quarkusio:main Jan 18, 2022
@quarkus-bot quarkus-bot bot added this to the 2.7 - main milestone Jan 18, 2022
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 18, 2022
@cescoffier cescoffier deleted the reactive-data-source-shared branch January 18, 2022 17:29
@Sanne
Copy link
Member

Sanne commented Jan 19, 2022

Sorry I might be slow (and I'm away) but I still don't understand the implications a Quarkus user should understand.

"shared" across what? What kind of usage pattern would require this to be set (or not)? The documentation I found is very specific for a Vert.x application - I'm not sure how that translates to a Quarkus application and we need to document that in a way which a typical Quarkus user can understand.

In particular I'd want to explore if we actually need a property, or if something should be enforced or inferred.

@gavinking
Copy link

@cescoffier I don't understand this concept either:

share the pool among datasources.

You mean, something like: connections to different databases might go into one big pool? Or something else?

@gavinking
Copy link

Whatever, the documentation should read something like:

/**
 * True when the database connection pool is shared among datasources.
 * 
 * If, false, the default, there is a separate connection pool for each named datasource.
 * Otherwise, if true, there is a single connection pool holding every pooled connection 
 * from any datasource, with a single FIFO eviction queue and blah blah blah blah blah.
 */

Where blah blah blah blah blah explains all the implications of enabling this pretty unintuitive setting.

tsegismont added a commit to tsegismont/quarkus that referenced this pull request Mar 29, 2022
…configuration

Follows-up on quarkusio#22292

The attribute is used by every other Reactive SQL Client recorder.
gsmet pushed a commit to gsmet/quarkus that referenced this pull request Mar 29, 2022
…configuration

Follows-up on quarkusio#22292

The attribute is used by every other Reactive SQL Client recorder.

(cherry picked from commit e7a8deb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants