From 6926a8b59e1da71c99774398536d021695028532 Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Wed, 21 Aug 2024 15:20:44 -0700 Subject: [PATCH 01/10] First integration test passing --- .../jdbc/HikariPooledConnectionProvider.java | 2 +- .../failover/FailoverConnectionPlugin.java | 31 +++++-- ..._advanced_jdbc_wrapper_messages.properties | 1 + .../container/tests/HikariTests.java | 87 ++++++++++++++++++- 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java b/wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java index 0dde3e47e..21c03853c 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java +++ b/wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java @@ -156,7 +156,7 @@ public HikariPooledConnectionProvider( public boolean acceptsUrl( @NonNull String protocol, @NonNull HostSpec hostSpec, @NonNull Properties props) { final RdsUrlType urlType = rdsUtils.identifyRdsType(hostSpec.getHost()); - return RdsUrlType.RDS_INSTANCE.equals(urlType); + return RdsUrlType.RDS_INSTANCE.equals(urlType) || hostSpec.getHost().contains(".proxied"); } @Override diff --git a/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java b/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java index 2cd76f5ed..3ee464cce 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java +++ b/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java @@ -767,15 +767,34 @@ public Connection connect( final boolean isInitialConnection, final JdbcCallable connectFunc) throws SQLException { - return connectInternal(driverProtocol, hostSpec, props, isInitialConnection, connectFunc); + return connectInternal(driverProtocol, hostSpec, props, isInitialConnection, connectFunc, false); } private Connection connectInternal(String driverProtocol, HostSpec hostSpec, Properties props, - boolean isInitialConnection, JdbcCallable connectFunc) + boolean isInitialConnection, JdbcCallable connectFunc, boolean isForceConnect) throws SQLException { - final Connection conn = - this.staleDnsHelper.getVerifiedConnection(isInitialConnection, this.hostListProviderService, - driverProtocol, hostSpec, props, connectFunc); + + Connection conn = null; + try { + conn = + this.staleDnsHelper.getVerifiedConnection(isInitialConnection, this.hostListProviderService, + driverProtocol, hostSpec, props, connectFunc); + } catch (final SQLException e) { + try { + if (isForceConnect || !shouldExceptionTriggerConnectionSwitch(e)) { + throw e; + } + + failover(this.pluginService.getCurrentHostSpec()); + } catch (FailoverSuccessSQLException failoverSuccessException) { + conn = this.pluginService.getCurrentConnection(); + } + } + + if (conn == null) { + // This should be unreachable, the above logic will either get a connection successfully or throw an exception. + throw new SQLException(Messages.get("Failover.unableToConnect")); + } if (isInitialConnection) { this.pluginService.refreshHostList(conn); @@ -792,6 +811,6 @@ public Connection forceConnect( final boolean isInitialConnection, final JdbcCallable forceConnectFunc) throws SQLException { - return connectInternal(driverProtocol, hostSpec, props, isInitialConnection, forceConnectFunc); + return connectInternal(driverProtocol, hostSpec, props, isInitialConnection, forceConnectFunc, true); } } diff --git a/wrapper/src/main/resources/aws_advanced_jdbc_wrapper_messages.properties b/wrapper/src/main/resources/aws_advanced_jdbc_wrapper_messages.properties index 7e7da6cb2..13f5579a4 100644 --- a/wrapper/src/main/resources/aws_advanced_jdbc_wrapper_messages.properties +++ b/wrapper/src/main/resources/aws_advanced_jdbc_wrapper_messages.properties @@ -147,6 +147,7 @@ ExecutionTimeConnectionPlugin.executionTime=Executed {0} in {1} nanos. Failover.transactionResolutionUnknownError=Transaction resolution unknown. Please re-configure session state if required and try restarting the transaction. Failover.connectionChangedError=The active SQL connection has changed due to a connection failure. Please re-configure session state if required. Failover.parameterValue={0}={1} +Failover.unableToConnect=Unable to establish a SQL connection due to an unexpected error. Failover.unableToConnectToWriter=Unable to establish SQL connection to the writer instance. Failover.unableToConnectToReader=Unable to establish SQL connection to the reader instance. Failover.detectedException=Detected an exception while executing a command: {0} diff --git a/wrapper/src/test/java/integration/container/tests/HikariTests.java b/wrapper/src/test/java/integration/container/tests/HikariTests.java index 4ac033f45..4c56cf5f2 100644 --- a/wrapper/src/test/java/integration/container/tests/HikariTests.java +++ b/wrapper/src/test/java/integration/container/tests/HikariTests.java @@ -17,7 +17,9 @@ package integration.container.tests; import static integration.util.AuroraTestUtility.executeWithTimeout; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static software.amazon.jdbc.PropertyDefinition.PLUGINS; @@ -57,8 +59,11 @@ import org.junit.jupiter.api.TestMethodOrder; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; +import software.amazon.jdbc.ConnectionProviderManager; +import software.amazon.jdbc.HikariPooledConnectionProvider; import software.amazon.jdbc.PropertyDefinition; import software.amazon.jdbc.ds.AwsWrapperDataSource; +import software.amazon.jdbc.hostlistprovider.RdsHostListProvider; import software.amazon.jdbc.plugin.efm2.HostMonitoringConnectionPlugin; import software.amazon.jdbc.plugin.failover.FailoverConnectionPlugin; import software.amazon.jdbc.plugin.failover.FailoverFailedSQLException; @@ -173,7 +178,7 @@ public void testFailoverLostConnection() throws SQLException { FAILOVER_TIMEOUT_MS.set(customProps, Integer.toString(1)); PropertyDefinition.SOCKET_TIMEOUT.set(customProps, String.valueOf(TimeUnit.SECONDS.toMillis(1))); - try (final HikariDataSource dataSource = createDataSource(customProps)) { + try (final HikariDataSource dataSource = createHikariDataSource(customProps)) { try (Connection conn = dataSource.getConnection()) { assertTrue(conn.isValid(5)); @@ -224,7 +229,7 @@ public void testEFMFailover() throws SQLException { LOGGER.fine("Instance to fail over to: " + readerIdentifier); ProxyHelper.enableConnectivity(writerIdentifier); - try (final HikariDataSource dataSource = createDataSource(null)) { + try (final HikariDataSource dataSource = createHikariDataSource(null)) { // Get a valid connection, then make it fail over to a different instance try (Connection conn = dataSource.getConnection()) { @@ -252,7 +257,83 @@ public void testEFMFailover() throws SQLException { } } - private HikariDataSource createDataSource(final Properties customProps) { + /** + * After successfully opening and returning a connection to the Hikari pool, getConnection() is called during + * failover. + */ + @TestTemplate + @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) + @EnableOnNumOfInstances(min = 2) + public void testInternalPoolsFailoverDuringGetConnection() throws SQLException, InterruptedException { + final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility(); + final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo(); + final List instances = proxyInfo.getInstances(); + final TestInstanceInfo writer = instances.get(0); + final String writerId = writer.getInstanceId(); + + setupInternalConnectionPools(); + final AwsWrapperDataSource ds = createWrapperDataSource(writer, proxyInfo); + + // Open connection and then return it to the pool + Connection conn = ds.getConnection(); + assertEquals(writerId, auroraUtil.queryInstanceId(conn)); + conn.close(); + + ProxyHelper.disableConnectivity(writer.getInstanceId()); + // Hikari's 'com.zaxxer.hikari.aliveBypassWindowMs' property is set to 500ms by default. We need to wait this long + // to trigger Hikari's validation attempts when HikariDatasource#getConnection is called. These attempts will fail + // and trigger failover. + TimeUnit.MILLISECONDS.sleep(500); + + // Driver will fail over internally and return a connection to another node. + conn = ds.getConnection(); + // Assert that we connected to a different node. + assertNotEquals(writerId, auroraUtil.queryInstanceId(conn)); + + ConnectionProviderManager.releaseResources(); + } + + private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo instanceInfo, TestProxyDatabaseInfo proxyInfo) { + Properties targetDataSourceProps = new Properties(); + targetDataSourceProps.setProperty(PropertyDefinition.PLUGINS.name, "auroraConnectionTracker,failover,efm"); + targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-reader"); + targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_TOPOLOGY_REFRESH_RATE_MS.name, String.valueOf(TimeUnit.MINUTES.toMillis(5))); + targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_INSTANCE_HOST_PATTERN.name, "?." + proxyInfo.getInstanceEndpointSuffix()); + targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_ID.name, "HikariTestsCluster "); + + AwsWrapperDataSource ds = new AwsWrapperDataSource(); + ds.setTargetDataSourceProperties(targetDataSourceProps); + ds.setJdbcProtocol(DriverHelper.getDriverProtocol()); + ds.setTargetDataSourceClassName(DriverHelper.getDataSourceClassname()); + ds.setServerName(instanceInfo.getHost()); + ds.setServerPort(String.valueOf(instanceInfo.getPort())); + ds.setDatabase(proxyInfo.getDefaultDbName()); + ds.setUser(proxyInfo.getUsername()); + ds.setPassword(proxyInfo.getPassword()); + + return ds; + } + + private static void setupInternalConnectionPools() { + final HikariPooledConnectionProvider provider = + new HikariPooledConnectionProvider((hostSpec, props) -> { + HikariConfig config = new HikariConfig(); + config.setMaximumPoolSize(30); + config.setMinimumIdle(2); + config.setIdleTimeout(TimeUnit.MINUTES.toMillis(15)); + config.setInitializationFailTimeout(-1); + config.setConnectionTimeout(1500); + config.setKeepaliveTime(TimeUnit.MINUTES.toMillis(3)); + config.setValidationTimeout(1000); + config.setMaxLifetime(TimeUnit.DAYS.toMillis(1)); + config.setReadOnly(true); + config.setAutoCommit(true); + return config; + }); + ConnectionProviderManager.setConnectionProvider(provider); + } + + private HikariDataSource createHikariDataSource(final Properties customProps) { final HikariConfig config = getConfig(customProps); final HikariDataSource dataSource = new HikariDataSource(config); From 8de6aff4cc30512f32caf07ef01df1c6505d0da8 Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Thu, 22 Aug 2024 17:39:05 -0700 Subject: [PATCH 02/10] Two more integration tests passing --- .../container/tests/HikariTests.java | 128 +++++++++++++++--- 1 file changed, 107 insertions(+), 21 deletions(-) diff --git a/wrapper/src/test/java/integration/container/tests/HikariTests.java b/wrapper/src/test/java/integration/container/tests/HikariTests.java index 4c56cf5f2..68a888692 100644 --- a/wrapper/src/test/java/integration/container/tests/HikariTests.java +++ b/wrapper/src/test/java/integration/container/tests/HikariTests.java @@ -49,6 +49,7 @@ import java.sql.Connection; import java.sql.SQLException; import java.sql.SQLTransientConnectionException; +import java.time.LocalTime; import java.util.Enumeration; import java.util.List; import java.util.Properties; @@ -258,13 +259,55 @@ public void testEFMFailover() throws SQLException { } /** - * After successfully opening and returning a connection to the Hikari pool, getConnection() is called during - * failover. + * After successfully opening and returning a connection to the Hikari pool, writer failover is triggered when + * getConnection is called. */ @TestTemplate @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) @EnableOnNumOfInstances(min = 2) - public void testInternalPoolsFailoverDuringGetConnection() throws SQLException, InterruptedException { + public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation() throws SQLException, InterruptedException { + final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility(); + final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo(); + final List instances = proxyInfo.getInstances(); + final TestInstanceInfo reader = instances.get(1); + final String readerId = reader.getInstanceId(); + + setupInternalConnectionPools(); + try { + final Properties targetDataSourceProps = new Properties(); + targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-writer"); + final AwsWrapperDataSource ds = createWrapperDataSource(reader, proxyInfo, targetDataSourceProps); + + // Open connection and then return it to the pool + Connection conn = ds.getConnection(); + assertEquals(readerId, auroraUtil.queryInstanceId(conn)); + conn.close(); + + ProxyHelper.disableConnectivity(reader.getInstanceId()); + // Hikari's 'com.zaxxer.hikari.aliveBypassWindowMs' property is set to 500ms by default. We need to wait this long + // to trigger Hikari's validation attempts when HikariDatasource#getConnection is called. These attempts will fail + // and trigger failover. + TimeUnit.MILLISECONDS.sleep(500); + + // Driver will fail over internally and return a connection to another node. + System.out.println("Calling getConnection at: " + LocalTime.now()); + conn = ds.getConnection(); + System.out.println("getConnection returned at: " + LocalTime.now()); + // Assert that we connected to a different node. + assertNotEquals(readerId, auroraUtil.queryInstanceId(conn)); + } finally { + ConnectionProviderManager.releaseResources(); + } + } + + /** + * After successfully opening and returning a connection to the Hikari pool, reader failover is triggered when + * getConnection is called. + */ + @TestTemplate + @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) + @EnableOnNumOfInstances(min = 2) + public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation() throws SQLException, InterruptedException { final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility(); final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo(); final List instances = proxyInfo.getInstances(); @@ -272,31 +315,74 @@ public void testInternalPoolsFailoverDuringGetConnection() throws SQLException, final String writerId = writer.getInstanceId(); setupInternalConnectionPools(); - final AwsWrapperDataSource ds = createWrapperDataSource(writer, proxyInfo); + try { + final Properties targetDataSourceProps = new Properties(); + targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-reader"); + final AwsWrapperDataSource ds = createWrapperDataSource(writer, proxyInfo, targetDataSourceProps); + + // Open connection and then return it to the pool + Connection conn = ds.getConnection(); + assertEquals(writerId, auroraUtil.queryInstanceId(conn)); + conn.close(); + + ProxyHelper.disableConnectivity(writer.getInstanceId()); + // Hikari's 'com.zaxxer.hikari.aliveBypassWindowMs' property is set to 500ms by default. We need to wait this long + // to trigger Hikari's validation attempts when HikariDatasource#getConnection is called. These attempts will fail + // and trigger failover. + TimeUnit.MILLISECONDS.sleep(500); + + // Driver will fail over internally and return a connection to another node. + System.out.println("Calling getConnection at: " + LocalTime.now()); + conn = ds.getConnection(); + System.out.println("getConnection returned at: " + LocalTime.now()); + // Assert that we connected to a different node. + assertNotEquals(writerId, auroraUtil.queryInstanceId(conn)); + } finally { + ConnectionProviderManager.releaseResources(); + } + } + + /** + * After successfully opening and returning a connection to the Hikari pool, writer failover is triggered when + * getConnection is called. Since the cluster only has one instance and the instance stays down, failover fails. + */ + @TestTemplate + @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) + @EnableOnNumOfInstances(max = 1) + public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_singleInstance() throws SQLException, InterruptedException { + final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility(); + final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo(); + final List instances = proxyInfo.getInstances(); + final TestInstanceInfo writer = instances.get(0); + final String writerId = writer.getInstanceId(); - // Open connection and then return it to the pool - Connection conn = ds.getConnection(); - assertEquals(writerId, auroraUtil.queryInstanceId(conn)); - conn.close(); + setupInternalConnectionPools(); + try { + final Properties targetDataSourceProps = new Properties(); + targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-writer"); + targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_TIMEOUT_MS.name, "5000"); + final AwsWrapperDataSource ds = createWrapperDataSource(writer, proxyInfo, targetDataSourceProps); - ProxyHelper.disableConnectivity(writer.getInstanceId()); - // Hikari's 'com.zaxxer.hikari.aliveBypassWindowMs' property is set to 500ms by default. We need to wait this long - // to trigger Hikari's validation attempts when HikariDatasource#getConnection is called. These attempts will fail - // and trigger failover. - TimeUnit.MILLISECONDS.sleep(500); + // Open connection and then return it to the pool + Connection conn = ds.getConnection(); + assertEquals(writerId, auroraUtil.queryInstanceId(conn)); + conn.close(); - // Driver will fail over internally and return a connection to another node. - conn = ds.getConnection(); - // Assert that we connected to a different node. - assertNotEquals(writerId, auroraUtil.queryInstanceId(conn)); + ProxyHelper.disableAllConnectivity(); + // Hikari's 'com.zaxxer.hikari.aliveBypassWindowMs' property is set to 500ms by default. We need to wait this long + // to trigger Hikari's validation attempts when HikariDatasource#getConnection is called. These attempts will fail + // and trigger failover. + TimeUnit.MILLISECONDS.sleep(500); - ConnectionProviderManager.releaseResources(); + // Driver will attempt to fail over internally, but the node is still down, so it fails. + assertThrows(FailoverFailedSQLException.class, ds::getConnection); + } finally { + ConnectionProviderManager.releaseResources(); + } } - private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo instanceInfo, TestProxyDatabaseInfo proxyInfo) { - Properties targetDataSourceProps = new Properties(); + private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo instanceInfo, TestProxyDatabaseInfo proxyInfo, Properties targetDataSourceProps) { targetDataSourceProps.setProperty(PropertyDefinition.PLUGINS.name, "auroraConnectionTracker,failover,efm"); - targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-reader"); targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_TOPOLOGY_REFRESH_RATE_MS.name, String.valueOf(TimeUnit.MINUTES.toMillis(5))); targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_INSTANCE_HOST_PATTERN.name, "?." + proxyInfo.getInstanceEndpointSuffix()); targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_ID.name, "HikariTestsCluster "); From 47d579b2f64a185341a2004c4ea31f43d932471d Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Fri, 23 Aug 2024 11:42:05 -0700 Subject: [PATCH 03/10] Integration tests passing --- .../failover/FailoverConnectionPlugin.java | 17 ++++-- .../container/tests/HikariTests.java | 55 +++++++++++-------- 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java b/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java index 3ee464cce..49ce63013 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java +++ b/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java @@ -84,6 +84,7 @@ public class FailoverConnectionPlugin extends AbstractConnectionPlugin { private final PluginService pluginService; protected final Properties properties; protected boolean enableFailoverSetting; + protected boolean enableConnectFailover; protected int failoverTimeoutMsSetting; protected int failoverClusterTopologyRefreshRateMsSetting; protected int failoverWriterReconnectIntervalMsSetting; @@ -137,6 +138,13 @@ public class FailoverConnectionPlugin extends AbstractConnectionPlugin { "enableClusterAwareFailover", "true", "Enable/disable cluster-aware failover logic"); + public static final AwsWrapperProperty ENABLE_CONNECT_FAILOVER = + new AwsWrapperProperty( + "enableConnectFailover", "false", + "Enable/disable cluster-aware failover if the initial connection to the database fails. " + + " Note that this may result in a connection to a different instance in the cluster than was specified " + + "by the URL."); + public static final AwsWrapperProperty FAILOVER_MODE = new AwsWrapperProperty( "failoverMode", null, @@ -353,6 +361,7 @@ public boolean isFailoverEnabled() { private void initSettings() { this.enableFailoverSetting = ENABLE_CLUSTER_AWARE_FAILOVER.getBoolean(this.properties); + this.enableConnectFailover = ENABLE_CONNECT_FAILOVER.getBoolean(this.properties); this.failoverTimeoutMsSetting = FAILOVER_TIMEOUT_MS.getInteger(this.properties); this.failoverClusterTopologyRefreshRateMsSetting = FAILOVER_CLUSTER_TOPOLOGY_REFRESH_RATE_MS.getInteger(this.properties); @@ -780,11 +789,11 @@ private Connection connectInternal(String driverProtocol, HostSpec hostSpec, Pro this.staleDnsHelper.getVerifiedConnection(isInitialConnection, this.hostListProviderService, driverProtocol, hostSpec, props, connectFunc); } catch (final SQLException e) { - try { - if (isForceConnect || !shouldExceptionTriggerConnectionSwitch(e)) { - throw e; - } + if (!this.enableConnectFailover || isForceConnect || !shouldExceptionTriggerConnectionSwitch(e)) { + throw e; + } + try { failover(this.pluginService.getCurrentHostSpec()); } catch (FailoverSuccessSQLException failoverSuccessException) { conn = this.pluginService.getCurrentConnection(); diff --git a/wrapper/src/test/java/integration/container/tests/HikariTests.java b/wrapper/src/test/java/integration/container/tests/HikariTests.java index 68a888692..07610ade2 100644 --- a/wrapper/src/test/java/integration/container/tests/HikariTests.java +++ b/wrapper/src/test/java/integration/container/tests/HikariTests.java @@ -49,7 +49,7 @@ import java.sql.Connection; import java.sql.SQLException; import java.sql.SQLTransientConnectionException; -import java.time.LocalTime; +import java.util.ArrayList; import java.util.Enumeration; import java.util.List; import java.util.Properties; @@ -265,7 +265,8 @@ public void testEFMFailover() throws SQLException { @TestTemplate @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) @EnableOnNumOfInstances(min = 2) - public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation() throws SQLException, InterruptedException { + public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation() + throws SQLException, InterruptedException { final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility(); final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo(); final List instances = proxyInfo.getInstances(); @@ -286,13 +287,11 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation() th ProxyHelper.disableConnectivity(reader.getInstanceId()); // Hikari's 'com.zaxxer.hikari.aliveBypassWindowMs' property is set to 500ms by default. We need to wait this long // to trigger Hikari's validation attempts when HikariDatasource#getConnection is called. These attempts will fail - // and trigger failover. + // and Hikari will throw an exception, which should trigger failover. TimeUnit.MILLISECONDS.sleep(500); // Driver will fail over internally and return a connection to another node. - System.out.println("Calling getConnection at: " + LocalTime.now()); conn = ds.getConnection(); - System.out.println("getConnection returned at: " + LocalTime.now()); // Assert that we connected to a different node. assertNotEquals(readerId, auroraUtil.queryInstanceId(conn)); } finally { @@ -301,13 +300,14 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation() th } /** - * After successfully opening and returning a connection to the Hikari pool, reader failover is triggered when + * After successfully opening and returning connections to the Hikari pool, reader failover is triggered when * getConnection is called. */ @TestTemplate @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) @EnableOnNumOfInstances(min = 2) - public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation() throws SQLException, InterruptedException { + public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation() + throws SQLException, InterruptedException { final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility(); final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo(); final List instances = proxyInfo.getInstances(); @@ -320,23 +320,29 @@ public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation() th targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-reader"); final AwsWrapperDataSource ds = createWrapperDataSource(writer, proxyInfo, targetDataSourceProps); - // Open connection and then return it to the pool - Connection conn = ds.getConnection(); - assertEquals(writerId, auroraUtil.queryInstanceId(conn)); - conn.close(); + // Open some connections. + List connections = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + Connection conn = ds.getConnection(); + connections.add(conn); + } + + // Return the opened connections to the pool. + for (Connection conn : connections) { + conn.close(); + } ProxyHelper.disableConnectivity(writer.getInstanceId()); // Hikari's 'com.zaxxer.hikari.aliveBypassWindowMs' property is set to 500ms by default. We need to wait this long // to trigger Hikari's validation attempts when HikariDatasource#getConnection is called. These attempts will fail - // and trigger failover. + // and Hikari will throw an exception, which should trigger failover. TimeUnit.MILLISECONDS.sleep(500); // Driver will fail over internally and return a connection to another node. - System.out.println("Calling getConnection at: " + LocalTime.now()); - conn = ds.getConnection(); - System.out.println("getConnection returned at: " + LocalTime.now()); - // Assert that we connected to a different node. - assertNotEquals(writerId, auroraUtil.queryInstanceId(conn)); + try (Connection conn = ds.getConnection()) { + // Assert that we connected to a different node. + assertNotEquals(writerId, auroraUtil.queryInstanceId(conn)); + } } finally { ConnectionProviderManager.releaseResources(); } @@ -349,7 +355,8 @@ public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation() th @TestTemplate @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) @EnableOnNumOfInstances(max = 1) - public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_singleInstance() throws SQLException, InterruptedException { + public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_singleInstance() + throws SQLException, InterruptedException { final AuroraTestUtility auroraUtil = AuroraTestUtility.getUtility(); final TestProxyDatabaseInfo proxyInfo = TestEnvironment.getCurrent().getInfo().getProxyDatabaseInfo(); final List instances = proxyInfo.getInstances(); @@ -371,7 +378,7 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_sing ProxyHelper.disableAllConnectivity(); // Hikari's 'com.zaxxer.hikari.aliveBypassWindowMs' property is set to 500ms by default. We need to wait this long // to trigger Hikari's validation attempts when HikariDatasource#getConnection is called. These attempts will fail - // and trigger failover. + // and Hikari will throw an exception, which should trigger failover. TimeUnit.MILLISECONDS.sleep(500); // Driver will attempt to fail over internally, but the node is still down, so it fails. @@ -381,10 +388,14 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_sing } } - private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo instanceInfo, TestProxyDatabaseInfo proxyInfo, Properties targetDataSourceProps) { + private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo instanceInfo, + TestProxyDatabaseInfo proxyInfo, Properties targetDataSourceProps) { targetDataSourceProps.setProperty(PropertyDefinition.PLUGINS.name, "auroraConnectionTracker,failover,efm"); - targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_TOPOLOGY_REFRESH_RATE_MS.name, String.valueOf(TimeUnit.MINUTES.toMillis(5))); - targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_INSTANCE_HOST_PATTERN.name, "?." + proxyInfo.getInstanceEndpointSuffix()); + targetDataSourceProps.setProperty(FailoverConnectionPlugin.ENABLE_CONNECT_FAILOVER.name, "true"); + targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_TOPOLOGY_REFRESH_RATE_MS.name, + String.valueOf(TimeUnit.MINUTES.toMillis(5))); + targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_INSTANCE_HOST_PATTERN.name, + "?." + proxyInfo.getInstanceEndpointSuffix()); targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_ID.name, "HikariTestsCluster "); AwsWrapperDataSource ds = new AwsWrapperDataSource(); From 4e8cfd6c72a683cd826025df50434f7400d7600b Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Fri, 23 Aug 2024 13:11:01 -0700 Subject: [PATCH 04/10] Only run new internal connection pool integration tests against Aurora --- .../src/test/java/integration/container/tests/HikariTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/wrapper/src/test/java/integration/container/tests/HikariTests.java b/wrapper/src/test/java/integration/container/tests/HikariTests.java index 07610ade2..84280e2dc 100644 --- a/wrapper/src/test/java/integration/container/tests/HikariTests.java +++ b/wrapper/src/test/java/integration/container/tests/HikariTests.java @@ -263,6 +263,7 @@ public void testEFMFailover() throws SQLException { * getConnection is called. */ @TestTemplate + @EnableOnDatabaseEngineDeployment(DatabaseEngineDeployment.AURORA) @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) @EnableOnNumOfInstances(min = 2) public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation() @@ -304,6 +305,7 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation() * getConnection is called. */ @TestTemplate + @EnableOnDatabaseEngineDeployment(DatabaseEngineDeployment.AURORA) @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) @EnableOnNumOfInstances(min = 2) public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation() @@ -353,6 +355,7 @@ public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation() * getConnection is called. Since the cluster only has one instance and the instance stays down, failover fails. */ @TestTemplate + @EnableOnDatabaseEngineDeployment(DatabaseEngineDeployment.AURORA) @EnableOnTestFeature({TestEnvironmentFeatures.NETWORK_OUTAGES_ENABLED}) @EnableOnNumOfInstances(max = 1) public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_singleInstance() From d0adee70d9129fe03ec96ce74991358a8e1c0b25 Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Mon, 26 Aug 2024 10:39:24 -0700 Subject: [PATCH 05/10] Add AcceptsUrlFunc to HikariPooledConnectionProvider constructor --- .../software/amazon/jdbc/AcceptsUrlFunc.java | 43 ++++++++++++++ .../jdbc/HikariPooledConnectionProvider.java | 52 ++++++++++++++++- .../container/tests/HikariTests.java | 57 +++++++++++++------ 3 files changed, 133 insertions(+), 19 deletions(-) create mode 100644 wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java diff --git a/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java b/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java new file mode 100644 index 000000000..27510c57a --- /dev/null +++ b/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java @@ -0,0 +1,43 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package software.amazon.jdbc; + +import java.util.Properties; + +@FunctionalInterface +public interface AcceptsUrlFunc { + + /** + * A function that can optionally be passed to the {@link HikariPooledConnectionProvider} + * to define when an internal connection pool should be created for a connection with the given {@link HostSpec} and + * {@link Properties}. + * + *

By default, the HikariPooledConnectionProvider will create an internal connection pool if the passed in + * {@link HostSpec} specifies a URL with a standard RDS instance format. If you would like to change this default + * behaviour, you can pass in a function with this interface to define your own behaviour. If the connection provider + * has been set to a {@link HikariPooledConnectionProvider} via + * {@link ConnectionProviderManager#setConnectionProvider(ConnectionProvider)}, this function will be called when the + * connect pipeline is used. If the function returns true, an internal connection pool will be created for the + * connection to the host specified by the given {@link HostSpec}. + * + * @param hostSpec the host details for the requested connection + * @param props the properties specified for the original connection through + * @return the key that should be used for the given host and properties. An internal connection + * pool will be created for each unique key returned by this function. + */ + boolean acceptsUrl(HostSpec hostSpec, Properties props); +} diff --git a/wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java b/wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java index 21c03853c..8c2cd1e13 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java +++ b/wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java @@ -69,6 +69,7 @@ public class HikariPooledConnectionProvider implements PooledConnectionProvider, private static long poolExpirationCheckNanos = TimeUnit.MINUTES.toNanos(30); private final HikariPoolConfigurator poolConfigurator; private final HikariPoolMapping poolMapping; + private final AcceptsUrlFunc acceptsUrlFunc; private final LeastConnectionsHostSelector leastConnectionsHostSelector; /** @@ -112,6 +113,7 @@ public HikariPooledConnectionProvider( HikariPoolConfigurator hikariPoolConfigurator, HikariPoolMapping mapping) { this.poolConfigurator = hikariPoolConfigurator; this.poolMapping = mapping; + this.acceptsUrlFunc = null; this.leastConnectionsHostSelector = new LeastConnectionsHostSelector(databasePools); } @@ -147,16 +149,64 @@ public HikariPooledConnectionProvider( long poolCleanupNanos) { this.poolConfigurator = hikariPoolConfigurator; this.poolMapping = mapping; + this.acceptsUrlFunc = null; poolExpirationCheckNanos = poolExpirationNanos; databasePools.setCleanupIntervalNanos(poolCleanupNanos); this.leastConnectionsHostSelector = new LeastConnectionsHostSelector(databasePools); } + /** + * {@link HikariPooledConnectionProvider} constructor. This class can be passed to + * {@link ConnectionProviderManager#setConnectionProvider} to enable internal connection pools for + * each database instance in a cluster. By maintaining internal connection pools, the driver can + * improve performance by reusing old {@link Connection} objects. + * + * @param hikariPoolConfigurator a function that returns a {@link HikariConfig} with specific + * Hikari configurations. By default, the + * {@link HikariPooledConnectionProvider} will configure the + * jdbcUrl, exceptionOverrideClassName, username, and password. Any + * additional configuration should be defined by passing in this + * parameter. If no additional configuration is desired, pass in a + * {@link HikariPoolConfigurator} that returns an empty + * HikariConfig. + * @param mapping a function that returns a String key used for the internal + * connection pool keys. An internal connection pool will be + * generated for each unique key returned by this function. + * @param acceptsUrlFunc a function that defines when an internal connection pool should be created for a + * requested connection. An internal connection pool will be created when the connect + * pipeline is being executed and this function returns true. + * @param poolExpirationNanos the amount of time that a pool should sit in the cache before + * being marked as expired for cleanup, in nanoseconds. Expired + * pools can still be used and will not be closed unless there + * are no active connections. + * @param poolCleanupNanos the interval defining how often expired connection pools + * should be cleaned up, in nanoseconds. Note that expired pools + * will not be closed unless there are no active connections. + */ + public HikariPooledConnectionProvider( + HikariPoolConfigurator hikariPoolConfigurator, + HikariPoolMapping mapping, + AcceptsUrlFunc acceptsUrlFunc, + long poolExpirationNanos, + long poolCleanupNanos) { + this.poolConfigurator = hikariPoolConfigurator; + this.poolMapping = mapping; + this.acceptsUrlFunc = acceptsUrlFunc; + poolExpirationCheckNanos = poolExpirationNanos; + databasePools.setCleanupIntervalNanos(poolCleanupNanos); + this.leastConnectionsHostSelector = new LeastConnectionsHostSelector(databasePools); + } + + @Override public boolean acceptsUrl( @NonNull String protocol, @NonNull HostSpec hostSpec, @NonNull Properties props) { + if (this.acceptsUrlFunc != null) { + return this.acceptsUrlFunc.acceptsUrl(hostSpec, props); + } + final RdsUrlType urlType = rdsUtils.identifyRdsType(hostSpec.getHost()); - return RdsUrlType.RDS_INSTANCE.equals(urlType) || hostSpec.getHost().contains(".proxied"); + return RdsUrlType.RDS_INSTANCE.equals(urlType); } @Override diff --git a/wrapper/src/test/java/integration/container/tests/HikariTests.java b/wrapper/src/test/java/integration/container/tests/HikariTests.java index 84280e2dc..68c31985d 100644 --- a/wrapper/src/test/java/integration/container/tests/HikariTests.java +++ b/wrapper/src/test/java/integration/container/tests/HikariTests.java @@ -60,7 +60,9 @@ import org.junit.jupiter.api.TestMethodOrder; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; +import software.amazon.jdbc.AcceptsUrlFunc; import software.amazon.jdbc.ConnectionProviderManager; +import software.amazon.jdbc.HikariPoolConfigurator; import software.amazon.jdbc.HikariPooledConnectionProvider; import software.amazon.jdbc.PropertyDefinition; import software.amazon.jdbc.ds.AwsWrapperDataSource; @@ -274,7 +276,7 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation() final TestInstanceInfo reader = instances.get(1); final String readerId = reader.getInstanceId(); - setupInternalConnectionPools(); + setupInternalConnectionPools(getInstanceUrlSubstring(reader.getHost())); try { final Properties targetDataSourceProps = new Properties(); targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-writer"); @@ -316,7 +318,7 @@ public void testInternalPools_driverReaderFailoverOnGetConnectionInvocation() final TestInstanceInfo writer = instances.get(0); final String writerId = writer.getInstanceId(); - setupInternalConnectionPools(); + setupInternalConnectionPools(getInstanceUrlSubstring(writer.getHost())); try { final Properties targetDataSourceProps = new Properties(); targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-reader"); @@ -366,7 +368,7 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_sing final TestInstanceInfo writer = instances.get(0); final String writerId = writer.getInstanceId(); - setupInternalConnectionPools(); + setupInternalConnectionPools(getInstanceUrlSubstring(writer.getHost())); try { final Properties targetDataSourceProps = new Properties(); targetDataSourceProps.setProperty(FailoverConnectionPlugin.FAILOVER_MODE.name, "strict-writer"); @@ -391,6 +393,16 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_sing } } + /** + * Given an instance URL, extracts the substring of the URL that is common to all instance URLs. For example, given + * "instance-1.ABC.cluster-XYZ.us-west-2.rds.amazonaws.com.proxied", returns + * ".ABC.cluster-XYZ.us-west-2.rds.amazonaws.com.proxied" + */ + private static String getInstanceUrlSubstring(String instanceUrl) { + int substringStart = instanceUrl.indexOf("."); + return instanceUrl.substring(substringStart); + } + private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo instanceInfo, TestProxyDatabaseInfo proxyInfo, Properties targetDataSourceProps) { targetDataSourceProps.setProperty(PropertyDefinition.PLUGINS.name, "auroraConnectionTracker,failover,efm"); @@ -414,22 +426,31 @@ private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo ins return ds; } - private static void setupInternalConnectionPools() { + private static void setupInternalConnectionPools(String instanceUrlSubstring) { + HikariPoolConfigurator hikariConfigurator = (hostSpec, props) -> { + HikariConfig config = new HikariConfig(); + config.setMaximumPoolSize(30); + config.setMinimumIdle(2); + config.setIdleTimeout(TimeUnit.MINUTES.toMillis(15)); + config.setInitializationFailTimeout(-1); + config.setConnectionTimeout(1500); + config.setKeepaliveTime(TimeUnit.MINUTES.toMillis(3)); + config.setValidationTimeout(1000); + config.setMaxLifetime(TimeUnit.DAYS.toMillis(1)); + config.setReadOnly(true); + config.setAutoCommit(true); + return config; + }; + + AcceptsUrlFunc acceptsUrlFunc = (hostSpec, props) -> hostSpec.getHost().contains(instanceUrlSubstring); + final HikariPooledConnectionProvider provider = - new HikariPooledConnectionProvider((hostSpec, props) -> { - HikariConfig config = new HikariConfig(); - config.setMaximumPoolSize(30); - config.setMinimumIdle(2); - config.setIdleTimeout(TimeUnit.MINUTES.toMillis(15)); - config.setInitializationFailTimeout(-1); - config.setConnectionTimeout(1500); - config.setKeepaliveTime(TimeUnit.MINUTES.toMillis(3)); - config.setValidationTimeout(1000); - config.setMaxLifetime(TimeUnit.DAYS.toMillis(1)); - config.setReadOnly(true); - config.setAutoCommit(true); - return config; - }); + new HikariPooledConnectionProvider( + hikariConfigurator, + null, + acceptsUrlFunc, + TimeUnit.MINUTES.toNanos(30), + TimeUnit.MINUTES.toNanos(10)); ConnectionProviderManager.setConnectionProvider(provider); } From faa4cdcbd601f4cc07aed1bdbe03712a766f9375 Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Mon, 26 Aug 2024 14:55:18 -0700 Subject: [PATCH 06/10] Update CHANGELOG and documentation with new connection property --- CHANGELOG.md | 3 +++ .../using-plugins/UsingTheFailoverPlugin.md | 1 + .../jdbc/plugin/failover/FailoverConnectionPlugin.java | 6 +++--- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b97d3b0e4..5cc432412 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/#semantic-versioning-200). +### :magic_wand: Added +- Logic and a connection property to enable driver failover when network exceptions occur in the connect pipeline (PR #1099)[https://github.com/aws/aws-advanced-jdbc-wrapper/pull/1099] + ## [2.3.9] - 2024-08-09 ### :bug: Fixed diff --git a/docs/using-the-jdbc-driver/using-plugins/UsingTheFailoverPlugin.md b/docs/using-the-jdbc-driver/using-plugins/UsingTheFailoverPlugin.md index 6de99f258..aa925fcb1 100644 --- a/docs/using-the-jdbc-driver/using-plugins/UsingTheFailoverPlugin.md +++ b/docs/using-the-jdbc-driver/using-plugins/UsingTheFailoverPlugin.md @@ -30,6 +30,7 @@ In addition to the parameters that you can configure for the underlying driver, | `failoverReaderConnectTimeoutMs` | Integer | No | Maximum allowed time in milliseconds to attempt to connect to a reader instance during a reader failover process. | `30000` | | `failoverTimeoutMs` | Integer | No | Maximum allowed time in milliseconds to attempt reconnecting to a new writer or reader instance after a cluster failover is initiated. | `300000` | | `failoverWriterReconnectIntervalMs` | Integer | No | Interval of time in milliseconds to wait between attempts to reconnect to a failed writer during a writer failover process. | `2000` | +| `enableConnectFailover` | Boolean | No | Enables/disables cluster-aware failover if the initial connection to the database fails due to a network exception. Note that this may result in a connection to a different instance in the cluster than was specified by the URL. | `false` | | ~~`keepSessionStateOnFailover`~~ | Boolean | No | This parameter is no longer available. If specified, it will be ignored by the driver. See [Session State](../SessionState.md) for more details. | `false` | | ~~`enableFailoverStrictReader`~~ | Boolean | No | This parameter is no longer available and, if specified, it will be ignored by the driver. See `failoverMode` (`reader-or-writer` or `strict-reader`) for more details. | | diff --git a/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java b/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java index 49ce63013..15fb51ca8 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java +++ b/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java @@ -141,9 +141,9 @@ public class FailoverConnectionPlugin extends AbstractConnectionPlugin { public static final AwsWrapperProperty ENABLE_CONNECT_FAILOVER = new AwsWrapperProperty( "enableConnectFailover", "false", - "Enable/disable cluster-aware failover if the initial connection to the database fails. " - + " Note that this may result in a connection to a different instance in the cluster than was specified " - + "by the URL."); + "Enable/disable cluster-aware failover if the initial connection to the database fails due to a " + + "network exception. Note that this may result in a connection to a different instance in the cluster " + + "than was specified by the URL."); public static final AwsWrapperProperty FAILOVER_MODE = new AwsWrapperProperty( From c535dfe3ac59365be31b5a7c9f5aba95e8a94ead Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Mon, 26 Aug 2024 15:23:30 -0700 Subject: [PATCH 07/10] Fix documentation --- .../src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java b/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java index 27510c57a..0233efc98 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java +++ b/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java @@ -35,9 +35,9 @@ public interface AcceptsUrlFunc { * connection to the host specified by the given {@link HostSpec}. * * @param hostSpec the host details for the requested connection - * @param props the properties specified for the original connection through - * @return the key that should be used for the given host and properties. An internal connection - * pool will be created for each unique key returned by this function. + * @param props the properties specified for the requested connection + * @return a boolean indicating whether an internal connection pool should be created for a connection specified by + * the given {@link HostSpec} and {@link Properties}. */ boolean acceptsUrl(HostSpec hostSpec, Properties props); } From a8a83abd20ef0909ebeb94d1f4d427d861d63771 Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Wed, 28 Aug 2024 09:54:43 -0700 Subject: [PATCH 08/10] Check for Hikari MariaDB network exception in MySQLExceptionHandler --- .../exceptions/MySQLExceptionHandler.java | 21 ++++++++++++++++++- .../container/tests/HikariTests.java | 7 ++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java b/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java index 65c9701e6..5308ded07 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java +++ b/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java @@ -21,6 +21,9 @@ public class MySQLExceptionHandler implements ExceptionHandler { public static final String SQLSTATE_ACCESS_ERROR = "28000"; + public static final String SYNTAX_ERROR_OR_ACCESS_VIOLATION = "42000"; + public static final String SET_NETWORK_TIMEOUT_ON_CLOSED_CONNECTION = + "setNetworkTimeout cannot be called on a closed connection"; @Override public boolean isNetworkException(final Throwable throwable) { @@ -28,7 +31,18 @@ public boolean isNetworkException(final Throwable throwable) { while (exception != null) { if (exception instanceof SQLException) { - return isNetworkException(((SQLException) exception).getSQLState()); + SQLException sqlException = (SQLException) exception; + + // Hikari throws a network exception with SQL state 42000 if: + // - the MariaDB driver is being used (the underlying driver determines the SQL state of the Hikari exception) + // - HikariDataSource#getConnection is called and the cached connection is broken due to server failover. + // + // The check for the Hikari MariaDB exception is added here because the exception handler is determined by the + // database dialect. Consequently, this exception handler is used when using the MariaDB driver against a MySQL + // database engine. + if (isNetworkException(sqlException.getSQLState()) || isHikariMariaDbNetworkException(sqlException)) { + return true; + } } else if (exception instanceof CJException) { return isNetworkException(((CJException) exception).getSQLState()); } @@ -39,6 +53,11 @@ public boolean isNetworkException(final Throwable throwable) { return false; } + private boolean isHikariMariaDbNetworkException(final SQLException sqlException) { + return sqlException.getSQLState().equals(SYNTAX_ERROR_OR_ACCESS_VIOLATION) + && sqlException.getMessage().contains(SET_NETWORK_TIMEOUT_ON_CLOSED_CONNECTION); + } + @Override public boolean isNetworkException(final String sqlState) { if (sqlState == null) { diff --git a/wrapper/src/test/java/integration/container/tests/HikariTests.java b/wrapper/src/test/java/integration/container/tests/HikariTests.java index 68c31985d..9bd4740d3 100644 --- a/wrapper/src/test/java/integration/container/tests/HikariTests.java +++ b/wrapper/src/test/java/integration/container/tests/HikariTests.java @@ -411,7 +411,12 @@ private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo ins String.valueOf(TimeUnit.MINUTES.toMillis(5))); targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_INSTANCE_HOST_PATTERN.name, "?." + proxyInfo.getInstanceEndpointSuffix()); - targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_ID.name, "HikariTestsCluster "); + targetDataSourceProps.setProperty(RdsHostListProvider.CLUSTER_ID.name, "HikariTestsCluster"); + + if (TestEnvironment.getCurrent().getCurrentDriver() == TestDriver.MARIADB + && TestEnvironment.getCurrent().getInfo().getRequest().getDatabaseEngine() == DatabaseEngine.MYSQL) { + targetDataSourceProps.setProperty("permitMysqlScheme", "1"); + } AwsWrapperDataSource ds = new AwsWrapperDataSource(); ds.setTargetDataSourceProperties(targetDataSourceProps); From fb2f910a7321a8ed493aeda925e6ef5558e77a4d Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Wed, 28 Aug 2024 10:01:39 -0700 Subject: [PATCH 09/10] Fix checkstyle errors --- .../amazon/jdbc/exceptions/MySQLExceptionHandler.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java b/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java index 5308ded07..b19a53a94 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java +++ b/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java @@ -53,11 +53,6 @@ public boolean isNetworkException(final Throwable throwable) { return false; } - private boolean isHikariMariaDbNetworkException(final SQLException sqlException) { - return sqlException.getSQLState().equals(SYNTAX_ERROR_OR_ACCESS_VIOLATION) - && sqlException.getMessage().contains(SET_NETWORK_TIMEOUT_ON_CLOSED_CONNECTION); - } - @Override public boolean isNetworkException(final String sqlState) { if (sqlState == null) { @@ -95,4 +90,9 @@ public boolean isLoginException(final String sqlState) { return SQLSTATE_ACCESS_ERROR.equals(sqlState); } + + private boolean isHikariMariaDbNetworkException(final SQLException sqlException) { + return sqlException.getSQLState().equals(SYNTAX_ERROR_OR_ACCESS_VIOLATION) + && sqlException.getMessage().contains(SET_NETWORK_TIMEOUT_ON_CLOSED_CONNECTION); + } } From 8d8db7307e6e7a912d832f33eafbde6fdc50db67 Mon Sep 17 00:00:00 2001 From: aaron-congo Date: Wed, 28 Aug 2024 13:36:27 -0700 Subject: [PATCH 10/10] Remove unnecessary static keywords, clean up docs and variable names --- .../software/amazon/jdbc/AcceptsUrlFunc.java | 20 ++++++------------- .../exceptions/MySQLExceptionHandler.java | 11 +++++----- .../container/tests/HikariTests.java | 6 +++--- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java b/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java index 0233efc98..65f84c513 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java +++ b/wrapper/src/main/java/software/amazon/jdbc/AcceptsUrlFunc.java @@ -22,22 +22,14 @@ public interface AcceptsUrlFunc { /** - * A function that can optionally be passed to the {@link HikariPooledConnectionProvider} - * to define when an internal connection pool should be created for a connection with the given {@link HostSpec} and - * {@link Properties}. - * - *

By default, the HikariPooledConnectionProvider will create an internal connection pool if the passed in - * {@link HostSpec} specifies a URL with a standard RDS instance format. If you would like to change this default - * behaviour, you can pass in a function with this interface to define your own behaviour. If the connection provider - * has been set to a {@link HikariPooledConnectionProvider} via - * {@link ConnectionProviderManager#setConnectionProvider(ConnectionProvider)}, this function will be called when the - * connect pipeline is used. If the function returns true, an internal connection pool will be created for the - * connection to the host specified by the given {@link HostSpec}. + * This function can be passed to a {@link ConnectionProvider} constructor to specify when the + * {@link ConnectionProvider} should be used to open a connection to the given {@link HostSpec} with the + * given {@link Properties}. * * @param hostSpec the host details for the requested connection - * @param props the properties specified for the requested connection - * @return a boolean indicating whether an internal connection pool should be created for a connection specified by - * the given {@link HostSpec} and {@link Properties}. + * @param props the properties for the requested connection + * @return a boolean indicating whether a {@link ConnectionProvider} should be used to open a connection to the given + * {@link HostSpec} with the given {@link Properties}. */ boolean acceptsUrl(HostSpec hostSpec, Properties props); } diff --git a/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java b/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java index b19a53a94..c6d7aeee1 100644 --- a/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java +++ b/wrapper/src/main/java/software/amazon/jdbc/exceptions/MySQLExceptionHandler.java @@ -21,7 +21,7 @@ public class MySQLExceptionHandler implements ExceptionHandler { public static final String SQLSTATE_ACCESS_ERROR = "28000"; - public static final String SYNTAX_ERROR_OR_ACCESS_VIOLATION = "42000"; + public static final String SQLSTATE_SYNTAX_ERROR_OR_ACCESS_VIOLATION = "42000"; public static final String SET_NETWORK_TIMEOUT_ON_CLOSED_CONNECTION = "setNetworkTimeout cannot be called on a closed connection"; @@ -33,9 +33,10 @@ public boolean isNetworkException(final Throwable throwable) { if (exception instanceof SQLException) { SQLException sqlException = (SQLException) exception; - // Hikari throws a network exception with SQL state 42000 if: - // - the MariaDB driver is being used (the underlying driver determines the SQL state of the Hikari exception) - // - HikariDataSource#getConnection is called and the cached connection is broken due to server failover. + // Hikari throws a network exception with SQL state 42000 if all the following points are true: + // - HikariDataSource#getConnection is called and the cached connection that was grabbed is broken due to server + // failover. + // - the MariaDB driver is being used (the underlying driver determines the SQL state of the Hikari exception). // // The check for the Hikari MariaDB exception is added here because the exception handler is determined by the // database dialect. Consequently, this exception handler is used when using the MariaDB driver against a MySQL @@ -92,7 +93,7 @@ public boolean isLoginException(final String sqlState) { } private boolean isHikariMariaDbNetworkException(final SQLException sqlException) { - return sqlException.getSQLState().equals(SYNTAX_ERROR_OR_ACCESS_VIOLATION) + return sqlException.getSQLState().equals(SQLSTATE_SYNTAX_ERROR_OR_ACCESS_VIOLATION) && sqlException.getMessage().contains(SET_NETWORK_TIMEOUT_ON_CLOSED_CONNECTION); } } diff --git a/wrapper/src/test/java/integration/container/tests/HikariTests.java b/wrapper/src/test/java/integration/container/tests/HikariTests.java index 9bd4740d3..e1290c44e 100644 --- a/wrapper/src/test/java/integration/container/tests/HikariTests.java +++ b/wrapper/src/test/java/integration/container/tests/HikariTests.java @@ -398,12 +398,12 @@ public void testInternalPools_driverWriterFailoverOnGetConnectionInvocation_sing * "instance-1.ABC.cluster-XYZ.us-west-2.rds.amazonaws.com.proxied", returns * ".ABC.cluster-XYZ.us-west-2.rds.amazonaws.com.proxied" */ - private static String getInstanceUrlSubstring(String instanceUrl) { + private String getInstanceUrlSubstring(String instanceUrl) { int substringStart = instanceUrl.indexOf("."); return instanceUrl.substring(substringStart); } - private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo instanceInfo, + private AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo instanceInfo, TestProxyDatabaseInfo proxyInfo, Properties targetDataSourceProps) { targetDataSourceProps.setProperty(PropertyDefinition.PLUGINS.name, "auroraConnectionTracker,failover,efm"); targetDataSourceProps.setProperty(FailoverConnectionPlugin.ENABLE_CONNECT_FAILOVER.name, "true"); @@ -431,7 +431,7 @@ private static AwsWrapperDataSource createWrapperDataSource(TestInstanceInfo ins return ds; } - private static void setupInternalConnectionPools(String instanceUrlSubstring) { + private void setupInternalConnectionPools(String instanceUrlSubstring) { HikariPoolConfigurator hikariConfigurator = (hostSpec, props) -> { HikariConfig config = new HikariConfig(); config.setMaximumPoolSize(30);