From e19a54c19d956d2157ad5b2fca7d5af6ea5b72b4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Thu, 23 Feb 2017 10:20:28 +0100 Subject: [PATCH] SONAR-7937 Monitor reloads JavaCommand to execute on restart --- .../org/sonar/process/monitor/Monitor.java | 32 +++++++-- .../sonar/process/monitor/MonitorTest.java | 70 +++++++++++++++---- .../main/java/org/sonar/application/App.java | 2 +- .../java/org/sonar/application/AppTest.java | 13 ++-- 4 files changed, 93 insertions(+), 24 deletions(-) diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java index 42dd1a20487..742aba3e86c 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.function.Supplier; import javax.annotation.CheckForNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,6 +60,8 @@ public class Monitor { private final TerminatorThread terminator = new TerminatorThread(); private final RestartRequestWatcherThread restartWatcher = new RestartRequestWatcherThread(); @CheckForNull + private Supplier> javaCommandsSupplier; + @CheckForNull private List javaCommands; @CheckForNull private JavaProcessLauncher launcher; @@ -138,10 +141,10 @@ public class Monitor { * @throws IllegalStateException if already started or if at least one process failed to start. In this case * all processes are terminated. No need to execute {@link #stop()} */ - public void start(List commands) throws InterruptedException { - if (commands.isEmpty()) { - throw new IllegalArgumentException("At least one command is required"); - } + public void start(Supplier> javaCommandsSupplier) throws InterruptedException { + this.javaCommandsSupplier = javaCommandsSupplier; + // load java commands now, to fail fast if need be + List commands = loadJavaCommands(); if (lifecycle.getState() != State.INIT) { throw new IllegalStateException("Can not start multiple times"); @@ -153,10 +156,28 @@ public class Monitor { // start watching for restart requested by child process restartWatcher.start(); - javaCommands = new ArrayList<>(commands); + this.javaCommands = new ArrayList<>(commands); startProcesses(); } + /** + * @throws IllegalArgumentException if supplied didn't provide at least one JavaCommand + */ + private List loadJavaCommands() { + List commands = this.javaCommandsSupplier.get(); + if (commands.isEmpty()) { + throw new IllegalArgumentException("At least one command is required"); + } + return commands; + } + + private void reloadJavaCommands() { + this.javaCommands = loadJavaCommands(); + } + + /** + * Starts the processes defined by the JavaCommand in {@link #javaCommands}/ + */ private void startProcesses() throws InterruptedException { // do no start any child process if not in state INIT or RESTARTING (a stop could be in progress too) if (lifecycle.tryToMoveTo(State.STARTING)) { @@ -364,6 +385,7 @@ public class Monitor { public void run() { stopProcesses(); try { + reloadJavaCommands(); startProcesses(); } catch (InterruptedException e) { // Startup was interrupted. Processes are being stopped asynchronously. diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java index 189b14824d9..175d99c41d1 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java @@ -27,6 +27,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.function.Supplier; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.StringUtils; import org.assertj.core.api.AbstractAssert; @@ -124,7 +125,7 @@ public class MonitorTest { public void fail_to_start_if_no_commands() throws Exception { underTest = newDefaultMonitor(tempDir); try { - underTest.start(Collections.emptyList()); + underTest.start(Collections::emptyList); fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage("At least one command is required"); @@ -134,10 +135,10 @@ public class MonitorTest { @Test public void fail_to_start_multiple_times() throws Exception { underTest = newDefaultMonitor(tempDir); - underTest.start(singletonList(newStandardProcessCommand())); + underTest.start(() -> singletonList(newStandardProcessCommand())); boolean failed = false; try { - underTest.start(singletonList(newStandardProcessCommand())); + underTest.start(() -> singletonList(newStandardProcessCommand())); } catch (IllegalStateException e) { failed = e.getMessage().equals("Can not start multiple times"); } @@ -150,7 +151,7 @@ public class MonitorTest { underTest = newDefaultMonitor(tempDir); HttpProcessClient client = new HttpProcessClient(tempDir, ProcessId.ELASTICSEARCH); // blocks until started - underTest.start(singletonList(client.newCommand())); + underTest.start(() -> singletonList(client.newCommand())); assertThat(client).isUp() .wasStartedBefore(System.currentTimeMillis()); @@ -169,7 +170,7 @@ public class MonitorTest { underTest = newDefaultMonitor(tempDir); HttpProcessClient p1 = new HttpProcessClient(tempDir, ProcessId.ELASTICSEARCH); HttpProcessClient p2 = new HttpProcessClient(tempDir, ProcessId.WEB_SERVER); - underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand())); + underTest.start(() -> Arrays.asList(p1.newCommand(), p2.newCommand())); // start p2 when p1 is fully started (ready) assertThat(p1) @@ -195,7 +196,7 @@ public class MonitorTest { underTest = newDefaultMonitor(tempDir); HttpProcessClient p1 = new HttpProcessClient(tempDir, ProcessId.ELASTICSEARCH); HttpProcessClient p2 = new HttpProcessClient(tempDir, ProcessId.WEB_SERVER); - underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand())); + underTest.start(() -> Arrays.asList(p1.newCommand(), p2.newCommand())); assertThat(p1).isUp(); assertThat(p2).isUp(); @@ -214,7 +215,7 @@ public class MonitorTest { underTest = newDefaultMonitor(tempDir); HttpProcessClient p1 = new HttpProcessClient(tempDir, ProcessId.ELASTICSEARCH); HttpProcessClient p2 = new HttpProcessClient(tempDir, ProcessId.WEB_SERVER); - underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand())); + underTest.start(() -> Arrays.asList(p1.newCommand(), p2.newCommand())); assertThat(p1).isUp(); assertThat(p2).isUp(); @@ -242,12 +243,57 @@ public class MonitorTest { verify(fileSystem, times(2)).reset(); } + @Test + public void restart_reloads_java_commands() throws Exception { + underTest = newDefaultMonitor(tempDir); + HttpProcessClient p1 = new HttpProcessClient(tempDir, ProcessId.ELASTICSEARCH); + HttpProcessClient p2 = new HttpProcessClient(tempDir, ProcessId.WEB_SERVER); + + // a supplier that will return p1 the first time it's called and then p2 all the time + Supplier> listSupplier = new Supplier>() { + private int counter = 0; + @Override + public List get() { + if (counter == 0) { + counter++; + return Collections.singletonList(p1.newCommand()); + } else { + return Collections.singletonList(p2.newCommand()); + } + } + }; + + underTest.start(listSupplier); + + assertThat(p1).isUp(); + assertThat(p2).isNotUp(); + + p1.restart(); + + assertThat(underTest.waitForOneRestart()).isTrue(); + + assertThat(p1) + .wasStarted(1) + .wasGracefullyTerminated(1); + assertThat(p2) + .wasStarted(1) + .isUp(); + + underTest.stop(); + assertThat(p1) + .wasStarted(1) + .wasGracefullyTerminated(1); + assertThat(p2) + .wasStarted(1) + .wasGracefullyTerminated(1); + } + @Test public void stop_all_processes_if_one_shutdowns() throws Exception { underTest = newDefaultMonitor(tempDir); HttpProcessClient p1 = new HttpProcessClient(tempDir, ProcessId.ELASTICSEARCH); HttpProcessClient p2 = new HttpProcessClient(tempDir, ProcessId.WEB_SERVER); - underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand())); + underTest.start(() -> Arrays.asList(p1.newCommand(), p2.newCommand())); assertThat(p1.isUp()).isTrue(); assertThat(p2.isUp()).isTrue(); @@ -271,7 +317,7 @@ public class MonitorTest { HttpProcessClient p1 = new HttpProcessClient(tempDir, ProcessId.ELASTICSEARCH); HttpProcessClient p2 = new HttpProcessClient(tempDir, ProcessId.WEB_SERVER, -1); try { - underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand())); + underTest.start(() -> Arrays.asList(p1.newCommand(), p2.newCommand())); fail(); } catch (Exception expected) { assertThat(p1) @@ -292,7 +338,7 @@ public class MonitorTest { .setClassName("org.sonar.process.test.Unknown"); try { - underTest.start(singletonList(command)); + underTest.start(() -> singletonList(command)); fail(); } catch (Exception e) { // expected @@ -306,7 +352,7 @@ public class MonitorTest { assertThat(underTest.hardStopWatcher).isNull(); HttpProcessClient p1 = new HttpProcessClient(tempDir, ProcessId.COMPUTE_ENGINE); - underTest.start(singletonList(p1.newCommand())); + underTest.start(() -> singletonList(p1.newCommand())); assertThat(underTest.hardStopWatcher).isNotNull(); assertThat(underTest.hardStopWatcher.isAlive()).isTrue(); @@ -575,7 +621,7 @@ public class MonitorTest { } } - private JavaCommand newStandardProcessCommand() throws IOException { + private JavaCommand newStandardProcessCommand() { return new JavaCommand(ProcessId.ELASTICSEARCH) .addClasspath(testJar.getAbsolutePath()) .setClassName("org.sonar.process.test.StandardProcess"); diff --git a/sonar-application/src/main/java/org/sonar/application/App.java b/sonar-application/src/main/java/org/sonar/application/App.java index a2e96cbd3ab..7271d593ad8 100644 --- a/sonar-application/src/main/java/org/sonar/application/App.java +++ b/sonar-application/src/main/java/org/sonar/application/App.java @@ -76,7 +76,7 @@ public class App implements Stoppable { } public void start(Props props) throws InterruptedException { - monitor.start(createCommands(props)); + monitor.start(() -> createCommands(props)); monitor.awaitTermination(); } diff --git a/sonar-application/src/test/java/org/sonar/application/AppTest.java b/sonar-application/src/test/java/org/sonar/application/AppTest.java index 12869dc3e08..e335cbd9c9d 100644 --- a/sonar-application/src/test/java/org/sonar/application/AppTest.java +++ b/sonar-application/src/test/java/org/sonar/application/AppTest.java @@ -23,6 +23,7 @@ import java.io.File; import java.io.IOException; import java.util.List; import java.util.Properties; +import java.util.function.Supplier; import org.apache.commons.io.FilenameUtils; import org.junit.Rule; import org.junit.Test; @@ -58,9 +59,9 @@ public class AppTest { Monitor monitor = mock(Monitor.class); App app = new App(monitor); app.start(props); - ArgumentCaptor> argument = newJavaCommandArgumentCaptor(); + ArgumentCaptor>> argument = newJavaCommandArgumentCaptor(); verify(monitor).start(argument.capture()); - assertThat(argument.getValue()).extracting("processId").containsExactly(ProcessId.ELASTICSEARCH, ProcessId.WEB_SERVER, ProcessId.COMPUTE_ENGINE); + assertThat(argument.getValue().get()).extracting("processId").containsExactly(ProcessId.ELASTICSEARCH, ProcessId.WEB_SERVER, ProcessId.COMPUTE_ENGINE); app.stopAsync(); verify(monitor).stop(); @@ -176,13 +177,13 @@ public class AppTest { Monitor monitor = mock(Monitor.class); App app = new App(monitor); app.start(props); - ArgumentCaptor> argument = newJavaCommandArgumentCaptor(); + ArgumentCaptor>> argument = newJavaCommandArgumentCaptor(); verify(monitor).start(argument.capture()); - return argument.getValue(); + return argument.getValue().get(); } - private ArgumentCaptor> newJavaCommandArgumentCaptor() { - Class> listClass = (Class>) (Class) List.class; + private ArgumentCaptor>> newJavaCommandArgumentCaptor() { + Class>> listClass = (Class>>) (Class) List.class; return ArgumentCaptor.forClass(listClass); } } -- 2.39.5