[
https://issues.apache.org/jira/browse/ARTEMIS-3808?focusedWorklogId=770113&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-770113
]
ASF GitHub Bot logged work on ARTEMIS-3808:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 13/May/22 10:21
Start Date: 13/May/22 10:21
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4061:
URL: https://github.com/apache/activemq-artemis/pull/4061#discussion_r872187352
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java:
##########
@@ -4479,5 +4487,68 @@ public void replay(String startScan, String endScan,
String address, String targ
server.replay(startScanDate, endScanDate, address, target, filter);
}
+
+ @Override
+ public void stopEmbeddedWebServer() throws Exception {
+ synchronized (embeddedWebServerLock) {
+ getEmbeddedWebServerComponent().stop(true);
+ }
+ }
+
+ @Override
+ public void startEmbeddedWebServer() throws Exception {
+ synchronized (embeddedWebServerLock) {
+ getEmbeddedWebServerComponent().start();
+ }
+ }
+
+ @Override
+ public void restartEmbeddedWebServer() throws Exception {
+
restartEmbeddedWebServer(ActiveMQDefaultConfiguration.getDefaultEmbeddedWebServerRestartTimeout());
+ }
+
+ @Override
+ public void restartEmbeddedWebServer(long timeout) throws Exception {
+ final CountDownLatch latch = new CountDownLatch(1);
+ /*
+ * This needs to be run in its own thread managed by the broker because
if it is run on a thread managed by Jetty
+ * (e.g. if it is invoked from the web console) then the thread will die
before Jetty can be restarted.
+ */
+ server.getThreadPool().execute(() -> {
+ try {
+ synchronized (embeddedWebServerLock) {
+ stopEmbeddedWebServer();
+ startEmbeddedWebServer();
+ }
+ latch.countDown();
+ } catch (Exception e) {
+ ActiveMQServerLogger.LOGGER.failedToRestartEmbeddedWebServer(e);
+ }
+ });
+ if (!latch.await(timeout, TimeUnit.MILLISECONDS)) {
+ throw
ActiveMQMessageBundle.BUNDLE.embeddedWebServerNotRestarted(timeout);
+ }
Review Comment:
This handles the waiting for completion, but misses the other aspect around
throwing the failure when the restarts start or stop fails. Right now it would
catch and log a failure on the executor, but the caller would then just sit
there for 5 seconds (or other given value) and time out even though it is
already known it failed and wont complete. It should just fail immediately,
tripping the latch and throwing based on the exception its currently logging.
You could add a simple mechanism to pass any exception back...or I see that
ActiveMQServer#getThreadPool() returns Executor but
ActiveMQServerImpl#getThreadPool() actually returns ExecutorService. If the
interface could be expanded perhaps then you could use submit/Callable for the
waiting and exception handling.
##########
artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java:
##########
@@ -363,9 +366,17 @@ public void stop() throws Exception {
}
@Override
- public void stop(boolean isShutdown) throws Exception {
+ public synchronized void stop(boolean isShutdown) throws Exception {
if (isShutdown) {
- internalStop();
+ ActiveMQWebLogger.LOGGER.stoppingEmbeddedWebServer();
+ server.stop();
+ server = null;
Review Comment:
This method should check that it is running first before it gets to doing
this, much like start() does. It would currently NPE if you called stop(true)
while it is already stopped, as e.g restart does, since _server_ would be null
but this assumes it is populated.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -4290,6 +4295,101 @@ public boolean isStarted() {
}
}
+ @Test
+ public void testManualStopStartEmbeddedWebServer() throws Exception {
+ FakeWebServerComponent fake = new FakeWebServerComponent();
+ server.addExternalComponent(fake, true);
+ Assert.assertTrue(fake.isStarted());
+
+ ActiveMQServerControl serverControl = createManagementControl();
+ serverControl.stopEmbeddedWebServer();
+ Assert.assertFalse(fake.isStarted());
+ serverControl.startEmbeddedWebServer();
+ Assert.assertTrue(fake.isStarted());
+ }
+
+ @Test
+ public void testRestartEmbeddedWebServer() throws Exception {
+ FakeWebServerComponent fake = new FakeWebServerComponent();
+ server.addExternalComponent(fake, true);
+ Assert.assertTrue(fake.isStarted());
+
+ ActiveMQServerControl serverControl = createManagementControl();
+ long time = System.currentTimeMillis();
+ Assert.assertTrue(time >= fake.getStartTime());
+ Assert.assertTrue(time > fake.getStopTime());
+ Thread.sleep(5);
+ serverControl.restartEmbeddedWebServer();
+ Assert.assertTrue(serverControl.isEmbeddedWebServerStarted());
+ Assert.assertTrue(time < fake.getStartTime());
+ Assert.assertTrue(time < fake.getStopTime());
+ }
+
+ @Test
+ public void testRestartEmbeddedWebServerTimeout() throws Exception {
+ FakeWebServerComponent fake = new FakeWebServerComponent(400);
+ server.addExternalComponent(fake, false);
+
+ ActiveMQServerControl serverControl = createManagementControl();
+ try {
+ serverControl.restartEmbeddedWebServer(100);
Review Comment:
Seems unnecessary to wait 100ms for timeout when the point is to timeout and
the other thread is only waiting 400ms to complete/record starting, might as
well make it something smaller. It would make the test quicker and less likely
to fail (by accidentally succeeding) on a busy system due to the whims of
thread scheduling and only having 300ms between the two values.
You could shorten the test and make it more reliable by having the
FakeWebServerComponent await on a latch you pass it instead, rather than
passing a small fixed sleep time. You can then give it something longer to wait
for to help ensure it does cause a timeout, and then trip it to complete from
the main test immediately after the restart call has actually timed out. Avoids
the other thread needing to dangling after the core test is actually finished,
and is more reliable in various ways (e.g right now its also susceptible to
spurious wakeups on its Thread.sleep() whereas the latch would handle the wait
properly).
Issue Time Tracking
-------------------
Worklog Id: (was: 770113)
Time Spent: 4h 50m (was: 4h 40m)
> Support starting/stopping the embedded web server via mangement
> ---------------------------------------------------------------
>
> Key: ARTEMIS-3808
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3808
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Time Spent: 4h 50m
> Remaining Estimate: 0h
>
> It would be useful to be able to cycle the embedded web server if, for
> example, one needed to renew the SSL certificates.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
