From ba02fccb77539f8642aa5d09511cda2fc998542e Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Mon, 31 Jan 2022 11:59:11 -0500 Subject: [PATCH] ARTEMIS-3664 Critical Analyzer should not HALT or STOP the broker during the startup --- .../utils/critical/CriticalAnalyzerImpl.java | 2 +- .../critical/CriticalAnalyzerAccessor.java | 26 +++++++ .../core/server/ActiveMQServerLogger.java | 4 + .../core/server/impl/ActiveMQServerImpl.java | 74 ++++++++++++------- .../impl/ActiveMQServerImplAccessor.java | 27 +++++++ .../impl/ActiveMQServerStartupTest.java | 70 ++++++++++++++++++ 6 files changed, 174 insertions(+), 29 deletions(-) create mode 100644 artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerAccessor.java create mode 100644 artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImplAccessor.java create mode 100644 artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerStartupTest.java diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerImpl.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerImpl.java index ecb543979c..528d15f488 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerImpl.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerImpl.java @@ -149,7 +149,7 @@ public class CriticalAnalyzerImpl implements CriticalAnalyzer { } } - private void fireActions(CriticalComponent component) { + protected void fireActions(CriticalComponent component) { for (CriticalAction action : actions) { try { action.run(component); diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerAccessor.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerAccessor.java new file mode 100644 index 0000000000..0066e99b8e --- /dev/null +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalAnalyzerAccessor.java @@ -0,0 +1,26 @@ +/** + * 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.critical; + +public class CriticalAnalyzerAccessor { + + public static void fireActions(CriticalAnalyzer impl, CriticalComponent component) { + ((CriticalAnalyzerImpl)impl).fireActions(component); + } + +} diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java index 6dd51b3638..b0676cd6f1 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java @@ -2216,4 +2216,8 @@ public interface ActiveMQServerLogger extends BasicLogger { @Message(id = 224115, value = "Address control unblock of address ''{0}''. Clients will be granted credit as normal.", format = Message.Format.MESSAGE_FORMAT) void unblockingViaControl(SimpleString addressName); + @LogMessage(level = Logger.Level.WARN) + @Message(id = 224116, value = "The component {0} is not responsive during start up. The Server may be taking too long to start", format = Message.Format.MESSAGE_FORMAT) + void tooLongToStart(Object component); + } 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 48d827fe70..e227f4bd2a 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 @@ -704,7 +704,13 @@ public class ActiveMQServerImpl implements ActiveMQServer { } } - private void initializeCriticalAnalyzer() throws Exception { + + private void takingLongToStart(Object criticalComponent) { + ActiveMQServerLogger.LOGGER.tooLongToStart(criticalComponent); + threadDump(); + } + + protected void initializeCriticalAnalyzer() throws Exception { // Some tests will play crazy frequenceistop/start CriticalAnalyzer analyzer = this.getCriticalAnalyzer(); @@ -735,49 +741,61 @@ public class ActiveMQServerImpl implements ActiveMQServer { case HALT: criticalAction = criticalComponent -> { - checkCriticalAnalyzerLogging(); - ActiveMQServerLogger.LOGGER.criticalSystemHalt(criticalComponent); + if (ActiveMQServerImpl.this.state == SERVER_STATE.STARTING) { + takingLongToStart(criticalComponent); + } else { + checkCriticalAnalyzerLogging(); + ActiveMQServerLogger.LOGGER.criticalSystemHalt(criticalComponent); - threadDump(); - sendCriticalNotification(criticalComponent); + threadDump(); + sendCriticalNotification(criticalComponent); - Runtime.getRuntime().halt(70); // Linux systems will have /usr/include/sysexits.h showing 70 as internal software error + Runtime.getRuntime().halt(70); // Linux systems will have /usr/include/sysexits.h showing 70 as internal software error + } }; break; case SHUTDOWN: criticalAction = criticalComponent -> { - checkCriticalAnalyzerLogging(); - ActiveMQServerLogger.LOGGER.criticalSystemShutdown(criticalComponent); + if (ActiveMQServerImpl.this.state == SERVER_STATE.STARTING) { + takingLongToStart(criticalComponent); + } else { + checkCriticalAnalyzerLogging(); + ActiveMQServerLogger.LOGGER.criticalSystemShutdown(criticalComponent); - threadDump(); + threadDump(); - // on the case of a critical failure, -1 cannot simply means forever. - // in case graceful is -1, we will set it to 30 seconds - sendCriticalNotification(criticalComponent); + // on the case of a critical failure, -1 cannot simply means forever. + // in case graceful is -1, we will set it to 30 seconds + sendCriticalNotification(criticalComponent); - // you can't stop from the check thread, - // nor can use an executor - Thread stopThread = new Thread() { - @Override - public void run() { - try { - ActiveMQServerImpl.this.stop(); - } catch (Throwable e) { - logger.warn(e.getMessage(), e); + // you can't stop from the check thread, + // nor can use an executor + Thread stopThread = new Thread() { + @Override + public void run() { + try { + ActiveMQServerImpl.this.stop(); + } catch (Throwable e) { + logger.warn(e.getMessage(), e); + } } - } - }; - stopThread.start(); + }; + stopThread.start(); + } }; break; case LOG: criticalAction = criticalComponent -> { - checkCriticalAnalyzerLogging(); - ActiveMQServerLogger.LOGGER.criticalSystemLog(criticalComponent); - threadDump(); - sendCriticalNotification(criticalComponent); + if (ActiveMQServerImpl.this.state == SERVER_STATE.STARTING) { + takingLongToStart(criticalComponent); + } else { + checkCriticalAnalyzerLogging(); + ActiveMQServerLogger.LOGGER.criticalSystemLog(criticalComponent); + threadDump(); + sendCriticalNotification(criticalComponent); + } }; break; } diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImplAccessor.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImplAccessor.java new file mode 100644 index 0000000000..73068739ca --- /dev/null +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImplAccessor.java @@ -0,0 +1,27 @@ +/** + * 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.server.impl; + +public class ActiveMQServerImplAccessor { + + + public static void initializeCriticalAnalyzer(ActiveMQServerImpl server) throws Exception { + server.initializeCriticalAnalyzer(); + } + +} diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerStartupTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerStartupTest.java new file mode 100644 index 0000000000..48fbb3787e --- /dev/null +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerStartupTest.java @@ -0,0 +1,70 @@ +/** + * 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.server.impl; + +import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.logs.AssertionLoggerHandler; +import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.apache.activemq.artemis.utils.critical.CriticalAnalyzerAccessor; +import org.apache.activemq.artemis.utils.critical.CriticalAnalyzerPolicy; +import org.apache.activemq.artemis.utils.critical.CriticalComponentImpl; +import org.junit.Assert; +import org.junit.Test; + +public class ActiveMQServerStartupTest extends ActiveMQTestBase { + + @Test + public void testTooLongToStartHalt() throws Exception { + testTooLongToStart(CriticalAnalyzerPolicy.HALT); + } + + @Test + public void testTooLongToStartShutdown() throws Exception { + testTooLongToStart(CriticalAnalyzerPolicy.SHUTDOWN); + } + + @Test + public void testTooLongToStartLOG() throws Exception { + testTooLongToStart(CriticalAnalyzerPolicy.LOG); + } + + private void testTooLongToStart(CriticalAnalyzerPolicy policy) throws Exception { + + AssertionLoggerHandler.startCapture(); + try { + + ConfigurationImpl configuration = new ConfigurationImpl(); + configuration.setCriticalAnalyzerPolicy(policy); + configuration.setCriticalAnalyzer(true); + configuration.setPersistenceEnabled(false); + ActiveMQServerImpl server = new ActiveMQServerImpl(configuration); + addServer(server); + server.start(); + // this will be faking a condition + server.setState(ActiveMQServer.SERVER_STATE.STARTING); + CriticalAnalyzerAccessor.fireActions(server.getCriticalAnalyzer(), new CriticalComponentImpl(server.getCriticalAnalyzer(), 2)); + Assert.assertTrue(AssertionLoggerHandler.findText("224116")); + Assert.assertEquals(ActiveMQServer.SERVER_STATE.STARTING, server.getState()); // should not be changed + server.stop(); + } finally { + AssertionLoggerHandler.stopCapture(); + } + } + +}