From f2daf496660f784d97d2b6949be323ed689bca6a Mon Sep 17 00:00:00 2001 From: Jacek Poreda Date: Tue, 18 Jul 2023 10:58:16 +0200 Subject: [PATCH] SONAR-19560 Silence hz inactive exception in scheduler --- .../ce/CeDistributedInformationImpl.java | 5 ++- .../ce/CeDistributedInformationImplTest.java | 25 ++++++++++- .../cluster/health/HealthStateRefresher.java | 8 +++- .../health/HealthStateRefresherTest.java | 41 +++++++++++-------- 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/CeDistributedInformationImpl.java b/server/sonar-ce/src/main/java/org/sonar/ce/CeDistributedInformationImpl.java index c6d23f4e94a..4ac9b89f85b 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/CeDistributedInformationImpl.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/CeDistributedInformationImpl.java @@ -19,6 +19,7 @@ */ package org.sonar.ce; +import com.hazelcast.core.HazelcastInstanceNotActiveException; import com.hazelcast.spi.exception.RetryableHazelcastException; import java.util.Map; import java.util.Set; @@ -82,8 +83,8 @@ public class CeDistributedInformationImpl implements CeDistributedInformation, S try { // Removing the worker UUIDs getClusteredWorkerUUIDs().remove(hazelcastMember.getUuid()); - } catch (RetryableHazelcastException e) { - LOGGER.debug("Unable to remove worker UUID from the list of active workers", e.getMessage()); + } catch (HazelcastInstanceNotActiveException | RetryableHazelcastException e) { + LOGGER.debug("Hazelcast is not active anymore", e); } } diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/CeDistributedInformationImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/CeDistributedInformationImplTest.java index 9e473a539a0..adde2711ffd 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/CeDistributedInformationImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/CeDistributedInformationImplTest.java @@ -21,6 +21,7 @@ package org.sonar.ce; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.hazelcast.core.HazelcastInstanceNotActiveException; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -28,18 +29,25 @@ import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.junit.Rule; import org.junit.Test; +import org.slf4j.event.Level; +import org.sonar.api.testfixtures.log.LogTester; import org.sonar.ce.taskprocessor.CeWorker; import org.sonar.ce.taskprocessor.CeWorkerFactory; import org.sonar.process.cluster.hz.HazelcastMember; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.data.MapEntry.entry; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.process.cluster.hz.HazelcastObjects.WORKER_UUIDS; public class CeDistributedInformationImplTest { + @Rule + public final LogTester logTester = new LogTester(); + private final UUID clientUUID1 = UUID.randomUUID(); private final UUID clientUUID2 = UUID.randomUUID(); private final UUID clientUUID3 = UUID.randomUUID(); @@ -101,7 +109,7 @@ public class CeDistributedInformationImplTest { try { ceDistributedInformation.broadcastWorkerUUIDs(); assertThat(modifiableWorkerMap).containsExactly( - entry(clientUUID1, ImmutableSet.of("a10", "a11"))); + entry(clientUUID1, Set.of("a10", "a11"))); } finally { ceDistributedInformation.stop(); } @@ -122,6 +130,19 @@ public class CeDistributedInformationImplTest { CeDistributedInformationImpl ceDistributedInformation = new CeDistributedInformationImpl(hzClientWrapper, mock(CeWorkerFactory.class)); ceDistributedInformation.stop(); assertThat(modifiableWorkerMap).containsExactlyInAnyOrderEntriesOf( - ImmutableMap.of(clientUUID2, ImmutableSet.of(w3), clientUUID3, ImmutableSet.of(w4, w5, w6))); + Map.of(clientUUID2, Set.of(w3), clientUUID3, ImmutableSet.of(w4, w5, w6))); } + + @Test + public void stop_whenThrowHazelcastInactiveException_shouldSilenceError() { + logTester.setLevel(Level.DEBUG); + when(hzClientWrapper.getReplicatedMap(any())).thenThrow(new HazelcastInstanceNotActiveException("Hazelcast is not active")); + + CeDistributedInformationImpl ceDistributedInformation = new CeDistributedInformationImpl(hzClientWrapper, mock(CeWorkerFactory.class)); + ceDistributedInformation.stop(); + + assertThat(logTester.logs(Level.DEBUG)).contains("Hazelcast is not active anymore"); + assertThat(logTester.logs(Level.ERROR)).isEmpty(); + } + } diff --git a/server/sonar-process/src/main/java/org/sonar/process/cluster/health/HealthStateRefresher.java b/server/sonar-process/src/main/java/org/sonar/process/cluster/health/HealthStateRefresher.java index 235b8e31817..20cd21498c5 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/cluster/health/HealthStateRefresher.java +++ b/server/sonar-process/src/main/java/org/sonar/process/cluster/health/HealthStateRefresher.java @@ -51,13 +51,17 @@ public class HealthStateRefresher implements Startable { NodeHealth nodeHealth = nodeHealthProvider.get(); sharedHealthState.writeMine(nodeHealth); } catch (HazelcastInstanceNotActiveException | RetryableHazelcastException e) { - LOG.debug("Hazelcast is no more active", e); + LOG.debug("Hazelcast is not active anymore", e); } catch (Throwable t) { LOG.error("An error occurred while attempting to refresh HealthState of the current node in the shared state:", t); } } public void stop() { - sharedHealthState.clearMine(); + try { + sharedHealthState.clearMine(); + } catch (HazelcastInstanceNotActiveException | RetryableHazelcastException e) { + LOG.debug("Hazelcast is not active anymore", e); + } } } diff --git a/server/sonar-process/src/test/java/org/sonar/process/cluster/health/HealthStateRefresherTest.java b/server/sonar-process/src/test/java/org/sonar/process/cluster/health/HealthStateRefresherTest.java index eafe00a8a76..93e47ffa42b 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/cluster/health/HealthStateRefresherTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/cluster/health/HealthStateRefresherTest.java @@ -28,7 +28,7 @@ import org.mockito.ArgumentCaptor; import org.sonar.process.LoggingRule; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; @@ -41,15 +41,15 @@ import static org.slf4j.event.Level.ERROR; public class HealthStateRefresherTest { @Rule - public LoggingRule logging = new LoggingRule(HealthStateRefresher.class); + public final LoggingRule logging = new LoggingRule(HealthStateRefresher.class); - private Random random = new Random(); - private NodeDetailsTestSupport testSupport = new NodeDetailsTestSupport(random); + private final Random random = new Random(); + private final NodeDetailsTestSupport testSupport = new NodeDetailsTestSupport(random); - private HealthStateRefresherExecutorService executorService = mock(HealthStateRefresherExecutorService.class); - private NodeHealthProvider nodeHealthProvider = mock(NodeHealthProvider.class); - private SharedHealthState sharedHealthState = mock(SharedHealthState.class); - private HealthStateRefresher underTest = new HealthStateRefresher(executorService, nodeHealthProvider, sharedHealthState); + private final HealthStateRefresherExecutorService executorService = mock(HealthStateRefresherExecutorService.class); + private final NodeHealthProvider nodeHealthProvider = mock(NodeHealthProvider.class); + private final SharedHealthState sharedHealthState = mock(SharedHealthState.class); + private final HealthStateRefresher underTest = new HealthStateRefresher(executorService, nodeHealthProvider, sharedHealthState); @Test public void start_adds_runnable_with_10_second_delay_and_initial_delay_putting_NodeHealth_from_provider_into_SharedHealthState() { @@ -79,15 +79,12 @@ public class HealthStateRefresherTest { verify(sharedHealthState).writeMine(nodeHealths[1]); verify(sharedHealthState).writeMine(nodeHealths[2]); - try { - runnable.run(); - } catch (IllegalStateException e) { - fail("Runnable should catch any Throwable"); - } + assertThatCode(runnable::run) + .doesNotThrowAnyException(); } @Test - public void stop_has_no_effect() { + public void stop_whenCalled_hasNoEffect() { underTest.stop(); verify(sharedHealthState).clearMine(); @@ -95,7 +92,19 @@ public class HealthStateRefresherTest { } @Test - public void do_not_log_errors_when_hazelcast_is_not_active() { + public void stop_whenThrowHazelcastInactiveException_shouldSilenceError() { + logging.setLevel(DEBUG); + SharedHealthState sharedHealthStateMock = mock(SharedHealthState.class); + doThrow(HazelcastInstanceNotActiveException.class).when(sharedHealthStateMock).clearMine(); + HealthStateRefresher underTest = new HealthStateRefresher(executorService, nodeHealthProvider, sharedHealthStateMock); + underTest.stop(); + + assertThat(logging.getLogs(ERROR)).isEmpty(); + assertThat(logging.hasLog(DEBUG, "Hazelcast is not active anymore")).isTrue(); + } + + @Test + public void start_whenHazelcastIsNotActive_shouldNotLogErrors() { logging.setLevel(DEBUG); doThrow(new HazelcastInstanceNotActiveException()).when(sharedHealthState).writeMine(any()); @@ -107,6 +116,6 @@ public class HealthStateRefresherTest { runnable.run(); assertThat(logging.getLogs(ERROR)).isEmpty(); - assertThat(logging.hasLog(DEBUG, "Hazelcast is no more active")).isTrue(); + assertThat(logging.hasLog(DEBUG, "Hazelcast is not active anymore")).isTrue(); } } -- 2.39.5