Mailing-List: contact commits-help@geode.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geode.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: bschuchardt@apache.org
To: commits@geode.incubator.apache.org
Message-Id: <bc8076e1278f4fdc97136f9ee8ef311e@git.apache.org>
Subject: incubator-geode git commit: GEODE-2073 CI Failure:
 GMSHealthMonitorJUnitTest.testHMNextNeighborAfterTimeout
Date: Wed, 16 Nov 2016 18:38:30 +0000 (UTC)
archived-at: Wed, 16 Nov 2016 18:38:37 -0000

Repository: incubator-geode
Updated Branches:
  refs/heads/develop 216a10917 -> bb7a5e218


GEODE-2073 CI Failure: GMSHealthMonitorJUnitTest.testHMNextNeighborAfterTimeout

I modified the test to wait longer and not require that the "next neighbor"
be an exact member but merely a different one than the initial "next neigbor".

Also, the diagnostic test in the exception thrown by this method was
including the JoinLeave membership view but that is null in these
tests.  Instead it needs to show the HealthMonitor's membership view.
I added a method to GMSHealthMonitor to allow access to its view.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/bb7a5e21
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/bb7a5e21
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/bb7a5e21

Branch: refs/heads/develop
Commit: bb7a5e218e446e296df8f71b3578daf152ab0c8c
Parents: 216a109
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
Authored: Wed Nov 16 10:26:08 2016 -0800
Committer: Bruce Schuchardt <bschuchardt@pivotal.io>
Committed: Wed Nov 16 10:27:22 2016 -0800

----------------------------------------------------------------------
 .../membership/gms/fd/GMSHealthMonitor.java     |  8 ++++
 .../gms/fd/GMSHealthMonitorJUnitTest.java       | 39 +++++++++++---------
 2 files changed, 30 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/bb7a5e21/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
index b04c6a1..e62fee8 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
@@ -845,6 +845,14 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler {
     setNextNeighbor(newView, null);
   }
 
+  /**
+   * this method is primarily for tests. The current view should be pulled from JoinLeave or the
+   * MembershipManager (which includes surprise members)
+   */
+  public synchronized NetView getView() {
+    return currentView;
+  }
+
   /***
    * This method sets next neighbour which it needs to watch in current view.
    *

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/bb7a5e21/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java
index 547a200..505c010 100644
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorJUnitTest.java
@@ -74,6 +74,7 @@ public class GMSHealthMonitorJUnitTest {
   final long memberTimeout = 1000l;
   private int[] portRange = new int[] {0, 65535};
   private boolean useGMSHealthMonitorTestClass = false;
+  private final int myAddressIndex = 3;
 
   @Before
   public void initMocks() throws UnknownHostException {
@@ -124,8 +125,8 @@ public class GMSHealthMonitorJUnitTest {
         mockMembers.add(mbr);
       }
     }
-    when(joinLeave.getMemberID()).thenReturn(mockMembers.get(3));
-    when(messenger.getMemberID()).thenReturn(mockMembers.get(3));
+    when(joinLeave.getMemberID()).thenReturn(mockMembers.get(myAddressIndex));
+    when(messenger.getMemberID()).thenReturn(mockMembers.get(myAddressIndex));
     gmsHealthMonitor = new GMSHealthMonitorTest();
     gmsHealthMonitor.init(services);
     gmsHealthMonitor.start();
@@ -161,30 +162,34 @@ public class GMSHealthMonitorJUnitTest {
   @Test
   public void testHMNextNeighborVerify() throws IOException {
     installAView();
-    Assert.assertEquals(mockMembers.get(4), gmsHealthMonitor.getNextNeighbor());
+    Assert.assertEquals(mockMembers.get(myAddressIndex + 1), gmsHealthMonitor.getNextNeighbor());
   }
 
-  @Category(FlakyTest.class) // GEODE-2073
+  // @Category(FlakyTest.class) // GEODE-2073
   @Test
   public void testHMNextNeighborAfterTimeout() throws Exception {
     System.out.println("testHMNextNeighborAfterTimeout starting");
+
     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() + memberTimeout + 1500;
-    InternalDistributedMember expected = mockMembers.get(5);
+    long giveup = System.currentTimeMillis() + (2 * memberTimeout) + 1500;
     InternalDistributedMember neighbor = gmsHealthMonitor.getNextNeighbor();
-    while (System.currentTimeMillis() < giveup && neighbor != expected) {
-      Thread.sleep(5);
+    while (System.currentTimeMillis() < giveup && neighbor == initialNeighbor) {
+      Thread.sleep(50);
       neighbor = gmsHealthMonitor.getNextNeighbor();
     }
 
-    // neighbor should change to 5th
+    // 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
+    // monitored has changed
     System.out.println("testHMNextNeighborAfterTimeout ending");
-    Assert.assertEquals(
-        "expected " + expected + " but found " + neighbor + ".  view=" + joinLeave.getView(),
-        expected, neighbor);
+    Assert.assertNotNull(gmsHealthMonitor.getView());
+    Assert.assertNotEquals("neighbor to not be " + neighbor + "; my ID is "
+        + mockMembers.get(myAddressIndex) + ";  view=" + gmsHealthMonitor.getView(),
+        initialNeighbor, neighbor);
   }
 
   /**
@@ -204,9 +209,9 @@ public class GMSHealthMonitorJUnitTest {
     }
     // neighbor should be same
     System.out.println("next neighbor is " + gmsHealthMonitor.getNextNeighbor() + "\nmy address is "
-        + mockMembers.get(3) + "\nview is " + joinLeave.getView());
+        + mockMembers.get(myAddressIndex) + "\nview is " + joinLeave.getView());
 
-    Assert.assertEquals(mockMembers.get(4), gmsHealthMonitor.getNextNeighbor());
+    Assert.assertEquals(mockMembers.get(myAddressIndex + 1), gmsHealthMonitor.getNextNeighbor());
   }
 
   /***
@@ -221,7 +226,7 @@ public class GMSHealthMonitorJUnitTest {
     Thread.sleep(3 * memberTimeout + 100);
 
     System.out.println("testSuspectMembersCalledThroughMemberCheckThread ending");
-    assertTrue(gmsHealthMonitor.isSuspectMember(mockMembers.get(4)));
+    assertTrue(gmsHealthMonitor.isSuspectMember(mockMembers.get(myAddressIndex + 1)));
     Assert.assertTrue(gmsHealthMonitor.getStats().getHeartbeatRequestsSent() > 0);
     Assert.assertTrue(gmsHealthMonitor.getStats().getSuspectsSent() > 0);
   }
@@ -231,7 +236,7 @@ public class GMSHealthMonitorJUnitTest {
     NetView v = new NetView(mockMembers.get(0), 2, mockMembers);
 
     // 3rd is current member
-    when(messenger.getMemberID()).thenReturn(mockMembers.get(3));
+    when(messenger.getMemberID()).thenReturn(mockMembers.get(myAddressIndex));
     gmsHealthMonitor.started();
 
     gmsHealthMonitor.installView(v);
@@ -400,7 +405,7 @@ public class GMSHealthMonitorJUnitTest {
                                                                                  // coordinator
     as.add(sr);
     SuspectMembersMessage sm = new SuspectMembersMessage(recipient, as);
-    sm.setSender(mockMembers.get(4));// member 4 sends suspect message
+    sm.setSender(mockMembers.get(myAddressIndex + 1));// member 4 sends suspect message
 
     gmsHealthMonitor.processMessage(sm);