From 7fa806c708a9849de927497ca20dd620b90f5500 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 22 Jun 2021 13:53:47 +0100 Subject: [PATCH 1/2] Remove stacktrace from recoverable discovery log warnings This is to reduce the noise in the logs when recoverable discovery exceptions occur. Complete discovery failures are expected to be reported separately. --- .../driver/internal/cluster/RediscoveryImpl.java | 11 +++++++---- .../driver/internal/cluster/RediscoveryTest.java | 12 ++++++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java index 52c76d784c..3352240582 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java @@ -53,13 +53,15 @@ import static org.neo4j.driver.internal.util.Futures.failedFuture; /** - * This class is used by all router tables to perform discovery. - * In other words, the methods in this class could be called by multiple threads concurrently. + * This class is used by all router tables to perform discovery. In other words, the methods in this class could be called by multiple threads concurrently. */ public class RediscoveryImpl implements Rediscovery { private static final String NO_ROUTERS_AVAILABLE = "Could not perform discovery for database '%s'. No routing server available."; private static final String RECOVERABLE_ROUTING_ERROR = "Failed to update routing table with server '%s'."; + private static final String RECOVERABLE_DISCOVERY_ERROR_WITH_SERVER = "Received a recoverable discovery error with server '%s', " + + "will continue discovery with other routing servers if available. " + + "Complete routing failures will be reported separately from this warning."; private final BoltServerAddress initialRouter; private final RoutingSettings settings; @@ -291,8 +293,9 @@ private ClusterComposition handleRoutingProcedureError( Throwable error, Routing // Retriable error happened during discovery. DiscoveryException discoveryError = new DiscoveryException( format( RECOVERABLE_ROUTING_ERROR, routerAddress ), error ); Futures.combineErrors( baseError, discoveryError ); // we record each failure here - logger.warn( format( "Received a recoverable discovery error with server '%s', will continue discovery with other routing servers if available.", - routerAddress ), discoveryError ); + String warningMessage = format( RECOVERABLE_DISCOVERY_ERROR_WITH_SERVER, routerAddress ); + logger.warn( warningMessage ); + logger.debug( warningMessage, discoveryError ); routingTable.forget( routerAddress ); return null; } diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java index 536684547d..0987774f0b 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java @@ -57,7 +57,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.startsWith; @@ -187,9 +186,14 @@ void shouldFailImmediatelyWhenClusterCompositionProviderReturnsFailure() ClusterComposition composition = await( rediscovery.lookupClusterComposition( table, pool, empty() ) ).getClusterComposition(); assertEquals( validComposition, composition ); - ArgumentCaptor argument = ArgumentCaptor.forClass( DiscoveryException.class ); - verify( logger ).warn( anyString(), argument.capture() ); - assertThat( argument.getValue().getCause(), equalTo( protocolError ) ); + ArgumentCaptor warningMessageCaptor = ArgumentCaptor.forClass( String.class ); + ArgumentCaptor debugMessageCaptor = ArgumentCaptor.forClass( String.class ); + ArgumentCaptor debugThrowableCaptor = ArgumentCaptor.forClass( DiscoveryException.class ); + verify( logger ).warn( warningMessageCaptor.capture() ); + verify( logger ).debug( debugMessageCaptor.capture(), debugThrowableCaptor.capture() ); + assertNotNull( warningMessageCaptor.getValue() ); + assertEquals( warningMessageCaptor.getValue(), debugMessageCaptor.getValue() ); + assertThat( debugThrowableCaptor.getValue().getCause(), equalTo( protocolError ) ); } @Test From 239a58daf331ad00188def3fac98ff3ef83e69f8 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Sat, 26 Jun 2021 10:41:23 +0100 Subject: [PATCH 2/2] Update class comment on RediscoveryImpl --- .../neo4j/driver/internal/cluster/Rediscovery.java | 13 +++++++++++++ .../driver/internal/cluster/RediscoveryImpl.java | 3 --- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java index 5faea2186b..553efb7511 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java @@ -26,8 +26,21 @@ import org.neo4j.driver.internal.BoltServerAddress; import org.neo4j.driver.internal.spi.ConnectionPool; +/** + * Provides cluster composition lookup capabilities and initial router address resolution. + */ public interface Rediscovery { + /** + * Fetches cluster composition using the provided routing table. + *

+ * Implementation must be thread safe to be called with distinct routing tables concurrently. The routing table instance may be modified. + * + * @param routingTable the routing table for cluster composition lookup + * @param connectionPool the connection pool for connection acquisition + * @param bookmark the bookmark that is presented to the server + * @return cluster composition lookup result + */ CompletionStage lookupClusterComposition( RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark ); List resolve() throws UnknownHostException; diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java index 3352240582..fe033acf1a 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java @@ -52,9 +52,6 @@ import static org.neo4j.driver.internal.util.Futures.completedWithNull; import static org.neo4j.driver.internal.util.Futures.failedFuture; -/** - * This class is used by all router tables to perform discovery. In other words, the methods in this class could be called by multiple threads concurrently. - */ public class RediscoveryImpl implements Rediscovery { private static final String NO_ROUTERS_AVAILABLE = "Could not perform discovery for database '%s'. No routing server available.";