You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2019/07/19 19:01:50 UTC
[activemq-artemis] branch master updated (5107eb9 -> 1c6dea8)
This is an automated email from the ASF dual-hosted git repository.
clebertsuconic pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git.
from 5107eb9 This closes #2761
new 89cc07c ARTEMIS-2408 Changing FileDescriptors rule as classRule
new 2d7d714 ARTEMIS-2428 Exposing timeout on configuration and changing it to 0 on the testsuite
new 1c6dea8 This closes #2762
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../remoting/impl/netty/TransportConstants.java | 34 ++++++++
.../impl/netty/TransportConstantTest.java} | 26 +++---
.../core/remoting/impl/netty/NettyAcceptor.java | 34 +++++++-
.../artemis/tests/util/ActiveMQTestBase.java | 5 +-
.../artemis/tests/util/NoProcessFilesBehind.java | 98 ++++------------------
.../activemq/artemis/uri/AcceptorParserTest.java | 19 +++++
pom.xml | 2 +-
7 files changed, 113 insertions(+), 105 deletions(-)
copy artemis-core-client/src/{main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NonClosingDefaultFileRegion.java => test/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstantTest.java} (63%)
[activemq-artemis] 02/03: ARTEMIS-2428 Exposing timeout on
configuration and changing it to 0 on the testsuite
Posted by 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}
[activemq-artemis] 01/03: ARTEMIS-2408 Changing FileDescriptors
rule as classRule
Posted by 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 89cc07c7d8bf12d83d7a81eb8f71ee9746c5fb8e
Author: Clebert Suconic <cl...@apache.org>
AuthorDate: Thu Jul 18 23:34:33 2019 -0400
ARTEMIS-2408 Changing FileDescriptors rule as classRule
---
.../artemis/tests/util/ActiveMQTestBase.java | 5 +-
.../artemis/tests/util/NoProcessFilesBehind.java | 98 ++++------------------
2 files changed, 17 insertions(+), 86 deletions(-)
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 19a3919..22b3173 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
@@ -171,8 +171,8 @@ public abstract class ActiveMQTestBase extends Assert {
@ClassRule
public static ThreadLeakCheckRule leakCheckRule = new ThreadLeakCheckRule();
- @Rule
- public NoProcessFilesBehind noProcessFilesBehind = new NoProcessFilesBehind(-1, 1000);
+ @ClassRule
+ public static NoProcessFilesBehind noProcessFilesBehind = new NoProcessFilesBehind(1000);
/** We should not under any circunstance create data outside of ./target
* if you have a test failing because because of this rule for any reason,
@@ -279,7 +279,6 @@ public abstract class ActiveMQTestBase extends Assert {
@After
public void tearDown() throws Exception {
- noProcessFilesBehind.tearDown();
closeAllSessionFactories();
closeAllServerLocatorsFactories();
diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/NoProcessFilesBehind.java b/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/NoProcessFilesBehind.java
index fe28ad7..e04b6ce 100644
--- a/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/NoProcessFilesBehind.java
+++ b/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/NoProcessFilesBehind.java
@@ -19,16 +19,18 @@ package org.apache.activemq.artemis.tests.util;
import java.io.BufferedReader;
import java.io.InputStreamReader;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import java.lang.management.ManagementFactory;
import java.lang.management.OperatingSystemMXBean;
import java.util.ArrayList;
-import java.util.Iterator;
import java.util.List;
import com.sun.management.UnixOperatingSystemMXBean;
import org.apache.activemq.artemis.nativo.jlibaio.LibaioContext;
import org.apache.activemq.artemis.utils.Wait;
import org.jboss.logging.Logger;
+import org.junit.Assert;
import org.junit.rules.TestWatcher;
import org.junit.runner.Description;
@@ -42,17 +44,13 @@ public class NoProcessFilesBehind extends TestWatcher {
/**
* -1 on maxVariance means no check
*/
- public NoProcessFilesBehind(int maxVariance, long maxFiles) {
+ public NoProcessFilesBehind(long maxFiles) {
- this.maxVariance = maxVariance;
this.maxFiles = maxFiles;
}
long fdBefore;
- int maxVariance;
long maxFiles;
- List<String> openFilesBefore;
- List<String> openFilesAfter;
static OperatingSystemMXBean os = ManagementFactory.getOperatingSystemMXBean();
@@ -68,7 +66,6 @@ public class NoProcessFilesBehind extends TestWatcher {
protected void starting(Description description) {
LibaioContext.isLoaded();
fdBefore = getOpenFD();
- openFilesBefore = getOpenFiles(true);
}
public static List<String> getOpenFiles(boolean filtered) {
@@ -92,27 +89,6 @@ public class NoProcessFilesBehind extends TestWatcher {
return openFiles;
}
-
- private static List<String> getDiffFiles(List<String> xOpenFiles, List<String> yOpenFiles) {
- ArrayList<String> diffOpenFiles = new ArrayList<>();
- for (String xOpenFile : xOpenFiles) {
- boolean found = false;
- Iterator<String> yOpenFilesIterator = yOpenFiles.iterator();
- String xOpenFileS = xOpenFile.replaceAll("\\s+", "\\\\s+");
-
- while (yOpenFilesIterator.hasNext() && !found) {
- String yOpenFileS = yOpenFilesIterator.next().replaceAll("\\s+", "\\\\s+");
- found = yOpenFileS.equals(xOpenFileS);
- }
-
- if (!found) {
- diffOpenFiles.add(xOpenFile);
- }
- }
-
- return diffOpenFiles;
- }
-
private static int getProcessId() throws ReflectiveOperationException {
java.lang.management.RuntimeMXBean runtime = java.lang.management.ManagementFactory.getRuntimeMXBean();
java.lang.reflect.Field jvmField = runtime.getClass().getDeclaredField("jvm");
@@ -123,9 +99,6 @@ public class NoProcessFilesBehind extends TestWatcher {
return (Integer) getProcessIdMethod.invoke(jvm);
}
- public void tearDown() {
- openFilesAfter = getOpenFiles(true);
- }
@Override
protected void failed(Throwable e, Description description) {
@@ -135,68 +108,27 @@ public class NoProcessFilesBehind extends TestWatcher {
protected void succeeded(Description description) {
}
- List<String> getVariance() {
-
- long fdAfter = getOpenFD();
-
- long variance = fdAfter - fdBefore;
-
- if (variance > 0) {
- List<String> currOpenFiles = getOpenFiles(true);
- List<String> diffOpenFiles = getDiffFiles(currOpenFiles, openFilesBefore);
- List<String> skippingOpenFiles = getDiffFiles(currOpenFiles, openFilesAfter);
- List<String> leavingOpenFiles = getDiffFiles(diffOpenFiles, skippingOpenFiles);
-
- return leavingOpenFiles;
- } else {
- return new ArrayList<>();
- }
- }
-
/**
* Override to tear down your specific external resource.
*/
@Override
protected void finished(Description description) {
- long fdAfter = getOpenFD();
- List<String> variance = getVariance();
-
- if (variance.size() > 0) {
- log.warn("test " + description.toString() + " is leaving " + variance.size() + " files open with a total number of files open = " + fdAfter);
- System.err.println("test " + description.toString() + " is leaving " + variance.size() + " files open with a total number of files open = " + fdAfter);
-
- for (String openFile : variance) {
- System.err.println(openFile);
+ Wait.waitFor(() -> getOpenFD() < maxFiles, 5000, 0);
+ if (getOpenFD() >= maxFiles) {
+ List<String> openFiles = getOpenFiles(true);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ boolean first = true;
+ for (String str : openFiles) {
+ if (!first) printWriter.print(", ");
+ first = false;
+ printWriter.print(str);
}
+ Assert.fail("Too many files open (" + maxFiles + "). A possible list: " + stringWriter.toString());
}
-
- if (maxVariance > 0) {
- VarianceCondition varianceCondition = new VarianceCondition();
- Wait.assertTrue("The test " + description.toString() + " is leaving " + varianceCondition.getVarianceSize() + " files open, which is more than " + maxVariance + " max open", varianceCondition, 5000, 0);
- }
-
Wait.assertTrue("Too many open files", () -> getOpenFD() < maxFiles, 5000, 0);
}
- class VarianceCondition implements Wait.Condition {
- private List<String> variance = null;
-
- public long getVarianceSize() {
- if (variance != null) {
- return variance.size();
- } else {
- return 0;
- }
- }
-
- @Override
- public boolean isSatisfied() throws Exception {
- variance = getVariance();
-
- return variance.size() < maxVariance;
- }
- }
-
}
[activemq-artemis] 03/03: This closes #2762
Posted by 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 1c6dea80dd407bdb8642a6187e79256db1c22256
Merge: 5107eb9 2d7d714
Author: Clebert Suconic <cl...@apache.org>
AuthorDate: Fri Jul 19 15:01:41 2019 -0400
This closes #2762
.../remoting/impl/netty/TransportConstants.java | 34 ++++++++
.../remoting/impl/netty/TransportConstantTest.java | 32 +++++++
.../core/remoting/impl/netty/NettyAcceptor.java | 34 +++++++-
.../artemis/tests/util/ActiveMQTestBase.java | 5 +-
.../artemis/tests/util/NoProcessFilesBehind.java | 98 ++++------------------
.../activemq/artemis/uri/AcceptorParserTest.java | 19 +++++
pom.xml | 2 +-
7 files changed, 135 insertions(+), 89 deletions(-)