Skip to content

Commit 44fc1fd

Browse files
authored
Fix reactive transaction function retry logic to retry on relevant resource cleanup failures (#1012)
* Fix reactive transaction function retry logic to retry on relevant resource cleanup failures (#1006) This update fixes reactive transaction function retry logic and makes it retry when retryable errors occur during resource cleanup. * Fixed mock logger in retry test
1 parent c02621f commit 44fc1fd

File tree

3 files changed

+29
-3
lines changed

3 files changed

+29
-3
lines changed

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

+6
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.neo4j.driver.Logging;
4141
import org.neo4j.driver.exceptions.AuthorizationExpiredException;
4242
import org.neo4j.driver.exceptions.ClientException;
43+
import org.neo4j.driver.exceptions.Neo4jException;
4344
import org.neo4j.driver.exceptions.ServiceUnavailableException;
4445
import org.neo4j.driver.exceptions.SessionExpiredException;
4546
import org.neo4j.driver.exceptions.TransientException;
@@ -182,6 +183,11 @@ private Retry exponentialBackoffRetryRx()
182183
contextView ->
183184
{
184185
Throwable throwable = retrySignal.failure();
186+
// Extract nested Neo4jException from not Neo4jException. Reactor usingWhen returns RuntimeException on async resource cleanup failure.
187+
if ( throwable != null && !(throwable instanceof Neo4jException) && throwable.getCause() instanceof Neo4jException )
188+
{
189+
throwable = throwable.getCause();
190+
}
185191
Throwable error = extractPossibleTerminationCause( throwable );
186192

187193
List<Throwable> errors = contextView.getOrDefault( "errors", null );

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

+23
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,29 @@ void doesRetryOnAuthorizationExpiredExceptionRx()
10081008
assertEquals( "Done", result );
10091009
}
10101010

1011+
@Test
1012+
void doesRetryOnAsyncResourceCleanupRuntimeExceptionRx()
1013+
{
1014+
Clock clock = mock( Clock.class );
1015+
Logging logging = mock( Logging.class );
1016+
Logger logger = mock( Logger.class );
1017+
when( logging.getLog( anyString() ) ).thenReturn( logger );
1018+
ExponentialBackoffRetryLogic logic = new ExponentialBackoffRetryLogic( RetrySettings.DEFAULT, eventExecutor, clock, logging );
1019+
1020+
AtomicBoolean exceptionThrown = new AtomicBoolean( false );
1021+
String result = await( Mono.from( logic.retryRx( Mono.fromSupplier( () ->
1022+
{
1023+
if ( exceptionThrown.compareAndSet( false, true ) )
1024+
{
1025+
throw new RuntimeException( "Async resource cleanup failed after",
1026+
authorizationExpiredException() );
1027+
}
1028+
return "Done";
1029+
} ) ) ) );
1030+
1031+
assertEquals( "Done", result );
1032+
}
1033+
10111034
@Test
10121035
void doesNotRetryOnRandomClientExceptionRx()
10131036
{

testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/StartTest.java

-3
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@ public class StartTest implements TestkitRequest
6666
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.TestSessionRun\\.test_discard_on_session_close_unfinished_result$",
6767
"Does not support partially consumed state" );
6868
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.NoRouting[^.]+\\.test_should_error_on_database_shutdown_using_tx_run$", "Session close throws error" );
69-
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.Routing[^.]+\\.test_should_retry_write_until_success_with_leader_shutdown_during_tx_using_tx_function$",
70-
"Commit failure leaks outside function" );
7169
REACTIVE_SKIP_PATTERN_TO_REASON.put(
7270
"^.*\\.Routing[^.]+\\.test_should_fail_when_reading_from_unexpectedly_interrupting_readers_on_run_using_tx_function$",
7371
"Rollback failures following commit failure" );
@@ -85,7 +83,6 @@ public class StartTest implements TestkitRequest
8583
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.TestDirectConnectionRecvTimeout\\..*$", skipMessage );
8684
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.TestRoutingConnectionRecvTimeout\\..*$", skipMessage );
8785
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.Routing[^.]+\\.test_should_successfully_acquire_rt_when_router_ip_changes$", skipMessage );
88-
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.Routing[^.]+\\.test_should_revert_to_initial_router_if_known_router_throws_protocol_errors$", skipMessage );
8986
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.TestRoutingConnectionRecvTimeout\\.test_timeout$", skipMessage );
9087
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.TestRoutingConnectionRecvTimeout\\.test_timeout_unmanaged_tx$", skipMessage );
9188
REACTIVE_SKIP_PATTERN_TO_REASON.put( "^.*\\.TestDisconnects\\.test_disconnect_session_on_tx_commit$", skipMessage );

0 commit comments

Comments
 (0)