From f680d9f71209d4f5a714120bad96e728dee91bca Mon Sep 17 00:00:00 2001 From: brusdev Date: Tue, 5 Nov 2019 22:44:52 +0100 Subject: [PATCH] ARTEMIS-2466 PageSyncTimer::timeSync isn't configurable using ASYNCIO Add the config parameter `page-sync-timeout` to set a customized value, because if the broker is configured to use ASYNCIO journal, the timeout has the same value of NIO default journal buffer timeout ie 3333333. --- .../activemq/artemis/cli/commands/Create.java | 15 +++++++++++++ .../artemis/cli/commands/etc/broker.xml | 4 +++- .../cli/commands/etc/page-sync-settings.txt | 2 ++ .../apache/activemq/cli/test/ArtemisTest.java | 21 +++++++++++++++++++ .../artemis/core/config/Configuration.java | 12 +++++++++++ .../core/config/impl/ConfigurationImpl.java | 13 ++++++++++++ .../impl/FileConfigurationParser.java | 2 ++ .../core/server/impl/ActiveMQServerImpl.java | 4 ++-- .../schema/artemis-configuration.xsd | 9 ++++++++ .../config/impl/ConfigurationImplTest.java | 9 ++++++++ .../impl/DefaultsFileConfigurationTest.java | 2 ++ .../impl/FileConfigurationParserTest.java | 12 +++++++++++ .../ConfigurationTest-full-config.xml | 1 + docs/user-manual/en/configuration-index.md | 1 + docs/user-manual/en/paging.md | 7 +++++++ 15 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/page-sync-settings.txt diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java index 36d0c3a75e..07456b5dc3 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java @@ -109,6 +109,7 @@ public class Create extends InputAbstract { public static final String ETC_GLOBAL_MAX_SPECIFIED_TXT = "etc/global-max-specified.txt"; public static final String ETC_GLOBAL_MAX_DEFAULT_TXT = "etc/global-max-default.txt"; + public static final String ETC_PAGE_SYNC_SETTINGS = "etc/page-sync-settings.txt"; public static final String ETC_JOLOKIA_ACCESS_XML = "jolokia-access.xml"; @Arguments(description = "The instance directory to hold the broker's configuration and data. Path must be writable.", required = true) @@ -1002,6 +1003,7 @@ public class Create extends InputAbstract { private void performAutoTune(HashMap filters, JournalType journalType, File dataFolder) { if (noAutoTune) { filters.put("${journal-buffer.settings}", ""); + filters.put("${page-sync.settings}", ""); } else { try { int writes = 250; @@ -1018,6 +1020,7 @@ public class Create extends InputAbstract { filters.put("${journal-buffer.settings}", readTextFile(ETC_JOURNAL_BUFFER_SETTINGS, syncFilter)); + filters.put("${page-sync.settings}", readTextFile(ETC_PAGE_SYNC_SETTINGS, syncFilter)); } else { long time = SyncCalculation.syncTest(dataFolder, 4096, writes, 5, verbose, !noJournalSync, false, "journal-test.tmp", ActiveMQDefaultConfiguration.getDefaultJournalMaxIoAio(), journalType); long nanoseconds = SyncCalculation.toNanos(time, writes, verbose); @@ -1034,11 +1037,23 @@ public class Create extends InputAbstract { " writes per millisecond, your journal-buffer-timeout will be " + nanoseconds); filters.put("${journal-buffer.settings}", readTextFile(ETC_JOURNAL_BUFFER_SETTINGS, syncFilter)); + + if (noJournalSync) { + syncFilter.put("${nanoseconds}", "0"); + } else if (journalType != JournalType.NIO) { + long nioTime = SyncCalculation.syncTest(dataFolder, 4096, writes, 5, verbose, !noJournalSync, false, "journal-test.tmp", ActiveMQDefaultConfiguration.getDefaultJournalMaxIoAio(), JournalType.NIO); + long nioNanoseconds = SyncCalculation.toNanos(nioTime, writes, verbose); + syncFilter.put("${nanoseconds}", Long.toString(nioNanoseconds)); + } + + filters.put("${page-sync.settings}", readTextFile(ETC_PAGE_SYNC_SETTINGS, syncFilter)); } + } catch (Exception e) { filters.put("${journal-buffer.settings}", ""); + filters.put("${page-sync.settings}", ""); e.printStackTrace(); System.err.println("Couldn't perform sync calculation, using default values"); } diff --git a/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/broker.xml b/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/broker.xml index 4472c4625c..f26d651ebf 100644 --- a/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/broker.xml +++ b/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/broker.xml @@ -73,7 +73,9 @@ ${jdbc} HALT -${global-max-section} + ${page-sync.settings} + + ${global-max-section} diff --git a/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/page-sync-settings.txt b/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/page-sync-settings.txt new file mode 100644 index 0000000000..e1b79fd487 --- /dev/null +++ b/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/page-sync-settings.txt @@ -0,0 +1,2 @@ + + ${nanoseconds} diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java index 6ffbf46069..b3f8f39c78 100644 --- a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java +++ b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java @@ -884,6 +884,27 @@ public class ArtemisTest extends CliTestBase { } + @Test + public void testAutoTune() throws Exception { + File instanceFolder = temporaryFolder.newFolder("autoTuneTest"); + + setupAuth(instanceFolder); + + // This is usually set when run from the command line via artemis.profile + Run.setEmbedded(true); + Artemis.main("create", instanceFolder.getAbsolutePath(), "--force", "--silent", "--no-web", "--require-login"); + System.setProperty("artemis.instance", instanceFolder.getAbsolutePath()); + + FileConfiguration fc = new FileConfiguration(); + FileDeploymentManager deploymentManager = new FileDeploymentManager(new File(instanceFolder, "./etc/broker.xml").toURI().toString()); + deploymentManager.addDeployable(fc); + + fc.setPageSyncTimeout(-1); + deploymentManager.readConfiguration(); + + Assert.assertNotEquals(-1, fc.getPageSyncTimeout()); + } + @Test public void testQstat() throws Exception { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java index 9b1a75cbc8..6f2c507b65 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java @@ -1193,6 +1193,18 @@ public interface Configuration { String getInternalNamingPrefix(); + /** + * Returns the timeout (in nanoseconds) used to sync pages. + *
+ * Default value is {@link org.apache.activemq.artemis.ArtemisConstants#DEFAULT_JOURNAL_BUFFER_TIMEOUT_NIO}. + */ + int getPageSyncTimeout(); + + /** + * Sets the timeout (in nanoseconds) used to sync pages. + */ + Configuration setPageSyncTimeout(int pageSyncTimeout); + /** * @param plugins */ diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java index 573ebe3a53..8fc7262d12 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java @@ -338,6 +338,8 @@ public class ConfigurationImpl implements Configuration, Serializable { private long criticalAnalyzerCheckPeriod = 0; // non set + private int pageSyncTimeout = ActiveMQDefaultConfiguration.getDefaultJournalBufferTimeoutNio(); + /** * Parent folder for all data folders. */ @@ -2348,6 +2350,17 @@ public class ConfigurationImpl implements Configuration, Serializable { return this; } + @Override + public int getPageSyncTimeout() { + return pageSyncTimeout; + } + + @Override + public ConfigurationImpl setPageSyncTimeout(final int pageSyncTimeout) { + this.pageSyncTimeout = pageSyncTimeout; + return this; + } + public static boolean checkoutDupCacheSize(final int windowSize, final int idCacheSize) { final int msgNumInFlight = windowSize / DEFAULT_JMS_MESSAGE_SIZE; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java index 9983ef08ec..648943c04e 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java @@ -677,6 +677,8 @@ public final class FileConfigurationParser extends XMLConfigurationUtil { config.setCriticalAnalyzerPolicy(CriticalAnalyzerPolicy.valueOf(getString(e, "critical-analyzer-policy", config.getCriticalAnalyzerPolicy().name(), Validators.NOT_NULL_OR_EMPTY))); + config.setPageSyncTimeout(getInteger(e, "page-sync-timeout", config.getJournalBufferTimeout_NIO(), Validators.GE_ZERO)); + parseAddressSettings(e, config); parseResourceLimits(e, config); 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 25486c42ba..f73407a54d 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 @@ -2603,9 +2603,9 @@ public class ActiveMQServerImpl implements ActiveMQServer { protected PagingStoreFactory getPagingStoreFactory() throws Exception { if (configuration.getStoreConfiguration() != null && configuration.getStoreConfiguration().getStoreType() == StoreConfiguration.StoreType.DATABASE) { DatabaseStorageConfiguration dbConf = (DatabaseStorageConfiguration) configuration.getStoreConfiguration(); - return new PagingStoreFactoryDatabase(dbConf, storageManager, configuration.getJournalBufferTimeout_NIO(), scheduledPool, ioExecutorFactory, false, shutdownOnCriticalIO, configuration.isReadWholePage()); + return new PagingStoreFactoryDatabase(dbConf, storageManager, configuration.getPageSyncTimeout(), scheduledPool, ioExecutorFactory, false, shutdownOnCriticalIO, configuration.isReadWholePage()); } - return new PagingStoreFactoryNIO(storageManager, configuration.getPagingLocation(), configuration.getJournalBufferTimeout_NIO(), scheduledPool, ioExecutorFactory, configuration.isJournalSyncNonTransactional(), shutdownOnCriticalIO, configuration.isReadWholePage()); + return new PagingStoreFactoryNIO(storageManager, configuration.getPagingLocation(), configuration.getPageSyncTimeout(), scheduledPool, ioExecutorFactory, configuration.isJournalSyncNonTransactional(), shutdownOnCriticalIO, configuration.isReadWholePage()); } /** diff --git a/artemis-server/src/main/resources/schema/artemis-configuration.xsd b/artemis-server/src/main/resources/schema/artemis-configuration.xsd index f99ca90605..724485429b 100644 --- a/artemis-server/src/main/resources/schema/artemis-configuration.xsd +++ b/artemis-server/src/main/resources/schema/artemis-configuration.xsd @@ -888,6 +888,15 @@ + + + + The timeout (in nanoseconds) used to sync pages. The exact default value + depend on whether the journal is ASYNCIO or NIO. + + + + diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java index ab4a7ea2a7..c1a2910278 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java @@ -89,6 +89,7 @@ public class ConfigurationImplTest extends ActiveMQTestBase { Assert.assertEquals(conf.getJournalLocation(), conf.getNodeManagerLockLocation()); Assert.assertNull(conf.getJournalDeviceBlockSize()); Assert.assertEquals(ActiveMQDefaultConfiguration.isDefaultReadWholePage(), conf.isReadWholePage()); + Assert.assertEquals(ActiveMQDefaultConfiguration.getDefaultJournalBufferTimeoutNio(), conf.getPageSyncTimeout()); } @Test @@ -277,6 +278,10 @@ public class ConfigurationImplTest extends ActiveMQTestBase { s = RandomUtil.randomString(); conf.setClusterPassword(s); Assert.assertEquals(s, conf.getClusterPassword()); + + i = RandomUtil.randomInt(); + conf.setPageSyncTimeout(i); + Assert.assertEquals(i, conf.getPageSyncTimeout()); } } @@ -480,6 +485,10 @@ public class ConfigurationImplTest extends ActiveMQTestBase { conf.setClusterPassword(s); Assert.assertEquals(s, conf.getClusterPassword()); + i = RandomUtil.randomInt(); + conf.setPageSyncTimeout(i); + Assert.assertEquals(i, conf.getPageSyncTimeout()); + conf.registerBrokerPlugin(new LoggingActiveMQServerPlugin()); Assert.assertEquals("ensure one plugin registered", 1, conf.getBrokerPlugins().size()); diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/DefaultsFileConfigurationTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/DefaultsFileConfigurationTest.java index 388d2cae45..7f16114f03 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/DefaultsFileConfigurationTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/DefaultsFileConfigurationTest.java @@ -137,6 +137,8 @@ public class DefaultsFileConfigurationTest extends ConfigurationImplTest { Assert.assertEquals(ActiveMQDefaultConfiguration.getDefaultGracefulShutdownTimeout(), conf.getGracefulShutdownTimeout()); Assert.assertEquals(ActiveMQDefaultConfiguration.getDefaultAmqpUseCoreSubscriptionNaming(), conf.isAmqpUseCoreSubscriptionNaming()); + + Assert.assertEquals(ActiveMQDefaultConfiguration.getDefaultJournalBufferTimeoutNio(), conf.getPageSyncTimeout()); } // Protected --------------------------------------------------------------------------------------------- diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java index 2b943d4094..2507cadfd1 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationParserTest.java @@ -351,6 +351,18 @@ public class FileConfigurationParserTest extends ActiveMQTestBase { assertEquals(expected, addressSettings.getMaxSizeBytesRejectThreshold()); } + @Test + public void testParsingPageSyncTimeout() throws Exception { + int expected = 1000; + FileConfigurationParser parser = new FileConfigurationParser(); + + String configStr = firstPart + String.format("%d\n", expected) + lastPart; + ByteArrayInputStream input = new ByteArrayInputStream(configStr.getBytes(StandardCharsets.UTF_8)); + + Configuration config = parser.parseMainConfig(input); + assertEquals(expected, config.getPageSyncTimeout()); + } + private static String firstPart = "" + "\n" + "ActiveMQ.main.config" + "\n" + "org.apache.activemq.artemis.integration.logging.Log4jLogDelegateFactory" + "\n" + diff --git a/artemis-server/src/test/resources/ConfigurationTest-full-config.xml b/artemis-server/src/test/resources/ConfigurationTest-full-config.xml index d282d1d61a..5c40d7ea02 100644 --- a/artemis-server/src/test/resources/ConfigurationTest-full-config.xml +++ b/artemis-server/src/test/resources/ConfigurationTest-full-config.xml @@ -369,6 +369,7 @@ www.apache.org ping-four ping-six + 1000 diff --git a/docs/user-manual/en/configuration-index.md b/docs/user-manual/en/configuration-index.md index 585677cf94..389d619129 100644 --- a/docs/user-manual/en/configuration-index.md +++ b/docs/user-manual/en/configuration-index.md @@ -159,6 +159,7 @@ log-delegate-factory-class-name | **deprecated** the name of the factory class t name | node name; used in topology notifications if set. | n/a [password-codec](masking-passwords.md) | the name of the class (and optional configuration properties) used to decode masked passwords. Only valid when `mask-password` is `true`. | n/a [page-max-concurrent-io](paging.md) | The max number of concurrent reads allowed on paging. | 5 +[page-sync-timeout](paging.md#page-sync-timeout) | The time in nanoseconds a page will be synced. | 3333333 for ASYNCIO; `journal-buffer-timeout` for NIO [read-whole-page](paging.md) | If true the whole page would be read, otherwise just seek and read while getting message. | `false` [paging-directory](paging.md#configuration)| the directory to store paged messages in. | `data/paging` [persist-delivery-count-before-delivery](undelivered-messages.md#delivery-count-persistence) | True means that the delivery count is persisted before delivery. False means that this only happens after a message has been cancelled. | `false` diff --git a/docs/user-manual/en/paging.md b/docs/user-manual/en/paging.md index 9177efe91c..bd8e7958fb 100644 --- a/docs/user-manual/en/paging.md +++ b/docs/user-manual/en/paging.md @@ -171,6 +171,13 @@ Once that limit is reached any message will be blocked. (unless the protocol doesn't support flow control on which case there will be an exception thrown and the connection for those clients dropped). +## Page Sync Timeout + +The pages are synced periodically and the sync period is configured through +`page-sync-timeout` in nanoseconds. When using NIO journal, by default has +the same value of `journal-buffer-timeout`. When using ASYNCIO, the default +should be `3333333`. + ## Example See the [Paging Example](examples.md#paging) which shows how to use paging with