SONAR-22990 improved the mechanism for locking the leader in data centern edition environment

8 files changed, 100 insertions, 10 deletions

diff --git a/server/sonar-main/src/main/java/org/sonar/application/AppState.java b/server/sonar-main/src/main/java/org/sonar/application/AppState.java
index d6bba371414..99d598821d5 100644
--- a/server/sonar-main/src/main/java/org/sonar/application/AppState.java
+++ b/server/sonar-main/src/main/java/org/sonar/application/AppState.java
@@ -48,6 +48,8 @@ public interface AppState extends AutoCloseable {
 
   boolean tryToLockWebLeader();
 
+  void tryToReleaseWebLeaderLock();
+
   void reset();
 
   void registerSonarQubeVersion(String sonarqubeVersion);
diff --git a/server/sonar-main/src/main/java/org/sonar/application/AppStateImpl.java b/server/sonar-main/src/main/java/org/sonar/application/AppStateImpl.java
index 0fd857a1238..d2651f1771b 100644
--- a/server/sonar-main/src/main/java/org/sonar/application/AppStateImpl.java
+++ b/server/sonar-main/src/main/java/org/sonar/application/AppStateImpl.java
@@ -56,6 +56,11 @@ public class AppStateImpl implements AppState {
   }
 
   @Override
+  public void tryToReleaseWebLeaderLock() {
+    webLeaderLocked.compareAndSet(true, false);
+  }
+
+  @Override
   public void reset() {
     webLeaderLocked.set(false);
     processes.clear();
@@ -80,4 +85,5 @@ public class AppStateImpl implements AppState {
   public void close() {
     // nothing to do
   }
+
 }
diff --git a/server/sonar-main/src/main/java/org/sonar/application/SchedulerImpl.java b/server/sonar-main/src/main/java/org/sonar/application/SchedulerImpl.java
index eacf5352a0d..32ca4242755 100644
--- a/server/sonar-main/src/main/java/org/sonar/application/SchedulerImpl.java
+++ b/server/sonar-main/src/main/java/org/sonar/application/SchedulerImpl.java
@@ -154,10 +154,10 @@ public class SchedulerImpl implements Scheduler, ManagedProcessEventListener, Pr
       }
       return;
     }
-    if (appState.isOperational(ProcessId.WEB_SERVER, false)) {
+    if (appState.tryToLockWebLeader()) {
+      tryToStartWebLeader(process);
+    } else if (appState.isOperational(ProcessId.WEB_SERVER, false)) {
       tryToStartProcess(process, () -> commandFactory.createWebCommand(false));
-    } else if (appState.tryToLockWebLeader()) {
-      tryToStartProcess(process, () -> commandFactory.createWebCommand(true));
     } else {
       Optional<String> leader = appState.getLeaderHostName();
       if (leader.isPresent()) {
@@ -168,6 +168,24 @@ public class SchedulerImpl implements Scheduler, ManagedProcessEventListener, Pr
     }
   }
 
+
+  /**
+   * Tries to start the web leader process. If the process fails to start, the web leader lock is released. If we would not release the lock
+   * then all nodes would need to be stopped and restarted.
+   */
+  private void tryToStartWebLeader(ManagedProcessHandler process) throws InterruptedException {
+    try {
+      boolean processStarted = tryToStartProcess(process, () -> commandFactory.createWebCommand(true));
+      if (!processStarted) {
+        appState.tryToReleaseWebLeaderLock();
+      }
+    } catch (InterruptedException e) {
+      logProcessStartFailure(process, e);
+      appState.tryToReleaseWebLeaderLock();
+      throw e;
+    }
+  }
+
   private void tryToStartCe() throws InterruptedException {
     ManagedProcessHandler process = processesById.get(ProcessId.COMPUTE_ENGINE);
     if (process != null && appState.isOperational(ProcessId.WEB_SERVER, true) && isEsOperational()) {
@@ -180,7 +198,7 @@ public class SchedulerImpl implements Scheduler, ManagedProcessEventListener, Pr
     return appState.isOperational(ProcessId.ELASTICSEARCH, requireLocalEs);
   }
 
-  private void tryToStartProcess(ManagedProcessHandler processHandler, Supplier<AbstractCommand> commandSupplier) throws InterruptedException {
+  private boolean tryToStartProcess(ManagedProcessHandler processHandler, Supplier<AbstractCommand> commandSupplier) throws InterruptedException {
     // starter or restarter thread was interrupted, we should not proceed with starting the process
     if (Thread.currentThread().isInterrupted()) {
       throw new InterruptedException();
@@ -192,13 +210,19 @@ public class SchedulerImpl implements Scheduler, ManagedProcessEventListener, Pr
         return processLauncher.launch(command);
       });
     } catch (RuntimeException e) {
-      // failed to start command -> do nothing
-      // the process failing to start will move directly to STOP state
-      // this early stop of the process will be picked up by onProcessStop (which calls hardStopAsync)
-      // through interface ProcessLifecycleListener#onProcessState implemented by SchedulerImpl
-      LOG.trace("Failed to start process [{}] (currentThread={})",
-        processHandler.getProcessId().getHumanReadableName(), Thread.currentThread().getName(), e);
+      logProcessStartFailure(processHandler, e);
+      return false;
     }
+    return true;
+  }
+
+  private static void logProcessStartFailure(ManagedProcessHandler processHandler, Exception e) {
+    // failed to start command -> do nothing
+    // the process failing to start will move directly to STOP state
+    // this early stop of the process will be picked up by onProcessStop (which calls hardStopAsync)
+    // through interface ProcessLifecycleListener#onProcessState implemented by SchedulerImpl
+    LOG.warn("Failed to start process [{}] (currentThread={})",
+      processHandler.getProcessId().getHumanReadableName(), Thread.currentThread().getName(), e);
   }
 
   @Override
@@ -211,6 +235,7 @@ public class SchedulerImpl implements Scheduler, ManagedProcessEventListener, Pr
 
   private void stopImpl() {
     try {
+      appState.tryToReleaseWebLeaderLock();
       stopAll();
       finalizeStop();
     } catch (InterruptedException e) {
diff --git a/server/sonar-main/src/main/java/org/sonar/application/cluster/ClusterAppStateImpl.java b/server/sonar-main/src/main/java/org/sonar/application/cluster/ClusterAppStateImpl.java
index 66154bd0c17..4917b27ac07 100644
--- a/server/sonar-main/src/main/java/org/sonar/application/cluster/ClusterAppStateImpl.java
+++ b/server/sonar-main/src/main/java/org/sonar/application/cluster/ClusterAppStateImpl.java
@@ -131,6 +131,21 @@ public class ClusterAppStateImpl implements ClusterAppState {
   }
 
   @Override
+  public void tryToReleaseWebLeaderLock() {
+    tryToReleaseWebLeaderLock(hzMember.getUuid());
+  }
+
+  /**
+   * Tries to release the lock of the cluster leader. It is safe to call this method even if one is not sure about the UUID of the leader.
+   * If all nodes call this method then we can be confident that the lock is released.
+   * @param uuidOfLeader - the UUID of the leader to release the lock. In case the UUID is not the leader's uuid this method has no effect.
+   */
+  private void tryToReleaseWebLeaderLock(UUID uuidOfLeader) {
+    IAtomicReference<UUID> leader = hzMember.getAtomicReference(LEADER);
+    leader.compareAndSet(uuidOfLeader, null);
+  }
+
+  @Override
   public void reset() {
     throw new IllegalStateException("state reset is not supported in cluster mode");
   }
@@ -267,6 +282,7 @@ public class ClusterAppStateImpl implements ClusterAppState {
     }
 
     private void removeOperationalProcess(UUID uuid) {
+      tryToReleaseWebLeaderLock(uuid);
       for (ClusterProcess clusterProcess : operationalProcesses.keySet()) {
         if (clusterProcess.getNodeUuid().equals(uuid)) {
           LOGGER.debug("Set node process off for [{}:{}] : ", clusterProcess.getNodeUuid(), clusterProcess.getProcessId());
diff --git a/server/sonar-main/src/test/java/org/sonar/application/AppStateImplTest.java b/server/sonar-main/src/test/java/org/sonar/application/AppStateImplTest.java
index 32e68923ca5..3847955569c 100644
--- a/server/sonar-main/src/test/java/org/sonar/application/AppStateImplTest.java
+++ b/server/sonar-main/src/test/java/org/sonar/application/AppStateImplTest.java
@@ -70,6 +70,15 @@ public class AppStateImplTest {
   }
 
   @Test
+  public void tryToReleaseWebLeaderLock_shouldReleaseLock() {
+    underTest.tryToLockWebLeader();
+
+    underTest.tryToReleaseWebLeaderLock();
+
+    assertThat(underTest.tryToLockWebLeader()).isTrue();
+  }
+
+  @Test
   public void reset_initializes_all_flags() {
     underTest.setOperational(ProcessId.ELASTICSEARCH);
     assertThat(underTest.tryToLockWebLeader()).isTrue();
diff --git a/server/sonar-main/src/test/java/org/sonar/application/SchedulerImplTest.java b/server/sonar-main/src/test/java/org/sonar/application/SchedulerImplTest.java
index 0a6ec85fcf3..14f7975d271 100644
--- a/server/sonar-main/src/test/java/org/sonar/application/SchedulerImplTest.java
+++ b/server/sonar-main/src/test/java/org/sonar/application/SchedulerImplTest.java
@@ -112,6 +112,18 @@ public class SchedulerImplTest {
   }
 
   @Test
+  public void schedule_whenWebLeaderFailsToStart_webLockIsReleased() throws InterruptedException {
+    TestAppSettings settings = new TestAppSettings();
+    SchedulerImpl underTest = newScheduler(settings, true);
+    clusterAppState.setOperational(ProcessId.ELASTICSEARCH);
+    processLauncher.makeStartupFail = WEB_SERVER;
+
+    underTest.schedule();
+
+    assertThat(clusterAppState.getWebLeaderLocked().get()).isFalse();
+  }
+
+  @Test
   public void start_and_stop_sequence_of_ES_WEB_CE_in_order() throws Exception {
     TestAppSettings settings = new TestAppSettings();
     SchedulerImpl underTest = newScheduler(settings, false);
diff --git a/server/sonar-main/src/test/java/org/sonar/application/TestAppState.java b/server/sonar-main/src/test/java/org/sonar/application/TestAppState.java
index 7ba6200df40..223f672abd0 100644
--- a/server/sonar-main/src/test/java/org/sonar/application/TestAppState.java
+++ b/server/sonar-main/src/test/java/org/sonar/application/TestAppState.java
@@ -67,6 +67,11 @@ public class TestAppState implements AppState {
   }
 
   @Override
+  public void tryToReleaseWebLeaderLock() {
+    webLeaderLocked.compareAndSet(true, false);
+  }
+
+  @Override
   public void reset() {
     webLeaderLocked.set(false);
     localProcesses.clear();
@@ -92,4 +97,8 @@ public class TestAppState implements AppState {
   public void close() {
     // nothing to do
   }
+
+  public AtomicBoolean getWebLeaderLocked() {
+    return webLeaderLocked;
+  }
 }
diff --git a/server/sonar-main/src/test/java/org/sonar/application/cluster/ClusterAppStateImplTest.java b/server/sonar-main/src/test/java/org/sonar/application/cluster/ClusterAppStateImplTest.java
index 422f202bafe..16a051fc934 100644
--- a/server/sonar-main/src/test/java/org/sonar/application/cluster/ClusterAppStateImplTest.java
+++ b/server/sonar-main/src/test/java/org/sonar/application/cluster/ClusterAppStateImplTest.java
@@ -77,6 +77,17 @@ public class ClusterAppStateImplTest {
   }
 
   @Test
+  public void tryToReleaseWebLeaderLock_shouldReleaseLock() {
+    try (ClusterAppStateImpl underTest = createClusterAppState()) {
+      underTest.tryToLockWebLeader();
+      assertThat(underTest.getLeaderHostName()).isPresent();
+
+      underTest.tryToReleaseWebLeaderLock();
+      assertThat(underTest.getLeaderHostName()).isEmpty();
+    }
+  }
+
+  @Test
   public void check_if_elasticsearch_is_operational_on_cluster() {
     AppStateListener listener = mock(AppStateListener.class);
     EsConnector esConnectorMock = mock(EsConnector.class);