Skip to content

Commit b40e53a

Browse files
committed
Fix reset of transaction functions
Transaction functions (`Session#readTransaction()` and `Session#writeTransaction()`) should respect `Session#reset()` call the same way other statement running operations do. They shouldn't continue retrying after `#reset()`. This commit makes transaction functions responsive to reset by removing excessive synchronization and adding status code checks so retries do not happen when transaction is explicitly terminated.
1 parent bec13b3 commit b40e53a

File tree

4 files changed

+463
-2
lines changed

4 files changed

+463
-2
lines changed

driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ public synchronized void onConnectionError( boolean recoverable )
244244
}
245245
}
246246

247-
private synchronized <T> T transaction( AccessMode mode, TransactionWork<T> work )
247+
private <T> T transaction( AccessMode mode, TransactionWork<T> work )
248248
{
249249
RetryDecision decision = null;
250250
List<Throwable> errors = null;

driver/src/main/java/org/neo4j/driver/internal/retry/ExponentialBackoff.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,25 @@ private static boolean canRetryOn( Throwable error )
150150
{
151151
return error instanceof SessionExpiredException ||
152152
error instanceof ServiceUnavailableException ||
153-
error instanceof TransientException;
153+
isTransientError( error );
154+
}
155+
156+
private static boolean isTransientError( Throwable error )
157+
{
158+
if ( error instanceof TransientException )
159+
{
160+
String code = ((TransientException) error).code();
161+
// Retries should not happen when transaction was explicitly terminated by the user.
162+
// Termination of transaction might result in two different error codes depending on where it was
163+
// terminated. These are really client errors but classification on the server is not entirely correct and
164+
// they are classified as transient.
165+
if ( "Neo.TransientError.Transaction.Terminated".equals( code ) ||
166+
"Neo.TransientError.Transaction.LockClientStopped".equals( code ) )
167+
{
168+
return false;
169+
}
170+
return true;
171+
}
172+
return false;
154173
}
155174
}

driver/src/test/java/org/neo4j/driver/internal/retry/ExponentialBackoffTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,32 @@ public void sleepsOnTransientException() throws Exception
253253
verify( clock ).sleep( 1 );
254254
}
255255

256+
@Test
257+
public void doesNothingWhenTransactionTerminatedError() throws Exception
258+
{
259+
Clock clock = mock( Clock.class );
260+
ExponentialBackoff backoff = newBackoff( 1, 1, 1, 0, clock );
261+
262+
TransientException exception = new TransientException( "Neo.TransientError.Transaction.Terminated", "" );
263+
ExponentialBackoffDecision decision = backoff.apply( exception, null );
264+
265+
assertFalse( decision.shouldRetry() );
266+
verify( clock, never() ).sleep( anyLong() );
267+
}
268+
269+
@Test
270+
public void doesNothingWhenTransactionLockClientStoppedError() throws Exception
271+
{
272+
Clock clock = mock( Clock.class );
273+
ExponentialBackoff backoff = newBackoff( 1, 1, 1, 0, clock );
274+
275+
TransientException exception = new TransientException( "Neo.TransientError.Transaction.LockClientStopped", "" );
276+
ExponentialBackoffDecision decision = backoff.apply( exception, null );
277+
278+
assertFalse( decision.shouldRetry() );
279+
verify( clock, never() ).sleep( anyLong() );
280+
}
281+
256282
@Test
257283
public void throwsWhenSleepInterrupted() throws Exception
258284
{

0 commit comments

Comments
 (0)