You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by wi...@apache.org on 2018/10/11 18:07:27 UTC
[geode] branch develop updated: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi… (#2589)
This is an automated email from the ASF dual-hosted git repository.
wirebaron pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 50efe8a GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi… (#2589)
50efe8a is described below
commit 50efe8a87ccc71203f31ae871f04bfd9f9e38c83
Author: Brian Rowe <br...@pivotal.io>
AuthorDate: Thu Oct 11 11:07:18 2018 -0700
GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi… (#2589)
Refactored several tests to use Awaitility instead of sleeping.
Removed other sleep calls that were not impacting their test case.
---
.../gms/fd/GMSHealthMonitorJUnitTest.java | 107 +++++++++------------
1 file changed, 43 insertions(+), 64 deletions(-)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java
index 6f4b86f..e3c9f9b 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java
@@ -23,6 +23,7 @@ import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_TTL;
import static org.apache.geode.distributed.ConfigurationProperties.MEMBER_TIMEOUT;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
@@ -218,14 +219,10 @@ public class GMSHealthMonitorJUnitTest {
installAView();
InternalDistributedMember initialNeighbor = mockMembers.get(myAddressIndex + 1);
- // allow the monitor to give up on the initial "next neighbor" and
- // move on to the one after it
- long giveup = System.currentTimeMillis() + (2 * memberTimeout) + 1500;
+ Awaitility.await("wait for new neighbor")
+ .atMost((2 * memberTimeout) + 1500, TimeUnit.MILLISECONDS)
+ .until(() -> gmsHealthMonitor.getNextNeighbor() != initialNeighbor);
InternalDistributedMember neighbor = gmsHealthMonitor.getNextNeighbor();
- while (System.currentTimeMillis() < giveup && neighbor == initialNeighbor) {
- Thread.sleep(50);
- neighbor = gmsHealthMonitor.getNextNeighbor();
- }
// neighbor should change. In order to not be a flaky test we don't demand
// that it be myAddressIndex+2 but just require that the neighbor being
@@ -243,20 +240,21 @@ public class GMSHealthMonitorJUnitTest {
@Test
public void testHMNextNeighborBeforeTimeout() throws IOException {
+ long startTime = System.currentTimeMillis();
installAView();
-
- // Should we remove these sleeps and force the checkmember directly instead of waiting?
- try {
- // member-timeout is 1000 ms. We initiate a check and choose
- // a new neighbor at 500 ms
- Thread.sleep(memberTimeout / GMSHealthMonitor.LOGICAL_INTERVAL - 100);
- } catch (InterruptedException e) {
- }
- // neighbor should be same
- System.out.println("next neighbor is " + gmsHealthMonitor.getNextNeighbor() + "\nmy address is "
+ final InternalDistributedMember neighbor = gmsHealthMonitor.getNextNeighbor();
+ System.out.println("next neighbor is " + neighbor + "\nmy address is "
+ mockMembers.get(myAddressIndex) + "\nview is " + joinLeave.getView());
+ assertEquals(mockMembers.get(myAddressIndex + 1), neighbor);
+ Awaitility.await("wait for new neighbor")
+ .atMost((2 * memberTimeout) + 1500, TimeUnit.MILLISECONDS)
+ .until(() -> gmsHealthMonitor.getNextNeighbor() != neighbor);
+ long endTime = System.currentTimeMillis();
- assertEquals(mockMembers.get(myAddressIndex + 1), gmsHealthMonitor.getNextNeighbor());
+ // member-timeout is 1000 ms. We initiate a check and choose
+ // a new neighbor at 500 ms
+ assertThat(memberTimeout / GMSHealthMonitor.LOGICAL_INTERVAL)
+ .isLessThanOrEqualTo(endTime - startTime);
}
/***
@@ -268,12 +266,12 @@ public class GMSHealthMonitorJUnitTest {
// when the view is installed we start a heartbeat timeout. After
// that expires we request a heartbeat
- Thread.sleep(3 * memberTimeout + 100);
+ Awaitility.await().atMost(3 * memberTimeout + 100, TimeUnit.MILLISECONDS)
+ .until(() -> (gmsHealthMonitor.isSuspectMember(mockMembers.get(myAddressIndex + 1))) &&
+ (gmsHealthMonitor.getStats().getHeartbeatRequestsSent() > 0) &&
+ (gmsHealthMonitor.getStats().getSuspectsSent() > 0));
System.out.println("testSuspectMembersCalledThroughMemberCheckThread ending");
- assertTrue(gmsHealthMonitor.isSuspectMember(mockMembers.get(myAddressIndex + 1)));
- Assert.assertTrue(gmsHealthMonitor.getStats().getHeartbeatRequestsSent() > 0);
- Assert.assertTrue(gmsHealthMonitor.getStats().getSuspectsSent() > 0);
}
private NetView installAView() {
@@ -303,17 +301,17 @@ public class GMSHealthMonitorJUnitTest {
*/
@Test
public void testSuspectMembersNotCalledThroughPingThreadBeforeTimeout() {
+ long startTime = System.currentTimeMillis();
installAView();
InternalDistributedMember neighbor = gmsHealthMonitor.getNextNeighbor();
- try {
- // member-timeout is 1000 ms
- // plus 100 ms for ack
- Thread.sleep(memberTimeout - 200);
- } catch (InterruptedException e) {
- }
+ Awaitility.await().atMost(3 * memberTimeout, TimeUnit.MILLISECONDS)
+ .until(() -> gmsHealthMonitor.isSuspectMember(neighbor));
+ long endTime = System.currentTimeMillis();
- assertFalse(gmsHealthMonitor.isSuspectMember(neighbor));
+ // member-timeout is 1000 ms
+ // plus 100 ms for ack
+ assertThat(memberTimeout + 100).isLessThanOrEqualTo(endTime - startTime);
}
/***
@@ -325,9 +323,10 @@ public class GMSHealthMonitorJUnitTest {
gmsHealthMonitor.suspect(mockMembers.get(1), "Not responding");
- Thread.sleep(GMSHealthMonitor.MEMBER_SUSPECT_COLLECTION_INTERVAL + 1000);
-
- verify(messenger, atLeastOnce()).send(any(SuspectMembersMessage.class));
+ Awaitility.await()
+ .atMost(GMSHealthMonitor.MEMBER_SUSPECT_COLLECTION_INTERVAL + 1000, TimeUnit.MILLISECONDS)
+ .untilAsserted(
+ () -> verify(messenger, atLeastOnce()).send(any(SuspectMembersMessage.class)));
Assert.assertTrue(gmsHealthMonitor.getStats().getSuspectsSent() > 0);
}
@@ -337,19 +336,11 @@ public class GMSHealthMonitorJUnitTest {
*/
@Test
public void testSuspectMembersNotCalledThroughSuspectThreadBeforeTimeout() {
-
installAView();
gmsHealthMonitor.suspect(mockMembers.get(1), "Not responding");
- try {
- // suspect thread timeout is 200 ms
- Thread.sleep(100l);
- } catch (InterruptedException e) {
- }
-
verify(messenger, atLeastOnce()).send(isA(SuspectMembersMessage.class));
-
Assert.assertTrue(gmsHealthMonitor.getStats().getSuspectsSent() > 0);
}
@@ -367,8 +358,6 @@ public class GMSHealthMonitorJUnitTest {
gmsHealthMonitor.installView(v);
- Thread.sleep(memberTimeout);
-
ArrayList<InternalDistributedMember> recipient = new ArrayList<InternalDistributedMember>();
recipient.add(mockMembers.get(0));
ArrayList<SuspectRequest> as = new ArrayList<SuspectRequest>();
@@ -412,17 +401,16 @@ public class GMSHealthMonitorJUnitTest {
SuspectMembersMessage sm = new SuspectMembersMessage(recipient, as);
sm.setSender(mockMembers.get(0));
+ long preProcess = System.currentTimeMillis();
gmsHealthMonitor.processMessage(sm);
- try {
- // this happens after final check, ping timeout
- Thread.sleep(memberTimeout - 100);
- } catch (InterruptedException e) {
- }
+ Awaitility.await("waiting for remove(member) to be invoked")
+ .atMost(3 * memberTimeout, TimeUnit.MILLISECONDS).untilAsserted(
+ () -> verify(joinLeave, atLeastOnce()).remove(any(InternalDistributedMember.class),
+ any(String.class)));
+ long postRemove = System.currentTimeMillis();
- System.out.println("testRemoveMemberNotCalledBeforeTimeout ending");
- verify(joinLeave, never()).remove(any(InternalDistributedMember.class), any(String.class));
- Assert.assertTrue(gmsHealthMonitor.getStats().getSuspectsReceived() > 0);
+ assertThat(memberTimeout).isLessThanOrEqualTo(postRemove - preProcess);
}
/***
@@ -440,8 +428,6 @@ public class GMSHealthMonitorJUnitTest {
gmsHealthMonitor.installView(v);
- Thread.sleep(memberTimeout / GMSHealthMonitor.LOGICAL_INTERVAL);
-
ArrayList<InternalDistributedMember> recipient = new ArrayList<InternalDistributedMember>();
recipient.add(mockMembers.get(0));
recipient.add(mockMembers.get(1));
@@ -454,11 +440,11 @@ public class GMSHealthMonitorJUnitTest {
gmsHealthMonitor.processMessage(sm);
- // this happens after final check, ping timeout = 1000 ms
- Thread.sleep(memberTimeout + 200);
+ Awaitility.await("waiting for remove(member) to be invoked")
+ .atMost(3 * memberTimeout, TimeUnit.MILLISECONDS).untilAsserted(
+ () -> verify(joinLeave, atLeastOnce()).remove(any(InternalDistributedMember.class),
+ any(String.class)));
- verify(joinLeave, atLeastOnce()).remove(any(InternalDistributedMember.class),
- any(String.class));
Assert.assertTrue(gmsHealthMonitor.getStats().getSuspectsReceived() > 0);
}
@@ -645,14 +631,7 @@ public class GMSHealthMonitorJUnitTest {
installAView();
gmsHealthMonitor.stop();
-
- try {
- // this happens after final check, membertimeout = 1000
- Thread.sleep(100l);
- } catch (InterruptedException e) {
- }
-
- assertTrue("HeathMonitor should have shutdown", gmsHealthMonitor.isShutdown());
+ Awaitility.await().until(() -> gmsHealthMonitor.isShutdown());
}