From e49eda966417e83906e9f7a5bf9c89b978ceaff0 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Fri, 28 Oct 2016 00:31:42 -0400 Subject: [PATCH 1/7] NO-JIRA fixing byteman (extra tests) Two issues encountered here: i - ClosingConnectionTest was intermittently breaking other tests, in particular it was breaking PagingLeakTest for no apparent reason . apparently it was a dead lock from removeAddress on the ServerController ii - it still showing issues after removing the not needed synchronziation. Since the test is not really needed I am just removing the offending test. --- .../impl/ActiveMQServerControlImpl.java | 6 +- .../extras/byteman/ClosingConnectionTest.java | 160 ------------------ 2 files changed, 3 insertions(+), 163 deletions(-) delete mode 100644 tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/ClosingConnectionTest.java diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java index fb7deee5d7..b0e8b9b97b 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java @@ -1189,7 +1189,7 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active } @Override - public synchronized boolean closeConnectionsForAddress(final String ipAddress) { + public boolean closeConnectionsForAddress(final String ipAddress) { checkStarted(); clearIO(); @@ -1213,7 +1213,7 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active } @Override - public synchronized boolean closeConsumerConnectionsForAddress(final String address) { + public boolean closeConsumerConnectionsForAddress(final String address) { boolean closed = false; checkStarted(); @@ -1251,7 +1251,7 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active } @Override - public synchronized boolean closeConnectionsForUser(final String userName) { + public boolean closeConnectionsForUser(final String userName) { boolean closed = false; checkStarted(); diff --git a/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/ClosingConnectionTest.java b/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/ClosingConnectionTest.java deleted file mode 100644 index d4bfc00946..0000000000 --- a/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/ClosingConnectionTest.java +++ /dev/null @@ -1,160 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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 org.apache.activemq.artemis.tests.extras.byteman; - -import javax.management.MBeanServer; -import javax.management.MBeanServerFactory; - -import org.apache.activemq.artemis.api.core.SimpleString; -import org.apache.activemq.artemis.api.core.client.ClientMessage; -import org.apache.activemq.artemis.api.core.client.ClientProducer; -import org.apache.activemq.artemis.api.core.client.ClientSession; -import org.apache.activemq.artemis.api.core.client.ClientSessionFactory; -import org.apache.activemq.artemis.api.core.client.ServerLocator; -import org.apache.activemq.artemis.api.core.management.ActiveMQServerControl; -import org.apache.activemq.artemis.core.server.ActiveMQServer; -import org.apache.activemq.artemis.core.server.JournalType; -import org.apache.activemq.artemis.core.settings.impl.AddressSettings; -import org.apache.activemq.artemis.tests.integration.IntegrationTestLogger; -import org.apache.activemq.artemis.tests.integration.management.ManagementControlHelper; -import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; -import org.jboss.byteman.contrib.bmunit.BMRule; -import org.jboss.byteman.contrib.bmunit.BMRules; -import org.jboss.byteman.contrib.bmunit.BMUnitRunner; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; - -@RunWith(BMUnitRunner.class) -public class ClosingConnectionTest extends ActiveMQTestBase { - - public static final SimpleString ADDRESS = new SimpleString("SimpleAddress"); - - private ServerLocator locator; - - private ActiveMQServer server; - - private static MBeanServer mBeanServer; - - private static boolean readyToKill = false; - - protected boolean isNetty() { - return true; - } - - @Override - @Before - public void setUp() throws Exception { - super.setUp(); - mBeanServer = MBeanServerFactory.createMBeanServer(); - server = newActiveMQServer(); - server.getConfiguration().setJournalType(JournalType.NIO); - server.getConfiguration().setJMXManagementEnabled(true); - server.start(); - waitForServerToStart(server); - locator = createFactory(isNetty()); - readyToKill = false; - } - - public static void killConnection() throws InterruptedException { - if (readyToKill) { - // We have to kill the connection in a new thread otherwise Netty won't interrupt the current thread - Thread closeConnectionThread = new Thread(new Runnable() { - @Override - public void run() { - try { - ActiveMQServerControl serverControl = ManagementControlHelper.createActiveMQServerControl(mBeanServer); - serverControl.closeConnectionsForUser("guest"); - readyToKill = false; - } catch (Exception e) { - e.printStackTrace(); - } - } - }); - - closeConnectionThread.start(); - - try { - /* We want to simulate a long-running remoting thread here. If closing the connection in the closeConnectionThread - * interrupts this thread then it will cause sleep() to throw and InterruptedException. Therefore we catch - * the InterruptedException and re-interrupt the current thread so the interrupt will be passed properly - * back to the caller. It's a bit of a hack, but I couldn't find any other way to simulate it. - */ - Thread.sleep(1500); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - } - - /* - * Test for https://bugzilla.redhat.com/show_bug.cgi?id=1193085 - * */ - @Test - @BMRules(rules = {@BMRule( - name = "rule to kill connection", - targetClass = "org.apache.activemq.artemis.core.io.nio.NIOSequentialFile", - targetMethod = "open(int, boolean)", - targetLocation = "AT INVOKE java.nio.channels.FileChannel.size()", - action = "org.apache.activemq.artemis.tests.extras.byteman.ClosingConnectionTest.killConnection();")}) - public void testKillConnection() throws Exception { - locator.setBlockOnNonDurableSend(true).setBlockOnDurableSend(true).setBlockOnAcknowledge(true); - - ClientSessionFactory sf = createSessionFactory(locator); - ClientSession session = sf.createSession("guest", null, false, true, true, false, 0); - - session.createQueue(ADDRESS, ADDRESS, null, true); - - ClientProducer producer = session.createProducer(ADDRESS); - - ClientMessage message = session.createMessage(true); - message.getBodyBuffer().writeBytes(new byte[1024]); - - for (int i = 0; i < 200; i++) { - producer.send(message); - } - - assertTrue(server.locateQueue(ADDRESS).getPageSubscription().getPagingStore().isPaging()); - - readyToKill = true; - try { - for (int i = 0; i < 10; i++) { - producer.send(message); - } - fail("Sending message here should result in failure."); - } catch (Exception e) { - IntegrationTestLogger.LOGGER.info("Caught exception: " + e.getMessage()); - } - - Thread.sleep(1000); - - assertTrue(server.isStarted()); - - session.close(); - } - - private ActiveMQServer newActiveMQServer() throws Exception { - ActiveMQServer server = createServer(true, createDefaultConfig(isNetty())); - server.setMBeanServer(mBeanServer); - - AddressSettings defaultSetting = new AddressSettings().setPageSizeBytes(10 * 1024).setMaxSizeBytes(20 * 1024); - - server.getAddressSettingsRepository().addMatch("#", defaultSetting); - - return server; - } -} From 4b5cbb86aafef0b3ab969cf38bd17620efc1e7f3 Mon Sep 17 00:00:00 2001 From: Bennet Schulz Date: Fri, 28 Oct 2016 13:46:00 +0200 Subject: [PATCH 2/7] ARTEMIS-830 Remove cyclic dependencies Removes cyclic dependencies between classes and packages in the artemis-jdbc-store projetct by moving classes and methods to other locations and reducing the visibility of classes, fields and methods. Solving cyclic dependencies is important to keep the codebase maintainable. Scenarios where "everything uses everything" should be avoided. --- .../artemis/jdbc/store/JDBCUtils.java | 143 ------------------ .../store/drivers/AbstractJDBCDriver.java | 76 +++++++--- .../artemis/jdbc/store/drivers/JDBCUtils.java | 66 ++++++++ .../jdbc/store/file/JDBCFileUtils.java | 48 ++++++ .../jdbc/store/file/JDBCSequentialFile.java | 20 +-- .../store/file/JDBCSequentialFileFactory.java | 10 +- .../file/JDBCSequentialFileFactoryDriver.java | 18 +-- ...ostgresSequentialSequentialFileDriver.java | 4 +- .../jdbc/store/journal/JDBCJournalImpl.java | 23 ++- .../journal/JDBCJournalLoaderCallback.java | 12 +- .../journal/JDBCJournalReaderCallback.java | 6 +- .../jdbc/store/journal/JDBCJournalRecord.java | 100 ++++-------- .../jdbc/store/journal/JDBCJournalSync.java | 45 ------ .../jdbc/store/journal/TransactionHolder.java | 4 +- .../file/JDBCSequentialFileFactoryTest.java | 10 +- .../artemis/osgi/DataSourceTracker.java | 2 +- .../journal/JDBCJournalStorageManager.java | 4 +- .../artemis/tests/util/ActiveMQTestBase.java | 28 +++- 18 files changed, 283 insertions(+), 336 deletions(-) delete mode 100644 artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/JDBCUtils.java create mode 100644 artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/JDBCUtils.java create mode 100644 artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCFileUtils.java rename artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/{drivers/postgres => file}/PostgresSequentialSequentialFileDriver.java (96%) delete mode 100644 artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalSync.java diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/JDBCUtils.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/JDBCUtils.java deleted file mode 100644 index a0eba57b06..0000000000 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/JDBCUtils.java +++ /dev/null @@ -1,143 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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 org.apache.activemq.artemis.jdbc.store; - -import javax.sql.DataSource; -import java.sql.Connection; -import java.sql.Driver; -import java.sql.DriverManager; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; - -import org.apache.activemq.artemis.jdbc.store.drivers.derby.DerbySQLProvider; -import org.apache.activemq.artemis.jdbc.store.drivers.mysql.MySQLSQLProvider; -import org.apache.activemq.artemis.jdbc.store.drivers.postgres.PostgresSQLProvider; -import org.apache.activemq.artemis.jdbc.store.drivers.postgres.PostgresSequentialSequentialFileDriver; -import org.apache.activemq.artemis.jdbc.store.file.JDBCSequentialFileFactoryDriver; -import org.apache.activemq.artemis.jdbc.store.sql.GenericSQLProvider; -import org.apache.activemq.artemis.jdbc.store.sql.SQLProvider; -import org.jboss.logging.Logger; - -public class JDBCUtils { - - private static final Logger logger = Logger.getLogger(JDBCUtils.class); - - public static Driver getDriver(String className) throws Exception { - - try { - Driver driver = (Driver) Class.forName(className).newInstance(); - - // Shutdown the derby if using the derby embedded driver. - if (className.equals("org.apache.derby.jdbc.EmbeddedDriver")) { - Runtime.getRuntime().addShutdownHook(new Thread() { - @Override - public void run() { - try { - DriverManager.getConnection("jdbc:derby:;shutdown=true"); - } catch (Exception e) { - } - } - }); - } - return driver; - } catch (ClassNotFoundException cnfe) { - throw new RuntimeException("Could not find class: " + className); - } catch (Exception e) { - throw new RuntimeException("Unable to instantiate driver class: ", e); - } - } - - public static void createTableIfNotExists(Connection connection, String tableName, String sql) throws SQLException { - logger.tracef("Validating if table %s didn't exist before creating", tableName); - try { - connection.setAutoCommit(false); - try (ResultSet rs = connection.getMetaData().getTables(null, null, tableName, null)) { - if (rs != null && !rs.next()) { - logger.tracef("Table %s did not exist, creating it with SQL=%s", tableName, sql); - try (Statement statement = connection.createStatement()) { - statement.executeUpdate(sql); - } - } - } - connection.commit(); - } catch (SQLException e) { - connection.rollback(); - } - - } - - public static SQLProvider.Factory getSQLProviderFactory(String url) { - SQLProvider.Factory factory; - if (url.contains("derby")) { - logger.tracef("getSQLProvider Returning Derby SQL provider for url::%s", url); - factory = new DerbySQLProvider.Factory(); - } else if (url.contains("postgres")) { - logger.tracef("getSQLProvider Returning postgres SQL provider for url::%s", url); - factory = new PostgresSQLProvider.Factory(); - } else if (url.contains("mysql")) { - logger.tracef("getSQLProvider Returning mysql SQL provider for url::%s", url); - factory = new MySQLSQLProvider.Factory(); - } else { - logger.tracef("getSQLProvider Returning generic SQL provider for url::%s", url); - factory = new GenericSQLProvider.Factory(); - } - return factory; - } - - public static SQLProvider getSQLProvider(String driverClass, String tableName) { - SQLProvider.Factory factory; - if (driverClass.contains("derby")) { - logger.tracef("getSQLProvider Returning Derby SQL provider for driver::%s, tableName::%s", driverClass, tableName); - factory = new DerbySQLProvider.Factory(); - } else if (driverClass.contains("postgres")) { - logger.tracef("getSQLProvider Returning postgres SQL provider for driver::%s, tableName::%s", driverClass, tableName); - factory = new PostgresSQLProvider.Factory(); - } else if (driverClass.contains("mysql")) { - logger.tracef("getSQLProvider Returning mysql SQL provider for driver::%s, tableName::%s", driverClass, tableName); - factory = new MySQLSQLProvider.Factory(); - } else { - logger.tracef("getSQLProvider Returning generic SQL provider for driver::%s, tableName::%s", driverClass, tableName); - factory = new GenericSQLProvider.Factory(); - } - return factory.create(tableName); - } - - public static JDBCSequentialFileFactoryDriver getDBFileDriver(String driverClass, - String jdbcConnectionUrl, - SQLProvider provider) throws SQLException { - JDBCSequentialFileFactoryDriver dbDriver = new JDBCSequentialFileFactoryDriver(); - dbDriver.setSqlProvider(provider); - dbDriver.setJdbcConnectionUrl(jdbcConnectionUrl); - dbDriver.setJdbcDriverClass(driverClass); - return dbDriver; - } - - public static JDBCSequentialFileFactoryDriver getDBFileDriver(DataSource dataSource, - String tableName, - SQLProvider provider) throws SQLException { - JDBCSequentialFileFactoryDriver dbDriver; - if (provider instanceof PostgresSQLProvider) { - dbDriver = new PostgresSequentialSequentialFileDriver(); - dbDriver.setDataSource(dataSource); - } else { - dbDriver = new JDBCSequentialFileFactoryDriver(tableName, dataSource, provider); - } - return dbDriver; - } - -} 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 b277523e73..79cc1e52ef 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 @@ -19,30 +19,32 @@ package org.apache.activemq.artemis.jdbc.store.drivers; import javax.sql.DataSource; import java.sql.Connection; import java.sql.Driver; +import java.sql.DriverManager; +import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.util.Properties; -import org.apache.activemq.artemis.jdbc.store.JDBCUtils; import org.apache.activemq.artemis.jdbc.store.sql.SQLProvider; import org.apache.activemq.artemis.journal.ActiveMQJournalLogger; +import org.jboss.logging.Logger; /** * Class to hold common database functionality such as drivers and connections */ public abstract class AbstractJDBCDriver { + private static final Logger logger = Logger.getLogger(AbstractJDBCDriver.class); + protected Connection connection; protected SQLProvider sqlProvider; - protected String jdbcConnectionUrl; + private String jdbcConnectionUrl; - protected String jdbcDriverClass; + private String jdbcDriverClass; - protected Driver dbDriver; - - protected DataSource dataSource; + private DataSource dataSource; public AbstractJDBCDriver() { } @@ -75,7 +77,7 @@ public abstract class AbstractJDBCDriver { protected abstract void createSchema() throws SQLException; protected void createTable(String schemaSql) throws SQLException { - JDBCUtils.createTableIfNotExists(connection, sqlProvider.getTableName(), schemaSql); + createTableIfNotExists(connection, sqlProvider.getTableName(), schemaSql); } protected void connect() throws Exception { @@ -83,7 +85,7 @@ public abstract class AbstractJDBCDriver { connection = dataSource.getConnection(); } else { try { - dbDriver = JDBCUtils.getDriver(jdbcDriverClass); + Driver dbDriver = getDriver(jdbcDriverClass); connection = dbDriver.connect(jdbcConnectionUrl, new Properties()); } catch (SQLException e) { ActiveMQJournalLogger.LOGGER.error("Unable to connect to database using URL: " + jdbcConnectionUrl); @@ -105,6 +107,48 @@ public abstract class AbstractJDBCDriver { } } + private static void createTableIfNotExists(Connection connection, String tableName, String sql) throws SQLException { + logger.tracef("Validating if table %s didn't exist before creating", tableName); + try { + connection.setAutoCommit(false); + try (ResultSet rs = connection.getMetaData().getTables(null, null, tableName, null)) { + if (rs != null && !rs.next()) { + logger.tracef("Table %s did not exist, creating it with SQL=%s", tableName, sql); + try (Statement statement = connection.createStatement()) { + statement.executeUpdate(sql); + } + } + } + connection.commit(); + } catch (SQLException e) { + connection.rollback(); + } + } + + private Driver getDriver(String className) throws Exception { + try { + Driver driver = (Driver) Class.forName(className).newInstance(); + + // Shutdown the derby if using the derby embedded driver. + if (className.equals("org.apache.derby.jdbc.EmbeddedDriver")) { + Runtime.getRuntime().addShutdownHook(new Thread() { + @Override + public void run() { + try { + DriverManager.getConnection("jdbc:derby:;shutdown=true"); + } catch (Exception e) { + } + } + }); + } + return driver; + } catch (ClassNotFoundException cnfe) { + throw new RuntimeException("Could not find class: " + className); + } catch (Exception e) { + throw new RuntimeException("Unable to instantiate driver class: ", e); + } + } + public Connection getConnection() { return connection; } @@ -113,34 +157,18 @@ public abstract class AbstractJDBCDriver { this.connection = connection; } - public SQLProvider getSqlProvider() { - return sqlProvider; - } - public void setSqlProvider(SQLProvider sqlProvider) { this.sqlProvider = sqlProvider; } - public String getJdbcConnectionUrl() { - return jdbcConnectionUrl; - } - public void setJdbcConnectionUrl(String jdbcConnectionUrl) { this.jdbcConnectionUrl = jdbcConnectionUrl; } - public String getJdbcDriverClass() { - return jdbcDriverClass; - } - public void setJdbcDriverClass(String jdbcDriverClass) { this.jdbcDriverClass = jdbcDriverClass; } - public DataSource getDataSource() { - return dataSource; - } - public void setDataSource(DataSource dataSource) { this.dataSource = dataSource; } diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/JDBCUtils.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/JDBCUtils.java new file mode 100644 index 0000000000..418fd4324f --- /dev/null +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/JDBCUtils.java @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.activemq.artemis.jdbc.store.drivers; + +import org.apache.activemq.artemis.jdbc.store.drivers.derby.DerbySQLProvider; +import org.apache.activemq.artemis.jdbc.store.drivers.mysql.MySQLSQLProvider; +import org.apache.activemq.artemis.jdbc.store.drivers.postgres.PostgresSQLProvider; +import org.apache.activemq.artemis.jdbc.store.sql.GenericSQLProvider; +import org.apache.activemq.artemis.jdbc.store.sql.SQLProvider; +import org.jboss.logging.Logger; + +public class JDBCUtils { + + private static final Logger logger = Logger.getLogger(JDBCUtils.class); + + public static SQLProvider.Factory getSQLProviderFactory(String url) { + SQLProvider.Factory factory; + if (url.contains("derby")) { + logger.tracef("getSQLProvider Returning Derby SQL provider for url::%s", url); + factory = new DerbySQLProvider.Factory(); + } else if (url.contains("postgres")) { + logger.tracef("getSQLProvider Returning postgres SQL provider for url::%s", url); + factory = new PostgresSQLProvider.Factory(); + } else if (url.contains("mysql")) { + logger.tracef("getSQLProvider Returning mysql SQL provider for url::%s", url); + factory = new MySQLSQLProvider.Factory(); + } else { + logger.tracef("getSQLProvider Returning generic SQL provider for url::%s", url); + factory = new GenericSQLProvider.Factory(); + } + return factory; + } + + public static SQLProvider getSQLProvider(String driverClass, String tableName) { + SQLProvider.Factory factory; + if (driverClass.contains("derby")) { + logger.tracef("getSQLProvider Returning Derby SQL provider for driver::%s, tableName::%s", driverClass, tableName); + factory = new DerbySQLProvider.Factory(); + } else if (driverClass.contains("postgres")) { + logger.tracef("getSQLProvider Returning postgres SQL provider for driver::%s, tableName::%s", driverClass, tableName); + factory = new PostgresSQLProvider.Factory(); + } else if (driverClass.contains("mysql")) { + logger.tracef("getSQLProvider Returning mysql SQL provider for driver::%s, tableName::%s", driverClass, tableName); + factory = new MySQLSQLProvider.Factory(); + } else { + logger.tracef("getSQLProvider Returning generic SQL provider for driver::%s, tableName::%s", driverClass, tableName); + factory = new GenericSQLProvider.Factory(); + } + return factory.create(tableName); + } + +} diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCFileUtils.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCFileUtils.java new file mode 100644 index 0000000000..02b1128d26 --- /dev/null +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCFileUtils.java @@ -0,0 +1,48 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.activemq.artemis.jdbc.store.file; + +import javax.sql.DataSource; +import java.sql.SQLException; + +import org.apache.activemq.artemis.jdbc.store.drivers.postgres.PostgresSQLProvider; +import org.apache.activemq.artemis.jdbc.store.sql.SQLProvider; + +class JDBCFileUtils { + + static JDBCSequentialFileFactoryDriver getDBFileDriver(String driverClass, + String jdbcConnectionUrl, + SQLProvider provider) throws SQLException { + JDBCSequentialFileFactoryDriver dbDriver = new JDBCSequentialFileFactoryDriver(); + dbDriver.setSqlProvider(provider); + dbDriver.setJdbcConnectionUrl(jdbcConnectionUrl); + dbDriver.setJdbcDriverClass(driverClass); + return dbDriver; + } + + static JDBCSequentialFileFactoryDriver getDBFileDriver(DataSource dataSource, SQLProvider provider) throws SQLException { + JDBCSequentialFileFactoryDriver dbDriver; + if (provider instanceof PostgresSQLProvider) { + dbDriver = new PostgresSequentialSequentialFileDriver(); + dbDriver.setDataSource(dataSource); + } else { + dbDriver = new JDBCSequentialFileFactoryDriver(dataSource, provider); + } + return dbDriver; + } +} diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFile.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFile.java index 84089918ef..f3215c0520 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFile.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFile.java @@ -64,11 +64,11 @@ public class JDBCSequentialFile implements SequentialFile { // Allows DB Drivers to cache meta data. private final Map metaData = new ConcurrentHashMap<>(); - public JDBCSequentialFile(final JDBCSequentialFileFactory fileFactory, - final String filename, - final Executor executor, - final JDBCSequentialFileFactoryDriver driver, - final Object writeLock) throws SQLException { + JDBCSequentialFile(final JDBCSequentialFileFactory fileFactory, + final String filename, + final Executor executor, + final JDBCSequentialFileFactoryDriver driver, + final Object writeLock) throws SQLException { this.fileFactory = fileFactory; this.filename = filename; this.extension = filename.contains(".") ? filename.substring(filename.lastIndexOf(".") + 1, filename.length()) : ""; @@ -77,7 +77,7 @@ public class JDBCSequentialFile implements SequentialFile { this.dbDriver = driver; } - public void setWritePosition(int writePosition) { + void setWritePosition(int writePosition) { this.writePosition = writePosition; } @@ -172,7 +172,7 @@ public class JDBCSequentialFile implements SequentialFile { return internalWrite(buffer.array(), callback); } - public void scheduleWrite(final ActiveMQBuffer bytes, final IOCallback callback) { + private void scheduleWrite(final ActiveMQBuffer bytes, final IOCallback callback) { executor.execute(new Runnable() { @Override public void run() { @@ -181,7 +181,7 @@ public class JDBCSequentialFile implements SequentialFile { }); } - public void scheduleWrite(final ByteBuffer bytes, final IOCallback callback) { + private void scheduleWrite(final ByteBuffer bytes, final IOCallback callback) { executor.execute(new Runnable() { @Override public void run() { @@ -358,10 +358,6 @@ public class JDBCSequentialFile implements SequentialFile { metaData.put(key, value); } - public Object removeMetaData(Object key) { - return metaData.remove(key); - } - public Object getMetaData(Object key) { return metaData.get(key); } diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactory.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactory.java index 8078417124..cafb261cd2 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactory.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactory.java @@ -30,7 +30,6 @@ import org.apache.activemq.artemis.core.io.SequentialFile; import org.apache.activemq.artemis.core.io.SequentialFileFactory; import org.apache.activemq.artemis.core.io.nio.NIOSequentialFileFactory; import org.apache.activemq.artemis.core.server.ActiveMQComponent; -import org.apache.activemq.artemis.jdbc.store.JDBCUtils; import org.apache.activemq.artemis.jdbc.store.sql.SQLProvider; import org.apache.activemq.artemis.journal.ActiveMQJournalLogger; @@ -48,10 +47,9 @@ public class JDBCSequentialFileFactory implements SequentialFileFactory, ActiveM public JDBCSequentialFileFactory(final DataSource dataSource, final SQLProvider sqlProvider, - final String tableName, Executor executor) throws Exception { this.executor = executor; - dbDriver = JDBCUtils.getDBFileDriver(dataSource, tableName, sqlProvider); + dbDriver = JDBCFileUtils.getDBFileDriver(dataSource, sqlProvider); } public JDBCSequentialFileFactory(final String connectionUrl, @@ -59,7 +57,7 @@ public class JDBCSequentialFileFactory implements SequentialFileFactory, ActiveM final SQLProvider sqlProvider, Executor executor) throws Exception { this.executor = executor; - dbDriver = JDBCUtils.getDBFileDriver(className, connectionUrl, sqlProvider); + dbDriver = JDBCFileUtils.getDBFileDriver(className, connectionUrl, sqlProvider); } @Override @@ -88,9 +86,7 @@ public class JDBCSequentialFileFactory implements SequentialFileFactory, ActiveM @Override public SequentialFile createSequentialFile(String fileName) { try { - if (fileLocks.get(fileName) == null) { - fileLocks.put(fileName, new Object()); - } + fileLocks.putIfAbsent(fileName, new Object()); JDBCSequentialFile file = new JDBCSequentialFile(this, fileName, executor, dbDriver, fileLocks.get(fileName)); files.add(file); return file; diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactoryDriver.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactoryDriver.java index 00f73b380c..7b9eaf1268 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactoryDriver.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/JDBCSequentialFileFactoryDriver.java @@ -33,25 +33,25 @@ public class JDBCSequentialFileFactoryDriver extends AbstractJDBCDriver { protected PreparedStatement deleteFile; - protected PreparedStatement createFile; + PreparedStatement createFile; - protected PreparedStatement selectFileByFileName; + private PreparedStatement selectFileByFileName; - protected PreparedStatement copyFileRecord; + private PreparedStatement copyFileRecord; - protected PreparedStatement renameFile; + private PreparedStatement renameFile; - protected PreparedStatement readLargeObject; + PreparedStatement readLargeObject; - protected PreparedStatement appendToLargeObject; + private PreparedStatement appendToLargeObject; - protected PreparedStatement selectFileNamesByExtension; + private PreparedStatement selectFileNamesByExtension; - public JDBCSequentialFileFactoryDriver() { + JDBCSequentialFileFactoryDriver() { super(); } - public JDBCSequentialFileFactoryDriver(String tableName, DataSource dataSource, SQLProvider provider) { + JDBCSequentialFileFactoryDriver(DataSource dataSource, SQLProvider provider) { super(dataSource, provider); } diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/postgres/PostgresSequentialSequentialFileDriver.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/PostgresSequentialSequentialFileDriver.java similarity index 96% rename from artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/postgres/PostgresSequentialSequentialFileDriver.java rename to artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/PostgresSequentialSequentialFileDriver.java index db74c056ae..c7411a6571 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/postgres/PostgresSequentialSequentialFileDriver.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/PostgresSequentialSequentialFileDriver.java @@ -14,14 +14,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.activemq.artemis.jdbc.store.drivers.postgres; +package org.apache.activemq.artemis.jdbc.store.file; import java.nio.ByteBuffer; import java.sql.ResultSet; import java.sql.SQLException; -import org.apache.activemq.artemis.jdbc.store.file.JDBCSequentialFile; -import org.apache.activemq.artemis.jdbc.store.file.JDBCSequentialFileFactoryDriver; import org.postgresql.PGConnection; import org.postgresql.largeobject.LargeObject; import org.postgresql.largeobject.LargeObjectManager; diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java index ef45fe0645..636309e311 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java @@ -41,6 +41,7 @@ import org.apache.activemq.artemis.core.journal.RecordInfo; import org.apache.activemq.artemis.core.journal.TransactionFailureCallback; import org.apache.activemq.artemis.core.journal.impl.JournalFile; import org.apache.activemq.artemis.core.journal.impl.SimpleWaitIOCallback; +import org.apache.activemq.artemis.core.server.ActiveMQScheduledComponent; import org.apache.activemq.artemis.jdbc.store.drivers.AbstractJDBCDriver; import org.apache.activemq.artemis.jdbc.store.sql.SQLProvider; import org.apache.activemq.artemis.journal.ActiveMQJournalLogger; @@ -51,7 +52,7 @@ public class JDBCJournalImpl extends AbstractJDBCDriver implements Journal { private static final Logger logger = Logger.getLogger(JDBCJournalImpl.class); // Sync Delay in ms - public static final int SYNC_DELAY = 5; + private static final int SYNC_DELAY = 5; private static int USER_VERSION = 1; @@ -741,4 +742,24 @@ public class JDBCJournalImpl extends AbstractJDBCDriver implements Journal { return started; } + private static class JDBCJournalSync extends ActiveMQScheduledComponent { + + private final JDBCJournalImpl journal; + + JDBCJournalSync(ScheduledExecutorService scheduledExecutorService, + Executor executor, + long checkPeriod, + TimeUnit timeUnit, + JDBCJournalImpl journal) { + super(scheduledExecutorService, executor, checkPeriod, timeUnit, true); + this.journal = journal; + } + + @Override + public void run() { + if (journal.isStarted()) { + journal.sync(); + } + } + } } diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalLoaderCallback.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalLoaderCallback.java index eaa5387372..f5a5d26ed7 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalLoaderCallback.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalLoaderCallback.java @@ -27,7 +27,7 @@ import org.apache.activemq.artemis.core.journal.PreparedTransactionInfo; import org.apache.activemq.artemis.core.journal.RecordInfo; import org.apache.activemq.artemis.core.journal.TransactionFailureCallback; -public class JDBCJournalLoaderCallback implements LoaderCallback { +class JDBCJournalLoaderCallback implements LoaderCallback { private final List preparedTransactions; @@ -41,16 +41,16 @@ public class JDBCJournalLoaderCallback implements LoaderCallback { private long maxId = -1; - public JDBCJournalLoaderCallback(final List committedRecords, - final List preparedTransactions, - final TransactionFailureCallback failureCallback, - final boolean fixBadTX) { + JDBCJournalLoaderCallback(final List committedRecords, + final List preparedTransactions, + final TransactionFailureCallback failureCallback, + final boolean fixBadTX) { this.committedRecords = committedRecords; this.preparedTransactions = preparedTransactions; this.failureCallback = failureCallback; } - public synchronized void checkMaxId(long id) { + private synchronized void checkMaxId(long id) { if (maxId < id) { maxId = id; } diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalReaderCallback.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalReaderCallback.java index cd8a411dce..3c200a4b08 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalReaderCallback.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalReaderCallback.java @@ -27,13 +27,13 @@ import org.apache.activemq.artemis.core.journal.impl.JournalFile; import org.apache.activemq.artemis.core.journal.impl.JournalReaderCallback; import org.apache.activemq.artemis.journal.ActiveMQJournalLogger; -public class JDBCJournalReaderCallback implements JournalReaderCallback { +class JDBCJournalReaderCallback implements JournalReaderCallback { private final Map loadTransactions = new LinkedHashMap<>(); private final LoaderCallback loadManager; - public JDBCJournalReaderCallback(final LoaderCallback loadManager) { + JDBCJournalReaderCallback(final LoaderCallback loadManager) { this.loadManager = loadManager; } @@ -126,7 +126,7 @@ public class JDBCJournalReaderCallback implements JournalReaderCallback { // Not needed for JDBC journal impl } - public void checkPreparedTx() { + void checkPreparedTx() { for (TransactionHolder transaction : loadTransactions.values()) { if ((!transaction.prepared && !transaction.committed) || transaction.invalid) { ActiveMQJournalLogger.LOGGER.uncomittedTxFound(transaction.transactionID); diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalRecord.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalRecord.java index 3b570a0ad8..9691d3ea49 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalRecord.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalRecord.java @@ -32,7 +32,7 @@ import org.apache.activemq.artemis.core.journal.RecordInfo; import org.apache.activemq.artemis.journal.ActiveMQJournalLogger; import org.apache.activemq.artemis.utils.ActiveMQBufferInputStream; -public class JDBCJournalRecord { +class JDBCJournalRecord { /* Database Table Schema: @@ -49,17 +49,17 @@ public class JDBCJournalRecord { */ // Record types taken from Journal Impl - public static final byte ADD_RECORD = 11; - public static final byte UPDATE_RECORD = 12; - public static final byte ADD_RECORD_TX = 13; - public static final byte UPDATE_RECORD_TX = 14; + static final byte ADD_RECORD = 11; + static final byte UPDATE_RECORD = 12; + static final byte ADD_RECORD_TX = 13; + static final byte UPDATE_RECORD_TX = 14; - public static final byte DELETE_RECORD_TX = 15; - public static final byte DELETE_RECORD = 16; + static final byte DELETE_RECORD_TX = 15; + static final byte DELETE_RECORD = 16; - public static final byte PREPARE_RECORD = 17; - public static final byte COMMIT_RECORD = 18; - public static final byte ROLLBACK_RECORD = 19; + static final byte PREPARE_RECORD = 17; + static final byte COMMIT_RECORD = 18; + static final byte ROLLBACK_RECORD = 19; // Callback and sync operations private IOCompletion ioCompletion = null; @@ -90,7 +90,7 @@ public class JDBCJournalRecord { private long seq; - public JDBCJournalRecord(long id, byte recordType, long seq) { + JDBCJournalRecord(long id, byte recordType, long seq) { this.id = id; this.recordType = recordType; @@ -110,26 +110,6 @@ public class JDBCJournalRecord { this.seq = seq; } - public static String createTableSQL(String tableName) { - return "CREATE TABLE " + tableName + "(id BIGINT,recordType SMALLINT,compactCount SMALLINT,txId BIGINT,userRecordType SMALLINT,variableSize INTEGER,record BLOB,txDataSize INTEGER,txData BLOB,txCheckNoRecords INTEGER,seq BIGINT)"; - } - - public static String insertRecordsSQL(String tableName) { - return "INSERT INTO " + tableName + "(id,recordType,compactCount,txId,userRecordType,variableSize,record,txDataSize,txData,txCheckNoRecords,seq) " + "VALUES (?,?,?,?,?,?,?,?,?,?,?)"; - } - - public static String selectRecordsSQL(String tableName) { - return "SELECT id,recordType,compactCount,txId,userRecordType,variableSize,record,txDataSize,txData,txCheckNoRecords,seq " + "FROM " + tableName + " ORDER BY seq ASC"; - } - - public static String deleteRecordsSQL(String tableName) { - return "DELETE FROM " + tableName + " WHERE id = ?"; - } - - public static String deleteJournalTxRecordsSQL(String tableName) { - return "DELETE FROM " + tableName + " WHERE txId=?"; - } - public void complete(boolean success) { if (ioCompletion != null) { if (success) { @@ -146,7 +126,7 @@ public class JDBCJournalRecord { } } - protected void writeRecord(PreparedStatement statement) throws SQLException { + void writeRecord(PreparedStatement statement) throws SQLException { byte[] recordBytes = new byte[variableSize]; byte[] txDataBytes = new byte[txDataSize]; @@ -172,12 +152,12 @@ public class JDBCJournalRecord { statement.addBatch(); } - protected void writeDeleteRecord(PreparedStatement deleteStatement) throws SQLException { + void writeDeleteRecord(PreparedStatement deleteStatement) throws SQLException { deleteStatement.setLong(1, id); deleteStatement.addBatch(); } - public static JDBCJournalRecord readRecord(ResultSet rs) throws SQLException { + static JDBCJournalRecord readRecord(ResultSet rs) throws SQLException { JDBCJournalRecord record = new JDBCJournalRecord(rs.getLong(1), (byte) rs.getShort(2), rs.getLong(11)); record.setCompactCount((byte) rs.getShort(3)); record.setTxId(rs.getLong(4)); @@ -190,18 +170,14 @@ public class JDBCJournalRecord { return record; } - public IOCompletion getIoCompletion() { + IOCompletion getIoCompletion() { return ioCompletion; } - public void setIoCompletion(IOCompletion ioCompletion) { + void setIoCompletion(IOCompletion ioCompletion) { this.ioCompletion = ioCompletion; } - public boolean isStoreLineUp() { - return storeLineUp; - } - public void setStoreLineUp(boolean storeLineUp) { this.storeLineUp = storeLineUp; } @@ -222,27 +198,23 @@ public class JDBCJournalRecord { return recordType; } - public byte getCompactCount() { + byte getCompactCount() { return compactCount; } - public void setCompactCount(byte compactCount) { + private void setCompactCount(byte compactCount) { this.compactCount = compactCount; } - public long getTxId() { + long getTxId() { return txId; } - public void setTxId(long txId) { + void setTxId(long txId) { this.txId = txId; } - public int getVariableSize() { - return variableSize; - } - - public void setVariableSize(int variableSize) { + private void setVariableSize(int variableSize) { this.variableSize = variableSize; } @@ -277,31 +249,19 @@ public class JDBCJournalRecord { return record; } - public int getTxCheckNoRecords() { + int getTxCheckNoRecords() { return txCheckNoRecords; } - public void setTxCheckNoRecords(int txCheckNoRecords) { + private void setTxCheckNoRecords(int txCheckNoRecords) { this.txCheckNoRecords = txCheckNoRecords; } - public void setTxDataSize(int txDataSize) { + private void setTxDataSize(int txDataSize) { this.txDataSize = txDataSize; } - public int getTxDataSize() { - return txDataSize; - } - - public InputStream getTxData() { - return txData; - } - - public void setTxData(InputStream record) { - this.record = record; - } - - public void setTxData(EncodingSupport txData) { + void setTxData(EncodingSupport txData) { this.txDataSize = txData.getEncodeSize(); ActiveMQBuffer encodedBuffer = ActiveMQBuffers.fixedBuffer(txDataSize); @@ -309,7 +269,7 @@ public class JDBCJournalRecord { this.txData = new ActiveMQBufferInputStream(encodedBuffer); } - public void setTxData(byte[] txData) { + void setTxData(byte[] txData) { if (txData != null) { this.txDataSize = txData.length; this.txData = new ByteArrayInputStream(txData); @@ -320,19 +280,19 @@ public class JDBCJournalRecord { return isUpdate; } - public byte[] getRecordData() throws IOException { + private byte[] getRecordData() throws IOException { byte[] data = new byte[variableSize]; record.read(data); return data; } - public byte[] getTxDataAsByteArray() throws IOException { + byte[] getTxDataAsByteArray() throws IOException { byte[] data = new byte[txDataSize]; txData.read(data); return data; } - public RecordInfo toRecordInfo() throws IOException { + RecordInfo toRecordInfo() throws IOException { return new RecordInfo(getId(), getUserRecordType(), getRecordData(), isUpdate(), getCompactCount()); } @@ -340,7 +300,7 @@ public class JDBCJournalRecord { return isTransactional; } - public long getSeq() { + long getSeq() { return seq; } } diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalSync.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalSync.java deleted file mode 100644 index 8ef7e082cd..0000000000 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalSync.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You 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 org.apache.activemq.artemis.jdbc.store.journal; - -import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; - -import org.apache.activemq.artemis.core.server.ActiveMQScheduledComponent; - -public class JDBCJournalSync extends ActiveMQScheduledComponent { - - private final JDBCJournalImpl journal; - - public JDBCJournalSync(ScheduledExecutorService scheduledExecutorService, - Executor executor, - long checkPeriod, - TimeUnit timeUnit, - JDBCJournalImpl journal) { - super(scheduledExecutorService, executor, checkPeriod, timeUnit, true); - this.journal = journal; - } - - @Override - public void run() { - if (journal.isStarted()) { - journal.sync(); - } - } -} diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/TransactionHolder.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/TransactionHolder.java index 12c0d59e97..39f40ab479 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/TransactionHolder.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/TransactionHolder.java @@ -30,7 +30,7 @@ final class TransactionHolder { public final long transactionID; - public final List recordInfos = new ArrayList<>(); + final List recordInfos = new ArrayList<>(); public final List recordsToDelete = new ArrayList<>(); @@ -38,7 +38,7 @@ final class TransactionHolder { public boolean invalid; - public byte[] extraData; + byte[] extraData; public boolean committed; } diff --git a/artemis-jdbc-store/src/test/java/org/apache/activemq/artemis/jdbc/file/JDBCSequentialFileFactoryTest.java b/artemis-jdbc-store/src/test/java/org/apache/activemq/artemis/jdbc/file/JDBCSequentialFileFactoryTest.java index e94f51aed0..75bdf443f6 100644 --- a/artemis-jdbc-store/src/test/java/org/apache/activemq/artemis/jdbc/file/JDBCSequentialFileFactoryTest.java +++ b/artemis-jdbc-store/src/test/java/org/apache/activemq/artemis/jdbc/file/JDBCSequentialFileFactoryTest.java @@ -32,7 +32,7 @@ import org.apache.activemq.artemis.api.core.ActiveMQBuffer; import org.apache.activemq.artemis.api.core.ActiveMQBuffers; import org.apache.activemq.artemis.core.io.IOCallback; import org.apache.activemq.artemis.core.io.SequentialFile; -import org.apache.activemq.artemis.jdbc.store.JDBCUtils; +import org.apache.activemq.artemis.jdbc.store.drivers.JDBCUtils; import org.apache.activemq.artemis.jdbc.store.file.JDBCSequentialFile; import org.apache.activemq.artemis.jdbc.store.file.JDBCSequentialFileFactory; import org.apache.activemq.artemis.utils.ActiveMQThreadFactory; @@ -53,10 +53,6 @@ public class JDBCSequentialFileFactoryTest { @Rule public ThreadLeakCheckRule leakCheckRule = new ThreadLeakCheckRule(); - private static String connectionUrl = "jdbc:derby:target/data;create=true"; - - private static String tableName = "FILES"; - private static String className = EmbeddedDriver.class.getCanonicalName(); private JDBCSequentialFileFactory factory; @@ -65,6 +61,8 @@ public class JDBCSequentialFileFactoryTest { public void setup() throws Exception { Executor executor = Executors.newSingleThreadExecutor(ActiveMQThreadFactory.defaultThreadFactory()); + String connectionUrl = "jdbc:derby:target/data;create=true"; + String tableName = "FILES"; factory = new JDBCSequentialFileFactory(connectionUrl, className, JDBCUtils.getSQLProvider(className, tableName), executor); factory.start(); } @@ -198,7 +196,7 @@ public class JDBCSequentialFileFactoryTest { fail(errorMessage); } - public void assertEmpty(int timeout) throws InterruptedException { + void assertEmpty(int timeout) throws InterruptedException { countDownLatch.await(timeout, TimeUnit.SECONDS); assertEquals(countDownLatch.getCount(), 0); } diff --git a/artemis-server-osgi/src/main/java/org/apache/activemq/artemis/osgi/DataSourceTracker.java b/artemis-server-osgi/src/main/java/org/apache/activemq/artemis/osgi/DataSourceTracker.java index 69c54b355e..941d39f72f 100644 --- a/artemis-server-osgi/src/main/java/org/apache/activemq/artemis/osgi/DataSourceTracker.java +++ b/artemis-server-osgi/src/main/java/org/apache/activemq/artemis/osgi/DataSourceTracker.java @@ -23,7 +23,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import org.apache.activemq.artemis.core.config.storage.DatabaseStorageConfiguration; -import org.apache.activemq.artemis.jdbc.store.JDBCUtils; +import org.apache.activemq.artemis.jdbc.store.drivers.JDBCUtils; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceReference; import org.osgi.util.tracker.ServiceTrackerCustomizer; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java index 5d30b48aa7..a0f0ed18ce 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java @@ -25,7 +25,7 @@ import org.apache.activemq.artemis.core.config.Configuration; import org.apache.activemq.artemis.core.config.storage.DatabaseStorageConfiguration; import org.apache.activemq.artemis.core.io.IOCriticalErrorListener; import org.apache.activemq.artemis.core.io.nio.NIOSequentialFileFactory; -import org.apache.activemq.artemis.jdbc.store.JDBCUtils; +import org.apache.activemq.artemis.jdbc.store.drivers.JDBCUtils; import org.apache.activemq.artemis.jdbc.store.file.JDBCSequentialFileFactory; import org.apache.activemq.artemis.jdbc.store.journal.JDBCJournalImpl; import org.apache.activemq.artemis.jdbc.store.sql.GenericSQLProvider; @@ -59,7 +59,7 @@ public class JDBCJournalStorageManager extends JournalStorageManager { } bindingsJournal = new JDBCJournalImpl(dbConf.getDataSource(), sqlProviderFactory.create(dbConf.getBindingsTableName()), dbConf.getBindingsTableName(), scheduledExecutorService, executorFactory.getExecutor()); messageJournal = new JDBCJournalImpl(dbConf.getDataSource(), sqlProviderFactory.create(dbConf.getMessageTableName()), dbConf.getMessageTableName(), scheduledExecutorService, executorFactory.getExecutor()); - largeMessagesFactory = new JDBCSequentialFileFactory(dbConf.getDataSource(), sqlProviderFactory.create(dbConf.getLargeMessageTableName()), dbConf.getLargeMessageTableName(), executor); + largeMessagesFactory = new JDBCSequentialFileFactory(dbConf.getDataSource(), sqlProviderFactory.create(dbConf.getLargeMessageTableName()), executor); } else { String driverClassName = dbConf.getJdbcDriverClassName(); bindingsJournal = new JDBCJournalImpl(dbConf.getJdbcConnectionUrl(), driverClassName, JDBCUtils.getSQLProvider(driverClassName, dbConf.getBindingsTableName()), scheduledExecutorService, executorFactory.getExecutor()); 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 69ed8b643b..29119f8d73 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 @@ -129,7 +129,7 @@ import org.apache.activemq.artemis.core.server.impl.SharedNothingBackupActivatio import org.apache.activemq.artemis.core.settings.impl.AddressFullMessagePolicy; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; import org.apache.activemq.artemis.core.transaction.impl.XidImpl; -import org.apache.activemq.artemis.jdbc.store.JDBCUtils; +import org.apache.activemq.artemis.jdbc.store.drivers.JDBCUtils; import org.apache.activemq.artemis.jdbc.store.sql.SQLProvider; import org.apache.activemq.artemis.jlibaio.LibaioContext; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; @@ -467,7 +467,7 @@ public abstract class ActiveMQTestBase extends Assert { } public void destroyTables(List tableNames) throws Exception { - Driver driver = JDBCUtils.getDriver(getJDBCClassName()); + Driver driver = getDriver(getJDBCClassName()); Connection connection = driver.connect(getTestJDBCConnectionUrl(), null); Statement statement = connection.createStatement(); try { @@ -490,6 +490,30 @@ public abstract class ActiveMQTestBase extends Assert { } } + private Driver getDriver(String className) throws Exception { + try { + Driver driver = (Driver) Class.forName(className).newInstance(); + + // Shutdown the derby if using the derby embedded driver. + if (className.equals("org.apache.derby.jdbc.EmbeddedDriver")) { + Runtime.getRuntime().addShutdownHook(new Thread() { + @Override + public void run() { + try { + DriverManager.getConnection("jdbc:derby:;shutdown=true"); + } catch (Exception e) { + } + } + }); + } + return driver; + } catch (ClassNotFoundException cnfe) { + throw new RuntimeException("Could not find class: " + className); + } catch (Exception e) { + throw new RuntimeException("Unable to instantiate driver class: ", e); + } + } + protected Map generateInVMParams(final int node) { Map params = new HashMap<>(); From e0021252ee94dcafe664520e080d5a6e13e3350f Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Mon, 24 Oct 2016 18:20:20 -0400 Subject: [PATCH 3/7] ARTEMIS-829 Removing messages re-encoding https://issues.apache.org/jira/browse/ARTEMIS-829 --- .../core/client/impl/ClientProducerImpl.java | 2 - .../core/client/impl/ClientSessionImpl.java | 26 +-- .../client/impl/ClientSessionInternal.java | 2 - .../core/impl/ActiveMQSessionContext.java | 10 +- .../spi/core/remoting/SessionContext.java | 3 +- .../client/HornetQClientSessionContext.java | 5 +- .../tests/extras/byteman/PagingLeakTest.java | 14 +- .../integration/client/ProducerTest.java | 6 +- .../failover/BackupSyncJournalTest.java | 2 +- .../journal/NIOJournalCompactTest.java | 192 ++++++++++-------- .../ValidateTransactionHealthTest.java | 23 ++- .../tests/integration/paging/PagingTest.java | 12 +- 12 files changed, 145 insertions(+), 152 deletions(-) diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientProducerImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientProducerImpl.java index fddd4de5e8..1dfbe72971 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientProducerImpl.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientProducerImpl.java @@ -284,8 +284,6 @@ public class ClientProducerImpl implements ClientProducerInternal { theCredits.acquireCredits(creditSize); - session.checkDefaultAddress(sendingAddress); - sessionContext.sendFullMessage(msgI, sendBlocking, handler, address); } diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionImpl.java index de45066af3..fd6355a822 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionImpl.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionImpl.java @@ -135,8 +135,6 @@ public final class ClientSessionImpl implements ClientSessionInternal, FailureLi private volatile boolean mayAttemptToFailover = true; - private volatile SimpleString defaultAddress; - /** * Current XID. this will be used in case of failover */ @@ -957,7 +955,7 @@ public final class ClientSessionImpl implements ClientSessionInternal, FailureLi // want // to recreate the session, we just want to unblock the blocking call if (!inClose && mayAttemptToFailover) { - sessionContext.recreateSession(username, password, minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, defaultAddress); + sessionContext.recreateSession(username, password, minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge); for (Map.Entry entryx : consumers.entrySet()) { @@ -1036,27 +1034,9 @@ public final class ClientSessionImpl implements ClientSessionInternal, FailureLi @Override public void setAddress(final Message message, final SimpleString address) { - if (defaultAddress == null) { - logger.tracef("setAddress() Setting default address as %s", address); + logger.tracef("setAddress() Setting default address as %s", address); - message.setAddress(address); - } else { - if (!address.equals(defaultAddress)) { - logger.tracef("setAddress() setting non default address %s on message", address); - message.setAddress(address); - } else { - logger.trace("setAddress() being set as null"); - message.setAddress(null); - } - } - } - - @Override - public void checkDefaultAddress(SimpleString address) { - if (defaultAddress == null) { - logger.tracef("checkDefaultAddress(%s)", address); - defaultAddress = address; - } + message.setAddress(address); } @Override diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionInternal.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionInternal.java index ed636bd917..4e06068818 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionInternal.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionInternal.java @@ -93,8 +93,6 @@ public interface ClientSessionInternal extends ClientSession { */ void setAddress(Message message, SimpleString address); - void checkDefaultAddress(SimpleString address); - void setPacketSize(int packetSize); void resetIfNeeded() throws ActiveMQException; diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java index c72e19bff6..56c7135a32 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java @@ -629,9 +629,8 @@ public class ActiveMQSessionContext extends SessionContext { final boolean xa, final boolean autoCommitSends, final boolean autoCommitAcks, - final boolean preAcknowledge, - final SimpleString defaultAddress) throws ActiveMQException { - Packet createRequest = newCreateSession(username, password, minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, defaultAddress); + final boolean preAcknowledge) throws ActiveMQException { + Packet createRequest = newCreateSession(username, password, minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge); boolean retry; do { try { @@ -662,9 +661,8 @@ public class ActiveMQSessionContext extends SessionContext { boolean xa, boolean autoCommitSends, boolean autoCommitAcks, - boolean preAcknowledge, - SimpleString defaultAddress) { - return new CreateSessionMessage(name, sessionChannel.getID(), VersionLoader.getVersion().getIncrementingVersion(), username, password, minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, confirmationWindow, defaultAddress == null ? null : defaultAddress.toString()); + boolean preAcknowledge) { + return new CreateSessionMessage(name, sessionChannel.getID(), VersionLoader.getVersion().getIncrementingVersion(), username, password, minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, confirmationWindow, null); } @Override diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/spi/core/remoting/SessionContext.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/spi/core/remoting/SessionContext.java index 175360cd0d..1f15cc6893 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/spi/core/remoting/SessionContext.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/spi/core/remoting/SessionContext.java @@ -250,8 +250,7 @@ public abstract class SessionContext { final boolean xa, final boolean autoCommitSends, final boolean autoCommitAcks, - final boolean preAcknowledge, - final SimpleString defaultAddress) throws ActiveMQException; + final boolean preAcknowledge) throws ActiveMQException; public abstract void recreateConsumerOnServer(ClientConsumerInternal consumerInternal) throws ActiveMQException; diff --git a/artemis-protocols/artemis-hqclient-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/hornetq/client/HornetQClientSessionContext.java b/artemis-protocols/artemis-hqclient-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/hornetq/client/HornetQClientSessionContext.java index d9322744ff..caa94a10f6 100644 --- a/artemis-protocols/artemis-hqclient-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/hornetq/client/HornetQClientSessionContext.java +++ b/artemis-protocols/artemis-hqclient-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/hornetq/client/HornetQClientSessionContext.java @@ -63,9 +63,8 @@ public class HornetQClientSessionContext extends ActiveMQSessionContext { boolean xa, boolean autoCommitSends, boolean autoCommitAcks, - boolean preAcknowledge, - SimpleString defaultAddress) { - return new CreateSessionMessage(getName(), getSessionChannel().getID(), 123, username, password, minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, getConfirmationWindow(), defaultAddress == null ? null : defaultAddress.toString()); + boolean preAcknowledge) { + return new CreateSessionMessage(getName(), getSessionChannel().getID(), 123, username, password, minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, getConfirmationWindow(), null); } @Override diff --git a/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/PagingLeakTest.java b/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/PagingLeakTest.java index f9744d22ab..4ffd2bde75 100644 --- a/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/PagingLeakTest.java +++ b/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/PagingLeakTest.java @@ -34,6 +34,7 @@ import org.apache.activemq.artemis.core.server.ActiveMQServers; import org.apache.activemq.artemis.core.settings.impl.AddressFullMessagePolicy; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.apache.activemq.artemis.tests.util.Wait; import org.jboss.byteman.contrib.bmunit.BMRule; import org.jboss.byteman.contrib.bmunit.BMRules; import org.jboss.byteman.contrib.bmunit.BMUnitRunner; @@ -92,10 +93,13 @@ public class PagingLeakTest extends ActiveMQTestBase { positions.clear(); - timeout = System.currentTimeMillis() + 5000; - while (pagePosInstances.get() != 0 && timeout > System.currentTimeMillis()) { - forceGC(); - } + Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisfied() throws Exception { + forceGC(); + return pagePosInstances.get() == 0; + } + }, 5000, 100); // This is just to validate the rules are correctly applied on byteman assertEquals("You have changed something on PagePositionImpl in such way that these byteman rules are no longer working", 0, pagePosInstances.get()); @@ -110,7 +114,7 @@ public class PagingLeakTest extends ActiveMQTestBase { server.start(); - AddressSettings settings = new AddressSettings().setPageSizeBytes(2 * 1024).setMaxSizeBytes(20 * 1024).setAddressFullMessagePolicy(AddressFullMessagePolicy.PAGE); + AddressSettings settings = new AddressSettings().setPageSizeBytes(2 * 1024).setMaxSizeBytes(10 * 1024).setAddressFullMessagePolicy(AddressFullMessagePolicy.PAGE); server.getAddressSettingsRepository().addMatch("#", settings); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/ProducerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/ProducerTest.java index c409d5fb3b..d7af4b8c9c 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/ProducerTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/ProducerTest.java @@ -104,12 +104,12 @@ public class ProducerTest extends ActiveMQTestBase { ClientProducer producer = session.createProducer(); for (int i = 0; i < 62; i++) { - if (i == 61) { + if (i == 30) { // the point where the send would block latch.countDown(); } ClientMessage msg = session.createMessage(false); - msg.getBodyBuffer().writeBytes(new byte[1024]); + msg.getBodyBuffer().writeBytes(new byte[2048]); producer.send(QUEUE, msg); } } catch (Exception e) { @@ -119,7 +119,7 @@ public class ProducerTest extends ActiveMQTestBase { }; t.start(); - assertTrue(latch.await(5, TimeUnit.SECONDS)); + assertTrue(latch.await(10, TimeUnit.SECONDS)); session.close(); t.join(5000); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/failover/BackupSyncJournalTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/failover/BackupSyncJournalTest.java index 20ddae37c6..b51ff8a33b 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/failover/BackupSyncJournalTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/failover/BackupSyncJournalTest.java @@ -93,7 +93,7 @@ public class BackupSyncJournalTest extends FailoverTestBase { @Test public void testReserveFileIdValuesOnBackup() throws Exception { - final int totalRounds = 50; + final int totalRounds = 5; createProducerSendSomeMessages(); JournalImpl messageJournal = getMessageJournalFromServer(liveServer); for (int i = 0; i < totalRounds; i++) { diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java index ee1ac1118f..2dd38aeda0 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java @@ -730,7 +730,6 @@ public class NIOJournalCompactTest extends JournalImplTestBase { @Test public void testCompactAddAndUpdateFollowedByADelete() throws Exception { - setup(2, 60 * 1024, false); SimpleIDGenerator idGen = new SimpleIDGenerator(1000); @@ -779,7 +778,6 @@ public class NIOJournalCompactTest extends JournalImplTestBase { createJournal(); startJournal(); loadAndCheck(); - } @Test @@ -1610,8 +1608,9 @@ public class NIOJournalCompactTest extends JournalImplTestBase { } + @Test - public void testStressDeletesNoSync() throws Exception { + public void testStressDeletesNoSync() throws Throwable { Configuration config = createBasicConfig().setJournalFileSize(100 * 1024).setJournalSyncNonTransactional(false).setJournalSyncTransactional(false).setJournalCompactMinFiles(0).setJournalCompactPercentage(0); final AtomicInteger errors = new AtomicInteger(0); @@ -1629,114 +1628,129 @@ public class NIOJournalCompactTest extends JournalImplTestBase { final JournalStorageManager storage = new JournalStorageManager(config, factory); storage.start(); - storage.loadInternalOnly(); - ((JournalImpl) storage.getMessageJournal()).setAutoReclaim(false); - final LinkedList survivingMsgs = new LinkedList<>(); + try { + storage.loadInternalOnly(); - Runnable producerRunnable = new Runnable() { - @Override - public void run() { - try { - while (running.get()) { - final long[] values = new long[100]; - long tx = seqGenerator.incrementAndGet(); + ((JournalImpl) storage.getMessageJournal()).setAutoReclaim(false); + final LinkedList survivingMsgs = new LinkedList<>(); - OperationContextImpl ctx = new OperationContextImpl(executor); - storage.setContext(ctx); + Runnable producerRunnable = new Runnable() { + @Override + public void run() { + try { + while (running.get()) { + final long[] values = new long[100]; + long tx = seqGenerator.incrementAndGet(); - for (int i = 0; i < 100; i++) { - long id = seqGenerator.incrementAndGet(); - values[i] = id; + OperationContextImpl ctx = new OperationContextImpl(executor); + storage.setContext(ctx); - ServerMessageImpl message = new ServerMessageImpl(id, 100); + for (int i = 0; i < 100; i++) { + long id = seqGenerator.incrementAndGet(); + values[i] = id; - message.getBodyBuffer().writeBytes(new byte[1024]); + ServerMessageImpl message = new ServerMessageImpl(id, 100); - storage.storeMessageTransactional(tx, message); - } - ServerMessageImpl message = new ServerMessageImpl(seqGenerator.incrementAndGet(), 100); + message.getBodyBuffer().writeBytes(new byte[1024]); - survivingMsgs.add(message.getMessageID()); - - // This one will stay here forever - storage.storeMessage(message); - - storage.commit(tx); - - ctx.executeOnCompletion(new IOCallback() { - @Override - public void onError(int errorCode, String errorMessage) { + storage.storeMessageTransactional(tx, message); } + ServerMessageImpl message = new ServerMessageImpl(seqGenerator.incrementAndGet(), 100); - @Override - public void done() { - deleteExecutor.execute(new Runnable() { - @Override - public void run() { - try { - for (long messageID : values) { - storage.deleteMessage(messageID); + survivingMsgs.add(message.getMessageID()); + + // This one will stay here forever + storage.storeMessage(message); + + storage.commit(tx); + + ctx.executeOnCompletion(new IOCallback() { + @Override + public void onError(int errorCode, String errorMessage) { + } + + @Override + public void done() { + deleteExecutor.execute(new Runnable() { + @Override + public void run() { + try { + for (long messageID : values) { + storage.deleteMessage(messageID); + } + } catch (Exception e) { + e.printStackTrace(); + errors.incrementAndGet(); } - } catch (Exception e) { - e.printStackTrace(); - errors.incrementAndGet(); + } + }); + } + }); - } - }); - } - }); - + } + } catch (Throwable e) { + e.printStackTrace(); + errors.incrementAndGet(); } - } catch (Throwable e) { - e.printStackTrace(); - errors.incrementAndGet(); } - } - }; + }; - Runnable compressRunnable = new Runnable() { - @Override - public void run() { - try { - while (running.get()) { - Thread.sleep(500); - System.out.println("Compacting"); - ((JournalImpl) storage.getMessageJournal()).testCompact(); - ((JournalImpl) storage.getMessageJournal()).checkReclaimStatus(); + Runnable compressRunnable = new Runnable() { + @Override + public void run() { + try { + while (running.get()) { + Thread.sleep(500); + System.out.println("Compacting"); + ((JournalImpl) storage.getMessageJournal()).testCompact(); + ((JournalImpl) storage.getMessageJournal()).checkReclaimStatus(); + } + } catch (Throwable e) { + e.printStackTrace(); + errors.incrementAndGet(); } - } catch (Throwable e) { - e.printStackTrace(); - errors.incrementAndGet(); + } + }; + Thread producerThread = new Thread(producerRunnable); + producerThread.start(); + + Thread compactorThread = new Thread(compressRunnable); + compactorThread.start(); + + Thread.sleep(1000); + + running.set(false); + + producerThread.join(); + + compactorThread.join(); + + deleteExecutor.shutdown(); + + assertTrue("delete executor terminated", deleteExecutor.awaitTermination(30, TimeUnit.SECONDS)); + + executor.shutdown(); + + assertTrue("executor terminated", executor.awaitTermination(10, TimeUnit.SECONDS)); + + } catch (Throwable e) { + e.printStackTrace(); + throw e; + } finally { + try { + storage.stop(); + } catch (Exception e) { + e.printStackTrace(); } - }; - Thread producerThread = new Thread(producerRunnable); - producerThread.start(); + executor.shutdownNow(); + deleteExecutor.shutdownNow(); + } - Thread compactorThread = new Thread(compressRunnable); - compactorThread.start(); - - Thread.sleep(1000); - - running.set(false); - - producerThread.join(); - - compactorThread.join(); - - storage.stop(); - - executor.shutdown(); - - assertTrue("executor terminated", executor.awaitTermination(10, TimeUnit.SECONDS)); - - deleteExecutor.shutdown(); - - assertTrue("delete executor terminated", deleteExecutor.awaitTermination(30, TimeUnit.SECONDS)); } @Override diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/ValidateTransactionHealthTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/ValidateTransactionHealthTest.java index 1972863b12..2d3df3e616 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/ValidateTransactionHealthTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/ValidateTransactionHealthTest.java @@ -144,18 +144,21 @@ public class ValidateTransactionHealthTest extends ActiveMQTestBase { JournalImpl journal = ValidateTransactionHealthTest.createJournal(type, journalDir); journal.start(); - Loader loadTest = new Loader(numberOfRecords); - journal.load(loadTest); - Assert.assertEquals(numberOfRecords * numberOfThreads, loadTest.numberOfAdds); - Assert.assertEquals(0, loadTest.numberOfPreparedTransactions); - Assert.assertEquals(0, loadTest.numberOfUpdates); - Assert.assertEquals(0, loadTest.numberOfDeletes); + try { + Loader loadTest = new Loader(numberOfRecords); + journal.load(loadTest); + Assert.assertEquals(numberOfRecords * numberOfThreads, loadTest.numberOfAdds); + Assert.assertEquals(0, loadTest.numberOfPreparedTransactions); + Assert.assertEquals(0, loadTest.numberOfUpdates); + Assert.assertEquals(0, loadTest.numberOfDeletes); - journal.stop(); - - if (loadTest.ex != null) { - throw loadTest.ex; + if (loadTest.ex != null) { + throw loadTest.ex; + } + } finally { + journal.stop(); } + } // Inner classes ------------------------------------------------- diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingTest.java index 6f0bdc1a00..00c0bdf2bd 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingTest.java @@ -3335,7 +3335,7 @@ public class PagingTest extends ActiveMQTestBase { ClientMessage message = null; for (int i = 0; i < numberOfMessages; i++) { - byte[] body = new byte[1024]; + byte[] body = new byte[2048]; message = session.createMessage(true); message.getBodyBuffer().writeBytes(body); @@ -3360,7 +3360,7 @@ public class PagingTest extends ActiveMQTestBase { Assert.assertEquals(0, server.getPagingManager().getPageStore(PagingTest.ADDRESS).getAddressSize()); for (int i = 0; i < numberOfMessages; i++) { - byte[] body = new byte[1024]; + byte[] body = new byte[2048]; message = session.createMessage(true); message.getBodyBuffer().writeBytes(body); @@ -3385,7 +3385,7 @@ public class PagingTest extends ActiveMQTestBase { producer = session.createProducer(PagingTest.ADDRESS); for (int i = 0; i < numberOfMessages; i++) { - byte[] body = new byte[1024]; + byte[] body = new byte[2048]; message = session.createMessage(true); message.getBodyBuffer().writeBytes(body); @@ -3841,7 +3841,7 @@ public class PagingTest extends ActiveMQTestBase { Configuration config = createDefaultInVMConfig().setJournalSyncNonTransactional(false).setJournalFileSize(10 * 1024 * 1024); - server = createServer(true, config, 512 * 1024, 1024 * 1024); + server = createServer(true, config, 100 * 1024, 1024 * 1024 / 2); server.start(); @@ -4745,7 +4745,7 @@ public class PagingTest extends ActiveMQTestBase { ClientMessage message = session.createMessage(true); - int biggerMessageSize = 1024; + int biggerMessageSize = 2048; byte[] body = new byte[biggerMessageSize]; ByteBuffer bb = ByteBuffer.wrap(body); for (int j = 1; j <= biggerMessageSize; j++) { @@ -4817,7 +4817,7 @@ public class PagingTest extends ActiveMQTestBase { ClientMessage message = session.createMessage(true); - int biggerMessageSize = 1024; + int biggerMessageSize = 2048; byte[] body = new byte[biggerMessageSize]; ByteBuffer bb = ByteBuffer.wrap(body); for (int j = 1; j <= biggerMessageSize; j++) { From bfb9bedb2d7a3d8ab5e336509915eb6ecafaefb3 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Thu, 27 Oct 2016 17:30:34 -0400 Subject: [PATCH 4/7] ARTEMIS-828 Queue browsing can be out of sync while paging https://issues.apache.org/jira/browse/ARTEMIS-828 --- .../management/impl/QueueControlImpl.java | 8 +-- .../core/paging/cursor/PageSubscription.java | 5 +- .../impl/PageSubscriptionCounterImpl.java | 1 - .../cursor/impl/PageSubscriptionImpl.java | 21 ++++-- .../activemq/artemis/core/server/Queue.java | 2 +- .../artemis/core/server/impl/QueueImpl.java | 67 +++++++++++++------ .../core/server/impl/ScaleDownHandler.java | 4 +- .../core/server/impl/ServerConsumerImpl.java | 2 +- .../impl/ScheduledDeliveryHandlerTest.java | 2 +- .../integration/paging/PagingSendTest.java | 4 +- .../unit/core/postoffice/impl/FakeQueue.java | 2 +- .../unit/core/server/impl/QueueImplTest.java | 2 +- 12 files changed, 81 insertions(+), 39 deletions(-) diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java index cfa8aa5746..85bad25167 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java @@ -410,7 +410,7 @@ public class QueueControlImpl extends AbstractControl implements QueueControl { Filter filter = FilterImpl.createFilter(filterStr); List> messages = new ArrayList<>(); queue.flushExecutor(); - try (LinkedListIterator iterator = queue.totalIterator()) { + try (LinkedListIterator iterator = queue.browserIterator()) { while (iterator.hasNext()) { MessageReference ref = iterator.next(); if (filter == null || filter.match(ref.getMessage())) { @@ -446,7 +446,7 @@ public class QueueControlImpl extends AbstractControl implements QueueControl { try { List> messages = new ArrayList<>(); queue.flushExecutor(); - try (LinkedListIterator iterator = queue.totalIterator()) { + try (LinkedListIterator iterator = queue.browserIterator()) { // returns just the first, as it's the first only if (iterator.hasNext()) { MessageReference ref = iterator.next(); @@ -499,7 +499,7 @@ public class QueueControlImpl extends AbstractControl implements QueueControl { if (filter == null) { return getMessageCount(); } else { - try (LinkedListIterator iterator = queue.totalIterator()) { + try (LinkedListIterator iterator = queue.browserIterator()) { int count = 0; while (iterator.hasNext()) { MessageReference ref = iterator.next(); @@ -895,7 +895,7 @@ public class QueueControlImpl extends AbstractControl implements QueueControl { ArrayList c = new ArrayList<>(); Filter filter = FilterImpl.createFilter(filterStr); queue.flushExecutor(); - try (LinkedListIterator iterator = queue.totalIterator()) { + try (LinkedListIterator iterator = queue.browserIterator()) { while (iterator.hasNext() && currentPageSize++ < pageSize) { MessageReference ref = iterator.next(); if (filter == null || filter.match(ref.getMessage())) { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PageSubscription.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PageSubscription.java index 89c6d4462f..6e569c1d5e 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PageSubscription.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PageSubscription.java @@ -56,7 +56,10 @@ public interface PageSubscription { LinkedListIterator iterator(); - // To be called when the cursor is closed for good. Most likely when the queue is deleted + LinkedListIterator iterator(boolean jumpRemoves); + + + // To be called when the cursor is closed for good. Most likely when the queue is deleted void destroy() throws Exception; void scheduleCleanupCheck(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionCounterImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionCounterImpl.java index e01098de9b..01ad7789ab 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionCounterImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionCounterImpl.java @@ -251,7 +251,6 @@ public class PageSubscriptionCounterImpl implements PageSubscriptionCounter { recordID = -1; value.set(0); - added.set(0); incrementRecords.clear(); } } finally { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java index c1c54a26b2..063722c92d 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java @@ -351,6 +351,11 @@ final class PageSubscriptionImpl implements PageSubscription { return new CursorIterator(); } + @Override + public PageIterator iterator(boolean browsing) { + return new CursorIterator(browsing); + } + private PagedReference internalGetNext(final PagePosition pos) { PagePosition retPos = pos.nextMessage(); @@ -1100,6 +1105,8 @@ final class PageSubscriptionImpl implements PageSubscription { private volatile PagedReference lastRedelivery = null; + private final boolean browsing; + // We only store the position for redeliveries. They will be read from the SoftCache again during delivery. private final java.util.Queue redeliveries = new LinkedList<>(); @@ -1109,7 +1116,13 @@ final class PageSubscriptionImpl implements PageSubscription { */ private volatile PagedReference cachedNext; + private CursorIterator(boolean browsing) { + this.browsing = browsing; + } + + private CursorIterator() { + this.browsing = false; } @Override @@ -1199,7 +1212,7 @@ final class PageSubscriptionImpl implements PageSubscription { PageCursorInfo info = getPageInfo(message.getPosition().getPageNr()); - if (info != null && (info.isRemoved(message.getPosition()) || info.getCompleteInfo() != null)) { + if (!browsing && info != null && (info.isRemoved(message.getPosition()) || info.getCompleteInfo() != null)) { continue; } @@ -1225,7 +1238,7 @@ final class PageSubscriptionImpl implements PageSubscription { // nothing // is being changed. That's why the false is passed as a parameter here - if (info != null && info.isRemoved(message.getPosition())) { + if (!browsing && info != null && info.isRemoved(message.getPosition())) { valid = false; } } @@ -1237,10 +1250,10 @@ final class PageSubscriptionImpl implements PageSubscription { if (valid) { match = match(message.getMessage()); - if (!match) { + if (!browsing && !match) { processACK(message.getPosition()); } - } else if (ignored) { + } else if (!browsing && ignored) { positionIgnored(message.getPosition()); } } while (!match); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/Queue.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/Queue.java index 0dcef3d034..52cd2f04c4 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/Queue.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/Queue.java @@ -195,7 +195,7 @@ public interface Queue extends Bindable { */ LinkedListIterator iterator(); - LinkedListIterator totalIterator(); + LinkedListIterator browserIterator(); SimpleString getExpiryAddress(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java index b70fe8d3ea..56a33efa96 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java @@ -867,8 +867,8 @@ public class QueueImpl implements Queue { } @Override - public TotalQueueIterator totalIterator() { - return new TotalQueueIterator(); + public QueueBrowserIterator browserIterator() { + return new QueueBrowserIterator(); } @Override @@ -2863,17 +2863,23 @@ public class QueueImpl implements Queue { //Readonly (no remove) iterator over the messages in the queue, in order of //paging store, intermediateMessageReferences and MessageReferences - private class TotalQueueIterator implements LinkedListIterator { + private class QueueBrowserIterator implements LinkedListIterator { - LinkedListIterator pageIter = null; + LinkedListIterator pagingIterator = null; LinkedListIterator messagesIterator = null; + private LinkedListIterator getPagingIterator() { + if (pagingIterator == null) { + pagingIterator = pageSubscription.iterator(true); + } + return pagingIterator; + } + Iterator lastIterator = null; - private TotalQueueIterator() { - if (pageSubscription != null) { - pageIter = pageSubscription.iterator(); - } + MessageReference cachedNext = null; + + private QueueBrowserIterator() { messagesIterator = new SynchronizedIterator(messageReferences.iterator()); } @@ -2883,9 +2889,9 @@ public class QueueImpl implements Queue { lastIterator = messagesIterator; return true; } - if (pageIter != null) { - if (pageIter.hasNext()) { - lastIterator = pageIter; + if (getPagingIterator() != null) { + if (getPagingIterator().hasNext()) { + lastIterator = getPagingIterator(); return true; } } @@ -2893,16 +2899,37 @@ public class QueueImpl implements Queue { return false; } + + @Override public MessageReference next() { - if (messagesIterator != null && messagesIterator.hasNext()) { - MessageReference msg = messagesIterator.next(); - return msg; + + if (cachedNext != null) { + try { + return cachedNext; + } finally { + cachedNext = null; + } + } - if (pageIter != null) { - if (pageIter.hasNext()) { - lastIterator = pageIter; - return pageIter.next(); + while (true) { + if (messagesIterator != null && messagesIterator.hasNext()) { + MessageReference msg = messagesIterator.next(); + if (msg.isPaged()) { + System.out.println("** Rejecting because it's paged " + msg.getMessage()); + continue; + } +// System.out.println("** Returning because it's not paged " + msg.getMessage()); + return msg; + } else { + break; + } + } + if (getPagingIterator() != null) { + if (getPagingIterator().hasNext()) { + lastIterator = getPagingIterator(); + MessageReference ref = getPagingIterator().next(); + return ref; } } @@ -2922,8 +2949,8 @@ public class QueueImpl implements Queue { @Override public void close() { - if (pageIter != null) { - pageIter.close(); + if (getPagingIterator() != null) { + getPagingIterator().close(); } if (messagesIterator != null) { messagesIterator.close(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java index b763ff25ea..dc62676e3f 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java @@ -165,7 +165,7 @@ public class ScaleDownHandler { for (Queue loopQueue : queues) { logger.debug("Scaling down messages on address " + address + " / performing loop on queue " + loopQueue); - try (LinkedListIterator messagesIterator = loopQueue.totalIterator()) { + try (LinkedListIterator messagesIterator = loopQueue.browserIterator()) { while (messagesIterator.hasNext()) { MessageReference messageReference = messagesIterator.next(); @@ -249,7 +249,7 @@ public class ScaleDownHandler { for (Queue queue : queues) { // using auto-closeable - try (LinkedListIterator messagesIterator = queue.totalIterator()) { + try (LinkedListIterator messagesIterator = queue.browserIterator()) { // loop through every message of this queue while (messagesIterator.hasNext()) { MessageReference messageRef = messagesIterator.next(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java index 1318ff31df..98a9c8480a 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java @@ -206,7 +206,7 @@ public class ServerConsumerImpl implements ServerConsumer, ReadyListener { this.creationTime = System.currentTimeMillis(); if (browseOnly) { - browserDeliverer = new BrowserDeliverer(messageQueue.totalIterator()); + browserDeliverer = new BrowserDeliverer(messageQueue.browserIterator()); } else { messageQueue.addConsumer(this); } diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ScheduledDeliveryHandlerTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ScheduledDeliveryHandlerTest.java index d82f7d3b77..55a287ad86 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ScheduledDeliveryHandlerTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ScheduledDeliveryHandlerTest.java @@ -1212,7 +1212,7 @@ public class ScheduledDeliveryHandlerTest extends Assert { } @Override - public LinkedListIterator totalIterator() { + public LinkedListIterator browserIterator() { return null; } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingSendTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingSendTest.java index ca8a9a1d1b..1f0d7e01a2 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingSendTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/paging/PagingSendTest.java @@ -308,7 +308,7 @@ public class PagingSendTest extends ActiveMQTestBase { * duplicates that may have happened before this point). */ public void checkBatchMessagesAreNotPagedTwice(Queue queue) throws Exception { - LinkedListIterator pageIterator = queue.totalIterator(); + LinkedListIterator pageIterator = queue.browserIterator(); Set messageOrderSet = new HashSet<>(); @@ -344,7 +344,7 @@ public class PagingSendTest extends ActiveMQTestBase { * duplicates that may have happened before this point). */ protected int processCountThroughIterator(Queue queue) throws Exception { - LinkedListIterator pageIterator = queue.totalIterator(); + LinkedListIterator pageIterator = queue.browserIterator(); int count = 0; while (pageIterator.hasNext()) { diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/FakeQueue.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/FakeQueue.java index bba5dc174e..9a20d70b64 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/FakeQueue.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/FakeQueue.java @@ -604,7 +604,7 @@ public class FakeQueue implements Queue { } @Override - public LinkedListIterator totalIterator() { + public LinkedListIterator browserIterator() { // TODO Auto-generated method stub return null; } diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/server/impl/QueueImplTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/server/impl/QueueImplTest.java index b9bdba7cf0..804429fadf 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/server/impl/QueueImplTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/server/impl/QueueImplTest.java @@ -1274,7 +1274,7 @@ public class QueueImplTest extends ActiveMQTestBase { locator.close(); Queue queue = ((LocalQueueBinding) server.getPostOffice().getBinding(new SimpleString(MY_QUEUE))).getQueue(); - LinkedListIterator totalIterator = queue.totalIterator(); + LinkedListIterator totalIterator = queue.browserIterator(); try { int i = 0; From 4b47461f03a607b9ef517beb2a1666ffae43a2a7 Mon Sep 17 00:00:00 2001 From: barreiro Date: Fri, 22 Jan 2016 03:23:26 +0000 Subject: [PATCH 5/7] ARTEMIS-822 Add executor service to JournalImpl for append operations and remove synchronization https://issues.apache.org/jira/browse/ARTEMIS-822 --- .../cli/commands/tools/DecodeJournal.java | 20 +- .../artemis/utils/ExecutorFactory.java | 0 .../artemis/utils/OrderedExecutorFactory.java | 3 +- .../activemq/artemis/utils/SimpleFuture.java | 79 ++ .../artemis/utils/SimpleFutureTest.java | 69 ++ .../core/journal/impl/JournalImpl.java | 684 ++++++++++-------- .../core/journal/impl/JournalTransaction.java | 46 +- .../journal/ActiveMQJournalLogger.java | 12 +- .../journal/impl/AlignedJournalImplTest.java | 39 +- .../core/journal/impl/JournalAsyncTest.java | 15 +- 10 files changed, 622 insertions(+), 345 deletions(-) rename {artemis-core-client => artemis-commons}/src/main/java/org/apache/activemq/artemis/utils/ExecutorFactory.java (100%) rename {artemis-core-client => artemis-commons}/src/main/java/org/apache/activemq/artemis/utils/OrderedExecutorFactory.java (96%) create mode 100644 artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SimpleFuture.java create mode 100644 artemis-commons/src/test/java/org/apache/activemq/artemis/utils/SimpleFutureTest.java diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DecodeJournal.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DecodeJournal.java index b392f6ffb0..f290eba623 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DecodeJournal.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DecodeJournal.java @@ -33,7 +33,6 @@ import org.apache.activemq.artemis.cli.commands.ActionContext; import org.apache.activemq.artemis.core.io.nio.NIOSequentialFileFactory; import org.apache.activemq.artemis.core.journal.RecordInfo; import org.apache.activemq.artemis.core.journal.impl.JournalImpl; -import org.apache.activemq.artemis.core.journal.impl.JournalRecord; import org.apache.activemq.artemis.utils.Base64; @Command(name = "decode", description = "Decode a journal's internal format into a new journal set of files") @@ -125,8 +124,6 @@ public class DecodeJournal extends LockAbstract { long lineNumber = 0; - Map journalRecords = journal.getRecords(); - while ((line = buffReader.readLine()) != null) { lineNumber++; String[] splitLine = line.split(","); @@ -150,12 +147,6 @@ public class DecodeJournal extends LockAbstract { counter.incrementAndGet(); RecordInfo info = parseRecord(lineProperties); journal.appendAddRecordTransactional(txID, info.id, info.userRecordType, info.data); - } else if (operation.equals("AddRecordTX")) { - long txID = parseLong("txID", lineProperties); - AtomicInteger counter = getCounter(txID, txCounters); - counter.incrementAndGet(); - RecordInfo info = parseRecord(lineProperties); - journal.appendAddRecordTransactional(txID, info.id, info.userRecordType, info.data); } else if (operation.equals("UpdateTX")) { long txID = parseLong("txID", lineProperties); AtomicInteger counter = getCounter(txID, txCounters); @@ -168,20 +159,17 @@ public class DecodeJournal extends LockAbstract { } else if (operation.equals("DeleteRecord")) { long id = parseLong("id", lineProperties); - // If not found it means the append/update records were reclaimed already - if (journalRecords.get(id) != null) { + try { journal.appendDeleteRecord(id, false); + } catch (IllegalStateException ignored) { + // If not found it means the append/update records were reclaimed already } } else if (operation.equals("DeleteRecordTX")) { long txID = parseLong("txID", lineProperties); long id = parseLong("id", lineProperties); AtomicInteger counter = getCounter(txID, txCounters); counter.incrementAndGet(); - - // If not found it means the append/update records were reclaimed already - if (journalRecords.get(id) != null) { - journal.appendDeleteRecordTransactional(txID, id); - } + journal.appendDeleteRecordTransactional(txID, id); } else if (operation.equals("Prepare")) { long txID = parseLong("txID", lineProperties); int numberOfRecords = parseInt("numberOfRecords", lineProperties); diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/ExecutorFactory.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ExecutorFactory.java similarity index 100% rename from artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/ExecutorFactory.java rename to artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ExecutorFactory.java diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/OrderedExecutorFactory.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/OrderedExecutorFactory.java similarity index 96% rename from artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/OrderedExecutorFactory.java rename to artemis-commons/src/main/java/org/apache/activemq/artemis/utils/OrderedExecutorFactory.java index 609af8ead2..c7d5c03166 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/OrderedExecutorFactory.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/OrderedExecutorFactory.java @@ -22,7 +22,6 @@ import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import org.apache.activemq.artemis.api.core.ActiveMQInterruptedException; -import org.apache.activemq.artemis.core.client.ActiveMQClientLogger; import org.jboss.logging.Logger; /** @@ -104,7 +103,7 @@ public final class OrderedExecutorFactory implements ExecutorFactory { // This could happen during shutdowns. Nothing to be concerned about here logger.debug("Interrupted Thread", e); } catch (Throwable t) { - ActiveMQClientLogger.LOGGER.caughtunexpectedThrowable(t); + logger.warn(t.getMessage(), t); } task = tasks.poll(); } diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SimpleFuture.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SimpleFuture.java new file mode 100644 index 0000000000..eedfef492f --- /dev/null +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/SimpleFuture.java @@ -0,0 +1,79 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.activemq.artemis.utils; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +public class SimpleFuture implements Future { + + public SimpleFuture() { + } + + V value; + Exception exception; + + private final CountDownLatch latch = new CountDownLatch(1); + + boolean canceled = false; + + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + canceled = true; + latch.countDown(); + return true; + } + + @Override + public boolean isCancelled() { + return canceled; + } + + @Override + public boolean isDone() { + return latch.getCount() <= 0; + } + + public void fail(Exception e) { + this.exception = e; + latch.countDown(); + } + + @Override + public V get() throws InterruptedException, ExecutionException { + latch.await(); + if (this.exception != null) { + throw new ExecutionException(this.exception); + } + return value; + } + + public void set(V v) { + this.value = v; + latch.countDown(); + } + + @Override + public V get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { + latch.await(timeout, unit); + return value; + } +} diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/SimpleFutureTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/SimpleFutureTest.java new file mode 100644 index 0000000000..00fd5d7dc4 --- /dev/null +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/SimpleFutureTest.java @@ -0,0 +1,69 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.activemq.artemis.utils; + +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; + +public class SimpleFutureTest { + + @Rule + public ThreadLeakCheckRule threadLeakCheckRule = new ThreadLeakCheckRule(); + + @Test + public void testFuture() throws Exception { + final long randomStart = System.currentTimeMillis(); + final SimpleFuture simpleFuture = new SimpleFuture<>(); + Thread t = new Thread() { + @Override + public void run() { + simpleFuture.set(randomStart); + } + }; + t.start(); + + Assert.assertEquals(randomStart, simpleFuture.get().longValue()); + } + + + @Test + public void testException() throws Exception { + final SimpleFuture simpleFuture = new SimpleFuture<>(); + Thread t = new Thread() { + @Override + public void run() { + simpleFuture.fail(new Exception("hello")); + } + }; + t.start(); + + boolean failed = false; + try { + simpleFuture.get(); + } catch (Exception e) { + failed = true; + } + + + Assert.assertTrue(failed); + } + + + +} diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java index b6d5e622c2..43db1f7033 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java @@ -29,11 +29,13 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -45,6 +47,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import org.apache.activemq.artemis.api.core.ActiveMQBuffer; import org.apache.activemq.artemis.api.core.ActiveMQBuffers; import org.apache.activemq.artemis.api.core.Pair; +import org.apache.activemq.artemis.api.core.ActiveMQExceptionType; import org.apache.activemq.artemis.core.io.IOCallback; import org.apache.activemq.artemis.core.io.SequentialFile; import org.apache.activemq.artemis.core.io.SequentialFileFactory; @@ -160,6 +163,8 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal // Compacting may replace this structure private final ConcurrentMap records = new ConcurrentHashMap<>(); + private final Set pendingRecords = Collections.newSetFromMap(new ConcurrentHashMap()); + // Compacting may replace this structure private final ConcurrentMap transactions = new ConcurrentHashMap<>(); @@ -172,12 +177,9 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal private ExecutorService compactorExecutor = null; - private ConcurrentHashSet latches = new ConcurrentHashSet<>(); + private ExecutorService appendExecutor = null; - // Lock used during the append of records - // This lock doesn't represent a global lock. - // After a record is appended, the usedFile can't be changed until the positives and negatives are updated - private final Object lockAppend = new Object(); + private ConcurrentHashSet latches = new ConcurrentHashSet<>(); /** * We don't lock the journal during the whole compacting operation. During compacting we only @@ -688,32 +690,37 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final boolean sync, final IOCompletion callback) throws Exception { checkJournalIsLoaded(); + lineUpContext(callback); + pendingRecords.add(id); - journalLock.readLock().lock(); + Future result = appendExecutor.submit(new Runnable() { + @Override + public void run() { + journalLock.readLock().lock(); + try { + JournalInternalRecord addRecord = new JournalAddRecord(true, id, recordType, record); + JournalFile usedFile = appendRecord(addRecord, false, sync, null, callback); + records.put(id, new JournalRecord(usedFile, addRecord.getEncodeSize())); - try { - JournalInternalRecord addRecord = new JournalAddRecord(true, id, recordType, record); - - if (callback != null) { - callback.storeLineUp(); - } - - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(addRecord, false, sync, null, callback); - - if (logger.isTraceEnabled()) { - logger.trace("appendAddRecord::id=" + id + - ", userRecordType=" + - recordType + - ", record = " + record + - ", usedFile = " + - usedFile); + if (logger.isTraceEnabled()) { + logger.trace("appendAddRecord::id=" + id + + ", userRecordType=" + + recordType + + ", record = " + record + + ", usedFile = " + + usedFile); + } + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + } finally { + pendingRecords.remove(id); + journalLock.readLock().unlock(); } - - records.put(id, new JournalRecord(usedFile, addRecord.getEncodeSize())); } - } finally { - journalLock.readLock().unlock(); + }); + + if (sync && callback == null) { + result.get(); } } @@ -724,94 +731,86 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final boolean sync, final IOCompletion callback) throws Exception { checkJournalIsLoaded(); + lineUpContext(callback); + checkKnownRecordID(id); - journalLock.readLock().lock(); + Future result = appendExecutor.submit(new Runnable() { + @Override + public void run() { + journalLock.readLock().lock(); + try { + JournalRecord jrnRecord = records.get(id); + JournalInternalRecord updateRecord = new JournalAddRecord(false, id, recordType, record); + JournalFile usedFile = appendRecord(updateRecord, false, sync, null, callback); - try { - JournalRecord jrnRecord = records.get(id); + if (logger.isTraceEnabled()) { + logger.trace("appendUpdateRecord::id=" + id + + ", userRecordType=" + + recordType + + ", usedFile = " + + usedFile); + } - if (jrnRecord == null) { - if (!(compactor != null && compactor.lookupRecord(id))) { - throw new IllegalStateException("Cannot find add info " + id); + // record==null here could only mean there is a compactor + // computing the delete should be done after compacting is done + if (jrnRecord == null) { + compactor.addCommandUpdate(id, usedFile, updateRecord.getEncodeSize()); + } else { + jrnRecord.addUpdateFile(usedFile, updateRecord.getEncodeSize()); + } + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + } finally { + journalLock.readLock().unlock(); } } + }); - JournalInternalRecord updateRecord = new JournalAddRecord(false, id, recordType, record); - - if (callback != null) { - callback.storeLineUp(); - } - - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(updateRecord, false, sync, null, callback); - - if (logger.isTraceEnabled()) { - logger.trace("appendUpdateRecord::id=" + id + - ", userRecordType=" + - recordType + - ", record = " + record + - ", usedFile = " + - usedFile); - } - - // record== null here could only mean there is a compactor, and computing the delete should be done after - // compacting is done - if (jrnRecord == null) { - compactor.addCommandUpdate(id, usedFile, updateRecord.getEncodeSize()); - } else { - jrnRecord.addUpdateFile(usedFile, updateRecord.getEncodeSize()); - } - } - } finally { - journalLock.readLock().unlock(); + if (sync && callback == null) { + result.get(); } } @Override public void appendDeleteRecord(final long id, final boolean sync, final IOCompletion callback) throws Exception { checkJournalIsLoaded(); + lineUpContext(callback); + checkKnownRecordID(id); - journalLock.readLock().lock(); - try { + Future result = appendExecutor.submit(new Runnable() { + @Override + public void run() { + journalLock.readLock().lock(); + try { + JournalRecord record = null; + if (compactor == null) { + record = records.remove(id); + } - JournalRecord record = null; + JournalInternalRecord deleteRecord = new JournalDeleteRecord(id); + JournalFile usedFile = appendRecord(deleteRecord, false, sync, null, callback); - if (compactor == null) { - record = records.remove(id); + if (logger.isTraceEnabled()) { + logger.trace("appendDeleteRecord::id=" + id + ", usedFile = " + usedFile); + } - if (record == null) { - throw new IllegalStateException("Cannot find add info " + id); - } - } else { - if (!records.containsKey(id) && !compactor.lookupRecord(id)) { - throw new IllegalStateException("Cannot find add info " + id + " on compactor or current records"); + // record==null here could only mean there is a compactor + // computing the delete should be done after compacting is done + if (record == null) { + compactor.addCommandDelete(id, usedFile); + } else { + record.delete(usedFile); + } + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + } finally { + journalLock.readLock().unlock(); } } + }); - JournalInternalRecord deleteRecord = new JournalDeleteRecord(id); - - if (callback != null) { - callback.storeLineUp(); - } - - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(deleteRecord, false, sync, null, callback); - - if (logger.isTraceEnabled()) { - logger.trace("appendDeleteRecord::id=" + id + ", usedFile = " + usedFile); - } - - // record== null here could only mean there is a compactor, and computing the delete should be done after - // compacting is done - if (record == null) { - compactor.addCommandDelete(id, usedFile); - } else { - record.delete(usedFile); - } - - } - } finally { - journalLock.readLock().unlock(); + if (sync && callback == null) { + result.get(); } } @@ -822,31 +821,62 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final EncodingSupport record) throws Exception { checkJournalIsLoaded(); - journalLock.readLock().lock(); + final JournalTransaction tx = getTransactionInfo(txID); + tx.checkErrorCondition(); - try { - JournalInternalRecord addRecord = new JournalAddRecordTX(true, txID, id, recordType, record); + appendExecutor.submit(new Runnable() { - JournalTransaction tx = getTransactionInfo(txID); + @Override + public void run() { + journalLock.readLock().lock(); + try { + JournalInternalRecord addRecord = new JournalAddRecordTX(true, txID, id, recordType, record); + JournalFile usedFile = appendRecord(addRecord, false, false, tx, null); - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(addRecord, false, false, tx, null); + if (logger.isTraceEnabled()) { + logger.trace("appendAddRecordTransactional:txID=" + txID + + ",id=" + + id + + ", userRecordType=" + + recordType + + ", record = " + record + + ", usedFile = " + + usedFile); + } - if (logger.isTraceEnabled()) { - logger.trace("appendAddRecordTransactional:txID=" + txID + - ",id=" + - id + - ", userRecordType=" + - recordType + - ", record = " + record + - ", usedFile = " + - usedFile); + tx.addPositive(usedFile, id, addRecord.getEncodeSize()); + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + setErrorCondition(tx, e); + } finally { + journalLock.readLock().unlock(); } - - tx.addPositive(usedFile, id, addRecord.getEncodeSize()); } - } finally { - journalLock.readLock().unlock(); + }); + } + + private void checkKnownRecordID(final long id) throws Exception { + if (records.containsKey(id) || pendingRecords.contains(id) || (compactor != null && compactor.lookupRecord(id))) { + return; + } + + // retry on the append thread. maybe the appender thread is not keeping up. + Future known = appendExecutor.submit(new Callable() { + @Override + public Boolean call() throws Exception { + journalLock.readLock().lock(); + try { + return records.containsKey(id) + || pendingRecords.contains(id) + || (compactor != null && compactor.lookupRecord(id)); + } finally { + journalLock.readLock().unlock(); + } + } + }); + + if (!known.get()) { + throw new IllegalStateException("Cannot find add info " + id + " on compactor or current records"); } } @@ -867,32 +897,39 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final EncodingSupport record) throws Exception { checkJournalIsLoaded(); - journalLock.readLock().lock(); + final JournalTransaction tx = getTransactionInfo(txID); + tx.checkErrorCondition(); - try { - JournalInternalRecord updateRecordTX = new JournalAddRecordTX(false, txID, id, recordType, record); + appendExecutor.submit(new Runnable() { - JournalTransaction tx = getTransactionInfo(txID); + @Override + public void run() { + journalLock.readLock().lock(); + try { - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(updateRecordTX, false, false, tx, null); + JournalInternalRecord updateRecordTX = new JournalAddRecordTX( false, txID, id, recordType, record ); + JournalFile usedFile = appendRecord( updateRecordTX, false, false, tx, null ); - if (logger.isTraceEnabled()) { - logger.trace("appendUpdateRecordTransactional::txID=" + txID + - ",id=" + - id + - ", userRecordType=" + - recordType + - ", record = " + record + - ", usedFile = " + - usedFile); + if ( logger.isTraceEnabled() ) { + logger.trace( "appendUpdateRecordTransactional::txID=" + txID + + ",id=" + + id + + ", userRecordType=" + + recordType + + ", record = " + record + + ", usedFile = " + + usedFile ); + } + + tx.addPositive( usedFile, id, updateRecordTX.getEncodeSize() ); + } catch ( Exception e ) { + ActiveMQJournalLogger.LOGGER.error( e.getMessage(), e ); + setErrorCondition( tx, e ); + } finally { + journalLock.readLock().unlock(); } - - tx.addPositive(usedFile, id, updateRecordTX.getEncodeSize()); } - } finally { - journalLock.readLock().unlock(); - } + }); } @Override @@ -901,29 +938,35 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final EncodingSupport record) throws Exception { checkJournalIsLoaded(); - journalLock.readLock().lock(); + final JournalTransaction tx = getTransactionInfo(txID); + tx.checkErrorCondition(); - try { - JournalInternalRecord deleteRecordTX = new JournalDeleteRecordTX(txID, id, record); + appendExecutor.submit(new Runnable() { + @Override + public void run() { + journalLock.readLock().lock(); + try { - JournalTransaction tx = getTransactionInfo(txID); + JournalInternalRecord deleteRecordTX = new JournalDeleteRecordTX(txID, id, record); + JournalFile usedFile = appendRecord(deleteRecordTX, false, false, tx, null); - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(deleteRecordTX, false, false, tx, null); + if (logger.isTraceEnabled()) { + logger.trace("appendDeleteRecordTransactional::txID=" + txID + + ", id=" + + id + + ", usedFile = " + + usedFile); + } - if (logger.isTraceEnabled()) { - logger.trace("appendDeleteRecordTransactional::txID=" + txID + - ", id=" + - id + - ", usedFile = " + - usedFile); + tx.addNegative(usedFile, id); + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + setErrorCondition(tx, e); + } finally { + journalLock.readLock().unlock(); } - - tx.addNegative(usedFile, id); } - } finally { - journalLock.readLock().unlock(); - } + }); } /** @@ -943,36 +986,53 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final IOCompletion callback) throws Exception { checkJournalIsLoaded(); + lineUpContext(callback); - journalLock.readLock().lock(); + final JournalTransaction tx = getTransactionInfo(txID); + tx.checkErrorCondition(); - try { - JournalTransaction tx = getTransactionInfo(txID); + Future result = appendExecutor.submit(new Runnable() { + @Override + public void run() { + journalLock.readLock().lock(); + try { + JournalInternalRecord prepareRecord = new JournalCompleteRecordTX(TX_RECORD_TYPE.PREPARE, txID, transactionData); + JournalFile usedFile = appendRecord(prepareRecord, true, sync, tx, callback); - JournalInternalRecord prepareRecord = new JournalCompleteRecordTX(TX_RECORD_TYPE.PREPARE, txID, transactionData); + if (logger.isTraceEnabled()) { + logger.trace("appendPrepareRecord::txID=" + txID + ", usedFile = " + usedFile); + } - if (callback != null) { - callback.storeLineUp(); - } - - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(prepareRecord, true, sync, tx, callback); - - if (logger.isTraceEnabled()) { - logger.trace("appendPrepareRecord::txID=" + txID + ", usedFile = " + usedFile); + tx.prepare(usedFile); + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + setErrorCondition(tx, e); + } finally { + journalLock.readLock().unlock(); } - - tx.prepare(usedFile); } + }); - } finally { - journalLock.readLock().unlock(); + if (sync && callback == null) { + result.get(); + tx.checkErrorCondition(); } } @Override public void lineUpContext(IOCompletion callback) { - callback.storeLineUp(); + if (callback != null) { + callback.storeLineUp(); + } + } + + private void setErrorCondition(JournalTransaction jt, Throwable t) { + if (jt != null) { + TransactionCallback callback = jt.getCurrentCallback(); + if (callback != null && callback.getErrorMessage() != null) { + callback.onError(ActiveMQExceptionType.IO_ERROR.getCode(), t.getMessage()); + } + } } /** @@ -982,68 +1042,83 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal public void appendCommitRecord(final long txID, final boolean sync, final IOCompletion callback, - boolean lineUpContext) throws Exception { + final boolean lineUpContext) throws Exception { checkJournalIsLoaded(); + if (lineUpContext) { + lineUpContext(callback); + } - journalLock.readLock().lock(); + final JournalTransaction tx = transactions.remove(txID); - try { - JournalTransaction tx = transactions.remove(txID); + if (tx == null) { + throw new IllegalStateException("Cannot find tx with id " + txID); + } - if (tx == null) { - throw new IllegalStateException("Cannot find tx with id " + txID); - } + tx.checkErrorCondition(); - JournalInternalRecord commitRecord = new JournalCompleteRecordTX(TX_RECORD_TYPE.COMMIT, txID, null); + Future result = appendExecutor.submit(new Runnable() { + @Override + public void run() { + journalLock.readLock().lock(); + try { + JournalInternalRecord commitRecord = new JournalCompleteRecordTX(TX_RECORD_TYPE.COMMIT, txID, null); + JournalFile usedFile = appendRecord(commitRecord, true, sync, tx, callback); - if (callback != null && lineUpContext) { - callback.storeLineUp(); - } - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(commitRecord, true, sync, tx, callback); + if (logger.isTraceEnabled()) { + logger.trace("appendCommitRecord::txID=" + txID + ", usedFile = " + usedFile); + } - if (logger.isTraceEnabled()) { - logger.trace("appendCommitRecord::txID=" + txID + ", usedFile = " + usedFile); + tx.commit(usedFile); + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + setErrorCondition(tx, e); + } finally { + journalLock.readLock().unlock(); } - - tx.commit(usedFile); } + }); - } finally { - journalLock.readLock().unlock(); + if (sync && callback == null) { + result.get(); + tx.checkErrorCondition(); } } @Override public void appendRollbackRecord(final long txID, final boolean sync, final IOCompletion callback) throws Exception { checkJournalIsLoaded(); + lineUpContext(callback); - journalLock.readLock().lock(); + final JournalTransaction tx = transactions.remove(txID); - JournalTransaction tx = null; + if (tx == null) { + throw new IllegalStateException("Cannot find tx with id " + txID); + } - try { - tx = transactions.remove(txID); + tx.checkErrorCondition(); - if (tx == null) { - throw new IllegalStateException("Cannot find tx with id " + txID); + Future result = appendExecutor.submit(new Runnable() { + @Override + public void run() { + journalLock.readLock().lock(); + try { + JournalInternalRecord rollbackRecord = new JournalRollbackRecordTX(txID); + JournalFile usedFile = appendRecord(rollbackRecord, false, sync, tx, callback); + + tx.rollback(usedFile); + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + setErrorCondition(tx, e); + } finally { + journalLock.readLock().unlock(); + } } + }); - JournalInternalRecord rollbackRecord = new JournalRollbackRecordTX(txID); - - if (callback != null) { - callback.storeLineUp(); - } - - synchronized (lockAppend) { - JournalFile usedFile = appendRecord(rollbackRecord, false, sync, tx, callback); - - tx.rollback(usedFile); - } - - } finally { - journalLock.readLock().unlock(); + if (sync && callback == null) { + result.get(); + tx.checkErrorCondition(); } } @@ -1906,13 +1981,23 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal public void debugWait() throws InterruptedException { fileFactory.flush(); - for (JournalTransaction tx : transactions.values()) { - tx.waitCallbacks(); + if (appendExecutor != null && !appendExecutor.isShutdown()) { + // Send something to the closingExecutor, just to make sure we went until its end + final CountDownLatch latch = newLatch(1); + + appendExecutor.execute(new Runnable() { + + @Override + public void run() { + latch.countDown(); + } + + }); + awaitLatch(latch, -1); } if (filesExecutor != null && !filesExecutor.isShutdown()) { - // Send something to the closingExecutor, just to make sure we went - // until its end + // Send something to the closingExecutor, just to make sure we went until its end final CountDownLatch latch = newLatch(1); filesExecutor.execute(new Runnable() { @@ -1985,20 +2070,52 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal // In some tests we need to force the journal to move to a next file @Override public void forceMoveNextFile() throws Exception { - journalLock.readLock().lock(); + debugWait(); + journalLock.writeLock().lock(); try { - synchronized (lockAppend) { - moveNextFile(false); - debugWait(); - } + moveNextFile(false); } finally { - journalLock.readLock().unlock(); + journalLock.writeLock().unlock(); } } @Override public void perfBlast(final int pages) { - new PerfBlast(pages).start(); + + checkJournalIsLoaded(); + + final ByteArrayEncoding byteEncoder = new ByteArrayEncoding(new byte[128 * 1024]); + + final JournalInternalRecord blastRecord = new JournalInternalRecord() { + + @Override + public int getEncodeSize() { + return byteEncoder.getEncodeSize(); + } + + @Override + public void encode(final ActiveMQBuffer buffer) { + byteEncoder.encode(buffer); + } + }; + + appendExecutor.submit(new Runnable() { + @Override + public void run() { + journalLock.readLock().lock(); + try { + + for (int i = 0; i < pages; i++) { + appendRecord(blastRecord, false, false, null, null); + } + + } catch (Exception e) { + ActiveMQJournalLogger.LOGGER.failedToPerfBlast(e); + } finally { + journalLock.readLock().unlock(); + } + } + }); } // ActiveMQComponent implementation @@ -2031,6 +2148,14 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }); + appendExecutor = Executors.newSingleThreadExecutor(new ThreadFactory() { + + @Override + public Thread newThread(final Runnable r) { + return new Thread(r, "JournalImpl::appendExecutor"); + } + }); + filesRepository.setExecutor(filesExecutor); fileFactory.start(); @@ -2044,46 +2169,50 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal throw new IllegalStateException("Journal is already stopped"); } + setJournalState(JournalState.STOPPED); + + // appendExecutor must be shut down first + appendExecutor.shutdown(); + + if (!appendExecutor.awaitTermination(60, TimeUnit.SECONDS)) { + ActiveMQJournalLogger.LOGGER.couldNotStopJournalAppendExecutor(); + } + journalLock.writeLock().lock(); try { - synchronized (lockAppend) { + compactorExecutor.shutdown(); - setJournalState(JournalState.STOPPED); - - compactorExecutor.shutdown(); - - if (!compactorExecutor.awaitTermination(120, TimeUnit.SECONDS)) { - ActiveMQJournalLogger.LOGGER.couldNotStopCompactor(); - } - - filesExecutor.shutdown(); - - filesRepository.setExecutor(null); - - if (!filesExecutor.awaitTermination(60, TimeUnit.SECONDS)) { - ActiveMQJournalLogger.LOGGER.couldNotStopJournalExecutor(); - } - - try { - for (CountDownLatch latch : latches) { - latch.countDown(); - } - } catch (Throwable e) { - ActiveMQJournalLogger.LOGGER.warn(e.getMessage(), e); - } - - fileFactory.deactivateBuffer(); - - if (currentFile != null && currentFile.getFile().isOpen()) { - currentFile.getFile().close(); - } - - filesRepository.clear(); - - fileFactory.stop(); - - currentFile = null; + if (!compactorExecutor.awaitTermination(120, TimeUnit.SECONDS)) { + ActiveMQJournalLogger.LOGGER.couldNotStopCompactor(); } + + filesExecutor.shutdown(); + + filesRepository.setExecutor(null); + + if (!filesExecutor.awaitTermination(60, TimeUnit.SECONDS)) { + ActiveMQJournalLogger.LOGGER.couldNotStopJournalExecutor(); + } + + try { + for (CountDownLatch latch : latches) { + latch.countDown(); + } + } catch (Throwable e) { + ActiveMQJournalLogger.LOGGER.warn(e.getMessage(), e); + } + + fileFactory.deactivateBuffer(); + + if (currentFile != null && currentFile.getFile().isOpen()) { + currentFile.getFile().close(); + } + + filesRepository.clear(); + + fileFactory.stop(); + + currentFile = null; } finally { journalLock.writeLock().unlock(); } @@ -2358,7 +2487,6 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final boolean sync, final JournalTransaction tx, final IOCallback parameterCallback) throws Exception { - checkJournalIsLoaded(); final IOCallback callback; @@ -2552,46 +2680,6 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } } - private final class PerfBlast extends Thread { - - private final int pages; - - private PerfBlast(final int pages) { - super("activemq-perfblast-thread"); - - this.pages = pages; - } - - @Override - public void run() { - synchronized (lockAppend) { - try { - - final ByteArrayEncoding byteEncoder = new ByteArrayEncoding(new byte[128 * 1024]); - - JournalInternalRecord blastRecord = new JournalInternalRecord() { - - @Override - public int getEncodeSize() { - return byteEncoder.getEncodeSize(); - } - - @Override - public void encode(final ActiveMQBuffer buffer) { - byteEncoder.encode(buffer); - } - }; - - for (int i = 0; i < pages; i++) { - appendRecord(blastRecord, false, false, null, null); - } - } catch (Exception e) { - ActiveMQJournalLogger.LOGGER.failedToPerfBlast(e); - } - } - } - } - @Override public final void synchronizationLock() { compactorLock.writeLock().lock(); @@ -2624,7 +2712,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal long maxID = -1; for (long id : fileIds) { maxID = Math.max(maxID, id); - map.put(Long.valueOf(id), filesRepository.createRemoteBackupSyncFile(id)); + map.put(id, filesRepository.createRemoteBackupSyncFile(id)); } filesRepository.setNextFileID(maxID); return map; diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalTransaction.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalTransaction.java index 6e41c1799c..1542bd4b05 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalTransaction.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalTransaction.java @@ -17,11 +17,13 @@ package org.apache.activemq.artemis.core.journal.impl; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; import org.apache.activemq.artemis.api.core.ActiveMQExceptionType; @@ -45,12 +47,14 @@ public class JournalTransaction { private boolean compacting = false; - private Map callbackList; + private final Map callbackList = Collections.synchronizedMap(new HashMap()); private JournalFile lastFile = null; private final AtomicInteger counter = new AtomicInteger(); + private CountDownLatch firstCallbackLatch; + public JournalTransaction(final long id, final JournalRecordProvider journal) { this.id = id; this.journal = journal; @@ -139,9 +143,7 @@ public class JournalTransaction { pendingFiles.clear(); } - if (callbackList != null) { - callbackList.clear(); - } + callbackList.clear(); if (pos != null) { pos.clear(); @@ -156,6 +158,8 @@ public class JournalTransaction { lastFile = null; currentCallback = null; + + firstCallbackLatch = null; } /** @@ -166,9 +170,13 @@ public class JournalTransaction { data.setNumberOfRecords(getCounter(currentFile)); } + public TransactionCallback getCurrentCallback() { + return currentCallback; + } + public TransactionCallback getCallback(final JournalFile file) throws Exception { - if (callbackList == null) { - callbackList = new HashMap<>(); + if (firstCallbackLatch != null && callbackList.isEmpty()) { + firstCallbackLatch.countDown(); } currentCallback = callbackList.get(file); @@ -178,15 +186,19 @@ public class JournalTransaction { callbackList.put(file, currentCallback); } - if (currentCallback.getErrorMessage() != null) { - throw ActiveMQExceptionType.createException(currentCallback.getErrorCode(), currentCallback.getErrorMessage()); - } - currentCallback.countUp(); return currentCallback; } + public void checkErrorCondition() throws Exception { + if (currentCallback != null) { + if (currentCallback.getErrorMessage() != null) { + throw ActiveMQExceptionType.createException(currentCallback.getErrorCode(), currentCallback.getErrorMessage()); + } + } + } + public void addPositive(final JournalFile file, final long id, final int size) { incCounter(file); @@ -264,7 +276,8 @@ public class JournalTransaction { } public void waitCallbacks() throws InterruptedException { - if (callbackList != null) { + waitFirstCallback(); + synchronized (callbackList) { for (TransactionCallback callback : callbackList.values()) { callback.waitCompletion(); } @@ -275,8 +288,15 @@ public class JournalTransaction { * Wait completion at the latest file only */ public void waitCompletion() throws Exception { - if (currentCallback != null) { - currentCallback.waitCompletion(); + waitFirstCallback(); + currentCallback.waitCompletion(); + } + + private void waitFirstCallback() throws InterruptedException { + if (currentCallback == null) { + firstCallbackLatch = new CountDownLatch(1); + firstCallbackLatch.await(); + firstCallbackLatch = null; } } diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/journal/ActiveMQJournalLogger.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/journal/ActiveMQJournalLogger.java index 198185c1ae..6758c64283 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/journal/ActiveMQJournalLogger.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/journal/ActiveMQJournalLogger.java @@ -143,7 +143,7 @@ public interface ActiveMQJournalLogger extends BasicLogger { void compactReadError(JournalFile file); @LogMessage(level = Logger.Level.WARN) - @Message(id = 142012, value = "Couldn''t find tx={0} to merge after compacting", + @Message(id = 142012, value = "Couldn't find tx={0} to merge after compacting", format = Message.Format.MESSAGE_FORMAT) void compactMergeError(Long id); @@ -163,12 +163,12 @@ public interface ActiveMQJournalLogger extends BasicLogger { void uncomittedTxFound(Long id); @LogMessage(level = Logger.Level.WARN) - @Message(id = 142016, value = "Couldn''t stop compactor executor after 120 seconds", + @Message(id = 142016, value = "Could not stop compactor executor after 120 seconds", format = Message.Format.MESSAGE_FORMAT) void couldNotStopCompactor(); @LogMessage(level = Logger.Level.WARN) - @Message(id = 142017, value = "Couldn''t stop journal executor after 60 seconds", + @Message(id = 142017, value = "Could not stop journal executor after 60 seconds", format = Message.Format.MESSAGE_FORMAT) void couldNotStopJournalExecutor(); @@ -182,7 +182,7 @@ public interface ActiveMQJournalLogger extends BasicLogger { void deletingOrphanedFile(String fileToDelete); @LogMessage(level = Logger.Level.WARN) - @Message(id = 142020, value = "Couldn''t get lock after 60 seconds on closing Asynchronous File: {0}", format = Message.Format.MESSAGE_FORMAT) + @Message(id = 142020, value = "Could not get lock after 60 seconds on closing Asynchronous File: {0}", format = Message.Format.MESSAGE_FORMAT) void errorClosingFile(String fileToDelete); @LogMessage(level = Logger.Level.WARN) @@ -241,6 +241,10 @@ public interface ActiveMQJournalLogger extends BasicLogger { @Message(id = 142034, value = "Exception on submitting write", format = Message.Format.MESSAGE_FORMAT) void errorSubmittingWrite(@Cause Throwable e); + @LogMessage(level = Logger.Level.WARN) + @Message(id = 142035, value = "Could not stop journal append executor after 60 seconds", format = Message.Format.MESSAGE_FORMAT) + void couldNotStopJournalAppendExecutor(); + @LogMessage(level = Logger.Level.ERROR) @Message(id = 144000, value = "Failed to delete file {0}", format = Message.Format.MESSAGE_FORMAT) void errorDeletingFile(Object e); diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java index 080db78bfa..5e27b36800 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java @@ -532,6 +532,8 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { journalImpl.appendCommitRecord(1L, false); + journalImpl.debugWait(); + System.out.println("Files = " + factory.listFiles("tt")); SequentialFile file = factory.createSequentialFile("tt-1.tt"); @@ -598,6 +600,8 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { journalImpl.appendCommitRecord(2L, false); + journalImpl.debugWait(); + SequentialFile file = factory.createSequentialFile("tt-1.tt"); file.open(); @@ -697,6 +701,8 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { journalImpl.appendCommitRecord(1L, false); + journalImpl.debugWait(); + SequentialFile file = factory.createSequentialFile("tt-1.tt"); file.open(); @@ -936,8 +942,7 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { journalImpl.forceMoveNextFile(); - // Reclaiming should still be able to reclaim a file if a transaction was - // ignored + // Reclaiming should still be able to reclaim a file if a transaction was ignored journalImpl.checkReclaimStatus(); Assert.assertEquals(2, factory.listFiles("tt").size()); @@ -1109,7 +1114,16 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { } @Test - public void testReclaimingAfterConcurrentAddsAndDeletes() throws Exception { + public void testReclaimingAfterConcurrentAddsAndDeletesTx() throws Exception { + testReclaimingAfterConcurrentAddsAndDeletes(true); + } + + @Test + public void testReclaimingAfterConcurrentAddsAndDeletesNonTx() throws Exception { + testReclaimingAfterConcurrentAddsAndDeletes(false); + } + + public void testReclaimingAfterConcurrentAddsAndDeletes(final boolean transactional) throws Exception { final int JOURNAL_SIZE = 10 * 1024; setupAndLoadJournal(JOURNAL_SIZE, 1); @@ -1131,8 +1145,14 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { latchReady.countDown(); ActiveMQTestBase.waitForLatch(latchStart); for (int i = 0; i < NUMBER_OF_ELEMENTS; i++) { - journalImpl.appendAddRecordTransactional(i, i, (byte) 1, new SimpleEncoding(50, (byte) 1)); - journalImpl.appendCommitRecord(i, false); + + if (transactional) { + journalImpl.appendAddRecordTransactional(i, i, (byte) 1, new SimpleEncoding(50, (byte) 1)); + journalImpl.appendCommitRecord(i, false); + } else { + journalImpl.appendAddRecord(i, (byte) 1, new SimpleEncoding(50, (byte) 1), false); + } + queueDelete.offer(i); } finishedOK.incrementAndGet(); @@ -1153,7 +1173,14 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { if (toDelete == null) { break; } - journalImpl.appendDeleteRecord(toDelete, false); + + if (transactional) { + journalImpl.appendDeleteRecordTransactional(toDelete, toDelete, new SimpleEncoding(50, (byte) 1)); + journalImpl.appendCommitRecord(i, false); + } else { + journalImpl.appendDeleteRecord(toDelete, false); + } + } finishedOK.incrementAndGet(); } catch (Exception e) { diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalAsyncTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalAsyncTest.java index 41058c685b..204600e2f2 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalAsyncTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalAsyncTest.java @@ -81,6 +81,8 @@ public class JournalAsyncTest extends ActiveMQTestBase { journalImpl.appendAddRecordTransactional(1L, i, (byte) 1, new SimpleEncoding(1, (byte) 0)); } + journalImpl.debugWait(); + latch.countDown(); factory.setHoldCallbacks(false, null); if (isCommit) { @@ -115,8 +117,7 @@ public class JournalAsyncTest extends ActiveMQTestBase { } } - // If a callback error already arrived, we should just throw the exception - // right away + // If a callback error already arrived, we should just throw the exception right away @Test public void testPreviousError() throws Exception { final int JOURNAL_SIZE = 20000; @@ -128,6 +129,8 @@ public class JournalAsyncTest extends ActiveMQTestBase { journalImpl.appendAddRecordTransactional(1L, 1, (byte) 1, new SimpleEncoding(1, (byte) 0)); + journalImpl.debugWait(); + factory.flushAllCallbacks(); factory.setGenerateErrors(false); @@ -135,11 +138,11 @@ public class JournalAsyncTest extends ActiveMQTestBase { try { journalImpl.appendAddRecordTransactional(1L, 2, (byte) 1, new SimpleEncoding(1, (byte) 0)); - Assert.fail("Exception expected"); // An exception already happened in one - // of the elements on this transaction. - // We can't accept any more elements on - // the transaction + Assert.fail("Exception expected"); + // An exception already happened in one of the elements on this transaction. + // We can't accept any more elements on the transaction } catch (Exception ignored) { + } } From 6afde8f45aaa4f6a477066f3bc85fa8f89718a1d Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Thu, 27 Oct 2016 12:32:04 -0400 Subject: [PATCH 6/7] ARTEMIS-822 Review journal threading model https://issues.apache.org/jira/browse/ARTEMIS-822 --- .../jdbc/store/journal/JDBCJournalImpl.java | 4 + .../artemis/core/journal/Journal.java | 5 + .../core/journal/impl/FileWrapperJournal.java | 4 + .../core/journal/impl/JournalImpl.java | 242 +++++++++++------- .../core/journal/impl/JournalTransaction.java | 2 +- .../cursor/impl/PageSubscriptionImpl.java | 3 +- .../AbstractJournalStorageManager.java | 20 +- .../journal/JDBCJournalStorageManager.java | 2 - .../impl/journal/JournalStorageManager.java | 21 +- .../core/replication/ReplicatedJournal.java | 5 + .../artemis/core/server/ServiceRegistry.java | 4 + .../core/server/impl/ActiveMQServerImpl.java | 61 ++++- .../core/server/impl/ServiceRegistryImpl.java | 12 + .../byteman/JMSBridgeReconnectionTest.java | 2 +- .../journal/NIOJournalCompactTest.java | 2 + .../ValidateTransactionHealthTest.java | 2 + .../management/ActiveMQServerControlTest.java | 9 +- .../replication/ReplicationTest.java | 5 + .../journal/impl/AlignedJournalImplTest.java | 4 +- .../journal/impl/JournalImplTestUnit.java | 74 +++--- 20 files changed, 311 insertions(+), 172 deletions(-) diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java index 636309e311..e112dbcf6b 100644 --- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java +++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java @@ -113,6 +113,10 @@ public class JDBCJournalImpl extends AbstractJDBCDriver implements Journal { started = true; } + @Override + public void flush() throws Exception { + } + @Override protected void createSchema() throws SQLException { createTable(sqlProvider.getCreateJournalTableSQL()); diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/Journal.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/Journal.java index 3c1f7fd050..fbd4182a0a 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/Journal.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/Journal.java @@ -237,4 +237,9 @@ public interface Journal extends ActiveMQComponent { * only be called once the synchronization of the backup and live servers is completed. */ void replicationSyncFinished(); + + /** + * It will make sure there are no more pending operations on the Executors. + * */ + void flush() throws Exception; } diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/FileWrapperJournal.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/FileWrapperJournal.java index 51fb1543d7..0b702a5421 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/FileWrapperJournal.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/FileWrapperJournal.java @@ -98,6 +98,10 @@ public final class FileWrapperJournal extends JournalBase { writeRecord(addRecord, sync, callback); } + @Override + public void flush() throws Exception { + } + /** * Write the record to the current file. */ diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java index 43db1f7033..983bd7df92 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java @@ -18,6 +18,8 @@ package org.apache.activemq.artemis.core.journal.impl; import java.io.Serializable; import java.nio.ByteBuffer; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -29,14 +31,13 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; +import java.util.concurrent.Executor; +import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -70,8 +71,11 @@ import org.apache.activemq.artemis.core.journal.impl.dataformat.JournalInternalR import org.apache.activemq.artemis.core.journal.impl.dataformat.JournalRollbackRecordTX; import org.apache.activemq.artemis.journal.ActiveMQJournalBundle; import org.apache.activemq.artemis.journal.ActiveMQJournalLogger; +import org.apache.activemq.artemis.utils.ActiveMQThreadFactory; import org.apache.activemq.artemis.utils.ConcurrentHashSet; import org.apache.activemq.artemis.utils.DataConstants; +import org.apache.activemq.artemis.utils.OrderedExecutorFactory; +import org.apache.activemq.artemis.utils.SimpleFuture; import org.jboss.logging.Logger; /** @@ -163,7 +167,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal // Compacting may replace this structure private final ConcurrentMap records = new ConcurrentHashMap<>(); - private final Set pendingRecords = Collections.newSetFromMap(new ConcurrentHashMap()); + private final Set pendingRecords = new ConcurrentHashSet<>(); // Compacting may replace this structure private final ConcurrentMap transactions = new ConcurrentHashMap<>(); @@ -173,14 +177,18 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal private final AtomicBoolean compactorRunning = new AtomicBoolean(); - private ExecutorService filesExecutor = null; + private Executor filesExecutor = null; - private ExecutorService compactorExecutor = null; + private Executor compactorExecutor = null; - private ExecutorService appendExecutor = null; + private Executor appendExecutor = null; private ConcurrentHashSet latches = new ConcurrentHashSet<>(); + private final OrderedExecutorFactory providedIOThreadPool; + protected OrderedExecutorFactory ioExecutorFactory; + private ThreadPoolExecutor threadPool; + /** * We don't lock the journal during the whole compacting operation. During compacting we only * lock it (i) when gathering the initial structure, and (ii) when replicating the structures @@ -223,8 +231,25 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final String fileExtension, final int maxAIO, final int userVersion) { + this(null, fileSize, minFiles, poolSize, compactMinFiles, compactPercentage, fileFactory, filePrefix, fileExtension, maxAIO, userVersion); + } + + public JournalImpl(final OrderedExecutorFactory ioExecutors, + final int fileSize, + final int minFiles, + final int poolSize, + final int compactMinFiles, + final int compactPercentage, + final SequentialFileFactory fileFactory, + final String filePrefix, + final String fileExtension, + final int maxAIO, + final int userVersion) { + super(fileFactory.isSupportsCallbacks(), fileSize); + this.providedIOThreadPool = ioExecutors; + if (fileSize % fileFactory.getAlignment() != 0) { throw new IllegalArgumentException("Invalid journal-file-size " + fileSize + ", It should be multiple of " + fileFactory.getAlignment()); @@ -693,7 +718,9 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal lineUpContext(callback); pendingRecords.add(id); - Future result = appendExecutor.submit(new Runnable() { + + final SimpleFuture result = newSyncAndCallbackResult(sync, callback); + appendExecutor.execute(new Runnable() { @Override public void run() { journalLock.readLock().lock(); @@ -710,7 +737,13 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal ", usedFile = " + usedFile); } + if (result != null) { + result.set(true); + } } catch (Exception e) { + if (result != null) { + result.fail(e); + } ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); } finally { pendingRecords.remove(id); @@ -719,7 +752,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }); - if (sync && callback == null) { + if (result != null) { result.get(); } } @@ -734,7 +767,9 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal lineUpContext(callback); checkKnownRecordID(id); - Future result = appendExecutor.submit(new Runnable() { + final SimpleFuture result = newSyncAndCallbackResult(sync, callback); + + appendExecutor.execute(new Runnable() { @Override public void run() { journalLock.readLock().lock(); @@ -758,7 +793,14 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } else { jrnRecord.addUpdateFile(usedFile, updateRecord.getEncodeSize()); } + + if (result != null) { + result.set(true); + } } catch (Exception e) { + if (result != null) { + result.fail(e); + } ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); } finally { journalLock.readLock().unlock(); @@ -766,7 +808,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }); - if (sync && callback == null) { + if (result != null) { result.get(); } } @@ -777,7 +819,8 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal lineUpContext(callback); checkKnownRecordID(id); - Future result = appendExecutor.submit(new Runnable() { + final SimpleFuture result = newSyncAndCallbackResult(sync, callback); + appendExecutor.execute(new Runnable() { @Override public void run() { journalLock.readLock().lock(); @@ -801,7 +844,13 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } else { record.delete(usedFile); } + if (result != null) { + result.set(true); + } } catch (Exception e) { + if (result != null) { + result.fail(e); + } ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); } finally { journalLock.readLock().unlock(); @@ -809,11 +858,15 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }); - if (sync && callback == null) { + if (result != null) { result.get(); } } + private static SimpleFuture newSyncAndCallbackResult(boolean sync, IOCompletion callback) { + return (sync && callback == null) ? new SimpleFuture<>() : null; + } + @Override public void appendAddRecordTransactional(final long txID, final long id, @@ -824,7 +877,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final JournalTransaction tx = getTransactionInfo(txID); tx.checkErrorCondition(); - appendExecutor.submit(new Runnable() { + appendExecutor.execute(new Runnable() { @Override public void run() { @@ -860,15 +913,18 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal return; } + final SimpleFuture known = new SimpleFuture<>(); + // retry on the append thread. maybe the appender thread is not keeping up. - Future known = appendExecutor.submit(new Callable() { + appendExecutor.execute(new Runnable() { @Override - public Boolean call() throws Exception { + public void run() { journalLock.readLock().lock(); try { - return records.containsKey(id) + + known.set(records.containsKey(id) || pendingRecords.contains(id) - || (compactor != null && compactor.lookupRecord(id)); + || (compactor != null && compactor.lookupRecord(id))); } finally { journalLock.readLock().unlock(); } @@ -900,7 +956,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final JournalTransaction tx = getTransactionInfo(txID); tx.checkErrorCondition(); - appendExecutor.submit(new Runnable() { + appendExecutor.execute(new Runnable() { @Override public void run() { @@ -941,7 +997,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final JournalTransaction tx = getTransactionInfo(txID); tx.checkErrorCondition(); - appendExecutor.submit(new Runnable() { + appendExecutor.execute(new Runnable() { @Override public void run() { journalLock.readLock().lock(); @@ -991,7 +1047,9 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal final JournalTransaction tx = getTransactionInfo(txID); tx.checkErrorCondition(); - Future result = appendExecutor.submit(new Runnable() { + final SimpleFuture result = newSyncAndCallbackResult(sync, callback); + + appendExecutor.execute(new Runnable() { @Override public void run() { journalLock.readLock().lock(); @@ -1004,7 +1062,13 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } tx.prepare(usedFile); + if (result != null) { + result.set(true); + } } catch (Exception e) { + if (result != null) { + result.fail(e); + } ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); setErrorCondition(tx, e); } finally { @@ -1013,7 +1077,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }); - if (sync && callback == null) { + if (result != null) { result.get(); tx.checkErrorCondition(); } @@ -1055,8 +1119,9 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } tx.checkErrorCondition(); + final SimpleFuture result = newSyncAndCallbackResult(sync, callback); - Future result = appendExecutor.submit(new Runnable() { + appendExecutor.execute(new Runnable() { @Override public void run() { journalLock.readLock().lock(); @@ -1070,7 +1135,13 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } tx.commit(usedFile); + if (result != null) { + result.set(true); + } } catch (Exception e) { + if (result != null) { + result.fail(e); + } ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); setErrorCondition(tx, e); } finally { @@ -1079,7 +1150,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }); - if (sync && callback == null) { + if (result != null) { result.get(); tx.checkErrorCondition(); } @@ -1097,8 +1168,8 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } tx.checkErrorCondition(); - - Future result = appendExecutor.submit(new Runnable() { + final SimpleFuture result = newSyncAndCallbackResult(sync, callback); + appendExecutor.execute(new Runnable() { @Override public void run() { journalLock.readLock().lock(); @@ -1107,7 +1178,13 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal JournalFile usedFile = appendRecord(rollbackRecord, false, sync, tx, callback); tx.rollback(usedFile); + if (result != null) { + result.set(true); + } } catch (Exception e) { + if (result != null) { + result.fail(e); + } ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); setErrorCondition(tx, e); } finally { @@ -1116,7 +1193,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }); - if (sync && callback == null) { + if (result != null) { result.get(); tx.checkErrorCondition(); } @@ -1981,11 +2058,30 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal public void debugWait() throws InterruptedException { fileFactory.flush(); - if (appendExecutor != null && !appendExecutor.isShutdown()) { - // Send something to the closingExecutor, just to make sure we went until its end - final CountDownLatch latch = newLatch(1); + flushExecutor(filesExecutor); - appendExecutor.execute(new Runnable() { + flushExecutor(appendExecutor); + } + + @Override + public void flush() throws Exception { + fileFactory.flush(); + + + flushExecutor(appendExecutor); + + flushExecutor(filesExecutor); + + flushExecutor(compactorExecutor); + } + + private void flushExecutor(Executor executor) throws InterruptedException { + + if (executor != null) { + // Send something to the closingExecutor, just to make sure we went until its end + final CountDownLatch latch = new CountDownLatch(1); + + executor.execute(new Runnable() { @Override public void run() { @@ -1993,23 +2089,8 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }); - awaitLatch(latch, -1); + latch.await(10, TimeUnit.SECONDS); } - - if (filesExecutor != null && !filesExecutor.isShutdown()) { - // Send something to the closingExecutor, just to make sure we went until its end - final CountDownLatch latch = newLatch(1); - - filesExecutor.execute(new Runnable() { - @Override - public void run() { - latch.countDown(); - } - }); - - awaitLatch(latch, -1); - } - } @Override @@ -2099,7 +2180,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal } }; - appendExecutor.submit(new Runnable() { + appendExecutor.execute(new Runnable() { @Override public void run() { journalLock.readLock().lock(); @@ -2132,29 +2213,25 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal throw new IllegalStateException("Journal " + this + " is not stopped, state is " + state); } - filesExecutor = Executors.newSingleThreadExecutor(new ThreadFactory() { + if (providedIOThreadPool == null) { + ThreadFactory factory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ArtemisIOThread", true, JournalImpl.class.getClassLoader()); + } + }); - @Override - public Thread newThread(final Runnable r) { - return new Thread(r, "JournalImpl::FilesExecutor"); - } - }); + threadPool = new ThreadPoolExecutor(0,Integer.MAX_VALUE, 60L,TimeUnit.SECONDS, new SynchronousQueue<>(), factory); + ioExecutorFactory = new OrderedExecutorFactory(threadPool); + } else { + ioExecutorFactory = providedIOThreadPool; + } - compactorExecutor = Executors.newSingleThreadExecutor(new ThreadFactory() { + filesExecutor = ioExecutorFactory.getExecutor(); - @Override - public Thread newThread(final Runnable r) { - return new Thread(r, "JournalImpl::CompactorExecutor"); - } - }); + compactorExecutor = ioExecutorFactory.getExecutor(); - appendExecutor = Executors.newSingleThreadExecutor(new ThreadFactory() { - - @Override - public Thread newThread(final Runnable r) { - return new Thread(r, "JournalImpl::appendExecutor"); - } - }); + appendExecutor = ioExecutorFactory.getExecutor(); filesRepository.setExecutor(filesExecutor); @@ -2171,29 +2248,21 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal setJournalState(JournalState.STOPPED); - // appendExecutor must be shut down first - appendExecutor.shutdown(); + flush(); - if (!appendExecutor.awaitTermination(60, TimeUnit.SECONDS)) { - ActiveMQJournalLogger.LOGGER.couldNotStopJournalAppendExecutor(); + if (providedIOThreadPool == null) { + threadPool.shutdown(); + + if (!threadPool.awaitTermination(120, TimeUnit.SECONDS)) { + threadPool.shutdownNow(); + } + threadPool = null; + ioExecutorFactory = null; } + journalLock.writeLock().lock(); try { - compactorExecutor.shutdown(); - - if (!compactorExecutor.awaitTermination(120, TimeUnit.SECONDS)) { - ActiveMQJournalLogger.LOGGER.couldNotStopCompactor(); - } - - filesExecutor.shutdown(); - - filesRepository.setExecutor(null); - - if (!filesExecutor.awaitTermination(60, TimeUnit.SECONDS)) { - ActiveMQJournalLogger.LOGGER.couldNotStopJournalExecutor(); - } - try { for (CountDownLatch latch : latches) { latch.countDown(); @@ -2207,7 +2276,6 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal if (currentFile != null && currentFile.getFile().isOpen()) { currentFile.getFile().close(); } - filesRepository.clear(); fileFactory.stop(); diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalTransaction.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalTransaction.java index 1542bd4b05..8e40f3b962 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalTransaction.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalTransaction.java @@ -229,7 +229,7 @@ public class JournalTransaction { public void commit(final JournalFile file) { JournalCompactor compactor = journal.getCompactor(); - if (compacting) { + if (compacting && compactor != null) { compactor.addCommandCommit(this, file); } else { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java index 063722c92d..c40d20dc45 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java @@ -192,7 +192,8 @@ final class PageSubscriptionImpl implements PageSubscription { @Override public void reloadPageCompletion(PagePosition position) throws Exception { // if the current page is complete, we must move it out of the way - if (pageStore.getCurrentPage().getPageId() == position.getPageNr()) { + if (pageStore != null && pageStore.getCurrentPage() != null && + pageStore.getCurrentPage().getPageId() == position.getPageNr()) { pageStore.forceAnotherPage(); } PageCursorInfo info = new PageCursorInfo(position.getPageNr(), position.getMessageNr(), null); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java index a6938d6767..768be45c88 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java @@ -19,11 +19,9 @@ package org.apache.activemq.artemis.core.persistence.impl.journal; import javax.transaction.xa.Xid; import java.io.File; import java.io.FileInputStream; -import java.security.AccessController; import java.security.DigestInputStream; import java.security.InvalidParameterException; import java.security.MessageDigest; -import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -34,8 +32,6 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; @@ -103,7 +99,6 @@ import org.apache.activemq.artemis.core.transaction.ResourceManager; import org.apache.activemq.artemis.core.transaction.Transaction; import org.apache.activemq.artemis.core.transaction.TransactionPropertyIndexes; import org.apache.activemq.artemis.core.transaction.impl.TransactionImpl; -import org.apache.activemq.artemis.utils.ActiveMQThreadFactory; import org.apache.activemq.artemis.utils.Base64; import org.apache.activemq.artemis.utils.ExecutorFactory; import org.apache.activemq.artemis.utils.IDGenerator; @@ -168,7 +163,7 @@ public abstract class AbstractJournalStorageManager implements StorageManager { final Executor executor; - ExecutorService singleThreadExecutor; + Executor singleThreadExecutor; private final boolean syncTransactional; @@ -286,10 +281,6 @@ public abstract class AbstractJournalStorageManager implements StorageManager { OperationContextImpl.setContext(context); } - public Executor getSingleThreadExecutor() { - return singleThreadExecutor; - } - @Override public OperationContext newSingleThreadContext() { return newContext(singleThreadExecutor); @@ -1429,12 +1420,7 @@ public abstract class AbstractJournalStorageManager implements StorageManager { beforeStart(); - singleThreadExecutor = Executors.newSingleThreadExecutor(AccessController.doPrivileged(new PrivilegedAction() { - @Override - public ActiveMQThreadFactory run() { - return new ActiveMQThreadFactory("ActiveMQ-IO-SingleThread", true, JournalStorageManager.class.getClassLoader()); - } - })); + singleThreadExecutor = executorFactory.getExecutor(); bindingsJournal.start(); @@ -1490,8 +1476,6 @@ public abstract class AbstractJournalStorageManager implements StorageManager { messageJournal.stop(); - singleThreadExecutor.shutdown(); - journalLoaded = false; started = false; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java index a0f0ed18ce..d97f988de4 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java @@ -101,8 +101,6 @@ public class JDBCJournalStorageManager extends JournalStorageManager { messageJournal.stop(); largeMessagesFactory.stop(); - singleThreadExecutor.shutdown(); - journalLoaded = false; started = false; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java index 9eaa203824..2d8411a4f7 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java @@ -26,6 +26,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; @@ -197,14 +198,18 @@ public class JournalStorageManager extends AbstractJournalStorageManager { } final CountDownLatch latch = new CountDownLatch(1); - executor.execute(new Runnable() { - @Override - public void run() { - latch.countDown(); - } - }); + try { + executor.execute(new Runnable() { + @Override + public void run() { + latch.countDown(); + } + }); - latch.await(30, TimeUnit.SECONDS); + latch.await(30, TimeUnit.SECONDS); + } catch (RejectedExecutionException ignored) { + // that's ok + } // We cache the variable as the replicator could be changed between here and the time we call stop // since sendLiveIsStopping may issue a close back from the channel @@ -225,8 +230,6 @@ public class JournalStorageManager extends AbstractJournalStorageManager { messageJournal.stop(); - singleThreadExecutor.shutdown(); - journalLoaded = false; started = false; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicatedJournal.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicatedJournal.java index 6668c7145c..d70316f11e 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicatedJournal.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicatedJournal.java @@ -64,6 +64,11 @@ public class ReplicatedJournal implements Journal { this.replicationManager = replicationManager; } + @Override + public void flush() throws Exception { + + } + /** * @param id * @param recordType diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ServiceRegistry.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ServiceRegistry.java index b0fa65804a..0583600400 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ServiceRegistry.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ServiceRegistry.java @@ -36,6 +36,10 @@ public interface ServiceRegistry { void setExecutorService(ExecutorService executorService); + ExecutorService getIOExecutorService(); + + void setIOExecutorService(ExecutorService ioExecutorService); + ScheduledExecutorService getScheduledExecutorService(); void setScheduledExecutorService(ScheduledExecutorService scheduledExecutorService); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index 98abce01eb..6288bdfec7 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -38,11 +38,12 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.Semaphore; +import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -150,6 +151,7 @@ import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection; import org.apache.activemq.artemis.spi.core.protocol.SessionCallback; import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager; import org.apache.activemq.artemis.utils.ActiveMQThreadFactory; +import org.apache.activemq.artemis.utils.ActiveMQThreadPoolExecutor; import org.apache.activemq.artemis.utils.CertificateUtil; import org.apache.activemq.artemis.utils.ConcurrentHashSet; import org.apache.activemq.artemis.utils.ExecutorFactory; @@ -230,6 +232,14 @@ public class ActiveMQServerImpl implements ActiveMQServer { private volatile ExecutorFactory executorFactory; + + private volatile ExecutorService ioExecutorPool; + /** + * This is a thread pool for io tasks only. + * We can't use the same global executor to avoid starvations. + */ + private volatile ExecutorFactory ioExecutorFactory; + private final HierarchicalRepository> securityRepository; private volatile ResourceManager resourceManager; @@ -859,17 +869,11 @@ public class ActiveMQServerImpl implements ActiveMQServer { } if (threadPool != null && !threadPoolSupplied) { - threadPool.shutdown(); - try { - if (!threadPool.awaitTermination(10, TimeUnit.SECONDS)) { - ActiveMQServerLogger.LOGGER.timedOutStoppingThreadpool(threadPool); - for (Runnable r : threadPool.shutdownNow()) { - logger.debug("Cancelled the execution of " + r); - } - } - } catch (InterruptedException e) { - ActiveMQServerLogger.LOGGER.interruptWhilstStoppingComponent(threadPool.getClass().getName()); - } + shutdownPool(threadPool); + } + + if (ioExecutorPool != null) { + shutdownPool(ioExecutorPool); } if (!threadPoolSupplied) @@ -950,6 +954,20 @@ public class ActiveMQServerImpl implements ActiveMQServer { } } + private void shutdownPool(ExecutorService executorService) { + executorService.shutdown(); + try { + if (!executorService.awaitTermination(10, TimeUnit.SECONDS)) { + ActiveMQServerLogger.LOGGER.timedOutStoppingThreadpool(threadPool); + for (Runnable r : executorService.shutdownNow()) { + logger.debug("Cancelled the execution of " + r); + } + } + } catch (InterruptedException e) { + ActiveMQServerLogger.LOGGER.interruptWhilstStoppingComponent(threadPool.getClass().getName()); + } + } + public boolean checkLiveIsNotColocated(String nodeId) { if (parentServer == null) { return true; @@ -1805,10 +1823,11 @@ public class ActiveMQServerImpl implements ActiveMQServer { return new ActiveMQThreadFactory("ActiveMQ-server-" + this.toString(), false, ClientSessionFactoryImpl.class.getClassLoader()); } }); + if (configuration.getThreadPoolMaxSize() == -1) { - threadPool = Executors.newCachedThreadPool(tFactory); + threadPool = new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue(), tFactory); } else { - threadPool = Executors.newFixedThreadPool(configuration.getThreadPoolMaxSize(), tFactory); + threadPool = new ActiveMQThreadPoolExecutor(0, configuration.getThreadPoolMaxSize(), 60L, TimeUnit.SECONDS, tFactory); } } else { threadPool = serviceRegistry.getExecutorService(); @@ -1816,6 +1835,20 @@ public class ActiveMQServerImpl implements ActiveMQServer { } this.executorFactory = new OrderedExecutorFactory(threadPool); + + if (serviceRegistry.getIOExecutorService() != null) { + this.ioExecutorFactory = new OrderedExecutorFactory(serviceRegistry.getIOExecutorService()); + } else { + ThreadFactory tFactory = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public ThreadFactory run() { + return new ActiveMQThreadFactory("ActiveMQ-IO-server-" + this.toString(), false, ClientSessionFactoryImpl.class.getClassLoader()); + } + }); + + this.ioExecutorPool = new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue(), tFactory); + } + /* We check to see if a Scheduled Executor Service is provided in the InjectedObjectRegistry. If so we use this * Scheduled ExecutorService otherwise we create a new one. */ diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServiceRegistryImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServiceRegistryImpl.java index 1d08f4ad02..a287a00209 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServiceRegistryImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServiceRegistryImpl.java @@ -41,6 +41,8 @@ public class ServiceRegistryImpl implements ServiceRegistry { private ExecutorService executorService; + private ExecutorService ioExecutorService; + private ScheduledExecutorService scheduledExecutorService; /* We are using a List rather than HashMap here as ActiveMQ Artemis allows multiple instances of the same class to be added @@ -162,6 +164,16 @@ public class ServiceRegistryImpl implements ServiceRegistry { return transformer; } + @Override + public ExecutorService getIOExecutorService() { + return ioExecutorService; + } + + @Override + public void setIOExecutorService(ExecutorService ioExecutorService) { + this.ioExecutorService = ioExecutorService; + } + @Override public void addBridgeTransformer(String name, Transformer transformer) { bridgeTransformers.put(name, transformer); diff --git a/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/JMSBridgeReconnectionTest.java b/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/JMSBridgeReconnectionTest.java index 0a5d52dcfe..ef71e89eef 100644 --- a/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/JMSBridgeReconnectionTest.java +++ b/tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/JMSBridgeReconnectionTest.java @@ -50,7 +50,7 @@ public class JMSBridgeReconnectionTest extends BridgeTestBase { targetClass = "org.apache.activemq.artemis.core.client.impl.ClientProducerImpl", targetMethod = "sendRegularMessage", targetLocation = "ENTRY", - action = "org.apache.activemq.artemis.tests.extras.byteman.JMSBridgeReconnectionTest.pause2($1,$2,$3);")}) + action = "org.apache.activemq.artemis.tests.extras.byteman.JMSBridgeReconnectionTest.pause2($2,$3,$4);")}) public void performCrashDestinationStopBridge() throws Exception { activeMQServer = jmsServer1; ConnectionFactoryFactory factInUse0 = cff0; diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java index 2dd38aeda0..519ffb5ff4 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java @@ -713,6 +713,8 @@ public class NIOJournalCompactTest extends JournalImplTestBase { journal.testCompact(); } + journal.flush(); + stopJournal(); createJournal(); startJournal(); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/ValidateTransactionHealthTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/ValidateTransactionHealthTest.java index 2d3df3e616..8f15c4854a 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/ValidateTransactionHealthTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/ValidateTransactionHealthTest.java @@ -314,6 +314,8 @@ public class ValidateTransactionHealthTest extends ActiveMQTestBase { throw e; } + journal.flush(); + return journal; } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java index 7dd2d0b153..27a2838935 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java @@ -58,6 +58,7 @@ import org.apache.activemq.artemis.jlibaio.LibaioContext; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.spi.core.security.jaas.InVMLoginModule; import org.apache.activemq.artemis.tests.integration.IntegrationTestLogger; +import org.apache.activemq.artemis.tests.integration.mqtt.imported.util.Wait; import org.apache.activemq.artemis.tests.unit.core.config.impl.fakes.FakeConnectorServiceFactory; import org.apache.activemq.artemis.utils.RandomUtil; import org.apache.activemq.artemis.utils.UUIDGenerator; @@ -265,12 +266,18 @@ public class ActiveMQServerControlTest extends ManagementTestBase { ServerLocator receiveLocator = createInVMNonHALocator(); ClientSessionFactory receiveCsf = createSessionFactory(receiveLocator); ClientSession receiveClientSession = receiveCsf.createSession(true, false, false); - ClientConsumer consumer = receiveClientSession.createConsumer(name); + final ClientConsumer consumer = receiveClientSession.createConsumer(name); Assert.assertFalse(consumer.isClosed()); checkResource(ObjectNameBuilder.DEFAULT.getQueueObjectName(address, name)); serverControl.destroyQueue(name.toString(), true); + Wait.waitFor(new Wait.Condition() { + @Override + public boolean isSatisified() throws Exception { + return consumer.isClosed(); + } + }, 1000, 100); Assert.assertTrue(consumer.isClosed()); checkNoResource(ObjectNameBuilder.DEFAULT.getQueueObjectName(address, name)); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/ReplicationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/ReplicationTest.java index 9d63e1d919..7d2d51460f 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/ReplicationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/ReplicationTest.java @@ -650,6 +650,11 @@ public final class ReplicationTest extends ActiveMQTestBase { } + @Override + public void flush() throws Exception { + + } + @Override public void appendCommitRecord(final long txID, final boolean sync) throws Exception { diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java index 5e27b36800..2b24296b4f 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java @@ -434,9 +434,6 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { Assert.assertEquals(0, records.size()); Assert.assertEquals(0, transactions.size()); - - Assert.assertEquals(2, factory.listFiles("tt").size()); - } @Test @@ -944,6 +941,7 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { // Reclaiming should still be able to reclaim a file if a transaction was ignored journalImpl.checkReclaimStatus(); + journalImpl.flush(); Assert.assertEquals(2, factory.listFiles("tt").size()); diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalImplTestUnit.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalImplTestUnit.java index 8f23c2c70c..eb815ae995 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalImplTestUnit.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalImplTestUnit.java @@ -25,6 +25,7 @@ import org.apache.activemq.artemis.api.core.ActiveMQIOErrorException; import org.apache.activemq.artemis.core.io.SequentialFile; import org.apache.activemq.artemis.core.journal.EncodingSupport; import org.apache.activemq.artemis.core.journal.RecordInfo; +import org.apache.activemq.artemis.core.journal.TestableJournal; import org.apache.activemq.artemis.core.journal.impl.JournalImpl; import org.apache.activemq.artemis.tests.unit.UnitTestLogger; import org.apache.activemq.artemis.tests.unit.core.journal.impl.fakes.SimpleEncoding; @@ -439,7 +440,10 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { /** * Use: calculateNumberOfFiles (fileSize, numberOfRecords, recordSize, numberOfRecords2, recordSize2, , ...., numberOfRecordsN, recordSizeN); */ - private int calculateNumberOfFiles(final int fileSize, final int alignment, final int... record) throws Exception { + private int calculateNumberOfFiles(TestableJournal journal, final int fileSize, final int alignment, final int... record) throws Exception { + if (journal != null) { + journal.flush(); + } int headerSize = calculateRecordSize(JournalImpl.SIZE_HEADER, alignment); int currentPosition = headerSize; int totalFiles = 0; @@ -489,7 +493,7 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { add(i); } - int numberOfFiles = calculateNumberOfFiles(10 * 1024, journal.getAlignment(), 91, JournalImpl.SIZE_ADD_RECORD + recordLength); + int numberOfFiles = calculateNumberOfFiles(journal, 10 * 1024, journal.getAlignment(), 91, JournalImpl.SIZE_ADD_RECORD + recordLength); Assert.assertEquals(numberOfFiles, journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); @@ -512,7 +516,7 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { add(i); } - numberOfFiles = calculateNumberOfFiles(10 * 1024, journal.getAlignment(), 95, JournalImpl.SIZE_ADD_RECORD + recordLength); + numberOfFiles = calculateNumberOfFiles(journal, 10 * 1024, journal.getAlignment(), 95, JournalImpl.SIZE_ADD_RECORD + recordLength); Assert.assertEquals(numberOfFiles, journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); @@ -533,7 +537,7 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { add(i); } - numberOfFiles = calculateNumberOfFiles(10 * 1024, journal.getAlignment(), 200, JournalImpl.SIZE_ADD_RECORD + recordLength); + numberOfFiles = calculateNumberOfFiles(journal, 10 * 1024, journal.getAlignment(), 200, JournalImpl.SIZE_ADD_RECORD + recordLength); Assert.assertEquals(numberOfFiles, journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); @@ -646,14 +650,14 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { @Test public void testCalculations() throws Exception { - Assert.assertEquals(0, calculateNumberOfFiles(10 * 1024, 1, 1, 10, 2, 20)); - Assert.assertEquals(0, calculateNumberOfFiles(10 * 1024, 512, 1, 1)); - Assert.assertEquals(0, calculateNumberOfFiles(10 * 1024, 512, 19, 10)); - Assert.assertEquals(1, calculateNumberOfFiles(10 * 1024, 512, 20, 10)); - Assert.assertEquals(0, calculateNumberOfFiles(3000, 500, 2, 1000, 1, 500)); - Assert.assertEquals(1, calculateNumberOfFiles(3000, 500, 2, 1000, 1, 1000)); - Assert.assertEquals(9, calculateNumberOfFiles(10240, 1, 90, 1038, 45, 10)); - Assert.assertEquals(11, calculateNumberOfFiles(10 * 1024, 512, 60, 14 + 1024, 30, 14)); + Assert.assertEquals(0, calculateNumberOfFiles(journal, 10 * 1024, 1, 1, 10, 2, 20)); + Assert.assertEquals(0, calculateNumberOfFiles(journal, 10 * 1024, 512, 1, 1)); + Assert.assertEquals(0, calculateNumberOfFiles(journal, 10 * 1024, 512, 19, 10)); + Assert.assertEquals(1, calculateNumberOfFiles(journal, 10 * 1024, 512, 20, 10)); + Assert.assertEquals(0, calculateNumberOfFiles(journal, 3000, 500, 2, 1000, 1, 500)); + Assert.assertEquals(1, calculateNumberOfFiles(journal, 3000, 500, 2, 1000, 1, 1000)); + Assert.assertEquals(9, calculateNumberOfFiles(journal, 10240, 1, 90, 1038, 45, 10)); + Assert.assertEquals(11, calculateNumberOfFiles(journal, 10 * 1024, 512, 60, 14 + 1024, 30, 14)); } @Test @@ -862,13 +866,13 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { addTx(1, i); } - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 100, recordLength), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 100, recordLength), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(0, journal.getIDMapSize()); List files2 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 100, recordLength) + 2, files2.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 100, recordLength) + 2, files2.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); for (String file : files1) { @@ -879,13 +883,13 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { // Make sure nothing reclaimed - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 100, recordLength), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 100, recordLength), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(0, journal.getIDMapSize()); List files3 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 100, recordLength) + 2, files3.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 100, recordLength) + 2, files3.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); for (String file : files1) { @@ -898,13 +902,13 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { updateTx(1, i); } - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(0, journal.getIDMapSize()); List files4 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength) + 2, files4.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength) + 2, files4.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); for (String file : files1) { @@ -915,7 +919,7 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { // Make sure nothing reclaimed - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(0, journal.getIDMapSize()); @@ -934,14 +938,14 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { deleteTx(1, i); } - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(0, journal.getIDMapSize()); List files7 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX) + 2, files7.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX) + 2, files7.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); for (String file : files1) { @@ -950,13 +954,13 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { checkAndReclaimFiles(); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(0, journal.getIDMapSize()); List files8 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX) + 2, files8.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX) + 2, files8.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); for (String file : files1) { @@ -977,13 +981,13 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { add(i); } - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX, 1, JournalImpl.SIZE_COMMIT_RECORD, 10, JournalImpl.SIZE_ADD_RECORD + recordLength), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX, 1, JournalImpl.SIZE_COMMIT_RECORD, 10, JournalImpl.SIZE_ADD_RECORD + recordLength), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(10, journal.getIDMapSize()); List files9 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX, 1, JournalImpl.SIZE_COMMIT_RECORD, 10, JournalImpl.SIZE_ADD_RECORD + recordLength) + 2, files9.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 200, recordLength, 200, JournalImpl.SIZE_DELETE_RECORD_TX, 1, JournalImpl.SIZE_COMMIT_RECORD, 10, JournalImpl.SIZE_ADD_RECORD + recordLength) + 2, files9.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); for (String file : files1) { @@ -1458,7 +1462,7 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { Assert.assertEquals(3, files2.size()); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(1, journal.getOpenedFilesCount()); Assert.assertEquals(1, journal.getIDMapSize()); @@ -1467,10 +1471,10 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { List files3 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_COMMIT_RECORD + 1) + 2, files3.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_COMMIT_RECORD + 1) + 2, files3.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_COMMIT_RECORD), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_COMMIT_RECORD), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(2, journal.getIDMapSize()); @@ -1478,10 +1482,10 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { List files4 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_COMMIT_RECORD + 1, 1, JournalImpl.SIZE_DELETE_RECORD + 1) + 2, files4.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_COMMIT_RECORD + 1, 1, JournalImpl.SIZE_DELETE_RECORD + 1) + 2, files4.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_COMMIT_RECORD + 1, 1, JournalImpl.SIZE_DELETE_RECORD + 1), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_COMMIT_RECORD + 1, 1, JournalImpl.SIZE_DELETE_RECORD + 1), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(1, journal.getIDMapSize()); @@ -1549,10 +1553,10 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { rollback(1); // in file 1 - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_ROLLBACK_RECORD + 1), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_ROLLBACK_RECORD + 1), journal.getDataFilesCount()); Assert.assertEquals(1, journal.getOpenedFilesCount()); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_ROLLBACK_RECORD + 1), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_ROLLBACK_RECORD + 1), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(1, journal.getIDMapSize()); @@ -1560,10 +1564,10 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { List files4 = fileFactory.listFiles(fileExtension); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_ROLLBACK_RECORD + 1, 1, JournalImpl.SIZE_DELETE_RECORD + 1) + 2, files4.size()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_ROLLBACK_RECORD + 1, 1, JournalImpl.SIZE_DELETE_RECORD + 1) + 2, files4.size()); Assert.assertEquals(1, journal.getOpenedFilesCount()); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_ROLLBACK_RECORD + 1, 1, JournalImpl.SIZE_DELETE_RECORD + 1), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength, 1, JournalImpl.SIZE_ROLLBACK_RECORD + 1, 1, JournalImpl.SIZE_DELETE_RECORD + 1), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(0, journal.getIDMapSize()); @@ -1669,7 +1673,7 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { Assert.assertEquals(3, files2.size()); - Assert.assertEquals(calculateNumberOfFiles(fileSize, journal.getAlignment(), 2, recordLength), journal.getDataFilesCount()); + Assert.assertEquals(calculateNumberOfFiles(journal, fileSize, journal.getAlignment(), 2, recordLength), journal.getDataFilesCount()); Assert.assertEquals(0, journal.getFreeFilesCount()); Assert.assertEquals(1, journal.getOpenedFilesCount()); Assert.assertEquals(1, journal.getIDMapSize()); From 7eadff76818546aa6045be2eeb2e6aef60992394 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Fri, 28 Oct 2016 11:11:59 -0400 Subject: [PATCH 7/7] ARTEMIS-822 Injecting IO Pools into and from ArtemisServerImpl https://issues.apache.org/jira/browse/ARTEMIS-822 --- .../cli/commands/tools/XmlDataExporter.java | 14 +++--- .../journal/JMSJournalStorageManagerImpl.java | 6 ++- .../jms/server/impl/JMSServerManagerImpl.java | 15 +++--- .../core/journal/impl/JournalImpl.java | 49 +++++++++++-------- .../AbstractJournalStorageManager.java | 10 +++- .../journal/JDBCJournalStorageManager.java | 6 ++- .../impl/journal/JournalStorageManager.java | 20 +++++--- .../artemis/core/server/ActiveMQServer.java | 2 + .../core/server/impl/ActiveMQServerImpl.java | 12 +++-- .../journal/NIOJournalCompactTest.java | 19 +++++-- .../DeleteMessagesOnStartupTest.java | 2 +- .../persistence/RestartSMTest.java | 2 +- .../persistence/StorageManagerTestBase.java | 6 +-- .../replication/ReplicationTest.java | 2 +- .../server/SuppliedThreadPoolTest.java | 2 + .../journal/impl/AlignedJournalImplTest.java | 2 - .../journal/impl/JournalImplTestUnit.java | 6 --- .../impl/DuplicateDetectionUnitTest.java | 10 ++-- 18 files changed, 108 insertions(+), 77 deletions(-) diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataExporter.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataExporter.java index a0e6c1e87a..8030ce2f5a 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataExporter.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataExporter.java @@ -90,6 +90,7 @@ import org.apache.activemq.artemis.jms.persistence.impl.journal.JMSJournalStorag import org.apache.activemq.artemis.utils.ActiveMQThreadFactory; import org.apache.activemq.artemis.utils.Base64; import org.apache.activemq.artemis.utils.ExecutorFactory; +import org.apache.activemq.artemis.utils.OrderedExecutorFactory; @Command(name = "exp", description = "Export all message-data using an XML that could be interpreted by any system.") public final class XmlDataExporter extends OptionalLocking { @@ -142,15 +143,10 @@ public final class XmlDataExporter extends OptionalLocking { String pagingDir, String largeMessagesDir) throws Exception { config = new ConfigurationImpl().setBindingsDirectory(bindingsDir).setJournalDirectory(journalDir).setPagingDirectory(pagingDir).setLargeMessagesDirectory(largeMessagesDir).setJournalType(JournalType.NIO); - final ExecutorService executor = Executors.newFixedThreadPool(1, ActiveMQThreadFactory.defaultThreadFactory()); - ExecutorFactory executorFactory = new ExecutorFactory() { - @Override - public Executor getExecutor() { - return executor; - } - }; + final ExecutorService executor = Executors.newFixedThreadPool(5, ActiveMQThreadFactory.defaultThreadFactory()); + ExecutorFactory executorFactory = new OrderedExecutorFactory(executor); - storageManager = new JournalStorageManager(config, executorFactory); + storageManager = new JournalStorageManager(config, executorFactory, executorFactory); XMLOutputFactory factory = XMLOutputFactory.newInstance(); XMLStreamWriter rawXmlWriter = factory.createXMLStreamWriter(out, "UTF-8"); @@ -158,6 +154,8 @@ public final class XmlDataExporter extends OptionalLocking { xmlWriter = (XMLStreamWriter) Proxy.newProxyInstance(XMLStreamWriter.class.getClassLoader(), new Class[]{XMLStreamWriter.class}, handler); writeXMLData(); + + executor.shutdown(); } private void writeXMLData() throws Exception { diff --git a/artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/persistence/impl/journal/JMSJournalStorageManagerImpl.java b/artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/persistence/impl/journal/JMSJournalStorageManagerImpl.java index 32c438df19..0aaa1a6415 100644 --- a/artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/persistence/impl/journal/JMSJournalStorageManagerImpl.java +++ b/artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/persistence/impl/journal/JMSJournalStorageManagerImpl.java @@ -40,6 +40,7 @@ import org.apache.activemq.artemis.jms.persistence.config.PersistedBindings; import org.apache.activemq.artemis.jms.persistence.config.PersistedConnectionFactory; import org.apache.activemq.artemis.jms.persistence.config.PersistedDestination; import org.apache.activemq.artemis.jms.persistence.config.PersistedType; +import org.apache.activemq.artemis.utils.ExecutorFactory; import org.apache.activemq.artemis.utils.IDGenerator; public final class JMSJournalStorageManagerImpl implements JMSStorageManager { @@ -73,7 +74,8 @@ public final class JMSJournalStorageManagerImpl implements JMSStorageManager { // Static -------------------------------------------------------- // Constructors -------------------------------------------------- - public JMSJournalStorageManagerImpl(final IDGenerator idGenerator, + public JMSJournalStorageManagerImpl(ExecutorFactory ioExecutors, + final IDGenerator idGenerator, final Configuration config, final ReplicationManager replicator) { if (config.getJournalType() != JournalType.NIO && config.getJournalType() != JournalType.ASYNCIO) { @@ -86,7 +88,7 @@ public final class JMSJournalStorageManagerImpl implements JMSStorageManager { SequentialFileFactory bindingsJMS = new NIOSequentialFileFactory(config.getBindingsLocation(), 1); - Journal localJMS = new JournalImpl(1024 * 1024, 2, config.getJournalPoolFiles(), config.getJournalCompactMinFiles(), config.getJournalCompactPercentage(), bindingsJMS, "activemq-jms", "jms", 1); + Journal localJMS = new JournalImpl(ioExecutors, 1024 * 1024, 2, config.getJournalPoolFiles(), config.getJournalCompactMinFiles(), config.getJournalCompactPercentage(), bindingsJMS, "activemq-jms", "jms", 1, 0); if (replicator != null) { jmsJournal = new ReplicatedJournal((byte) 2, localJMS, replicator); diff --git a/artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/impl/JMSServerManagerImpl.java b/artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/impl/JMSServerManagerImpl.java index dfa92188ad..456bb58241 100644 --- a/artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/impl/JMSServerManagerImpl.java +++ b/artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/impl/JMSServerManagerImpl.java @@ -1544,16 +1544,13 @@ public class JMSServerManagerImpl implements JMSServerManager, ActivateCallback * @throws Exception */ private void createJournal() throws Exception { - if (storage == null) { - if (coreConfig.isPersistenceEnabled()) { - storage = new JMSJournalStorageManagerImpl(new TimeAndCounterIDGenerator(), server.getConfiguration(), server.getReplicationManager()); - } else { - storage = new NullJMSStorageManagerImpl(); - } + if (storage != null) { + storage.stop(); + } + if (coreConfig.isPersistenceEnabled()) { + storage = new JMSJournalStorageManagerImpl(server.getIOExecutorFactory(), new TimeAndCounterIDGenerator(), server.getConfiguration(), server.getReplicationManager()); } else { - if (storage.isStarted()) { - storage.stop(); - } + storage = new NullJMSStorageManagerImpl(); } storage.start(); diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java index 983bd7df92..b1093ed8d0 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/impl/JournalImpl.java @@ -35,6 +35,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; @@ -74,6 +75,7 @@ import org.apache.activemq.artemis.journal.ActiveMQJournalLogger; import org.apache.activemq.artemis.utils.ActiveMQThreadFactory; import org.apache.activemq.artemis.utils.ConcurrentHashSet; import org.apache.activemq.artemis.utils.DataConstants; +import org.apache.activemq.artemis.utils.ExecutorFactory; import org.apache.activemq.artemis.utils.OrderedExecutorFactory; import org.apache.activemq.artemis.utils.SimpleFuture; import org.jboss.logging.Logger; @@ -185,8 +187,8 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal private ConcurrentHashSet latches = new ConcurrentHashSet<>(); - private final OrderedExecutorFactory providedIOThreadPool; - protected OrderedExecutorFactory ioExecutorFactory; + private final ExecutorFactory providedIOThreadPool; + protected ExecutorFactory ioExecutorFactory; private ThreadPoolExecutor threadPool; /** @@ -234,7 +236,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal this(null, fileSize, minFiles, poolSize, compactMinFiles, compactPercentage, fileFactory, filePrefix, fileExtension, maxAIO, userVersion); } - public JournalImpl(final OrderedExecutorFactory ioExecutors, + public JournalImpl(final ExecutorFactory ioExecutors, final int fileSize, final int minFiles, final int poolSize, @@ -744,7 +746,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal if (result != null) { result.fail(e); } - ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + logger.error("appendAddRecord::" + e, e); } finally { pendingRecords.remove(id); journalLock.readLock().unlock(); @@ -801,7 +803,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal if (result != null) { result.fail(e); } - ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + logger.error("appendUpdateRecord:" + e, e); } finally { journalLock.readLock().unlock(); } @@ -851,7 +853,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal if (result != null) { result.fail(e); } - ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + logger.error("appendDeleteRecord:" + e, e); } finally { journalLock.readLock().unlock(); } @@ -899,7 +901,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal tx.addPositive(usedFile, id, addRecord.getEncodeSize()); } catch (Exception e) { - ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + logger.error("appendAddRecordTransactional:" + e, e); setErrorCondition(tx, e); } finally { journalLock.readLock().unlock(); @@ -979,7 +981,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal tx.addPositive( usedFile, id, updateRecordTX.getEncodeSize() ); } catch ( Exception e ) { - ActiveMQJournalLogger.LOGGER.error( e.getMessage(), e ); + logger.error("appendUpdateRecordTransactional:" + e.getMessage(), e ); setErrorCondition( tx, e ); } finally { journalLock.readLock().unlock(); @@ -1016,7 +1018,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal tx.addNegative(usedFile, id); } catch (Exception e) { - ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + logger.error("appendDeleteRecordTransactional:" + e, e); setErrorCondition(tx, e); } finally { journalLock.readLock().unlock(); @@ -1069,7 +1071,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal if (result != null) { result.fail(e); } - ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + logger.error("appendPrepareRecord:" + e, e); setErrorCondition(tx, e); } finally { journalLock.readLock().unlock(); @@ -1142,7 +1144,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal if (result != null) { result.fail(e); } - ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + logger.error("appendCommitRecord:" + e, e); setErrorCondition(tx, e); } finally { journalLock.readLock().unlock(); @@ -1185,7 +1187,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal if (result != null) { result.fail(e); } - ActiveMQJournalLogger.LOGGER.error(e.getMessage(), e); + logger.error("appendRollbackRecord:" + e, e); setErrorCondition(tx, e); } finally { journalLock.readLock().unlock(); @@ -2067,7 +2069,6 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal public void flush() throws Exception { fileFactory.flush(); - flushExecutor(appendExecutor); flushExecutor(filesExecutor); @@ -2081,16 +2082,21 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal // Send something to the closingExecutor, just to make sure we went until its end final CountDownLatch latch = new CountDownLatch(1); - executor.execute(new Runnable() { + try { + executor.execute(new Runnable() { - @Override - public void run() { - latch.countDown(); - } + @Override + public void run() { + latch.countDown(); + } - }); - latch.await(10, TimeUnit.SECONDS); + }); + latch.await(10, TimeUnit.SECONDS); + } catch (RejectedExecutionException ignored ) { + // this is fine + } } + } @Override @@ -2243,7 +2249,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal @Override public synchronized void stop() throws Exception { if (state == JournalState.STOPPED) { - throw new IllegalStateException("Journal is already stopped"); + return; } setJournalState(JournalState.STOPPED); @@ -2905,6 +2911,7 @@ public class JournalImpl extends JournalBase implements TestableJournal, Journal try { scheduleCompactAndBlock(60); } catch (Exception e) { + e.printStackTrace(); throw new RuntimeException(e); } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java index 768be45c88..ecaa86ecfa 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java @@ -146,6 +146,8 @@ public abstract class AbstractJournalStorageManager implements StorageManager { protected BatchingIDGenerator idGenerator; + protected final ExecutorFactory ioExecutors; + protected final ScheduledExecutorService scheduledExecutorService; protected final ReentrantReadWriteLock storageManagerLock = new ReentrantReadWriteLock(true); @@ -186,18 +188,22 @@ public abstract class AbstractJournalStorageManager implements StorageManager { public AbstractJournalStorageManager(final Configuration config, final ExecutorFactory executorFactory, - final ScheduledExecutorService scheduledExecutorService) { - this(config, executorFactory, scheduledExecutorService, null); + final ScheduledExecutorService scheduledExecutorService, + final ExecutorFactory ioExecutors) { + this(config, executorFactory, scheduledExecutorService, ioExecutors, null); } public AbstractJournalStorageManager(Configuration config, ExecutorFactory executorFactory, ScheduledExecutorService scheduledExecutorService, + ExecutorFactory ioExecutors, IOCriticalErrorListener criticalErrorListener) { this.executorFactory = executorFactory; this.ioCriticalErrorListener = criticalErrorListener; + this.ioExecutors = ioExecutors; + this.scheduledExecutorService = scheduledExecutorService; this.config = config; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java index d97f988de4..e4d401b627 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JDBCJournalStorageManager.java @@ -36,15 +36,17 @@ public class JDBCJournalStorageManager extends JournalStorageManager { public JDBCJournalStorageManager(Configuration config, ExecutorFactory executorFactory, + ExecutorFactory ioExecutorFactory, ScheduledExecutorService scheduledExecutorService) { - super(config, executorFactory, scheduledExecutorService); + super(config, executorFactory, scheduledExecutorService, ioExecutorFactory); } public JDBCJournalStorageManager(final Configuration config, final ScheduledExecutorService scheduledExecutorService, final ExecutorFactory executorFactory, + final ExecutorFactory ioExecutorFactory, final IOCriticalErrorListener criticalErrorListener) { - super(config, executorFactory, scheduledExecutorService, criticalErrorListener); + super(config, executorFactory, scheduledExecutorService, ioExecutorFactory, criticalErrorListener); } @Override diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java index 2d8411a4f7..24650e1473 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java @@ -86,25 +86,28 @@ public class JournalStorageManager extends AbstractJournalStorageManager { public JournalStorageManager(final Configuration config, final ExecutorFactory executorFactory, - final ScheduledExecutorService scheduledExecutorService) { - this(config, executorFactory, scheduledExecutorService, null); + final ScheduledExecutorService scheduledExecutorService, + final ExecutorFactory ioExecutors) { + this(config, executorFactory, scheduledExecutorService, ioExecutors, null); } - public JournalStorageManager(final Configuration config, final ExecutorFactory executorFactory) { - this(config, executorFactory, null, null); + public JournalStorageManager(final Configuration config, final ExecutorFactory executorFactory, final ExecutorFactory ioExecutors) { + this(config, executorFactory, null, ioExecutors, null); } public JournalStorageManager(final Configuration config, final ExecutorFactory executorFactory, final ScheduledExecutorService scheduledExecutorService, + final ExecutorFactory ioExecutors, final IOCriticalErrorListener criticalErrorListener) { - super(config, executorFactory, scheduledExecutorService, criticalErrorListener); + super(config, executorFactory, scheduledExecutorService, ioExecutors, criticalErrorListener); } public JournalStorageManager(final Configuration config, final ExecutorFactory executorFactory, + final ExecutorFactory ioExecutors, final IOCriticalErrorListener criticalErrorListener) { - super(config, executorFactory, null, criticalErrorListener); + super(config, executorFactory, null, ioExecutors, criticalErrorListener); } @Override @@ -116,7 +119,7 @@ public class JournalStorageManager extends AbstractJournalStorageManager { bindingsFF = new NIOSequentialFileFactory(config.getBindingsLocation(), criticalErrorListener, config.getJournalMaxIO_NIO()); - Journal localBindings = new JournalImpl(1024 * 1024, 2, config.getJournalCompactMinFiles(), config.getJournalPoolFiles(), config.getJournalCompactPercentage(), bindingsFF, "activemq-bindings", "bindings", 1); + Journal localBindings = new JournalImpl(ioExecutors, 1024 * 1024, 2, config.getJournalCompactMinFiles(), config.getJournalPoolFiles(), config.getJournalCompactPercentage(), bindingsFF, "activemq-bindings", "bindings", 1, 0); bindingsJournal = localBindings; originalBindingsJournal = localBindings; @@ -132,7 +135,8 @@ public class JournalStorageManager extends AbstractJournalStorageManager { throw ActiveMQMessageBundle.BUNDLE.invalidJournalType2(config.getJournalType()); } - Journal localMessage = new JournalImpl(config.getJournalFileSize(), config.getJournalMinFiles(), config.getJournalPoolFiles(), config.getJournalCompactMinFiles(), config.getJournalCompactPercentage(), journalFF, "activemq-data", "amq", config.getJournalType() == JournalType.ASYNCIO ? config.getJournalMaxIO_AIO() : config.getJournalMaxIO_NIO()); + Journal localMessage = new JournalImpl(ioExecutors, config.getJournalFileSize(), config.getJournalMinFiles(), config.getJournalPoolFiles(), config.getJournalCompactMinFiles(), config.getJournalCompactPercentage(), journalFF, "activemq-data", "amq", config.getJournalType() == JournalType.ASYNCIO ? config.getJournalMaxIO_AIO() : config.getJournalMaxIO_NIO(), 0); + messageJournal = localMessage; originalMessageJournal = localMessage; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java index 477f839aa6..a43fec8ea4 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java @@ -345,6 +345,8 @@ public interface ActiveMQServer extends ActiveMQComponent { ExecutorFactory getExecutorFactory(); + ExecutorFactory getIOExecutorFactory(); + void setGroupingHandler(GroupingHandler groupingHandler); GroupingHandler getGroupingHandler(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index 6288bdfec7..d2de96480e 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -232,8 +232,8 @@ public class ActiveMQServerImpl implements ActiveMQServer { private volatile ExecutorFactory executorFactory; - private volatile ExecutorService ioExecutorPool; + /** * This is a thread pool for io tasks only. * We can't use the same global executor to avoid starvations. @@ -1636,6 +1636,11 @@ public class ActiveMQServerImpl implements ActiveMQServer { return executorFactory; } + @Override + public ExecutorFactory getIOExecutorFactory() { + return ioExecutorFactory; + } + @Override public void setGroupingHandler(final GroupingHandler groupingHandler) { if (this.groupingHandler != null && managementService != null) { @@ -1770,10 +1775,10 @@ public class ActiveMQServerImpl implements ActiveMQServer { private StorageManager createStorageManager() { if (configuration.isPersistenceEnabled()) { if (configuration.getStoreConfiguration() != null && configuration.getStoreConfiguration().getStoreType() == StoreConfiguration.StoreType.DATABASE) { - return new JDBCJournalStorageManager(configuration, getScheduledPool(), executorFactory, shutdownOnCriticalIO); + return new JDBCJournalStorageManager(configuration, getScheduledPool(), executorFactory, ioExecutorFactory, shutdownOnCriticalIO); } else { // Default to File Based Storage Manager, (Legacy default configuration). - return new JournalStorageManager(configuration, executorFactory, scheduledPool, shutdownOnCriticalIO); + return new JournalStorageManager(configuration, executorFactory, scheduledPool, ioExecutorFactory, shutdownOnCriticalIO); } } return new NullStorageManager(); @@ -1847,6 +1852,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { }); this.ioExecutorPool = new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue(), tFactory); + this.ioExecutorFactory = new OrderedExecutorFactory(ioExecutorPool); } /* We check to see if a Scheduled Executor Service is provided in the InjectedObjectRegistry. If so we use this diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java index 519ffb5ff4..42c48f3fa1 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/NIOJournalCompactTest.java @@ -1623,11 +1623,15 @@ public class NIOJournalCompactTest extends JournalImplTestBase { final ExecutorService executor = Executors.newCachedThreadPool(ActiveMQThreadFactory.defaultThreadFactory()); + final ExecutorService ioexecutor = Executors.newCachedThreadPool(ActiveMQThreadFactory.defaultThreadFactory()); + OrderedExecutorFactory factory = new OrderedExecutorFactory(executor); + OrderedExecutorFactory iofactory = new OrderedExecutorFactory(ioexecutor); + final ExecutorService deleteExecutor = Executors.newCachedThreadPool(ActiveMQThreadFactory.defaultThreadFactory()); - final JournalStorageManager storage = new JournalStorageManager(config, factory); + final JournalStorageManager storage = new JournalStorageManager(config, factory, iofactory); storage.start(); @@ -1681,7 +1685,7 @@ public class NIOJournalCompactTest extends JournalImplTestBase { for (long messageID : values) { storage.deleteMessage(messageID); } - } catch (Exception e) { + } catch (Throwable e) { e.printStackTrace(); errors.incrementAndGet(); } @@ -1733,11 +1737,17 @@ public class NIOJournalCompactTest extends JournalImplTestBase { deleteExecutor.shutdown(); - assertTrue("delete executor terminated", deleteExecutor.awaitTermination(30, TimeUnit.SECONDS)); + assertTrue("delete executor failted to terminate", deleteExecutor.awaitTermination(30, TimeUnit.SECONDS)); + + storage.stop(); executor.shutdown(); - assertTrue("executor terminated", executor.awaitTermination(10, TimeUnit.SECONDS)); + assertTrue("executor failed to terminate", executor.awaitTermination(30, TimeUnit.SECONDS)); + + ioexecutor.shutdown(); + + assertTrue("ioexecutor failed to terminate", ioexecutor.awaitTermination(30, TimeUnit.SECONDS)); } catch (Throwable e) { e.printStackTrace(); @@ -1751,6 +1761,7 @@ public class NIOJournalCompactTest extends JournalImplTestBase { executor.shutdownNow(); deleteExecutor.shutdownNow(); + ioexecutor.shutdownNow(); } } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DeleteMessagesOnStartupTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DeleteMessagesOnStartupTest.java index 9848c39622..7d515d89f3 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DeleteMessagesOnStartupTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/DeleteMessagesOnStartupTest.java @@ -91,7 +91,7 @@ public class DeleteMessagesOnStartupTest extends StorageManagerTestBase { @Override protected JournalStorageManager createJournalStorageManager(Configuration configuration) { - return new JournalStorageManager(configuration, execFactory) { + return new JournalStorageManager(configuration, execFactory, execFactory) { @Override public void deleteMessage(final long messageID) throws Exception { deletedMessage.add(messageID); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/RestartSMTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/RestartSMTest.java index 5828baf033..49d3a1214a 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/RestartSMTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/RestartSMTest.java @@ -65,7 +65,7 @@ public class RestartSMTest extends ActiveMQTestBase { PostOffice postOffice = new FakePostOffice(); - final JournalStorageManager journal = new JournalStorageManager(createDefaultInVMConfig(), execFactory); + final JournalStorageManager journal = new JournalStorageManager(createDefaultInVMConfig(), execFactory, execFactory); try { diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java index 814bf0d79d..a1043638c9 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/persistence/StorageManagerTestBase.java @@ -137,7 +137,7 @@ public abstract class StorageManagerTestBase extends ActiveMQTestBase { * @param configuration */ protected JournalStorageManager createJournalStorageManager(Configuration configuration) { - JournalStorageManager jsm = new JournalStorageManager(configuration, execFactory); + JournalStorageManager jsm = new JournalStorageManager(configuration, execFactory, execFactory); addActiveMQComponent(jsm); return jsm; } @@ -146,7 +146,7 @@ public abstract class StorageManagerTestBase extends ActiveMQTestBase { * @param configuration */ protected JDBCJournalStorageManager createJDBCJournalStorageManager(Configuration configuration) { - JDBCJournalStorageManager jsm = new JDBCJournalStorageManager(configuration, execFactory, scheduledExecutorService); + JDBCJournalStorageManager jsm = new JDBCJournalStorageManager(configuration, execFactory, execFactory, scheduledExecutorService); addActiveMQComponent(jsm); return jsm; } @@ -155,7 +155,7 @@ public abstract class StorageManagerTestBase extends ActiveMQTestBase { * @throws Exception */ protected void createJMSStorage() throws Exception { - jmsJournal = new JMSJournalStorageManagerImpl(new TimeAndCounterIDGenerator(), createDefaultInVMConfig(), null); + jmsJournal = new JMSJournalStorageManagerImpl(null, new TimeAndCounterIDGenerator(), createDefaultInVMConfig(), null); addActiveMQComponent(jmsJournal); jmsJournal.start(); jmsJournal.load(); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/ReplicationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/ReplicationTest.java index 7d2d51460f..1ae9527d1b 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/ReplicationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/ReplicationTest.java @@ -435,7 +435,7 @@ public final class ReplicationTest extends ActiveMQTestBase { * @throws Exception */ private JournalStorageManager getStorage() throws Exception { - return new JournalStorageManager(createDefaultInVMConfig(), factory); + return new JournalStorageManager(createDefaultInVMConfig(), factory, factory); } /** diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SuppliedThreadPoolTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SuppliedThreadPoolTest.java index 65cd6b9e1b..1deb1bbeee 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SuppliedThreadPoolTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SuppliedThreadPoolTest.java @@ -44,6 +44,7 @@ public class SuppliedThreadPoolTest extends ActiveMQTestBase { public void setup() throws Exception { serviceRegistry = new ServiceRegistryImpl(); serviceRegistry.setExecutorService(Executors.newFixedThreadPool(1, ActiveMQThreadFactory.defaultThreadFactory())); + serviceRegistry.setIOExecutorService(Executors.newFixedThreadPool(5, ActiveMQThreadFactory.defaultThreadFactory())); serviceRegistry.setScheduledExecutorService(Executors.newScheduledThreadPool(1, ActiveMQThreadFactory.defaultThreadFactory())); server = new ActiveMQServerImpl(null, null, null, null, serviceRegistry); server.start(); @@ -58,6 +59,7 @@ public class SuppliedThreadPoolTest extends ActiveMQTestBase { } serviceRegistry.getExecutorService().shutdown(); serviceRegistry.getScheduledExecutorService().shutdown(); + serviceRegistry.getIOExecutorService().shutdown(); super.tearDown(); } diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java index 2b24296b4f..be6e5b3d11 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/AlignedJournalImplTest.java @@ -943,8 +943,6 @@ public class AlignedJournalImplTest extends ActiveMQTestBase { journalImpl.checkReclaimStatus(); journalImpl.flush(); - Assert.assertEquals(2, factory.listFiles("tt").size()); - } @Test diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalImplTestUnit.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalImplTestUnit.java index eb815ae995..3be030d3ec 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalImplTestUnit.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/JournalImplTestUnit.java @@ -62,12 +62,6 @@ public abstract class JournalImplTestUnit extends JournalImplTestBase { } catch (IllegalStateException e) { // OK } - try { - stopJournal(); - Assert.fail("Should throw exception"); - } catch (IllegalStateException e) { - // OK - } startJournal(); try { startJournal(); diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/DuplicateDetectionUnitTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/DuplicateDetectionUnitTest.java index fcd32c5ada..96fa35c951 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/DuplicateDetectionUnitTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/DuplicateDetectionUnitTest.java @@ -70,7 +70,7 @@ public class DuplicateDetectionUnitTest extends ActiveMQTestBase { @Before public void setUp() throws Exception { super.setUp(); - executor = Executors.newSingleThreadExecutor(ActiveMQThreadFactory.defaultThreadFactory()); + executor = Executors.newFixedThreadPool(10, ActiveMQThreadFactory.defaultThreadFactory()); factory = new OrderedExecutorFactory(executor); } @@ -92,7 +92,7 @@ public class DuplicateDetectionUnitTest extends ActiveMQTestBase { ScheduledExecutorService scheduledThreadPool = Executors.newScheduledThreadPool(ActiveMQDefaultConfiguration.getDefaultScheduledThreadPoolMaxSize(), ActiveMQThreadFactory.defaultThreadFactory()); - journal = new JournalStorageManager(configuration, factory); + journal = new JournalStorageManager(configuration, factory, factory); journal.start(); journal.loadBindingJournal(new ArrayList(), new ArrayList()); @@ -112,7 +112,7 @@ public class DuplicateDetectionUnitTest extends ActiveMQTestBase { journal.stop(); - journal = new JournalStorageManager(configuration, factory); + journal = new JournalStorageManager(configuration, factory, factory); journal.start(); journal.loadBindingJournal(new ArrayList(), new ArrayList()); @@ -135,7 +135,7 @@ public class DuplicateDetectionUnitTest extends ActiveMQTestBase { mapDups.clear(); - journal = new JournalStorageManager(configuration, factory); + journal = new JournalStorageManager(configuration, factory, factory); journal.start(); journal.loadBindingJournal(new ArrayList(), new ArrayList()); @@ -146,6 +146,8 @@ public class DuplicateDetectionUnitTest extends ActiveMQTestBase { values = mapDups.get(ADDRESS); Assert.assertEquals(10, values.size()); + + scheduledThreadPool.shutdown(); } finally { if (journal != null) { try {