From 542c6c6f062a1897362606a62ca0ebe8938fd796 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 5 Sep 2017 11:12:49 +0200 Subject: [PATCH] SONAR-8741 ignore NodeHealth in HZ older than 30 seconds --- .../cluster/health/SharedHealthStateImpl.java | 15 +++- .../health/SharedHealthStateImplTest.java | 87 +++++++++++++++---- 2 files changed, 86 insertions(+), 16 deletions(-) diff --git a/server/sonar-cluster/src/main/java/org/sonar/cluster/health/SharedHealthStateImpl.java b/server/sonar-cluster/src/main/java/org/sonar/cluster/health/SharedHealthStateImpl.java index a045598d234..7286f4de9de 100644 --- a/server/sonar-cluster/src/main/java/org/sonar/cluster/health/SharedHealthStateImpl.java +++ b/server/sonar-cluster/src/main/java/org/sonar/cluster/health/SharedHealthStateImpl.java @@ -34,7 +34,7 @@ import static java.util.Objects.requireNonNull; public class SharedHealthStateImpl implements SharedHealthState { private static final String SQ_HEALTH_STATE_REPLICATED_MAP_IDENTIFIER = "sq_health_state"; private static final Logger LOG = Loggers.get(SharedHealthStateImpl.class); - private static final int TIMEOUT_30_SECONDS = 30 * 60 * 1000; + private static final int TIMEOUT_30_SECONDS = 30 * 1000; private final HazelcastClient hazelcastClient; @@ -65,9 +65,12 @@ public class SharedHealthStateImpl implements SharedHealthState { @Override public Set readAll() { + long clusterTime = hazelcastClient.getClusterTime(); + long timeout = clusterTime - TIMEOUT_30_SECONDS; Map sqHealthState = readReplicatedMap(); Set hzMemberUUIDs = hazelcastClient.getMemberUuids(); Set existingNodeHealths = sqHealthState.entrySet().stream() + .filter(outOfDate(timeout)) .filter(ofNonExistentMember(hzMemberUUIDs)) .map(entry -> entry.getValue().getNodeHealth()) .collect(Collectors.toSet()); @@ -77,6 +80,16 @@ public class SharedHealthStateImpl implements SharedHealthState { return ImmutableSet.copyOf(existingNodeHealths); } + private static Predicate> outOfDate(long timeout) { + return entry -> { + boolean res = entry.getValue().getTimestamp() > timeout; + if (!res && LOG.isTraceEnabled()) { + LOG.trace("Ignoring NodeHealth of member {} because it is too old", entry.getKey()); + } + return res; + }; + } + private static Predicate> ofNonExistentMember(Set hzMemberUUIDs) { return entry -> { boolean res = hzMemberUUIDs.contains(entry.getKey()); diff --git a/server/sonar-cluster/src/test/java/org/sonar/cluster/health/SharedHealthStateImplTest.java b/server/sonar-cluster/src/test/java/org/sonar/cluster/health/SharedHealthStateImplTest.java index 5c4eecc07e6..9c3c5d92e9c 100644 --- a/server/sonar-cluster/src/test/java/org/sonar/cluster/health/SharedHealthStateImplTest.java +++ b/server/sonar-cluster/src/test/java/org/sonar/cluster/health/SharedHealthStateImplTest.java @@ -19,7 +19,7 @@ */ package org.sonar.cluster.health; -import java.util.Collections; +import com.google.common.collect.ImmutableSet; import java.util.HashMap; import java.util.Map; import java.util.Random; @@ -32,6 +32,7 @@ import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; import org.sonar.cluster.localclient.HazelcastClient; +import static java.util.Collections.singleton; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.doReturn; @@ -51,6 +52,7 @@ public class SharedHealthStateImplTest { public LogTester logTester = new LogTester(); private final Random random = new Random(); + private long clusterTime = 99 + Math.abs(random.nextInt(9621)); private HazelcastClient hazelcastClient = Mockito.mock(HazelcastClient.class); private SharedHealthStateImpl underTest = new SharedHealthStateImpl(hazelcastClient); @@ -96,14 +98,14 @@ public class SharedHealthStateImplTest { } @Test - public void readAll_returns_all_NodeHealth_in_map_sq_health_state_for_existing_client_uuids() { + public void readAll_returns_all_NodeHealth_in_map_sq_health_state_for_existing_client_uuids_aged_less_than_30_seconds() { NodeHealth[] nodeHealths = IntStream.range(0, 1 + random.nextInt(6)).mapToObj(i -> randomNodeHealth()).toArray(NodeHealth[]::new); Map allNodeHealths = new HashMap<>(); Map expected = new HashMap<>(); String randomUuidBase = randomAlphanumeric(5); for (int i = 0; i < nodeHealths.length; i++) { String memberUuid = randomUuidBase + i; - TimestampedNodeHealth timestampedNodeHealth = new TimestampedNodeHealth(nodeHealths[i], random.nextLong()); + TimestampedNodeHealth timestampedNodeHealth = new TimestampedNodeHealth(nodeHealths[i], clusterTime - random.nextInt(30 * 1000)); allNodeHealths.put(memberUuid, timestampedNodeHealth); if (random.nextBoolean()) { expected.put(memberUuid, nodeHealths[i]); @@ -111,44 +113,99 @@ public class SharedHealthStateImplTest { } doReturn(allNodeHealths).when(hazelcastClient).getReplicatedMap(MAP_SQ_HEALTH_STATE); when(hazelcastClient.getMemberUuids()).thenReturn(expected.keySet()); + when(hazelcastClient.getClusterTime()).thenReturn(clusterTime); assertThat(underTest.readAll()) .containsOnly(expected.values().stream().toArray(NodeHealth[]::new)); assertThat(logTester.logs()).isEmpty(); } + @Test + public void readAll_ignores_NodeHealth_of_30_seconds_before_cluster_time() { + NodeHealth nodeHealth = randomNodeHealth(); + Map map = new HashMap<>(); + String memberUuid = randomAlphanumeric(5); + TimestampedNodeHealth timestampedNodeHealth = new TimestampedNodeHealth(nodeHealth, clusterTime - 30 * 1000); + map.put(memberUuid, timestampedNodeHealth); + doReturn(map).when(hazelcastClient).getReplicatedMap(MAP_SQ_HEALTH_STATE); + when(hazelcastClient.getMemberUuids()).thenReturn(map.keySet()); + when(hazelcastClient.getClusterTime()).thenReturn(clusterTime); + + assertThat(underTest.readAll()).isEmpty(); + } + + @Test + public void readAll_ignores_NodeHealth_of_more_than_30_seconds_before_cluster_time() { + NodeHealth nodeHealth = randomNodeHealth(); + Map map = new HashMap<>(); + String memberUuid = randomAlphanumeric(5); + TimestampedNodeHealth timestampedNodeHealth = new TimestampedNodeHealth(nodeHealth, clusterTime - 30 * 1000 - random.nextInt(99)); + map.put(memberUuid, timestampedNodeHealth); + doReturn(map).when(hazelcastClient).getReplicatedMap(MAP_SQ_HEALTH_STATE); + when(hazelcastClient.getMemberUuids()).thenReturn(map.keySet()); + when(hazelcastClient.getClusterTime()).thenReturn(clusterTime); + + assertThat(underTest.readAll()).isEmpty(); + } + @Test public void readAll_logs_map_sq_health_state_content_and_the_content_effectively_returned_if_TRACE() { logTester.setLevel(LoggerLevel.TRACE); Map map = new HashMap<>(); String uuid = randomAlphanumeric(44); NodeHealth nodeHealth = randomNodeHealth(); - map.put(uuid, new TimestampedNodeHealth(nodeHealth, random.nextLong())); - when(hazelcastClient.getClusterTime()).thenReturn(random.nextLong()); - when(hazelcastClient.getMemberUuids()).thenReturn(Collections.singleton(uuid)); + map.put(uuid, new TimestampedNodeHealth(nodeHealth, clusterTime - 1)); + when(hazelcastClient.getClusterTime()).thenReturn(clusterTime); + when(hazelcastClient.getMemberUuids()).thenReturn(singleton(uuid)); doReturn(map).when(hazelcastClient).getReplicatedMap(MAP_SQ_HEALTH_STATE); underTest.readAll(); assertThat(logTester.logs()).hasSize(1); - assertThat(logTester.logs(LoggerLevel.TRACE)).containsOnly("Reading " + new HashMap<>(map) + " and keeping " + Collections.singleton(nodeHealth)); + assertThat(logTester.logs(LoggerLevel.TRACE)).containsOnly("Reading " + new HashMap<>(map) + " and keeping " + singleton(nodeHealth)); + } + + @Test + public void readAll_logs_message_for_each_non_existing_member_ignored_if_TRACE() { + logTester.setLevel(LoggerLevel.TRACE); + Map map = new HashMap<>(); + String memberUuid1 = randomAlphanumeric(44); + String memberUuid2 = randomAlphanumeric(44); + map.put(memberUuid1, new TimestampedNodeHealth(randomNodeHealth(), clusterTime - 1)); + map.put(memberUuid2, new TimestampedNodeHealth(randomNodeHealth(), clusterTime - 1)); + when(hazelcastClient.getClusterTime()).thenReturn(clusterTime); + doReturn(map).when(hazelcastClient).getReplicatedMap(MAP_SQ_HEALTH_STATE); + + underTest.readAll(); + + assertThat(logTester.logs()).hasSize(3); + assertThat(logTester.logs(LoggerLevel.TRACE)) + .containsOnly( + "Reading " + new HashMap<>(map) + " and keeping []", + "Ignoring NodeHealth of member " + memberUuid1 + " because it is not part of the cluster at the moment", + "Ignoring NodeHealth of member " + memberUuid2 + " because it is not part of the cluster at the moment"); } @Test - public void readAll_logs_content_of_non_existing_member_was_ignored_if_TRACE() { + public void readAll_logs_message_for_each_timed_out_NodeHealth_ignored_if_TRACE() { logTester.setLevel(LoggerLevel.TRACE); Map map = new HashMap<>(); - String memberUuid = randomAlphanumeric(44); - map.put(memberUuid, new TimestampedNodeHealth(randomNodeHealth(), random.nextLong())); - when(hazelcastClient.getClusterTime()).thenReturn(random.nextLong()); + String memberUuid1 = randomAlphanumeric(44); + String memberUuid2 = randomAlphanumeric(44); + map.put(memberUuid1, new TimestampedNodeHealth(randomNodeHealth(), clusterTime - 30 * 1000)); + map.put(memberUuid2, new TimestampedNodeHealth(randomNodeHealth(), clusterTime - 30 * 1000)); doReturn(map).when(hazelcastClient).getReplicatedMap(MAP_SQ_HEALTH_STATE); + when(hazelcastClient.getMemberUuids()).thenReturn(ImmutableSet.of(memberUuid1, memberUuid2)); + when(hazelcastClient.getClusterTime()).thenReturn(clusterTime); underTest.readAll(); - assertThat(logTester.logs()).hasSize(2); - assertThat(logTester.logs(LoggerLevel.TRACE)).containsOnly( - "Reading " + map + " and keeping []", - "Ignoring NodeHealth of member " + memberUuid + " because it is not part of the cluster at the moment"); + assertThat(logTester.logs()).hasSize(3); + assertThat(logTester.logs(LoggerLevel.TRACE)) + .containsOnly( + "Reading " + new HashMap<>(map) + " and keeping []", + "Ignoring NodeHealth of member " + memberUuid1 + " because it is too old", + "Ignoring NodeHealth of member " + memberUuid2 + " because it is too old"); } @Test -- 2.39.5