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

minor cleanups and logging improvements related to logical connections #9879

Merged
merged 1 commit into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@
import jakarta.transaction.Synchronization;
import org.checkerframework.checker.nullness.qual.Nullable;

import static org.hibernate.cfg.JdbcSettings.CONNECTION_PROVIDER_DISABLES_AUTOCOMMIT;
import static org.hibernate.cfg.ValidationSettings.JAKARTA_VALIDATION_MODE;
import static org.jboss.logging.Logger.Level.DEBUG;
import static org.jboss.logging.Logger.Level.ERROR;
import static org.jboss.logging.Logger.Level.INFO;
import static org.jboss.logging.Logger.Level.TRACE;
import static org.jboss.logging.Logger.Level.WARN;

/**
Expand Down Expand Up @@ -562,7 +565,7 @@ void cannotResolveNonNullableTransientDependencies(
void explicitSkipLockedLockCombo();

@LogMessage(level = INFO)
@Message(value = "'jakarta.persistence.validation.mode' named multiple values: %s", id = 448)
@Message(value = "'" + JAKARTA_VALIDATION_MODE + "' named multiple values: %s", id = 448)
void multipleValidationModes(String modes);

@LogMessage(level = WARN)
Expand Down Expand Up @@ -722,22 +725,22 @@ void cannotResolveNonNullableTransientDependencies(
void flushAndEvictOnRemove(String entityName);

@LogMessage(level = ERROR)
@Message(value = "Illegal argument on static metamodel field injection : %s#%s; expected type : %s; encountered type : %s", id = 15007)
@Message(value = "Illegal argument on static metamodel field injection: %s#%s; expected type: %s; encountered type: %s", id = 15007)
void illegalArgumentOnStaticMetamodelFieldInjection(
String name,
String name2,
String name3,
String name4);

@LogMessage(level = WARN)
@Message(value = "Unable to locate static metamodel field : %s#%s; this may or may not indicate a problem with the static metamodel", id = 15011)
@Message(value = "Unable to locate static metamodel field: %s#%s; this may or may not indicate a problem with the static metamodel", id = 15011)
void unableToLocateStaticMetamodelField(
String name,
String name2);

@LogMessage(level = DEBUG)
@Message(value = "Returning null (as required by JPA spec) rather than throwing EntityNotFoundException, " +
"as the entity (type=%s, id=%s) does not exist", id = 15013)
@Message(value = "Returning null (as required by JPA spec) rather than throwing EntityNotFoundException " +
"since the entity of type '%s' with id [%s] does not exist", id = 15013)
void ignoringEntityNotFound(String entityName, String identifier);

@LogMessage(level = DEBUG)
Expand All @@ -754,6 +757,27 @@ void unableToLocateStaticMetamodelField(
)
void duplicatedPersistenceUnitName(String name);

@LogMessage(level = DEBUG)
@Message(
id = 455,
value =
"'" + CONNECTION_PROVIDER_DISABLES_AUTOCOMMIT + "' " +
"""
was enabled. This setting should only be enabled when JDBC Connections obtained by Hibernate \
from the ConnectionProvider have auto-commit disabled. Enabling this setting when connections \
have auto-commit enabled leads to execution of SQL operations outside of any JDBC transaction.\
"""
)
void connectionProviderDisablesAutoCommitEnabled();

@LogMessage(level = TRACE)
@Message(value = "Closing logical connection", id = 456)
void closingLogicalConnection();

@LogMessage(level = TRACE)
@Message(value = "Logical connection closed", id = 457)
void logicalConnectionClosed();

@LogMessage(level = DEBUG)
@Message(
id = 401,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
import org.hibernate.engine.jdbc.connections.spi.JdbcConnectionAccess;
import org.hibernate.engine.jdbc.spi.JdbcServices;
import org.hibernate.engine.jdbc.spi.SqlExceptionHelper;
import org.hibernate.internal.CoreLogging;
import org.hibernate.internal.CoreMessageLogger;
import org.hibernate.resource.jdbc.LogicalConnection;
import org.hibernate.resource.jdbc.ResourceRegistry;
import org.hibernate.resource.jdbc.spi.JdbcEventHandler;
import org.hibernate.resource.jdbc.spi.JdbcSessionContext;
import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode;

import org.jboss.logging.Logger;

import static org.hibernate.ConnectionAcquisitionMode.IMMEDIATELY;
import static org.hibernate.ConnectionReleaseMode.AFTER_STATEMENT;
import static org.hibernate.ConnectionReleaseMode.BEFORE_TRANSACTION_COMPLETION;
Expand All @@ -37,7 +37,7 @@
* @author Steve Ebersole
*/
public class LogicalConnectionManagedImpl extends AbstractLogicalConnectionImplementor {
private static final Logger log = Logger.getLogger( LogicalConnectionManagedImpl.class );
private static final CoreMessageLogger log = CoreLogging.messageLogger( LogicalConnectionManagedImpl.class );

private final transient JdbcConnectionAccess jdbcConnectionAccess;
private final transient JdbcEventHandler jdbcEventHandler;
Expand Down Expand Up @@ -72,13 +72,7 @@ public LogicalConnectionManagedImpl(

this.providerDisablesAutoCommit = jdbcSessionContext.doesConnectionProviderDisableAutoCommit();
if ( providerDisablesAutoCommit ) {
log.debug(
"`hibernate.connection.provider_disables_autocommit` was enabled. This setting should only be " +
"enabled when you are certain that the Connections given to Hibernate by the " +
"ConnectionProvider have auto-commit disabled. Enabling this setting when the " +
"Connections do not have auto-commit disabled will lead to Hibernate executing " +
"SQL operations outside of any JDBC/SQL transaction."
);
log.connectionProviderDisablesAutoCommitEnabled();
}
}

Expand All @@ -98,12 +92,10 @@ public LogicalConnectionManagedImpl(
private PhysicalConnectionHandlingMode determineConnectionHandlingMode(
PhysicalConnectionHandlingMode connectionHandlingMode,
JdbcConnectionAccess jdbcConnectionAccess) {
if ( connectionHandlingMode.getReleaseMode() == AFTER_STATEMENT
&& !jdbcConnectionAccess.supportsAggressiveRelease() ) {
return DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION;
}

return connectionHandlingMode;
return connectionHandlingMode.getReleaseMode() == AFTER_STATEMENT
&& !jdbcConnectionAccess.supportsAggressiveRelease()
? DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION
: connectionHandlingMode;
}

private LogicalConnectionManagedImpl(
Expand Down Expand Up @@ -159,7 +151,6 @@ public Connection getPhysicalConnection() {
@Override
public void afterStatement() {
super.afterStatement();

if ( connectionHandlingMode.getReleaseMode() == AFTER_STATEMENT ) {
if ( getResourceRegistry().hasRegisteredResources() ) {
log.debug( "Skipping aggressive release of JDBC Connection after-statement due to held resources" );
Expand All @@ -183,7 +174,6 @@ public void beforeTransactionCompletion() {
@Override
public void afterTransaction() {
super.afterTransaction();

if ( connectionHandlingMode.getReleaseMode() != ON_CLOSE ) {
// NOTE : we check for !ON_CLOSE here (rather than AFTER_TRANSACTION) to also catch:
// - AFTER_STATEMENT cases that were circumvented due to held resources
Expand All @@ -199,51 +189,48 @@ public Connection manualDisconnect() {
if ( closed ) {
throw new ResourceClosedException( "Logical connection is closed" );
}
final Connection c = physicalConnection;
final Connection connection = physicalConnection;
releaseConnection();
return c;
return connection;
}

@Override
public void manualReconnect(Connection suppliedConnection) {
if ( closed ) {
throw new ResourceClosedException( "Logical connection is closed" );
}

throw new IllegalStateException( "Cannot manually reconnect unless Connection was originally supplied by user" );
}

private void releaseConnection() {
final Connection localVariableConnection = this.physicalConnection;
if ( localVariableConnection == null ) {
return;
}

// We need to set the connection to null before we release resources,
// in order to prevent recursion into this method.
// Recursion can happen when we release resources and when batch statements are in progress:
// when releasing resources, we'll abort the batch statement,
// which will trigger "logicalConnection.afterStatement()",
// which in some configurations will release the connection.
this.physicalConnection = null;
try {
final Connection localVariableConnection = physicalConnection;
if ( localVariableConnection != null ) {
// We need to set the connection to null before we release resources,
// in order to prevent recursion into this method.
// Recursion can happen when we release resources and when batch statements are in progress:
// when releasing resources, we'll abort the batch statement,
// which will trigger "logicalConnection.afterStatement()",
// which in some configurations will release the connection.
physicalConnection = null;
try {
getResourceRegistry().releaseResources();
if ( !localVariableConnection.isClosed() ) {
sqlExceptionHelper.logAndClearWarnings( localVariableConnection );
try {
getResourceRegistry().releaseResources();
if ( !localVariableConnection.isClosed() ) {
sqlExceptionHelper.logAndClearWarnings( localVariableConnection );
}
}
finally {
jdbcEventHandler.jdbcConnectionReleaseStart();
jdbcConnectionAccess.releaseConnection( localVariableConnection );
}
}
catch (SQLException e) {
throw sqlExceptionHelper.convert( e, "Unable to release JDBC Connection" );
}
finally {
jdbcEventHandler.jdbcConnectionReleaseStart();
jdbcConnectionAccess.releaseConnection( localVariableConnection );
jdbcEventHandler.jdbcConnectionReleaseEnd();
}
}
catch (SQLException e) {
throw sqlExceptionHelper.convert( e, "Unable to release JDBC Connection" );
}
finally {
jdbcEventHandler.jdbcConnectionReleaseEnd();
}
}

@Override
Expand All @@ -261,20 +248,17 @@ public static LogicalConnectionManagedImpl deserialize(

@Override
public Connection close() {
if ( closed ) {
return null;
}

getResourceRegistry().releaseResources();

log.trace( "Closing logical connection" );
try {
releaseConnection();
}
finally {
// no matter what
closed = true;
log.trace( "Logical connection closed" );
if ( !closed ) {
getResourceRegistry().releaseResources();
log.closingLogicalConnection();
try {
releaseConnection();
}
finally {
// no matter what
closed = true;
log.logicalConnectionClosed();
}
}
return null;
}
Expand All @@ -300,7 +284,6 @@ public void begin() {
protected void afterCompletion() {
resetConnection( initiallyAutoCommit );
initiallyAutoCommit = false;

afterTransaction();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,19 @@
import java.io.ObjectOutputStream;
import java.sql.Connection;

import org.hibernate.internal.CoreLogging;
import org.hibernate.internal.CoreMessageLogger;
import org.hibernate.resource.jdbc.LogicalConnection;
import org.hibernate.resource.jdbc.ResourceRegistry;
import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode;

import org.jboss.logging.Logger;
import static org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode.IMMEDIATE_ACQUISITION_AND_HOLD;

/**
* @author Steve Ebersole
*/
public class LogicalConnectionProvidedImpl extends AbstractLogicalConnectionImplementor {
private static final Logger log = Logger.getLogger( LogicalConnection.class );
private static final CoreMessageLogger log = CoreLogging.messageLogger( LogicalConnection.class );

private transient Connection providedConnection;
private final boolean initiallyAutoCommit;
Expand All @@ -30,7 +32,6 @@ public LogicalConnectionProvidedImpl(Connection providedConnection, ResourceRegi
if ( providedConnection == null ) {
throw new IllegalArgumentException( "Provided Connection cannot be null" );
}

this.providedConnection = providedConnection;
this.initiallyAutoCommit = determineInitialAutoCommitMode( providedConnection );
}
Expand All @@ -43,7 +44,7 @@ private LogicalConnectionProvidedImpl(boolean closed, boolean initiallyAutoCommi

@Override
public PhysicalConnectionHandlingMode getConnectionHandlingMode() {
return PhysicalConnectionHandlingMode.IMMEDIATE_ACQUISITION_AND_HOLD;
return IMMEDIATE_ACQUISITION_AND_HOLD;
}

@Override
Expand All @@ -53,17 +54,15 @@ public boolean isOpen() {

@Override
public Connection close() {
log.trace( "Closing logical connection" );

log.closingLogicalConnection();
getResourceRegistry().releaseResources();

try {
return providedConnection;
}
finally {
providedConnection = null;
closed = true;
log.trace( "Logical connection closed" );
log.logicalConnectionClosed();
}
}

Expand Down Expand Up @@ -99,14 +98,13 @@ public Connection manualDisconnect() {
return providedConnection;
}
finally {
this.providedConnection = null;
providedConnection = null;
}
}

@Override
public void manualReconnect(Connection connection) {
errorIfClosed();

if ( connection == null ) {
throw new IllegalArgumentException( "cannot reconnect using a null connection" );
}
Expand All @@ -131,7 +129,6 @@ protected Connection getConnectionForTransactionManagement() {
@Override
protected void afterCompletion() {
afterTransaction();

resetConnection( initiallyAutoCommit );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,18 @@ public interface ConnectionHelper {
*
* @param needsAutoCommit Should connection be forced to auto-commit
* if not already.
* @throws SQLException
*/
public void prepare(boolean needsAutoCommit) throws SQLException;
void prepare(boolean needsAutoCommit) throws SQLException;

/**
* Get a reference to the connection we are using.
*
* @return The JDBC connection.
* @throws SQLException
*/
public Connection getConnection() throws SQLException;
Connection getConnection() throws SQLException;

/**
* Release any resources held by this helper.
*
* @throws SQLException
*/
public void release() throws SQLException;
void release() throws SQLException;
}
Loading