From c2a3bd3b485b46faadbfedf59e07d57dc6f06f3e Mon Sep 17 00:00:00 2001 From: Jan Schatteman Date: Tue, 7 Mar 2023 23:35:03 +0100 Subject: [PATCH] HHH-16249 - Add test for issue Disable batching in a stateless session when no transaction is active Signed-off-by: Jan Schatteman --- .../java/org/hibernate/jdbc/Expectation.java | 2 +- .../entity/mutation/DeleteCoordinator.java | 4 +- .../entity/mutation/InsertCoordinator.java | 14 +- .../mutation/UpdateCoordinatorStandard.java | 4 +- .../BatchSizeAndStatelessSessionTest.java | 132 ++++++++++++++++++ 5 files changed, 147 insertions(+), 9 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchSizeAndStatelessSessionTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java b/hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java index 9a72bc6dab..54b32789f1 100644 --- a/hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java +++ b/hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java @@ -18,7 +18,7 @@ import org.hibernate.HibernateException; public interface Expectation { /** - * Is it acceptable to combiner this expectation with statement batching? + * Is it acceptable to combine this expectation with statement batching? * * @return True if batching can be combined with this expectation; false otherwise. */ diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/DeleteCoordinator.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/DeleteCoordinator.java index 7b1914d6e3..3e96f7d060 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/DeleteCoordinator.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/DeleteCoordinator.java @@ -133,7 +133,9 @@ public class DeleteCoordinator extends AbstractMutationCoordinator { return session.getFactory() .getServiceRegistry() .getService( MutationExecutorService.class ) - .createExecutor( () -> batchKey, group, session ); + .createExecutor( ( session.getTransactionCoordinator() != null && + session.getTransactionCoordinator().isTransactionActive() ? () -> batchKey : () -> null ), + group, session ); } protected void applyLocking( diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java index f0b120d967..424a7c39fd 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java @@ -49,17 +49,17 @@ import static org.hibernate.generator.EventType.INSERT; @Internal public class InsertCoordinator extends AbstractMutationCoordinator { private final MutationOperationGroup staticInsertGroup; - private final BasicBatchKey insertBatchKey; + private final BasicBatchKey batchKey; public InsertCoordinator(AbstractEntityPersister entityPersister, SessionFactoryImplementor factory) { super( entityPersister, factory ); if ( entityPersister.hasInsertGeneratedProperties() ) { // disable batching in case of insert generated properties - insertBatchKey = null; + batchKey = null; } else { - insertBatchKey = new BasicBatchKey( + batchKey = new BasicBatchKey( entityPersister.getEntityName() + "#INSERT", null ); @@ -80,7 +80,7 @@ public class InsertCoordinator extends AbstractMutationCoordinator { } public BasicBatchKey getInsertBatchKey() { - return insertBatchKey; + return batchKey; } /** @@ -301,11 +301,13 @@ public class InsertCoordinator extends AbstractMutationCoordinator { } } - private MutationExecutor executor(SharedSessionContractImplementor session, MutationOperationGroup insertGroup) { + private MutationExecutor executor(SharedSessionContractImplementor session, MutationOperationGroup group) { return session.getFactory() .getServiceRegistry() .getService( MutationExecutorService.class ) - .createExecutor( () -> insertBatchKey, insertGroup, session ); + .createExecutor( ( session.getTransactionCoordinator() != null && + session.getTransactionCoordinator().isTransactionActive() ? () -> batchKey : () -> null ), + group, session ); } protected static TableInclusionChecker getTableInclusionChecker(InsertValuesAnalysis insertValuesAnalysis) { diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java index ab7a8e6941..9d6bdac060 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java @@ -970,7 +970,9 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple return session.getSessionFactory() .getServiceRegistry() .getService( MutationExecutorService.class ) - .createExecutor( () -> batchKey, group, session ); + .createExecutor( ( session.getTransactionCoordinator() != null && + session.getTransactionCoordinator().isTransactionActive() ? () -> batchKey : () -> null ), + group, session ); } protected MutationOperationGroup generateDynamicUpdateGroup( diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchSizeAndStatelessSessionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchSizeAndStatelessSessionTest.java new file mode 100644 index 0000000000..756b9ec95e --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchSizeAndStatelessSessionTest.java @@ -0,0 +1,132 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.batch; + +import org.hibernate.Session; +import org.hibernate.StatelessSession; +import org.hibernate.query.SelectionQuery; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author Jan Schatteman + */ +@DomainModel( + annotatedClasses = { BatchSizeAndStatelessSessionTest.TestEntity.class } +) +@SessionFactory +@TestForIssue( jiraKey = "HHH-16249") +public class BatchSizeAndStatelessSessionTest { + + private final String countQuery = "select count(id) from TestEntity"; + private final int batchSize = 3; + private final int total = 10; + + @AfterEach + public void cleanup( SessionFactoryScope scope ) { + scope.inTransaction( + session -> session.createMutationQuery( "delete from TestEntity" ).executeUpdate() + ); + } + + @Test + public void testBatchWithStatelessSessionTx( SessionFactoryScope scope ) { + scope.inStatelessTransaction( + ss -> { + SelectionQuery query = ss.createSelectionQuery( countQuery, Long.class ); + ss.setJdbcBatchSize( batchSize ); + long intermediateCount = 0; + for ( int i = 1; i <= total; i++ ) { + ss.insert( new TestEntity(i) ); + long count = query.getSingleResult(); + // This should be batched, so the count should remain 0 or a multiple of the batch size and only change + // when a batch is executed + if ( i % batchSize == 0 ) { + assertEquals( i, count ); + intermediateCount += batchSize; + } else { + assertEquals( intermediateCount, count ); + } + } + } + ); + + checkTotal( scope ); + } + + @Test + public void testBatchWithStatelessSessionNoTx( SessionFactoryScope scope ) { + scope.inStatelessSession( + ss -> { + ss.setJdbcBatchSize( batchSize ); + SelectionQuery query = ss.createSelectionQuery( countQuery, Long.class ); + for ( int i = 1; i <= total; i++ ) { + ss.insert( new TestEntity( i ) ); + long count = query.getSingleResult(); + // There shouldn't be any batching here, so the count should go up one at a time + assertEquals( i, count ); + } + } + ); + + checkTotal( scope ); + } + + @Test + public void testBatchWithStatelessSessionInParentTx( SessionFactoryScope scope ) { + scope.inSession( + s -> { + s.beginTransaction(); + try (StatelessSession ss = s.getSessionFactory().openStatelessSession()) { + SelectionQuery query = ss.createSelectionQuery( countQuery, Long.class ); + ss.setJdbcBatchSize(batchSize); + for ( int i = 1; i <= total; i++ ) { + ss.insert( new TestEntity(i) ); + long count = query.getSingleResult(); + // Even though it's inside a parent Tx, there's no batching here, so the count should go up one at a time + assertEquals( i, count ); + } + } + s.getTransaction().commit(); + } + ); + + checkTotal( scope ); + } + + private void checkTotal(SessionFactoryScope scope) { + scope.inSession( + s -> { + SelectionQuery q = s.createSelectionQuery( countQuery, Long.class ); + assertEquals( total, q.getSingleResult() ); + } + ); + } + + @Entity( name = "TestEntity" ) + public static class TestEntity { + @Id + int id; + + public TestEntity() { + } + + public TestEntity( int id ) { + this.id = id; + } + } +}