From 769101ac69a5b27d370fd2a65f1d3f9c07e24244 Mon Sep 17 00:00:00 2001 From: franz1981 Date: Mon, 19 Oct 2020 18:09:52 +0200 Subject: [PATCH] ARTEMIS-2955 commons-dbcp2 performance issue with Derby Embedded DBMS This commit is fixing: - a missing commit that can make leak a connection - restricting default specific commons-dbcp2 to the default data source - setting poolPreparedStatements true by default - configured embedded Derby to be in-memory to speedup tests --- .../jdbc/store/drivers/AbstractJDBCDriver.java | 1 + .../store/drivers/JDBCConnectionProvider.java | 18 ++++++++++++++++-- .../storage/DatabaseStorageConfiguration.java | 12 ++++++++++-- .../artemis/tests/util/ActiveMQTestBase.java | 10 +++++++++- 4 files changed, 36 insertions(+), 5 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 ab89fc43db..77f9ee5410 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 @@ -132,6 +132,7 @@ public abstract class AbstractJDBCDriver { logger.tracef("Table %s did exist but is empty. Starting initialization.", tableName); } else { logger.tracef("Table %s did exist but is empty. Initialization completed: no initialization statements left.", tableName); + connection.commit(); } } } catch (SQLException e) { diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/JDBCConnectionProvider.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/JDBCConnectionProvider.java index 72e971adc2..9f972d39d8 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/JDBCConnectionProvider.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/JDBCConnectionProvider.java @@ -24,6 +24,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.activemq.artemis.jdbc.store.logging.LoggingConnection; +import org.apache.activemq.artemis.jdbc.store.sql.PropertySQLProvider; import org.apache.activemq.artemis.journal.ActiveMQJournalLogger; import org.jboss.logging.Logger; @@ -34,19 +35,31 @@ public class JDBCConnectionProvider { private Executor networkTimeoutExecutor; private int networkTimeoutMillis; private boolean supportNetworkTimeout; + private final String user; + private final String password; public JDBCConnectionProvider(DataSource dataSource) { + this(dataSource, null, null); + } + + public JDBCConnectionProvider(DataSource dataSource, String user, String password) { this.dataSource = dataSource; this.networkTimeoutExecutor = null; this.networkTimeoutMillis = -1; this.supportNetworkTimeout = true; + this.user = user; + this.password = password; addDerbyShutdownHook(); } public synchronized Connection getConnection() throws SQLException { Connection connection; try { - connection = dataSource.getConnection(); + if (user != null || password != null) { + connection = dataSource.getConnection(user, password); + } else { + connection = dataSource.getConnection(); + } if (logger.isTraceEnabled() && !(connection instanceof LoggingConnection)) { connection = new LoggingConnection(connection, logger); } @@ -92,7 +105,8 @@ public class JDBCConnectionProvider { public void addDerbyShutdownHook() { // Shutdown the derby if using the derby embedded driver. try (Connection connection = getConnection()) { - if (connection.getMetaData().getDriverName().equals("org.apache.derby.jdbc.EmbeddedDriver")) { + PropertySQLProvider.Factory.SQLDialect sqlDialect = PropertySQLProvider.Factory.investigateDialect(connection); + if (sqlDialect == PropertySQLProvider.Factory.SQLDialect.DERBY) { if (shutAdded.compareAndSet(false, true)) { Runtime.getRuntime().addShutdownHook(new ShutdownDerby()); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/storage/DatabaseStorageConfiguration.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/storage/DatabaseStorageConfiguration.java index 76d8e4e21b..8ea933e7ce 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/storage/DatabaseStorageConfiguration.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/storage/DatabaseStorageConfiguration.java @@ -151,7 +151,8 @@ public class DatabaseStorageConfiguration implements StoreConfiguration { */ private DataSource getDataSource() { if (dataSource == null) { - if (dataSourceProperties.isEmpty()) { + // the next settings are going to be applied only if the datasource is the default one + if (dataSourceProperties.isEmpty() && ActiveMQDefaultConfiguration.getDefaultDataSourceClassName().equals(dataSourceClassName)) { addDataSourceProperty("driverClassName", jdbcDriverClassName); addDataSourceProperty("url", jdbcConnectionUrl); if (jdbcUser != null) { @@ -162,6 +163,8 @@ public class DatabaseStorageConfiguration implements StoreConfiguration { } // Let the pool to have unbounded number of connections by default to prevent connection starvation addDataSourceProperty("maxTotal", "-1"); + // Let the pool to have unbounded number of cached prepared statements to save the initialization cost + addDataSourceProperty("poolPreparedStatements", "true"); } dataSource = JDBCDataSourceUtils.getDataSource(dataSourceClassName, dataSourceProperties); } @@ -179,7 +182,12 @@ public class DatabaseStorageConfiguration implements StoreConfiguration { public JDBCConnectionProvider getConnectionProvider() { if (connectionProvider == null) { - connectionProvider = new JDBCConnectionProvider(getDataSource()); + // commons-dbcp2 doesn't support DataSource::getConnection(user, password) + if (dataSourceClassName == ActiveMQDefaultConfiguration.getDefaultDataSourceClassName()) { + connectionProvider = new JDBCConnectionProvider(getDataSource()); + } else { + connectionProvider = new JDBCConnectionProvider(getDataSource(), getJdbcUser(), getJdbcPassword()); + } } return connectionProvider; } diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java b/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java index b538f8a8b0..be6d2429bc 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java @@ -257,6 +257,10 @@ public abstract class ActiveMQTestBase extends Assert { @After public void shutdownDerby() { + try { + DriverManager.getConnection("jdbc:derby:" + getEmbeddedDataBaseName() + ";destroy=true"); + } catch (Exception ignored) { + } try { DriverManager.getConnection("jdbc:derby:;shutdown=true"); } catch (Exception ignored) { @@ -837,8 +841,12 @@ public abstract class ActiveMQTestBase extends Assert { return testDir; } + private String getEmbeddedDataBaseName() { + return "memory:" + getTestDir(); + } + protected final String getTestJDBCConnectionUrl() { - return System.getProperty("jdbc.connection.url", "jdbc:derby:" + getTestDir() + File.separator + "derby;create=true"); + return System.getProperty("jdbc.connection.url", "jdbc:derby:" + getEmbeddedDataBaseName() + ";create=true"); } protected final String getJDBCClassName() {