From 8c1ec12b33170689156317d6c452da0caaa13f10 Mon Sep 17 00:00:00 2001 From: Howard Gao Date: Wed, 2 Nov 2016 21:27:20 +0800 Subject: [PATCH 1/2] ARTEMIS-835 Quick restart of broker will leave tmp dir dirty - Added a timeout wait on each web context's tmp dir in WebServerComponent.stop() method. --- .../activemq/artemis/utils/TimeUtils.java | 17 ++++++ .../activemq/artemis/utils/TimeUnitsTest.java | 57 +++++++++++++++++++ .../activemq/artemis/ActiveMQWebLogger.java | 5 ++ .../artemis/component/WebServerComponent.java | 31 +++++++++- 4 files changed, 107 insertions(+), 3 deletions(-) rename {artemis-server => artemis-commons}/src/main/java/org/apache/activemq/artemis/utils/TimeUtils.java (85%) create mode 100644 artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TimeUnitsTest.java diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/utils/TimeUtils.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TimeUtils.java similarity index 85% rename from artemis-server/src/main/java/org/apache/activemq/artemis/utils/TimeUtils.java rename to artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TimeUtils.java index 5209a489b1..f2819acc6b 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/utils/TimeUtils.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/TimeUtils.java @@ -16,6 +16,7 @@ */ package org.apache.activemq.artemis.utils; + import java.text.DecimalFormat; import java.text.DecimalFormatSymbols; import java.text.NumberFormat; @@ -72,4 +73,20 @@ public final class TimeUtils { return s; } + public static boolean waitOnBoolean(boolean expected, long timeout, CheckMethod check) { + long timeLeft = timeout; + long interval = 10; + while (check.check() != expected && timeLeft > 0) { + try { + Thread.sleep(interval); + } catch (InterruptedException e) { + } + timeLeft -= interval; + } + return check.check() == expected; + } + + public interface CheckMethod { + boolean check(); + } } diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TimeUnitsTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TimeUnitsTest.java new file mode 100644 index 0000000000..ae09f9c066 --- /dev/null +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TimeUnitsTest.java @@ -0,0 +1,57 @@ +/* + * 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.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class TimeUnitsTest { + + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + + @Test + public void testWaitOnBoolean() throws IOException { + File tmpFile = folder.newFile("myfile.txt"); + assertTrue(tmpFile.exists()); + long begin = System.currentTimeMillis(); + boolean result = TimeUtils.waitOnBoolean(false, 2000, tmpFile::exists); + long end = System.currentTimeMillis(); + + assertFalse(result); + assertTrue(tmpFile.exists()); + //ideally the sleep time should > 2000. + System.out.println("actually waiting time: " + (end - begin)); + assertTrue((end - begin) >= 2000); + tmpFile.delete(); + begin = System.currentTimeMillis(); + result = TimeUtils.waitOnBoolean(false, 5000, tmpFile::exists); + end = System.currentTimeMillis(); + + assertTrue(result); + assertFalse(tmpFile.exists()); + + assertTrue((end - begin) < 5000); + } +} diff --git a/artemis-web/src/main/java/org/apache/activemq/artemis/ActiveMQWebLogger.java b/artemis-web/src/main/java/org/apache/activemq/artemis/ActiveMQWebLogger.java index 4200955624..e4fd85445c 100644 --- a/artemis-web/src/main/java/org/apache/activemq/artemis/ActiveMQWebLogger.java +++ b/artemis-web/src/main/java/org/apache/activemq/artemis/ActiveMQWebLogger.java @@ -22,6 +22,8 @@ import org.jboss.logging.annotations.LogMessage; import org.jboss.logging.annotations.Message; import org.jboss.logging.annotations.MessageLogger; +import java.io.File; + /** * Logger Code 24 * @@ -52,4 +54,7 @@ public interface ActiveMQWebLogger extends BasicLogger { @Message(id = 241002, value = "Artemis Jolokia REST API available at {0}", format = Message.Format.MESSAGE_FORMAT) void jolokiaAvailable(String bind); + @LogMessage(level = Logger.Level.WARN) + @Message(id = 244003, value = "Temporary file not deleted on shutdown: {0}", format = Message.Format.MESSAGE_FORMAT) + void tmpFileNotDeleted(File tmpdir); } \ No newline at end of file diff --git a/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java b/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java index 699dce36c7..a34d17926f 100644 --- a/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java +++ b/artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java @@ -16,16 +16,20 @@ */ package org.apache.activemq.artemis.component; +import java.io.File; import java.io.IOException; import java.net.URI; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; import org.apache.activemq.artemis.ActiveMQWebLogger; import org.apache.activemq.artemis.components.ExternalComponent; import org.apache.activemq.artemis.dto.AppDTO; import org.apache.activemq.artemis.dto.ComponentDTO; import org.apache.activemq.artemis.dto.WebServerDTO; +import org.apache.activemq.artemis.utils.TimeUtils; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; @@ -46,6 +50,7 @@ public class WebServerComponent implements ExternalComponent { private WebServerDTO webServerConfig; private URI uri; private String jolokiaUrl; + private List webContexts; @Override public void configure(ComponentDTO config, String artemisInstance, String artemisHome) throws Exception { @@ -87,9 +92,11 @@ public class WebServerComponent implements ExternalComponent { Path warDir = Paths.get(artemisHome != null ? artemisHome : ".").resolve(webServerConfig.path).toAbsolutePath(); - if (webServerConfig.apps != null) { + if (webServerConfig.apps != null && webServerConfig.apps.size() > 0) { + webContexts = new ArrayList<>(); for (AppDTO app : webServerConfig.apps) { - deployWar(app.url, app.war, warDir); + WebAppContext webContext = deployWar(app.url, app.war, warDir); + webContexts.add(webContext); if (app.war.startsWith("jolokia")) { jolokiaUrl = webServerConfig.bind + "/" + app.url; } @@ -121,6 +128,23 @@ public class WebServerComponent implements ExternalComponent { @Override public void stop() throws Exception { server.stop(); + if (webContexts != null) { + File tmpdir = null; + for (WebAppContext context : webContexts) { + tmpdir = context.getTempDirectory(); + + if (tmpdir != null && !context.isPersistTempDirectory()) { + //tmpdir will be removed by deleteOnExit() + //somehow when broker is stopped and restarted quickly + //this tmpdir won't get deleted sometimes + boolean fileDeleted = TimeUtils.waitOnBoolean(false, 10000, tmpdir::exists); + if (!fileDeleted) { + ActiveMQWebLogger.LOGGER.tmpFileNotDeleted(tmpdir); + } + } + } + webContexts.clear(); + } } @Override @@ -128,7 +152,7 @@ public class WebServerComponent implements ExternalComponent { return server != null && server.isStarted(); } - private void deployWar(String url, String warFile, Path warDirectory) throws IOException { + private WebAppContext deployWar(String url, String warFile, Path warDirectory) throws IOException { WebAppContext webapp = new WebAppContext(); if (url.startsWith("/")) { webapp.setContextPath(url); @@ -138,5 +162,6 @@ public class WebServerComponent implements ExternalComponent { webapp.setWar(warDirectory.resolve(warFile).toString()); handlers.addHandler(webapp); + return webapp; } } From 8b25f09c3bf4dae5509d05c71febce39f8290037 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Wed, 2 Nov 2016 13:43:35 -0400 Subject: [PATCH 2/2] ARTEMIS-835 speed up test --- .../org/apache/activemq/artemis/utils/TimeUnitsTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TimeUnitsTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TimeUnitsTest.java index ae09f9c066..32913c7e11 100644 --- a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TimeUnitsTest.java +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TimeUnitsTest.java @@ -36,14 +36,13 @@ public class TimeUnitsTest { File tmpFile = folder.newFile("myfile.txt"); assertTrue(tmpFile.exists()); long begin = System.currentTimeMillis(); - boolean result = TimeUtils.waitOnBoolean(false, 2000, tmpFile::exists); + boolean result = TimeUtils.waitOnBoolean(false, 100, tmpFile::exists); long end = System.currentTimeMillis(); assertFalse(result); assertTrue(tmpFile.exists()); //ideally the sleep time should > 2000. - System.out.println("actually waiting time: " + (end - begin)); - assertTrue((end - begin) >= 2000); + assertTrue((end - begin) >= 100); tmpFile.delete(); begin = System.currentTimeMillis(); result = TimeUtils.waitOnBoolean(false, 5000, tmpFile::exists);