8000 HHH-6780 - Wrong Query timeout calculation · JavaInCloud/hibernate-orm@9a7924d · GitHub
[go: up one dir, main page]
Skip to content

Commit 9a7924d

Browse files
sebersolesebersole
committed
HHH-6780 - Wrong Query timeout calculation
1 parent d00c9c8 commit 9a7924d

File tree

8 files changed

+175
-54
lines changed

8 files changed

+175
-54
lines changed

‎hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.jboss.logging.Logger;
3333

3434
import org.hibernate.HibernateException;
35+
import org.hibernate.TransactionException;
3536
import org.hibernate.engine.jdbc.batch.spi.Batch;
3637
import org.hibernate.engine.jdbc.batch.spi.BatchBuilder;
3738
import org.hibernate.engine.jdbc.batch.spi.BatchKey;
@@ -56,15 +57,17 @@
5657
* @author Steve Ebersole
5758
*/
5859
public class JdbcCoordinatorImpl implements JdbcCoordinator {
59-
60-
private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, JdbcCoordinatorImpl.class.getName());
60+
private static final CoreMessageLogger LOG = Logger.getMessageLogger(
61+
CoreMessageLogger.class, JdbcCoordinatorImpl.class.getName()
62+
);
6163

6264
private transient TransactionCoordinatorImpl transactionCoordinator;
63-
6465
private final transient LogicalConnectionImpl logicalConnection;
6566

6667
private transient Batch currentBatch;
6768

69+
private transient long transactionTimeOutInstant = -1;
70+
6871
public JdbcCoordinatorImpl(
6972
Connection userSuppliedConnection,
7073
TransactionCoordinatorImpl transactionCoordinator) {
@@ -153,6 +156,14 @@ public Batch getBatch(BatchKey key) {
153156
return currentBatch;
154157
}
155158

159+
@Override
160+
public void executeBatch() {
161+
if ( currentBatch != null ) {
162+
currentBatch.execute();
163+
currentBatch.release(); // needed?
164+
}
165+
}
166+
156167
@Override
157168
public void abortBatch() {
158169
if ( currentBatch != null ) {
@@ -171,20 +182,26 @@ public StatementPreparer getStatementPreparer() {
171182
}
172183

173184
@Override
174-
public void setTransactionTimeOut(int timeOut) {
175-
getStatementPreparer().setTransactionTimeOut( timeOut );
185+
public void setTransactionTimeOut(int seconds) {
186+
transactionTimeOutInstant = System.currentTimeMillis() + ( seconds * 1000 );
176187
}
177188

178-
/**
179-
* To be called after local transaction completion. Used to conditionally
180-
* release the JDBC connection aggressively if the configured release mode
181-
* indicates.
182-
*/
189+
@Override
190+
public int determineRemainingTransactionTimeOutPeriod() {
191+
if ( transactionTimeOutInstant < 0 ) {
192+
return -1;
193+
}
194+
final int secondsRemaining = (int) ((transactionTimeOutInstant - System.currentTimeMillis()) / 1000);
195+
if ( secondsRemaining <= 0 ) {
196+
throw new TransactionException( "transaction timeout expired" );
197+
}
198+
return secondsRemaining;
199+
}
200+
201+
@Override
183202
public void afterTransaction() {
184203
logicalConnection.afterTransaction();
185-
if ( statementPreparer != null ) {
186-
statementPreparer.unsetTransactionTimeOut();
187-
}
204+
transactionTimeOutInstant = -1;
188205
}
189206

190207
@Override
@@ -210,13 +227,6 @@ public <T> T coordinateWork(WorkExecutorVisitable<T> work) {
210227
}
211228
}
212229

213-
public void executeBatch() {
214-
if ( currentBatch != null ) {
215-
currentBatch.execute();
216-
currentBatch.release(); // needed?
217-
}
218-
}
219-
220230
@Override
221231
public void cancelLastQuery() {
222232
logicalConnection.getResourceRegistry().cancelLastQuery();

‎hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/LogicalConnectionImpl.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,7 @@ public Connection getDistinctConnectionProxy() {
188188
return buildConnectionProxy();
189189
}
190190

191-
/**
192-
* {@inheritDoc}
193-
*/
191+
@Override
194192
public Connection close() {
195193
LOG.trace( "Closing logical connection" );
196194
Connection c = isUserSuppliedConnection ? physicalConnection : null;

‎hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/StatementPreparerImpl.java

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030

3131
import org.hibernate.AssertionFailure;
3232
import org.hibernate.ScrollMode;
33-
import org.hibernate.TransactionException;
3433
import org.hibernate.cfg.Settings;
3534
import org.hibernate.engine.jdbc.spi.LogicalConnectionImplementor;
3635
import org.hibernate.engine.jdbc.spi.SqlExceptionHelper;
@@ -41,7 +40,6 @@
4140
* @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
4241
*/
4342
class StatementPreparerImpl implements StatementPreparer {
44-
private long transactionTimeOut = -1;
4543
private JdbcCoordinatorImpl jdbcCoordinator;
4644

4745
StatementPreparerImpl(JdbcCoordinatorImpl jdbcCoordinator) {
@@ -156,16 +154,6 @@ public PreparedStatement doPrepare() throws SQLException {
156154
}
157155
}
158156

159-
@Override
160-
public void setTransactionTimeOut(int timeOut) {
161-
transactionTimeOut = timeOut;
162-
}
163-
164-
@Override
165-
public void unsetTransactionTimeOut() {
166-
transactionTimeOut = -1;
167-
}
168-
169157
private abstract class StatementPreparationTemplate {
170158
protected final String sql;
171159

@@ -191,17 +179,11 @@ public void postProcess(PreparedStatement preparedStatement) throws SQLException
191179
}
192180

193181
private void setStatementTimeout(PreparedStatement preparedStatement) throws SQLException {
194-
if ( transactionTimeOut > 0 ) {
195-
int timeout = (int) ( transactionTimeOut - ( System.currentTimeMillis() / 1000 ) );
196-
if ( timeout <= 0 ) {
197-
throw new TransactionException( "transaction timeout expired" );
198-
}
199-
else {
200-
preparedStatement.setQueryTimeout( timeout );
201-
}
182+
final int remainingTransactionTimeOutPeriod = jdbcCoordinator.determineRemainingTransactionTimeOutPeriod();
183+
if ( remainingTransactionTimeOutPeriod > 0 ) {
184+
preparedStatement.setQueryTimeout( remainingTransactionTimeOutPeriod );
202185
}
203186
}
204-
205187
}
206188

207189
private abstract class QueryStatementPreparationTemplate extends StatementPreparationTemplate {

‎hibernate-core/src/main/java/org/hibernate/engine/jdbc/spi/JdbcCoordinator.java

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ public interface JdbcCoordinator extends Serializable {
6060
*/
6161
public Batch getBatch(BatchKey key);
6262

63+
/**
64+
* Execute the currently managed batch (if any)
65+
*/
66+
public void executeBatch();
67+
68+
/**
69+
* Abort the currently managed batch (if any)
70+
*/
6371
public void abortBatch();
6472

6573
/**
@@ -82,16 +90,51 @@ public interface JdbcCoordinator extends Serializable {
8290
*/
8391
public void flushEnding();
8492

93+
/**
94+
* Close this coordinator and release and resources.
95+
*
96+
* @return The {@link Connection} associated with the managed {@link #getLogicalConnection() logical connection}
97+
*
98+
* @see {@link LogicalConnection#close()}
99+
*/
85100
public Connection close();
86101

102+
/**
103+
* Signals the end of transaction.
104+
* <p/>
105+
* Intended for use from the transaction coordinator, after local transaction completion. Used to conditionally
106+
* release the JDBC connection aggressively if the configured release mode indicates.
107+
*/
87108
public void afterTransaction();
88109

110+
/**
111+
* Perform the requested work handling exceptions, coordinating and handling return processing.
112+
*
113+
* @param work The work to be performed.
114+
* @param <T> The result type.
115+
* @return The work result.
116+
*/
89117
public <T> T coordinateWork(WorkExecutorVisitable<T> work);
90118

91-
public void executeBatch();
92-
119+
/**
120+
* Attempt to cancel the last query sent to the JDBC driver.
121+
*/
93122
public void cancelLastQuery();
94123

95-
public void setTransactionTimeOut(int timeout);
124+
/**
125+
* Set the effective transaction timeout period for the current transaction, in seconds.
126+
*
127+
* @param seconds The number of seconds before a time out should occur.
128+
*/
129+
public void setTransactionTimeOut(int seconds);
96130

131+
/**
132+
* Calculate the amount of time, in seconds, still remaining before transaction timeout occurs.
133+
*
134+
* @return The number of seconds remaining until until a transaction timeout occurs. A negative value indicates
135+
* no timeout was requested.
136+
*
137+
* @throws org.hibernate.TransactionException Indicates the time out period has already been exceeded.
138+
*/
139+
public int determineRemainingTransactionTimeOutPeriod();
97140
}

‎hibernate-core/src/main/java/org/hibernate/engine/jdbc/spi/LogicalConnection.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,14 @@ public interface LogicalConnection extends Serializable {
8080
* Release the underlying connection and clean up any other resources associated
8181
* with this logical connection.
8282
* <p/>
83-
* This leaves the logical connection in a "no longer useable" state.
83+
* This leaves the logical connection in a "no longer usable" state.
8484
*
85-
* @return The physical connection which was being used.
85+
* @return The application-supplied connection, or {@code null} if Hibernate was managing connection.
8686
*/
8787
public Connection close();
8888

89+
/**
90+
* Signals the end of current transaction in which this logical connection operated.
91+
*/
8992
public void afterTransaction();
9093
}

‎hibernate-core/src/main/java/org/hibernate/engine/jdbc/spi/StatementPreparer.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import org.hibernate.ScrollMode;
2929

3030
/**
31+
* Contracting for preparing SQL statements
32+
*
3133
* @author Steve Ebersole
3234
*/
3335
public interface StatementPreparer {
@@ -91,7 +93,4 @@ public interface StatementPreparer {
9193
* @return the prepared statement
9294
*/
9395
public PreparedStatement prepareQueryStatement(String sql, boolean isCallable, ScrollMode scrollMode);
94-
95-
public void setTransactionTimeOut(int timeout);
96-
public void unsetTransactionTimeOut();
9796
}

‎hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionCoordinatorImpl.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ public class TransactionCoordinatorImpl implements TransactionCoordinator {
6666

6767
private final transient TransactionContext transactionContext;
6868
private final transient JdbcCoordinatorImpl jdbcCoordinator;
69-
private final transient TransactionFactory transactionFactory;
70-
private final transient TransactionEnvironment transactionEnvironment;
69+
private final transient TransactionFactory transactionFactory;
70+
private final transient TransactionEnvironment transactionEnvironment;
7171

7272
private final transient List<TransactionObserver> observers;
7373
private final transient SynchronizationRegistryImpl synchronizationRegistry;

‎hibernate-core/src/matrix/java/org/hibernate/test/tm/TransactionTimeoutTest.java

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Hibernate, Relational Persistence for Idiomatic Java
3+
*
4+
* Copyright (c) 2011, Red Hat Inc. or third-party contributors as
5+
* indicated by the @author tags or express copyright attribution
6+
* statements applied by the authors. All third-party contributions are
7+
* distributed under license by Red Hat Inc.
8+
*
9+
* This copyrighted material is made available to anyone wishing to use, modify,
10+
* copy, or redistribute it subject to the terms and conditions of the GNU
11+
* Lesser General Public License, as published by the Free Software Foundation.
12+
*
13+
* This program is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
15+
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
16+
* for more details.
17+
*
18+
* You should have received a copy of the GNU Lesser General Public License
19+
* along with this distribution; if not, write to:
20+
* Free Software Foundation, Inc.
21+
* 51 Franklin Street, Fifth Floor
22+
* Boston, MA 02110-1301 USA
23+
*/
24+
package org.hibernate.test.tm;
25+
26+
import org.junit.Test;
27+
28+
import org.hibernate.Session;
29+
import org.hibernate.Transaction;
30+
import org.hibernate.TransactionException;
31+
import org.hibernate.engine.spi.SessionImplementor;
32+
import org.hibernate.engine.transaction.internal.jdbc.JdbcTransaction;
33+
34+
import org.hibernate.test.jdbc.Person;
35+
import org.hibernate.testing.TestForIssue;
36+
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
37+
38+
import static org.junit.Assert.assertEquals;
39+
import static org.junit.Assert.assertNotSame;
40+
41+
/**
42+
* @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
43+
*/
44+
@TestForIssue(jiraKey = "HHH-6780")
45+
public class TransactionTimeoutTest extends BaseCoreFunctionalTestCase {
46+
@Override
47+
public String[] getMappings() {
48+
return new String[] {"jdbc/Mappings.hbm.xml"};
49+
}
50+
51+
@Test
52+
public void testJdbcCoordinatorTransactionTimeoutCheck() {
53+
Session session = openSession();
54+
Transaction transaction = session.getTransaction();
55+
transaction.setTimeout( 2 );
56+
assertEquals( -1, ((SessionImplementor)session).getTransactionCoordinator().getJdbcCoordinator().determineRemainingTransactionTimeOutPeriod() );
57+
transaction.begin();
58+
assertNotSame( -1, ((SessionImplementor)session).getTransactionCoordinator().getJdbcCoordinator().determineRemainingTransactionTimeOutPeriod() );
59+
transaction.commit();
60+
session.close();
61+
}
62+
63+
@Test(expected = TransactionException.class)
64+
public void testTransactionTimeoutFailure() throws InterruptedException {
65+
Session session = openSession();
66+
Transaction transaction = session.getTransaction();
67+
transaction.setTimeout( 1 );
68+
assertEquals( -1, ((SessionImplementor)session).getTransactionCoordinator().getJdbcCoordinator().determineRemainingTransactionTimeOutPeriod() );
69+
transaction.begin();
70+
Thread.sleep( 1000 );
71+
session.persist( new Person( "Lukasz", "Antoniak" ) );
72+
transaction.commit();
73+
session.close();
74+
}
75+
76+
@Test
77+
public void testTransactionTimeoutSuccess() {
78+
Session session = openSession();
79+
Transaction transaction = session.getTransaction();
80+
transaction.setTimeout( 2 );
81+
transaction.begin();
82+
session.persist( new Person( "Lukasz", "Antoniak" ) );
83+
transaction.commit();
84+
session.close();
85+
}
86+
}

0 commit comments

Comments
 (0)
0