Skip to content

Commit

Permalink
HHH-17989 - Fix for StatisticsImplementor.closeStatement() never called
Browse files Browse the repository at this point in the history
Signed-off-by: Jan Schatteman <[email protected]>
  • Loading branch information
jrenaat committed Oct 24, 2024
1 parent 7bf6004 commit edac5cf
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.hibernate.mapping.PrimaryKey;
import org.hibernate.mapping.Table;
import org.hibernate.service.ServiceRegistry;
import org.hibernate.stat.spi.StatisticsImplementor;
import org.hibernate.type.BasicTypeRegistry;
import org.hibernate.type.StandardBasicTypes;
import org.hibernate.type.Type;
Expand Down Expand Up @@ -624,13 +625,20 @@ private PreparedStatement prepareStatement(
logger.logStatement( sql, FormatStyle.BASIC.getFormatter() );
final EventManager eventManager = session.getEventManager();
final HibernateMonitoringEvent creationEvent = eventManager.beginJdbcPreparedStatementCreationEvent();
final StatisticsImplementor stats = session.getFactory().getStatistics();
try {
listener.jdbcPrepareStatementStart();
if ( stats != null && stats.isStatisticsEnabled() ) {
stats.prepareStatement();
}
return connection.prepareStatement( sql );
}
finally {
eventManager.completeJdbcPreparedStatementCreationEvent( creationEvent, sql );
listener.jdbcPrepareStatementEnd();
if ( stats != null && stats.isStatisticsEnabled() ) {
stats.closeStatement();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.hibernate.internal.CoreMessageLogger;
import org.hibernate.jdbc.AbstractReturningWork;
import org.hibernate.mapping.Table;
import org.hibernate.stat.spi.StatisticsImplementor;
import org.hibernate.type.StandardBasicTypes;

import org.jboss.logging.Logger;
Expand Down Expand Up @@ -231,13 +232,20 @@ private PreparedStatement prepareStatement(
logger.logStatement( sql, FormatStyle.BASIC.getFormatter() );
final EventManager eventManager = session.getEventManager();
final HibernateMonitoringEvent creationEvent = eventManager.beginJdbcPreparedStatementCreationEvent();
final StatisticsImplementor stats = session.getFactory().getStatistics();
try {
statsCollector.jdbcPrepareStatementStart();
if ( stats != null && stats.isStatisticsEnabled() ) {
stats.prepareStatement();
}
return connection.prepareStatement( sql );
}
finally {
eventManager.completeJdbcPreparedStatementCreationEvent( creationEvent, sql );
statsCollector.jdbcPrepareStatementEnd();
if ( stats != null && stats.isStatisticsEnabled() ) {
stats.closeStatement();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ public void jdbcPrepareStatementStart() {
if ( sessionListener != null ) {
sessionListener.jdbcPrepareStatementStart();
}

if ( statistics != null && statistics.isStatisticsEnabled() ) {
statistics.prepareStatement();
}
}

public void jdbcPrepareStatementEnd() {
Expand All @@ -88,7 +92,7 @@ public void jdbcPrepareStatementEnd() {
}

if ( statistics != null && statistics.isStatisticsEnabled() ) {
statistics.prepareStatement();
statistics.closeStatement();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import org.hibernate.dialect.Dialect;
import org.hibernate.stat.spi.StatisticsImplementor;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.ServiceRegistry;
Expand All @@ -20,6 +21,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;

import static org.hibernate.cfg.StatisticsSettings.GENERATE_STATISTICS;
import static org.hibernate.graph.GraphSemantic.FETCH;
Expand All @@ -32,22 +34,31 @@
public class StatelessSessionStatisticsTest {
@Test
void test(SessionFactoryScope scope) {
StatisticsImplementor statistics = scope.getSessionFactory().getStatistics();
final StatisticsImplementor statistics = scope.getSessionFactory().getStatistics();
final Dialect dialect = scope.fromSession( session -> session.getDialect() );
final boolean isSybaseOrMysql = dialect.getClass().getName().toLowerCase( Locale.ROOT ).split( "sybase|mysql" ).length == 2;
int stmtCount = isSybaseOrMysql ? 4 : 3;

assertEquals(0, statistics.getEntityInsertCount());
assertEquals(0, statistics.getEntityUpdateCount());
assertEquals(0, statistics.getEntityDeleteCount());
assertEquals(0, statistics.getEntityLoadCount());
assertEquals(0, statistics.getPrepareStatementCount());
Person person = new Person();
person.name = "Gavin";
person.handles.add("@1ovthafew");
scope.inStatelessTransaction(s -> s.insert(person));
assertEquals(1, statistics.getEntityInsertCount());
assertEquals(1, statistics.getCollectionRecreateCount());
assertEquals(stmtCount, statistics.getPrepareStatementCount());
assertEquals(stmtCount, statistics.getCloseStatementCount());
scope.inStatelessSession(s -> s.get(Person.class, person.id));
assertEquals(1, statistics.getEntityLoadCount());
assertEquals(0, statistics.getEntityFetchCount());
assertEquals(1, statistics.getCollectionLoadCount());
assertEquals(0, statistics.getCollectionFetchCount());
assertEquals(++stmtCount, statistics.getPrepareStatementCount());
assertEquals(stmtCount, statistics.getCloseStatementCount());
person.name = "Gavin King";
scope.inStatelessTransaction(s -> s.update(person));
assertEquals(1, statistics.getEntityUpdateCount());
Expand All @@ -56,25 +67,37 @@ void test(SessionFactoryScope scope) {
assertEquals(2, statistics.getEntityLoadCount());
assertEquals(2, statistics.getCollectionLoadCount());
assertEquals(0, statistics.getCollectionFetchCount());
assertEquals(stmtCount+=4, statistics.getPrepareStatementCount());
assertEquals(stmtCount, statistics.getCloseStatementCount());
scope.inStatelessSession(s -> s.get(s.createEntityGraph(Person.class), FETCH, person.id));
assertEquals(3, statistics.getEntityLoadCount());
assertEquals(2, statistics.getCollectionLoadCount());
assertEquals(0, statistics.getCollectionFetchCount());
assertEquals(++stmtCount, statistics.getPrepareStatementCount());
assertEquals(stmtCount, statistics.getCloseStatementCount());
scope.inStatelessSession(s -> s.fetch(s.get(s.createEntityGraph(Person.class), FETCH, person.id).handles));
assertEquals(4, statistics.getEntityLoadCount());
assertEquals(3, statistics.getCollectionLoadCount());
assertEquals(1, statistics.getCollectionFetchCount());
assertEquals(stmtCount+=2, statistics.getPrepareStatementCount());
assertEquals(stmtCount, statistics.getCloseStatementCount());
scope.inStatelessSession(s -> s.createQuery("from Person", Person.class).getSingleResult());
assertEquals(5, statistics.getEntityLoadCount());
assertEquals(4, statistics.getCollectionLoadCount());
assertEquals(2, statistics.getCollectionFetchCount());
assertEquals(stmtCount+=2, statistics.getPrepareStatementCount());
assertEquals(stmtCount, statistics.getCloseStatementCount());
person.handles.add("hello world");
scope.inStatelessTransaction(s -> s.upsert(person));
assertEquals(2, statistics.getCollectionUpdateCount());
assertEquals(stmtCount+=4, statistics.getPrepareStatementCount());
assertEquals(stmtCount, statistics.getCloseStatementCount());
scope.inStatelessTransaction(s -> s.delete(person));
assertEquals(1, statistics.getEntityDeleteCount());
assertEquals(1, statistics.getCollectionRemoveCount());
assertEquals(4, statistics.getTransactionCount());
assertEquals(stmtCount+=2, statistics.getPrepareStatementCount());
assertEquals(stmtCount, statistics.getCloseStatementCount());
}

@Entity(name="Person")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
@DomainModel
@SessionFactory
@ServiceRegistry(
settingProviders = @SettingProvider(provider = StatisticsWithNoCachingTest.RegionFacrotySettingProvider.class, settingName = AvailableSettings.CACHE_REGION_FACTORY)
settingProviders = @SettingProvider(provider = StatisticsWithNoCachingTest.RegionFactorySettingProvider.class, settingName = AvailableSettings.CACHE_REGION_FACTORY)
)
public class StatisticsWithNoCachingTest {

public static class RegionFacrotySettingProvider implements SettingProvider.Provider<String> {
public static class RegionFactorySettingProvider implements SettingProvider.Provider<String> {

@Override
public String getSetting() {
Expand Down

0 comments on commit edac5cf

Please sign in to comment.