From 469d6baa934211242781cfcb23a07c9cb1dcc21a Mon Sep 17 00:00:00 2001 From: Francesco Nigro Date: Mon, 9 Apr 2018 16:26:37 +0200 Subject: [PATCH] ARTEMIS-1788 JDBC HA should use JDBC Network Timeout JdbcNodeManager is configured to use the same network timeout value of the journal and to validate all the timeout values related to a correct HA behaviour. --- .../store/drivers/AbstractJDBCDriver.java | 3 + .../server/impl/jdbc/JdbcNodeManager.java | 86 ++++++++++++++----- .../impl/jdbc/JdbcSharedStateManager.java | 23 +++++ docs/user-manual/en/persistence.md | 1 + 4 files changed, 90 insertions(+), 23 deletions(-) diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/AbstractJDBCDriver.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/AbstractJDBCDriver.java index b0be3aedfa..62c9501e14 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/AbstractJDBCDriver.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/AbstractJDBCDriver.java @@ -141,6 +141,9 @@ public abstract class AbstractJDBCDriver { throw e; } } + if (this.networkTimeoutMillis >= 0 && this.networkTimeoutExecutor == null) { + logger.warn("Unable to set a network timeout on the JDBC connection: networkTimeoutExecutor is null"); + } if (this.networkTimeoutMillis >= 0 && this.networkTimeoutExecutor != null) { try { connection.setNetworkTimeout(this.networkTimeoutExecutor, this.networkTimeoutMillis); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcNodeManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcNodeManager.java index 55f75dc4d9..fb76f0434d 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcNodeManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcNodeManager.java @@ -60,6 +60,7 @@ public final class JdbcNodeManager extends NodeManager { ScheduledExecutorService scheduledExecutorService, ExecutorFactory executorFactory, IOCriticalErrorListener ioCriticalErrorListener) { + validateTimeoutConfiguration(configuration); if (configuration.getDataSource() != null) { final SQLProvider.Factory sqlProviderFactory; if (configuration.getSqlProviderFactory() != null) { @@ -69,6 +70,7 @@ public final class JdbcNodeManager extends NodeManager { } final String brokerId = java.util.UUID.randomUUID().toString(); return usingDataSource(brokerId, + configuration.getJdbcNetworkTimeout(), configuration.getJdbcLockExpirationMillis(), configuration.getJdbcLockRenewPeriodMillis(), configuration.getJdbcLockAcquisitionTimeoutMillis(), @@ -82,6 +84,7 @@ public final class JdbcNodeManager extends NodeManager { final SQLProvider sqlProvider = JDBCUtils.getSQLProvider(configuration.getJdbcDriverClassName(), configuration.getNodeManagerStoreTableName(), SQLProvider.DatabaseStoreType.NODE_MANAGER); final String brokerId = java.util.UUID.randomUUID().toString(); return usingConnectionUrl(brokerId, + configuration.getJdbcNetworkTimeout(), configuration.getJdbcLockExpirationMillis(), configuration.getJdbcLockRenewPeriodMillis(), configuration.getJdbcLockAcquisitionTimeoutMillis(), @@ -95,18 +98,25 @@ public final class JdbcNodeManager extends NodeManager { } } - static JdbcNodeManager usingDataSource(String brokerId, - long lockExpirationMillis, - long lockRenewPeriodMillis, - long lockAcquisitionTimeoutMillis, - long maxAllowedMillisFromDbTime, - DataSource dataSource, - SQLProvider provider, - ScheduledExecutorService scheduledExecutorService, - ExecutorFactory executorFactory, - IOCriticalErrorListener ioCriticalErrorListener) { + private static JdbcNodeManager usingDataSource(String brokerId, + int networkTimeoutMillis, + long lockExpirationMillis, + long lockRenewPeriodMillis, + long lockAcquisitionTimeoutMillis, + long maxAllowedMillisFromDbTime, + DataSource dataSource, + SQLProvider provider, + ScheduledExecutorService scheduledExecutorService, + ExecutorFactory executorFactory, + IOCriticalErrorListener ioCriticalErrorListener) { return new JdbcNodeManager( - () -> JdbcSharedStateManager.usingDataSource(brokerId, lockExpirationMillis, maxAllowedMillisFromDbTime, dataSource, provider), + () -> JdbcSharedStateManager.usingDataSource(brokerId, + networkTimeoutMillis, + executorFactory == null ? null : executorFactory.getExecutor(), + lockExpirationMillis, + maxAllowedMillisFromDbTime, + dataSource, + provider), false, lockRenewPeriodMillis, lockAcquisitionTimeoutMillis, @@ -115,19 +125,27 @@ public final class JdbcNodeManager extends NodeManager { ioCriticalErrorListener); } - public static JdbcNodeManager usingConnectionUrl(String brokerId, - long lockExpirationMillis, - long lockRenewPeriodMillis, - long lockAcquisitionTimeoutMillis, - long maxAllowedMillisFromDbTime, - String jdbcUrl, - String driverClass, - SQLProvider provider, - ScheduledExecutorService scheduledExecutorService, - ExecutorFactory executorFactory, - IOCriticalErrorListener ioCriticalErrorListener) { + private static JdbcNodeManager usingConnectionUrl(String brokerId, + int networkTimeoutMillis, + long lockExpirationMillis, + long lockRenewPeriodMillis, + long lockAcquisitionTimeoutMillis, + long maxAllowedMillisFromDbTime, + String jdbcUrl, + String driverClass, + SQLProvider provider, + ScheduledExecutorService scheduledExecutorService, + ExecutorFactory executorFactory, + IOCriticalErrorListener ioCriticalErrorListener) { return new JdbcNodeManager( - () -> JdbcSharedStateManager.usingConnectionUrl(brokerId, lockExpirationMillis, maxAllowedMillisFromDbTime, jdbcUrl, driverClass, provider), + () -> JdbcSharedStateManager.usingConnectionUrl(brokerId, + networkTimeoutMillis, + executorFactory == null ? null : executorFactory.getExecutor(), + lockExpirationMillis, + maxAllowedMillisFromDbTime, + jdbcUrl, + driverClass, + provider), false, lockRenewPeriodMillis, lockAcquisitionTimeoutMillis, @@ -136,6 +154,28 @@ public final class JdbcNodeManager extends NodeManager { ioCriticalErrorListener); } + private static void validateTimeoutConfiguration(DatabaseStorageConfiguration configuration) { + final long lockExpiration = configuration.getJdbcLockExpirationMillis(); + if (lockExpiration <= 0) { + throw new IllegalArgumentException("jdbc-lock-expiration should be positive"); + } + final long lockRenewPeriod = configuration.getJdbcLockRenewPeriodMillis(); + if (lockRenewPeriod <= 0) { + throw new IllegalArgumentException("jdbc-lock-renew-period should be positive"); + } + if (lockRenewPeriod >= lockExpiration) { + throw new IllegalArgumentException("jdbc-lock-renew-period should be < jdbc-lock-expiration"); + } + final int networkTimeout = configuration.getJdbcNetworkTimeout(); + if (networkTimeout >= 0) { + if (networkTimeout > lockExpiration) { + logger.warn("jdbc-network-timeout isn't properly configured: the recommended value is <= jdbc-lock-expiration"); + } + } else { + logger.warn("jdbc-network-timeout isn't properly configured: the recommended value is <= jdbc-lock-expiration"); + } + } + private JdbcNodeManager(Supplier sharedStateManagerFactory, boolean replicatedBackup, long lockRenewPeriodMillis, diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcSharedStateManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcSharedStateManager.java index d14de7a411..a8b07e9309 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcSharedStateManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcSharedStateManager.java @@ -24,6 +24,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.sql.Timestamp; +import java.util.concurrent.Executor; import java.util.function.Supplier; import org.apache.activemq.artemis.jdbc.store.drivers.AbstractJDBCDriver; @@ -52,11 +53,14 @@ final class JdbcSharedStateManager extends AbstractJDBCDriver implements SharedS private long timeDifferenceMillisFromDb = 0; public static JdbcSharedStateManager usingDataSource(String holderId, + int networkTimeout, + Executor networkTimeoutExecutor, long locksExpirationMillis, long maxAllowedMillisFromDbTime, DataSource dataSource, SQLProvider provider) { final JdbcSharedStateManager sharedStateManager = new JdbcSharedStateManager(holderId, locksExpirationMillis, maxAllowedMillisFromDbTime); + sharedStateManager.setNetworkTimeout(networkTimeoutExecutor, networkTimeout); sharedStateManager.setDataSource(dataSource); sharedStateManager.setSqlProvider(provider); try { @@ -73,7 +77,26 @@ final class JdbcSharedStateManager extends AbstractJDBCDriver implements SharedS String jdbcConnectionUrl, String jdbcDriverClass, SQLProvider provider) { + return JdbcSharedStateManager.usingConnectionUrl(holderId, + -1, + null, + locksExpirationMillis, + maxAllowedMillisFromDbTime, + jdbcConnectionUrl, + jdbcDriverClass, + provider); + } + + public static JdbcSharedStateManager usingConnectionUrl(String holderId, + int networkTimeout, + Executor networkTimeoutExecutor, + long locksExpirationMillis, + long maxAllowedMillisFromDbTime, + String jdbcConnectionUrl, + String jdbcDriverClass, + SQLProvider provider) { final JdbcSharedStateManager sharedStateManager = new JdbcSharedStateManager(holderId, locksExpirationMillis, maxAllowedMillisFromDbTime); + sharedStateManager.setNetworkTimeout(networkTimeoutExecutor, networkTimeout); sharedStateManager.setJdbcConnectionUrl(jdbcConnectionUrl); sharedStateManager.setJdbcDriverClass(jdbcDriverClass); sharedStateManager.setSqlProvider(provider); diff --git a/docs/user-manual/en/persistence.md b/docs/user-manual/en/persistence.md index e852a61328..11396adae3 100644 --- a/docs/user-manual/en/persistence.md +++ b/docs/user-manual/en/persistence.md @@ -456,6 +456,7 @@ To configure Apache ActiveMQ Artemis to use a database for persisting messages a The JDBC network connection timeout in milliseconds. The default value is 20000 milliseconds (ie 20 seconds). + When using a shared store it is recommended to set it less then or equal to `jdbc-lock-expiration`. - `jdbc-lock-renew-period`