From a800ef206373e1495a1d1132e413147bcbde6192 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 12 Sep 2014 14:23:07 +0200 Subject: [PATCH] SONAR-4898 add timeout to RMI calls on isReady() --- .../org/sonar/process/monitor/JavaCommand.java | 6 ++++++ .../java/org/sonar/process/monitor/Monitor.java | 5 ++++- .../sonar/process/monitor/RmiJmxConnector.java | 17 +++++++++++++++-- .../org/sonar/process/monitor/Timeouts.java | 15 +++++++++++++++ .../sonar/process/monitor/JavaCommandTest.java | 6 +++--- .../org/sonar/process/monitor/TimeoutsTest.java | 3 +++ .../main/java/org/sonar/application/App.java | 2 +- 7 files changed, 47 insertions(+), 7 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 dd564d77247..e950c92c58c 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 @@ -71,6 +71,9 @@ public class JavaCommand { return this; } + /** + * Shortcut to set the java option -Djava.io.tmpdir + */ public JavaCommand setTempDir(File tempDir) { this.javaOptions.add("-Djava.io.tmpdir=" + tempDir.getAbsolutePath()); return this; @@ -80,6 +83,9 @@ public class JavaCommand { return jmxPort; } + /** + * Current mandatory to be set + */ public JavaCommand setJmxPort(int jmxPort) { this.jmxPort = jmxPort; return this; 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 5a59a11df74..badc63b6da5 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 @@ -128,7 +128,10 @@ public class Monitor { try { ready = jmxConnector.isReady(processRef); } catch (Exception ignored) { - LoggerFactory.getLogger(getClass()).error("Fail to request JMX (isReady)", ignored); + // failed to send request, probably because RMI server is still not alive + // trying again, as long as process is alive + // TODO could be improved to have a STARTING timeout (to be implemented in monitor or + // in child process ?) } try { Thread.sleep(300L); diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java index d8889d9c45e..2ca0a295b87 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java @@ -91,8 +91,21 @@ class RmiJmxConnector implements JmxConnector { } @Override - public boolean isReady(ProcessRef processRef) { - return mbeans.get(processRef).isReady(); + public boolean isReady(final ProcessRef processRef) { + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future future = executor.submit(new Callable() { + @Override + public Boolean call() throws Exception { + return mbeans.get(processRef).isReady(); + } + }); + return future.get(timeouts.getMonitorIsReadyTimeout(), TimeUnit.MILLISECONDS); + } catch (Exception e) { + throw new IllegalStateException("Fail send JMX request (isReady)", e); + } finally { + executor.shutdownNow(); + } } @Override diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Timeouts.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Timeouts.java index 30ab3f5ce0c..68934f21066 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Timeouts.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Timeouts.java @@ -27,6 +27,7 @@ class Timeouts { private long terminationTimeout = 120000L; private long jmxConnectionTimeout = 30000L; private long monitorPingInterval = 3000L; + private long monitorIsReadyTimeout = 10000L; private long autokillPingTimeout = 60000L; private long autokillPingInterval = 3000L; @@ -58,6 +59,20 @@ class Timeouts { this.monitorPingInterval = l; } + /** + * [monitor] Timeout of isReady request + */ + long getMonitorIsReadyTimeout() { + return monitorIsReadyTimeout; + } + + /** + * @see #getMonitorIsReadyTimeout() + */ + void setMonitorIsReadyTimeout(long l) { + this.monitorIsReadyTimeout = l; + } + /** * [monitored process] maximum age of last received ping before process autokills */ 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 66654b0f1fc..766f24c86fc 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,7 +44,7 @@ public class JavaCommandTest { command.setJmxPort(1234); command.setClassName("org.sonar.ElasticSearch"); - command.setEnvVariable("BUILD_ID", "1000"); + command.setEnvVariable("JAVA_COMMAND_TEST", "1000"); File tempDir = temp.newFolder(); command.setTempDir(tempDir); File workDir = temp.newFolder(); @@ -59,10 +59,10 @@ public class JavaCommandTest { assertThat(command.getWorkDir()).isSameAs(workDir); assertThat(command.getJmxPort()).isEqualTo(1234); assertThat(command.getClassName()).isEqualTo("org.sonar.ElasticSearch"); - assertThat(command.getEnvVariables().get("BUILD_ID")).isEqualTo("1000"); // copy current env variables - assertThat(command.getEnvVariables().size()).isGreaterThan(1); + assertThat(command.getEnvVariables().get("JAVA_COMMAND_TEST")).isEqualTo("1000"); + assertThat(command.getEnvVariables().size()).isEqualTo(System.getenv().size() + 1); } @Test diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/TimeoutsTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/TimeoutsTest.java index 5a1d5b590d4..620a59929fc 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/TimeoutsTest.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/TimeoutsTest.java @@ -33,6 +33,7 @@ public class TimeoutsTest { assertThat(timeouts.getAutokillPingTimeout()).isGreaterThan(1000L); assertThat(timeouts.getTerminationTimeout()).isGreaterThan(1000L); assertThat(timeouts.getJmxConnectionTimeout()).isGreaterThan(1000L); + assertThat(timeouts.getMonitorIsReadyTimeout()).isGreaterThan(1000L); } @Test @@ -43,11 +44,13 @@ public class TimeoutsTest { timeouts.setTerminationTimeout(3L); timeouts.setJmxConnectionTimeout(4L); timeouts.setMonitorPingInterval(5L); + timeouts.setMonitorIsReadyTimeout(6L); assertThat(timeouts.getAutokillPingInterval()).isEqualTo(1L); assertThat(timeouts.getAutokillPingTimeout()).isEqualTo(2L); assertThat(timeouts.getTerminationTimeout()).isEqualTo(3L); assertThat(timeouts.getJmxConnectionTimeout()).isEqualTo(4L); assertThat(timeouts.getMonitorPingInterval()).isEqualTo(5L); + assertThat(timeouts.getMonitorIsReadyTimeout()).isEqualTo(6L); } } 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 42c4bba84ce..600828ec1db 100644 --- a/sonar-application/src/main/java/org/sonar/application/App.java +++ b/sonar-application/src/main/java/org/sonar/application/App.java @@ -56,7 +56,7 @@ public class App implements ProcessMXBean { monitor.awaitTermination(); } - private List createCommands(Props props) { + List createCommands(Props props) { List commands = new ArrayList(); File homeDir = props.nonNullValueAsFile("sonar.path.home"); File tempDir = props.nonNullValueAsFile("sonar.path.temp"); -- 2.39.5