SONAR-15230 Hazelcast clean up

4 files changed, 39 insertions, 83 deletions

diff --git a/server/sonar-main/src/main/java/org/sonar/application/AppStateFactory.java b/server/sonar-main/src/main/java/org/sonar/application/AppStateFactory.java
index 74541528cb2..026abbcdaec 100644
--- a/server/sonar-main/src/main/java/org/sonar/application/AppStateFactory.java
+++ b/server/sonar-main/src/main/java/org/sonar/application/AppStateFactory.java
@@ -34,7 +34,6 @@ import org.sonar.process.Props;
 import org.sonar.process.cluster.hz.HazelcastMember;
 import org.sonar.process.cluster.hz.HazelcastMemberBuilder;
 
-import static java.util.Arrays.asList;
 import static org.sonar.process.ProcessProperties.Property.CLUSTER_HZ_HOSTS;
 import static org.sonar.process.ProcessProperties.Property.CLUSTER_KUBERNETES;
 import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_HOST;
@@ -66,7 +65,7 @@ public class AppStateFactory {
     boolean isRunningOnKubernetes = props.valueAsBoolean(CLUSTER_KUBERNETES.getKey(), Boolean.parseBoolean(CLUSTER_KUBERNETES.getDefaultValue()));
     HazelcastMemberBuilder builder = new HazelcastMemberBuilder(isRunningOnKubernetes ? KUBERNETES : TCP_IP)
       .setNetworkInterface(props.nonNullValue(CLUSTER_NODE_HOST.getKey()))
-      .setMembers(asList(props.nonNullValue(CLUSTER_HZ_HOSTS.getKey()).split(",")))
+      .setMembers(props.nonNullValue(CLUSTER_HZ_HOSTS.getKey()))
       .setNodeName(props.nonNullValue(CLUSTER_NODE_NAME.getKey()))
       .setPort(Integer.parseInt(props.nonNullValue(CLUSTER_NODE_HZ_PORT.getKey())))
       .setProcessId(ProcessId.APP);
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 6d22bc6b9b9..11fd460a885 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
@@ -152,7 +152,7 @@ public class SchedulerImpl implements Scheduler, ManagedProcessEventListener, Pr
     if (process == null) {
       return;
     }
-    if (!isEsClientStartable()) {
+    if (!isEsOperational()) {
       if (firstWaitingEsLog.getAndSet(false)) {
         LOG.info("Waiting for Elasticsearch to be up and running");
       }
@@ -174,12 +174,12 @@ public class SchedulerImpl implements Scheduler, ManagedProcessEventListener, Pr
 
   private void tryToStartCe() throws InterruptedException {
     ManagedProcessHandler process = processesById.get(ProcessId.COMPUTE_ENGINE);
-    if (process != null && appState.isOperational(ProcessId.WEB_SERVER, true) && isEsClientStartable()) {
+    if (process != null && appState.isOperational(ProcessId.WEB_SERVER, true) && isEsOperational()) {
       tryToStartProcess(process, commandFactory::createCeCommand);
     }
   }
 
-  private boolean isEsClientStartable() {
+  private boolean isEsOperational() {
     boolean requireLocalEs = ClusterSettings.isLocalElasticsearchEnabled(settings);
     return appState.isOperational(ProcessId.ELASTICSEARCH, requireLocalEs);
   }
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 bcc81f2e3fb..ee4dd2eb43b 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
@@ -20,13 +20,12 @@
 package org.sonar.application.cluster;
 
 import com.hazelcast.cluster.Member;
+import com.hazelcast.cluster.MembershipAdapter;
 import com.hazelcast.cluster.MembershipEvent;
-import com.hazelcast.cluster.MembershipListener;
+import com.hazelcast.core.EntryAdapter;
 import com.hazelcast.core.EntryEvent;
-import com.hazelcast.core.EntryListener;
 import com.hazelcast.core.HazelcastInstanceNotActiveException;
 import com.hazelcast.cp.IAtomicReference;
-import com.hazelcast.map.MapEvent;
 import com.hazelcast.replicatedmap.ReplicatedMap;
 import java.util.ArrayList;
 import java.util.EnumMap;
@@ -35,7 +34,6 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicReference;
 import org.elasticsearch.cluster.health.ClusterHealthStatus;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -105,7 +103,11 @@ public class ClusterAppStateImpl implements ClusterAppState {
     }
 
     if (processId.equals(ProcessId.ELASTICSEARCH)) {
-      return isElasticSearchAvailable();
+      boolean operational = isElasticSearchOperational();
+      if (!operational) {
+        asyncWaitForEsToBecomeOperational();
+      }
+      return operational;
     }
 
     for (Map.Entry<ClusterProcess, Boolean> entry : operationalProcesses.entrySet()) {
@@ -203,14 +205,10 @@ public class ClusterAppStateImpl implements ClusterAppState {
     }
   }
 
-  private boolean isElasticSearchAvailable() {
-    boolean available = esConnector.getClusterHealthStatus()
+  private boolean isElasticSearchOperational() {
+    return esConnector.getClusterHealthStatus()
       .filter(t -> ClusterHealthStatus.GREEN.equals(t) || ClusterHealthStatus.YELLOW.equals(t))
       .isPresent();
-    if (!available) {
-      asyncWaitForEsToBecomeOperational();
-    }
-    return available;
   }
 
   private void asyncWaitForEsToBecomeOperational() {
@@ -228,14 +226,13 @@ public class ClusterAppStateImpl implements ClusterAppState {
 
     @Override
     public void run() {
-      if (isElasticSearchAvailable()) {
-        listeners.forEach(l -> l.onAppStateOperational(ProcessId.ELASTICSEARCH));
-        esPoolingThreadRunning.set(false);
-        return;
-      }
+      while (true) {
+        if (isElasticSearchOperational()) {
+          esPoolingThreadRunning.set(false);
+          listeners.forEach(l -> l.onAppStateOperational(ProcessId.ELASTICSEARCH));
+          return;
+        }
 
-      LOGGER.info("Waiting for ElasticSearch to be up and running");
-      do {
         try {
           Thread.sleep(5_000);
         } catch (InterruptedException e) {
@@ -243,16 +240,11 @@ public class ClusterAppStateImpl implements ClusterAppState {
           Thread.currentThread().interrupt();
           return;
         }
-        if (isElasticSearchAvailable()) {
-          listeners.forEach(l -> l.onAppStateOperational(ProcessId.ELASTICSEARCH));
-          esPoolingThreadRunning.set(false);
-          return;
-        }
-      } while (true);
+      }
     }
   }
 
-  private class OperationalProcessListener implements EntryListener<ClusterProcess, Boolean> {
+  private class OperationalProcessListener extends EntryAdapter<ClusterProcess, Boolean> {
     @Override
     public void entryAdded(EntryEvent<ClusterProcess, Boolean> event) {
       if (event.getValue()) {
@@ -261,44 +253,14 @@ public class ClusterAppStateImpl implements ClusterAppState {
     }
 
     @Override
-    public void entryRemoved(EntryEvent<ClusterProcess, Boolean> event) {
-      // Ignore it
-    }
-
-    @Override
     public void entryUpdated(EntryEvent<ClusterProcess, Boolean> event) {
       if (event.getValue()) {
         listeners.forEach(appStateListener -> appStateListener.onAppStateOperational(event.getKey().getProcessId()));
       }
     }
-
-    @Override
-    public void entryEvicted(EntryEvent<ClusterProcess, Boolean> event) {
-      // Ignore it
-    }
-
-    @Override
-    public void mapCleared(MapEvent event) {
-      // Ignore it
-    }
-
-    @Override
-    public void mapEvicted(MapEvent event) {
-      // Ignore it
-    }
-
-    @Override
-    public void entryExpired(EntryEvent<ClusterProcess, Boolean> event) {
-      // Ignore it
-    }
   }
 
-  private class NodeDisconnectedListener implements MembershipListener {
-    @Override
-    public void memberAdded(MembershipEvent membershipEvent) {
-      // Nothing to do
-    }
-
+  private class NodeDisconnectedListener extends MembershipAdapter {
     @Override
     public void memberRemoved(MembershipEvent membershipEvent) {
       removeOperationalProcess(membershipEvent.getMember().getUuid());
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 16dce4d4bc7..8a0deba5315 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
@@ -25,7 +25,6 @@ import org.elasticsearch.cluster.health.ClusterHealthStatus;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.DisableOnDebug;
-import org.junit.rules.ExpectedException;
 import org.junit.rules.TestRule;
 import org.junit.rules.Timeout;
 import org.sonar.application.AppStateListener;
@@ -39,6 +38,7 @@ import org.sonar.process.cluster.hz.HazelcastMemberBuilder;
 import org.sonar.process.cluster.hz.JoinConfigurationType;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
@@ -49,17 +49,14 @@ import static org.sonar.process.cluster.hz.HazelcastObjects.SONARQUBE_VERSION;
 public class ClusterAppStateImplTest {
 
   @Rule
-  public ExpectedException expectedException = ExpectedException.none();
-
-  @Rule
   public TestRule safeguardTimeout = new DisableOnDebug(Timeout.seconds(60));
 
   @Test
   public void tryToLockWebLeader_returns_true_only_for_the_first_call() {
     try (ClusterAppStateImpl underTest = new ClusterAppStateImpl(new TestAppSettings(), newHzMember(),
       mock(EsConnector.class), mock(AppNodesClusterHostsConsistency.class))) {
-      assertThat(underTest.tryToLockWebLeader()).isEqualTo(true);
-      assertThat(underTest.tryToLockWebLeader()).isEqualTo(false);
+      assertThat(underTest.tryToLockWebLeader()).isTrue();
+      assertThat(underTest.tryToLockWebLeader()).isFalse();
     }
   }
 
@@ -72,10 +69,10 @@ public class ClusterAppStateImplTest {
       underTest.setOperational(ProcessId.ELASTICSEARCH);
       verify(listener, timeout(20_000)).onAppStateOperational(ProcessId.ELASTICSEARCH);
 
-      assertThat(underTest.isOperational(ProcessId.ELASTICSEARCH, true)).isEqualTo(true);
-      assertThat(underTest.isOperational(ProcessId.APP, true)).isEqualTo(false);
-      assertThat(underTest.isOperational(ProcessId.WEB_SERVER, true)).isEqualTo(false);
-      assertThat(underTest.isOperational(ProcessId.COMPUTE_ENGINE, true)).isEqualTo(false);
+      assertThat(underTest.isOperational(ProcessId.ELASTICSEARCH, true)).isTrue();
+      assertThat(underTest.isOperational(ProcessId.APP, true)).isFalse();
+      assertThat(underTest.isOperational(ProcessId.WEB_SERVER, true)).isFalse();
+      assertThat(underTest.isOperational(ProcessId.COMPUTE_ENGINE, true)).isFalse();
     }
   }
 
@@ -130,10 +127,9 @@ public class ClusterAppStateImplTest {
   @Test
   public void reset_always_throws_ISE() {
     try (ClusterAppStateImpl underTest = createClusterAppState()) {
-      expectedException.expect(IllegalStateException.class);
-      expectedException.expectMessage("state reset is not supported in cluster mode");
-
-      underTest.reset();
+      assertThatThrownBy(underTest::reset)
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessageContaining("state reset is not supported in cluster mode");
     }
   }
 
@@ -144,11 +140,10 @@ public class ClusterAppStateImplTest {
       // Register first version
       underTest.getHazelcastMember().getAtomicReference(SONARQUBE_VERSION).set("6.6.0.1111");
 
-      expectedException.expect(IllegalStateException.class);
-      expectedException.expectMessage("The local version 6.7.0.9999 is not the same as the cluster 6.6.0.1111");
-
       // Registering a second different version must trigger an exception
-      underTest.registerSonarQubeVersion("6.7.0.9999");
+      assertThatThrownBy(() -> underTest.registerSonarQubeVersion("6.7.0.9999"))
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessageContaining("The local version 6.7.0.9999 is not the same as the cluster 6.6.0.1111");
     }
   }
 
@@ -158,11 +153,10 @@ public class ClusterAppStateImplTest {
       // Register first version
       underTest.getHazelcastMember().getAtomicReference(CLUSTER_NAME).set("goodClusterName");
 
-      expectedException.expect(MessageException.class);
-      expectedException.expectMessage("This node has a cluster name [badClusterName], which does not match [goodClusterName] from the cluster");
-
       // Registering a second different cluster name must trigger an exception
-      underTest.registerClusterName("badClusterName");
+      assertThatThrownBy(() -> underTest.registerClusterName("badClusterName"))
+        .isInstanceOf(MessageException.class)
+        .hasMessage("This node has a cluster name [badClusterName], which does not match [goodClusterName] from the cluster");
     }
   }
 
@@ -199,6 +193,7 @@ public class ClusterAppStateImplTest {
       .setProcessId(ProcessId.COMPUTE_ENGINE)
       .setNodeName("bar")
       .setPort(NetworkUtilsImpl.INSTANCE.getNextLoopbackAvailablePort())
+      .setMembers(loopback.getHostAddress())
       .setNetworkInterface(loopback.getHostAddress())
       .build();
   }