From 2d7d714260297054ad38c4a52e43dcd4170cec27 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Fri, 19 Jul 2019 11:57:53 -0400 Subject: [PATCH] ARTEMIS-2428 Exposing timeout on configuration and changing it to 0 on the testsuite --- .../impl/netty/TransportConstants.java | 34 +++++++++++++++++++ .../impl/netty/TransportConstantTest.java | 32 +++++++++++++++++ .../remoting/impl/netty/NettyAcceptor.java | 34 +++++++++++++++++-- .../artemis/uri/AcceptorParserTest.java | 19 +++++++++++ pom.xml | 2 +- 5 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 artemis-core-client/src/test/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstantTest.java diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java index 50da17950a..54b2061c19 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java @@ -22,9 +22,12 @@ import java.util.Set; import io.netty.util.Version; import org.apache.activemq.artemis.api.config.ActiveMQDefaultConfiguration; +import org.jboss.logging.Logger; public class TransportConstants { + private static final Logger logger = Logger.getLogger(TransportConstants.class); + public static final String SSL_ENABLED_PROP_NAME = "sslEnabled"; public static final String SSL_KRB5_CONFIG_PROP_NAME = "sslKrb5Config"; @@ -281,6 +284,35 @@ public class TransportConstants { public static final int DEFAULT_HANDSHAKE_TIMEOUT = 10; + public static final String QUIET_PERIOD = "quietPeriod"; + + /** We let this to be defined as a System Variable, as we need a different timeout over our testsuite. + * When running on a real server, this is the default we want. + * When running on a test suite, we need it to be 0, You should see a property on the main pom.xml. + */ + public static final int DEFAULT_QUIET_PERIOD = parseDefaultVariable("DEFAULT_QUIET_PERIOD", 100); + + public static final String SHUTDOWN_TIMEOUT = "shutdownTimeout"; + + /** We let this to be defined as a System Variable, as we need a different timeout over our testsuite. + * When running on a real server, this is the default we want. + * When running on a test suite, we need it to be 0, You should see a property on the main pom.xml */ + public static final int DEFAULT_SHUTDOWN_TIMEOUT = parseDefaultVariable("DEFAULT_SHUTDOWN_TIMEOUT", 3_000); + + private static int parseDefaultVariable(String variableName, int defaultValue) { + String variable = System.getProperty(TransportConstants.class.getName() + "." + variableName); + if (variable != null) { + try { + return Integer.parseInt(variable); + } catch (Throwable ignored) { + logger.debug(ignored); + } + } + + return defaultValue; + } + + static { Set allowableAcceptorKeys = new HashSet<>(); allowableAcceptorKeys.add(TransportConstants.SSL_ENABLED_PROP_NAME); @@ -332,6 +364,8 @@ public class TransportConstants { allowableAcceptorKeys.add(TransportConstants.CRL_PATH_PROP_NAME); allowableAcceptorKeys.add(TransportConstants.HANDSHAKE_TIMEOUT); allowableAcceptorKeys.add(TransportConstants.SSL_PROVIDER); + allowableAcceptorKeys.add(TransportConstants.SHUTDOWN_TIMEOUT); + allowableAcceptorKeys.add(TransportConstants.QUIET_PERIOD); ALLOWABLE_ACCEPTOR_KEYS = Collections.unmodifiableSet(allowableAcceptorKeys); diff --git a/artemis-core-client/src/test/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstantTest.java b/artemis-core-client/src/test/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstantTest.java new file mode 100644 index 0000000000..344ab7c295 --- /dev/null +++ b/artemis-core-client/src/test/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstantTest.java @@ -0,0 +1,32 @@ +/* + * 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.core.remoting.impl.netty; + +import org.junit.Assert; +import org.junit.Test; + +public class TransportConstantTest { + + /** We change the default on the main pom.xml + * This is just validating the pom still works */ + @Test + public void testDefaultOnPom() { + Assert.assertEquals("It is expected to have the default at 0 on the testsuite", 0, TransportConstants.DEFAULT_QUIET_PERIOD); + Assert.assertEquals("It is expected to have the default at 0 on the testsuite", 0, TransportConstants.DEFAULT_SHUTDOWN_TIMEOUT); + } +} diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java index 418e9a90bf..b9fc72cb8c 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java @@ -101,6 +101,9 @@ import org.jboss.logging.Logger; */ public class NettyAcceptor extends AbstractAcceptor { + private static final Logger logger = Logger.getLogger(NettyAcceptor.class); + + public static String INVM_ACCEPTOR_TYPE = "IN-VM"; public static String NIO_ACCEPTOR_TYPE = "NIO"; public static String EPOLL_ACCEPTOR_TYPE = "EPOLL"; @@ -200,6 +203,12 @@ public class NettyAcceptor extends AbstractAcceptor { private NotificationService notificationService; + /** The amount of time we wait before new tasks are added during a shutdown period. */ + private int quietPeriod; + + /** The total amount of time we wait before a hard shutdown. */ + private int shutdownTimeout; + private boolean paused; private BatchFlusher flusher; @@ -216,7 +225,6 @@ public class NettyAcceptor extends AbstractAcceptor { private Map extraConfigs; - private static final Logger logger = Logger.getLogger(NettyAcceptor.class); final AtomicBoolean warningPrinted = new AtomicBoolean(false); @@ -261,6 +269,10 @@ public class NettyAcceptor extends AbstractAcceptor { this.protocolsString = getProtocols(protocolMap); + this.quietPeriod = ConfigurationHelper.getIntProperty(TransportConstants.QUIET_PERIOD, TransportConstants.DEFAULT_QUIET_PERIOD, configuration); + + this.shutdownTimeout = ConfigurationHelper.getIntProperty(TransportConstants.SHUTDOWN_TIMEOUT, TransportConstants.DEFAULT_SHUTDOWN_TIMEOUT, configuration); + host = ConfigurationHelper.getStringProperty(TransportConstants.HOST_PROP_NAME, TransportConstants.DEFAULT_HOST, configuration); port = ConfigurationHelper.getIntProperty(TransportConstants.PORT_PROP_NAME, TransportConstants.DEFAULT_PORT, configuration); if (sslEnabled) { @@ -719,7 +731,7 @@ public class NettyAcceptor extends AbstractAcceptor { // Shutdown the EventLoopGroup if no new task was added for 100ms or if // 3000ms elapsed. - eventLoopGroup.shutdownGracefully(100, 3000, TimeUnit.MILLISECONDS).addListener(f -> callback.run()); + eventLoopGroup.shutdownGracefully(quietPeriod, shutdownTimeout, TimeUnit.MILLISECONDS).addListener(f -> callback.run()); eventLoopGroup = null; } @@ -787,6 +799,24 @@ public class NettyAcceptor extends AbstractAcceptor { return new ActiveMQServerChannelHandler(channelGroup, handler, new Listener(), failureExecutor); } + public int getQuietPeriod() { + return quietPeriod; + } + + public NettyAcceptor setQuietPeriod(int quietPeriod) { + this.quietPeriod = quietPeriod; + return this; + } + + public int getShutdownTimeout() { + return shutdownTimeout; + } + + public NettyAcceptor setShutdownTimeout(int shutdownTimeout) { + this.shutdownTimeout = shutdownTimeout; + return this; + } + private static String getProtocols(Map protocolManager) { StringBuilder sb = new StringBuilder(); if (protocolManager != null) { diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/uri/AcceptorParserTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/uri/AcceptorParserTest.java index 527e967af9..0d8e206dfc 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/uri/AcceptorParserTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/uri/AcceptorParserTest.java @@ -17,10 +17,14 @@ package org.apache.activemq.artemis.uri; +import java.util.HashMap; import java.util.List; import org.apache.activemq.artemis.api.core.TransportConfiguration; import org.apache.activemq.artemis.core.config.ConfigurationUtils; +import org.apache.activemq.artemis.core.remoting.impl.netty.NettyAcceptor; +import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants; +import org.apache.activemq.artemis.utils.ConfigurationHelper; import org.junit.Assert; import org.junit.Test; @@ -36,6 +40,21 @@ public class AcceptorParserTest { } } + @Test + public void testAcceptorShutdownTimeout() { + List configs = ConfigurationUtils.parseAcceptorURI("test", "tcp://localhost:8080?quietPeriod=33;shutdownTimeout=55"); + + Assert.assertEquals(1, configs.size()); + + Assert.assertEquals(33, ConfigurationHelper.getIntProperty(TransportConstants.QUIET_PERIOD, -1, configs.get(0).getParams())); + Assert.assertEquals(55, ConfigurationHelper.getIntProperty(TransportConstants.SHUTDOWN_TIMEOUT, -1, configs.get(0).getParams())); + + NettyAcceptor nettyAcceptor = new NettyAcceptor("name", null, configs.get(0).getParams(), null, null, null, null, new HashMap<>()); + + Assert.assertEquals(33, nettyAcceptor.getQuietPeriod()); + Assert.assertEquals(55, nettyAcceptor.getShutdownTimeout()); + } + @Test public void testAcceptorWithQueryParamEscapes() throws Exception { List configs = ConfigurationUtils.parseAcceptorURI("test", "tcp://0.0.0.0:5672?tcpSendBufferSize=1048576;tcpReceiveBufferSize=1048576;virtualTopicConsumerWildcards=Consumer.*.%3E%3B2"); diff --git a/pom.xml b/pom.xml index 4c79a0644c..23570dfd2a 100644 --- a/pom.xml +++ b/pom.xml @@ -169,7 +169,7 @@ --> - -Dbrokerconfig.maxDiskUsage=100 -Djava.util.logging.manager=org.jboss.logmanager.LogManager + -Dbrokerconfig.maxDiskUsage=100 -Dorg.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants.DEFAULT_QUIET_PERIOD=0 -Dorg.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants.DEFAULT_SHUTDOWN_TIMEOUT=0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -Dlogging.configuration="file:${activemq.basedir}/tests/config/logging.properties" -Djava.library.path="${activemq.basedir}/target/bin/lib/linux-x86_64:${activemq.basedir}/target/bin/lib/linux-i686" -Djgroups.bind_addr=localhost -Dorg.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory.localBindAddress=localhost -Djava.net.preferIPv4Stack=true -Dbasedir=${basedir}