[activemq-artemis] 02/03: ARTEMIS-2428 Exposing timeout on configuration and changing it to 0 on the testsuite

[cl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

clebertsuconic pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git

commit 2d7d714260297054ad38c4a52e43dcd4170cec27
Author: Clebert Suconic <cl...@apache.org>
AuthorDate: Fri Jul 19 11:57:53 2019 -0400

    ARTEMIS-2428 Exposing timeout on configuration and changing it to 0 on the testsuite
---
 .../remoting/impl/netty/TransportConstants.java    | 34 ++++++++++++++++++++++
 .../remoting/impl/netty/TransportConstantTest.java | 32 ++++++++++++++++++++
 .../core/remoting/impl/netty/NettyAcceptor.java    | 34 ++++++++++++++++++++--
 .../activemq/artemis/uri/AcceptorParserTest.java   | 19 ++++++++++++
 pom.xml                                            |  2 +-
 5 files changed, 118 insertions(+), 3 deletions(-)

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 50da179..54b2061 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<String> 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 0000000..344ab7c
--- /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 418e9a9..b9fc72c 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<String, Object> 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<String, ProtocolManager> 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 527e967..0d8e206 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;
 
@@ -37,6 +41,21 @@ public class AcceptorParserTest {
    }
 
    @Test
+   public void testAcceptorShutdownTimeout() {
+      List<TransportConfiguration> 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<TransportConfiguration> 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 4c79a06..23570df 100644
--- a/pom.xml
+++ b/pom.xml
@@ -169,7 +169,7 @@
 
       -->
 
-      <activemq-surefire-argline>-Dbrokerconfig.maxDiskUsage=100 -Djava.util.logging.manager=org.jboss.logmanager.LogManager
+      <activemq-surefire-argline>-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}