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.

Sign up for GitHub

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

Jump to bottom

Connection handling fixes #14305

Merged
merged 7 commits into from
Feb 25, 2021
Merged

Connection handling fixes #14305

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1deece0
Test the transaction lifecycle (session flushes, session closing, ...)
yrodiere Jan 13, 2021
373042f
Test warnings when executing StoredProcedureQuery (#13273)
yrodiere Jan 14, 2021
13696e9
Set Hibernate ORM connection handling strategy to BEFORE_TRANSACTION_…
Sanne May 1, 2020
d39f754
Simplify connection handling setup and add a note about #7242
yrodiere Jan 13, 2021
a488c71
Let Hibernate ORM handle the flush/close after a transaction
yrodiere Jan 12, 2021
3756ff8
Let the session join JTA transactions implicitly
yrodiere Jan 13, 2021
d9ed7bc
Allow setting a different connection handling mode and auto-close mod…
yrodiere Jan 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions .../runtime/src/main/java/io/quarkus/hibernate/orm/runtime/boot/FastBootMetadataBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,20 +242,18 @@ private MergedSettings mergeSettings(PersistenceUnitDescriptor persistenceUnit)
//Agroal already does disable auto-commit, so Hibernate ORM should trust that:
cfg.put(AvailableSettings.CONNECTION_PROVIDER_DISABLES_AUTOCOMMIT, Boolean.TRUE.toString());

/**
/*
* Set CONNECTION_HANDLING to DELAYED_ACQUISITION_AND_RELEASE_BEFORE_TRANSACTION_COMPLETION
* as it generally performs better, at no known drawbacks.
* This is a new mode in Hibernate ORM, it might become the default in the future.
*
* Note: other connection handling modes lead to leaked resources, statements in particular.
* See https://github.com/quarkusio/quarkus/issues/7242, https://github.com/quarkusio/quarkus/issues/13273
*
* @see org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode
*/
{
final Object explicitSetting = cfg.get(AvailableSettings.CONNECTION_HANDLING);
if (explicitSetting == null) {
cfg.put(AvailableSettings.CONNECTION_HANDLING,
PhysicalConnectionHandlingMode.DELAYED_ACQUISITION_AND_RELEASE_BEFORE_TRANSACTION_COMPLETION);
}
}
cfg.putIfAbsent(AvailableSettings.CONNECTION_HANDLING,
PhysicalConnectionHandlingMode.DELAYED_ACQUISITION_AND_RELEASE_BEFORE_TRANSACTION_COMPLETION);
Copy link
Member

Choose a reason for hiding this comment

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

N.B. that semantics is not the same: the key can be defined while the value is set to null. No? Or is the change intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

If it's a problem here, it's probably also a problem just above, where we use cfg.containsKey to check whether a property is defined or not.

If we're going to worry about this, I think what we need to ask is: how would we end up in a situation where a property is set, but its value is null? From what I understand the only way for properties to be pre-set here is from a persistence.xml. I'm not sure if you can set something to null from persistence.xml?


if (readBooleanConfigurationValue(cfg, WRAP_RESULT_SETS)) {
LOG.warn("Wrapping result sets is not supported. Setting " + WRAP_RESULT_SETS + " to false.");
Expand Down