diff options
author | Sébastien Lesaint <sebastien.lesaint@sonarsource.com> | 2016-01-08 18:01:57 +0100 |
---|---|---|
committer | Sébastien Lesaint <sebastien.lesaint@sonarsource.com> | 2016-01-13 13:42:43 +0100 |
commit | 5db076096966170cde636fa2c41d7777abca193e (patch) | |
tree | 650eb1d38a41d78f76f363ef3a748b785107d3ec /server/sonar-process-monitor | |
parent | 255e54d582d02ab72d1b33f440656fb2d5ae9f8c (diff) | |
download | sonarqube-5db076096966170cde636fa2c41d7777abca193e.tar.gz sonarqube-5db076096966170cde636fa2c41d7777abca193e.zip |
SONAR-7168 fix stop during restart not working
lifeCycle transition from RESTARTING to STOPPING should actually _not_ be allowed because it can occur when restarting child processes (the WatcherThreads detects that stop and tries to shutdown all other processes), this fixes the issue by adding a HARD_STOPPING state, representing a not gracefull stop, to which transition from RESTARTING is allowed
adds class AllProcessesCommands which implements access to sharedMemory for any process and is now used as the underlying implementation of DefaultProcessCommands. This class allows using a single IO to access sharedMemory from App
Diffstat (limited to 'server/sonar-process-monitor')
6 files changed, 147 insertions, 79 deletions
diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaCommand.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaCommand.java index 9187e0305ea..8a5551039d5 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaCommand.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaCommand.java @@ -52,8 +52,6 @@ public class JavaCommand { private final Map<String, String> envVariables = new HashMap<>(System.getenv()); - private File tempDir = null; - private int processIndex = -1; public JavaCommand(String key) { @@ -78,15 +76,6 @@ public class JavaCommand { return this; } - public File getTempDir() { - return tempDir; - } - - public JavaCommand setTempDir(File tempDir) { - this.tempDir = tempDir; - return this; - } - public List<String> getJavaOptions() { return javaOptions; } diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaProcessLauncher.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaProcessLauncher.java index 6a2eb83bb44..9bf969cd944 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaProcessLauncher.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaProcessLauncher.java @@ -19,13 +19,6 @@ */ package org.sonar.process.monitor; -import org.apache.commons.lang.StringUtils; -import org.slf4j.LoggerFactory; -import org.sonar.process.DefaultProcessCommands; -import org.sonar.process.ProcessCommands; -import org.sonar.process.ProcessEntryPoint; -import org.sonar.process.ProcessUtils; - import java.io.File; import java.io.FileOutputStream; import java.io.OutputStream; @@ -33,20 +26,33 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Properties; +import org.apache.commons.lang.StringUtils; +import org.slf4j.LoggerFactory; +import org.sonar.process.AllProcessesCommands; +import org.sonar.process.ProcessCommands; +import org.sonar.process.ProcessEntryPoint; +import org.sonar.process.ProcessUtils; public class JavaProcessLauncher { private final Timeouts timeouts; + private final File tempDir; + private final AllProcessesCommands allProcessesCommands; - public JavaProcessLauncher(Timeouts timeouts) { + public JavaProcessLauncher(Timeouts timeouts, File tempDir, AllProcessesCommands allProcessesCommands) { this.timeouts = timeouts; + this.tempDir = tempDir; + this.allProcessesCommands = allProcessesCommands; + } + + public void close() { + allProcessesCommands.close(); } ProcessRef launch(JavaCommand command) { Process process = null; try { - // cleanup existing monitor files - ProcessCommands commands = new DefaultProcessCommands(command.getTempDir(), command.getProcessIndex()); + ProcessCommands commands = allProcessesCommands.getProcessCommand(command.getProcessIndex(), true); ProcessBuilder processBuilder = create(command); LoggerFactory.getLogger(getClass()).info("Launch process[{}]: {}", @@ -69,7 +75,7 @@ public class JavaProcessLauncher { commands.add(buildJavaPath()); commands.addAll(javaCommand.getJavaOptions()); // TODO warning - does it work if temp dir contains a whitespace ? - commands.add(String.format("-Djava.io.tmpdir=%s", javaCommand.getTempDir().getAbsolutePath())); + commands.add(String.format("-Djava.io.tmpdir=%s", tempDir.getAbsolutePath())); commands.addAll(buildClasspath(javaCommand)); commands.add(javaCommand.getClassName()); commands.add(buildPropertiesFile(javaCommand).getAbsolutePath()); @@ -101,7 +107,7 @@ public class JavaProcessLauncher { props.setProperty(ProcessEntryPoint.PROPERTY_PROCESS_KEY, javaCommand.getKey()); props.setProperty(ProcessEntryPoint.PROPERTY_PROCESS_INDEX, Integer.toString(javaCommand.getProcessIndex())); props.setProperty(ProcessEntryPoint.PROPERTY_TERMINATION_TIMEOUT, String.valueOf(timeouts.getTerminationTimeout())); - props.setProperty(ProcessEntryPoint.PROPERTY_SHARED_PATH, javaCommand.getTempDir().getAbsolutePath()); + props.setProperty(ProcessEntryPoint.PROPERTY_SHARED_PATH, tempDir.getAbsolutePath()); OutputStream out = new FileOutputStream(propertiesFile); props.store(out, String.format("Temporary properties file for command [%s]", javaCommand.getKey())); out.close(); 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 749389d1054..7dcf8ede0d0 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 @@ -19,6 +19,7 @@ */ package org.sonar.process.monitor; +import java.io.File; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -27,6 +28,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.sonar.process.AllProcessesCommands; import org.sonar.process.Lifecycle; import org.sonar.process.Lifecycle.State; import org.sonar.process.ProcessCommands; @@ -37,9 +39,11 @@ public class Monitor { private static final Logger LOG = LoggerFactory.getLogger(Monitor.class); private static final Timeouts TIMEOUTS = new Timeouts(); private static final long WATCH_DELAY_MS = 500L; + private static final int CURRENT_PROCESS_NUMBER = 0; private static int restartorInstanceCounter = 0; + private final AllProcessesCommands allProcessesCommands; private final JavaProcessLauncher launcher; private final SystemExit systemExit; @@ -53,15 +57,18 @@ public class Monitor { private List<JavaCommand> javaCommands; @CheckForNull private RestartorThread restartor; + @CheckForNull + HardStopWatcherThread hardStopWatcher; static int nextProcessId = 1; - Monitor(JavaProcessLauncher launcher, SystemExit exit) { - this.launcher = launcher; + Monitor(File tempDir, SystemExit exit) { + this.allProcessesCommands = new AllProcessesCommands(tempDir); + this.launcher = new JavaProcessLauncher(TIMEOUTS, tempDir, allProcessesCommands); this.systemExit = exit; } - public static Monitor create() { - return new Monitor(new JavaProcessLauncher(TIMEOUTS), new SystemExit()); + public static Monitor create(File tempDir) { + return new Monitor(tempDir, new SystemExit()); } /** @@ -108,6 +115,12 @@ public class Monitor { } } + public void watchForHardStop() { + ProcessCommands processCommand = this.allProcessesCommands.getProcessCommand(CURRENT_PROCESS_NUMBER, true); + this.hardStopWatcher = new HardStopWatcherThread(processCommand); + this.hardStopWatcher.start(); + } + private void monitor(ProcessRef processRef) { // physically watch if process is alive WatcherThread watcherThread = new WatcherThread(processRef, this); @@ -145,7 +158,9 @@ public class Monitor { boolean waitForOneRestart() { boolean restartRequested = awaitChildProcessesTermination(); + trace("finished waiting, restartRequested=" + restartRequested); if (restartRequested) { + trace("awaitTermination(restartor)=" + restartor); awaitTermination(restartor); } return restartRequested; @@ -176,8 +191,8 @@ public class Monitor { * Blocks until all processes are terminated. */ public void stop() { - trace("start stop async..."); - stopAsync(); + trace("start hard stop async..."); + stopAsync(State.HARD_STOPPING); trace("await termination of terminator..."); awaitTermination(terminator); cleanAfterTermination(); @@ -187,17 +202,18 @@ public class Monitor { private void cleanAfterTermination() { trace("go to STOPPED..."); - // safeguard if TerminatorThread is buggy and stop restartWatcher if (lifecycle.tryToMoveTo(State.STOPPED)) { trace("await termination of restartWatcher..."); - // wait for restartWatcher to cleanly stop - awaitTermination(restartWatcher); + // wait for restartWatcher and hardStopWatcher to cleanly stop + awaitTermination(restartWatcher, hardStopWatcher); trace("restartWatcher done"); // removing shutdown hook to avoid called stop() unnecessarily unless already in shutdownHook if (!systemExit.isInShutdownHook()) { trace("removing shutdown hook..."); Runtime.getRuntime().removeShutdownHook(shutdownHook); } + // cleanly close JavaLauncher + launcher.close(); } } @@ -207,7 +223,11 @@ public class Monitor { * this call will be blocking until the previous request finishes. */ public void stopAsync() { - if (lifecycle.tryToMoveTo(State.STOPPING)) { + stopAsync(State.STOPPING); + } + + private void stopAsync(State stoppingState) { + if (lifecycle.tryToMoveTo(stoppingState)) { terminator.start(); } } @@ -288,12 +308,38 @@ public class Monitor { } + public class HardStopWatcherThread extends Thread { + private final ProcessCommands processCommands; + + public HardStopWatcherThread(ProcessCommands processCommands) { + super("Hard stop watcher"); + this.processCommands = processCommands; + } + + @Override + public void run() { + while (lifecycle.getState() != Lifecycle.State.STOPPED) { + if (processCommands.askedForStop()) { + trace("Stopping process"); + Monitor.this.stop(); + } else { + try { + Thread.sleep(WATCH_DELAY_MS); + } catch (InterruptedException ignored) { + // keep watching + } + } + } + } + + } + private void stopProcesses() { - List<WatcherThread> watcherThreads = new ArrayList<>(this.watcherThreads); + List<WatcherThread> watcherThreadsCopy = new ArrayList<>(this.watcherThreads); // create a copy and reverse it to terminate in reverse order of startup (dependency order) - Collections.reverse(watcherThreads); + Collections.reverse(watcherThreadsCopy); - for (WatcherThread watcherThread : watcherThreads) { + for (WatcherThread watcherThread : watcherThreadsCopy) { ProcessRef ref = watcherThread.getProcessRef(); if (!ref.isStopped()) { LOG.info("{} is stopping", ref); @@ -339,6 +385,12 @@ public class Monitor { } } + private static void awaitTermination(Thread... threads) { + for (Thread thread : threads) { + awaitTermination(thread); + } + } + private static void awaitTermination(@Nullable Thread t) { if (t == null) { return; diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaCommandTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaCommandTest.java index cc376db2d35..5e36ef6c797 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaCommandTest.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaCommandTest.java @@ -44,8 +44,6 @@ public class JavaCommandTest { command.setClassName("org.sonar.ElasticSearch"); command.setEnvVariable("JAVA_COMMAND_TEST", "1000"); - File tempDir = temp.newFolder(); - command.setTempDir(tempDir); File workDir = temp.newFolder(); command.setWorkDir(workDir); command.addClasspath("lib/*.jar"); @@ -56,7 +54,6 @@ public class JavaCommandTest { assertThat(command.getClasspath()).containsOnly("lib/*.jar", "conf/*.xml"); assertThat(command.getJavaOptions()).containsOnly("-Xmx128m"); assertThat(command.getWorkDir()).isSameAs(workDir); - assertThat(command.getTempDir()).isSameAs(tempDir); assertThat(command.getClassName()).isEqualTo("org.sonar.ElasticSearch"); // copy current env variables diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaProcessLauncherTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaProcessLauncherTest.java index a4c2c82ca64..cdd723c6fb4 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaProcessLauncherTest.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaProcessLauncherTest.java @@ -19,17 +19,25 @@ */ package org.sonar.process.monitor; +import java.io.File; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.sonar.process.AllProcessesCommands; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; public class JavaProcessLauncherTest { + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + @Test - public void fail_to_launch() { + public void fail_to_launch() throws Exception { + File tempDir = temp.newFolder(); JavaCommand command = new JavaCommand("test"); - JavaProcessLauncher launcher = new JavaProcessLauncher(new Timeouts()); + JavaProcessLauncher launcher = new JavaProcessLauncher(new Timeouts(), tempDir, new AllProcessesCommands(tempDir)); try { // command is not correct (missing options), java.lang.ProcessBuilder#start() // throws an exception 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 a9937311e1e..038d9aa64e1 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 @@ -32,6 +32,7 @@ import org.apache.commons.lang.StringUtils; import org.assertj.core.api.AbstractAssert; import org.assertj.core.internal.Longs; import org.junit.After; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; @@ -90,6 +91,14 @@ public class MonitorTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); + private File tempDir; + + @Before + public void setUp() throws Exception { + tempDir = temp.newFolder(); + + } + /** * Safeguard */ @@ -104,8 +113,8 @@ public class MonitorTest { } @Test - public void fail_to_start_if_no_commands() { - monitor = newDefaultMonitor(); + public void fail_to_start_if_no_commands() throws Exception { + monitor = newDefaultMonitor(tempDir); try { monitor.start(Collections.<JavaCommand>emptyList()); fail(); @@ -116,7 +125,7 @@ public class MonitorTest { @Test public void fail_to_start_multiple_times() throws Exception { - monitor = newDefaultMonitor(); + monitor = newDefaultMonitor(tempDir); monitor.start(Arrays.asList(newStandardProcessCommand())); boolean failed = false; try { @@ -130,8 +139,8 @@ public class MonitorTest { @Test public void start_then_stop_gracefully() throws Exception { - monitor = newDefaultMonitor(); - HttpProcessClient client = new HttpProcessClient("test"); + monitor = newDefaultMonitor(tempDir); + HttpProcessClient client = new HttpProcessClient(tempDir, "test"); // blocks until started monitor.start(Arrays.asList(client.newCommand())); @@ -148,9 +157,9 @@ public class MonitorTest { @Test public void start_then_stop_sequence_of_commands() throws Exception { - monitor = newDefaultMonitor(); - HttpProcessClient p1 = new HttpProcessClient("p1"); - HttpProcessClient p2 = new HttpProcessClient("p2"); + monitor = newDefaultMonitor(tempDir); + HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1"); + HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2"); monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand())); // start p2 when p1 is fully started (ready) @@ -173,9 +182,9 @@ public class MonitorTest { @Test public void stop_all_processes_if_monitor_shutdowns() throws Exception { - monitor = newDefaultMonitor(); - HttpProcessClient p1 = new HttpProcessClient("p1"); - HttpProcessClient p2 = new HttpProcessClient("p2"); + monitor = newDefaultMonitor(tempDir); + HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1"); + HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2"); monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand())); assertThat(p1).isReady(); assertThat(p2).isReady(); @@ -190,9 +199,9 @@ public class MonitorTest { @Test public void restart_all_processes_if_one_asks_for_restart() throws Exception { - monitor = newDefaultMonitor(); - HttpProcessClient p1 = new HttpProcessClient("p1"); - HttpProcessClient p2 = new HttpProcessClient("p2"); + monitor = newDefaultMonitor(tempDir); + HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1"); + HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2"); monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand())); assertThat(p1).isReady(); @@ -221,9 +230,9 @@ public class MonitorTest { @Test public void stop_all_processes_if_one_shutdowns() throws Exception { - monitor = newDefaultMonitor(); - HttpProcessClient p1 = new HttpProcessClient("p1"); - HttpProcessClient p2 = new HttpProcessClient("p2"); + monitor = newDefaultMonitor(tempDir); + HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1"); + HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2"); monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand())); assertThat(p1.isReady()).isTrue(); assertThat(p2.isReady()).isTrue(); @@ -242,9 +251,9 @@ public class MonitorTest { @Test public void stop_all_processes_if_one_fails_to_start() throws Exception { - monitor = newDefaultMonitor(); - HttpProcessClient p1 = new HttpProcessClient("p1"); - HttpProcessClient p2 = new HttpProcessClient("p2", -1); + monitor = newDefaultMonitor(tempDir); + HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1"); + HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2", -1); try { monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand())); fail(); @@ -260,11 +269,11 @@ public class MonitorTest { } @Test - public void test_too_many_processes() { + public void test_too_many_processes() throws Exception { while (Monitor.getNextProcessId() < ProcessCommands.MAX_PROCESSES - 1) { } try { - newDefaultMonitor(); + newDefaultMonitor(tempDir); } catch (IllegalStateException e) { assertThat(e).hasMessageStartingWith("The maximum number of processes launched has been reached "); } finally { @@ -274,11 +283,10 @@ public class MonitorTest { @Test public void fail_to_start_if_bad_class_name() throws Exception { - monitor = newDefaultMonitor(); + monitor = newDefaultMonitor(tempDir); JavaCommand command = new JavaCommand("test") .addClasspath(testJar.getAbsolutePath()) - .setClassName("org.sonar.process.test.Unknown") - .setTempDir(temp.newFolder()); + .setClassName("org.sonar.process.test.Unknown"); try { monitor.start(Arrays.asList(command)); @@ -289,9 +297,19 @@ public class MonitorTest { } } - private Monitor newDefaultMonitor() { - Timeouts timeouts = new Timeouts(); - return new Monitor(new JavaProcessLauncher(timeouts), exit); + @Test + public void watchForHardStop_adds_a_hardStopWatcher_thread_and_starts_it() throws Exception { + Monitor monitor = newDefaultMonitor(tempDir); + assertThat(monitor.hardStopWatcher).isNull(); + + monitor.watchForHardStop(); + + assertThat(monitor.hardStopWatcher).isNotNull(); + assertThat(monitor.hardStopWatcher.isAlive()).isTrue(); + } + + private Monitor newDefaultMonitor(File tempDir) throws IOException { + return new Monitor(tempDir, exit); } /** @@ -302,16 +320,16 @@ public class MonitorTest { private final String commandKey; private final File tempDir; - private HttpProcessClient(String commandKey) throws IOException { - this(commandKey, NetworkUtils.freePort()); + private HttpProcessClient(File tempDir, String commandKey) throws IOException { + this(tempDir, commandKey, NetworkUtils.freePort()); } /** * Use httpPort=-1 to make server fail to start */ - private HttpProcessClient(String commandKey, int httpPort) throws IOException { + private HttpProcessClient(File tempDir, String commandKey, int httpPort) throws IOException { + this.tempDir = tempDir; this.commandKey = commandKey; - this.tempDir = temp.newFolder(commandKey); this.httpPort = httpPort; } @@ -319,8 +337,7 @@ public class MonitorTest { return new JavaCommand(commandKey) .addClasspath(testJar.getAbsolutePath()) .setClassName("org.sonar.process.test.HttpProcess") - .setArgument("httpPort", String.valueOf(httpPort)) - .setTempDir(tempDir); + .setArgument("httpPort", String.valueOf(httpPort)); } /** @@ -386,7 +403,7 @@ public class MonitorTest { private List<Long> readTimeFromFile(String filename) { try { - File file = new File(tempDir, filename); + File file = new File(tempDir, httpPort + "-" + filename); if (file.isFile() && file.exists()) { String[] split = StringUtils.split(FileUtils.readFileToString(file), ','); List<Long> res = new ArrayList<>(split.length); @@ -402,7 +419,7 @@ public class MonitorTest { } private boolean fileExists(String filename) { - File file = new File(tempDir, filename); + File file = new File(tempDir, httpPort + "-" + filename); return file.isFile() && file.exists(); } } @@ -542,8 +559,7 @@ public class MonitorTest { private JavaCommand newStandardProcessCommand() throws IOException { return new JavaCommand("standard") .addClasspath(testJar.getAbsolutePath()) - .setClassName("org.sonar.process.test.StandardProcess") - .setTempDir(temp.newFolder()); + .setClassName("org.sonar.process.test.StandardProcess"); } } |