SONAR-4898 add timeout to RMI calls on isReady()

6 files changed, 46 insertions, 6 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<Boolean> future = executor.submit(new Callable<Boolean>() {
+        @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;
 
@@ -59,6 +60,20 @@ class Timeouts {
   }
 
   /**
+   * [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
    */
   long getAutokillPingTimeout() {
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);
   }
 }