Repository: geode
Updated Branches:
  refs/heads/develop c6b941fd3 -> 3474fa7ab


GEODE-2497 surprise members are never timed out during startup

Merge of 8d45ca22737282abe279d3c863478f904f2e1926 and
a6dfa4ca630a82fcf92942a834f8255e86d2bfcb from feature/GEODE-2497.

Moved the creation of the timer to GMSMembershipManager.started()

Removed write-lock in timer-creation method since it's only called from
one place now

Altered the way that the timer-creation method finds the
InternalDistributedSystem.  The old way of using getAnyInstance() was
the primary source of the problem since it returns null until startup
is completed.

Altered the surprise-member unit test to ensure that it's using the
timer and not relying on installation of a new membership view to clean
things up.

Altered the surprise-member unit test to run faster.  It now completes in
under 10 seconds.

This closes #402


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

Branch: refs/heads/develop
Commit: 135bd77a11c10609310838bb77cf9bb57d38fee3
Parents: c6b941f
Author: Bruce Schuchardt <bschucha...@pivotal.io>
Authored: Tue Feb 21 16:07:51 2017 -0800
Committer: Bruce Schuchardt <bschucha...@pivotal.io>
Committed: Tue Feb 21 16:30:19 2017 -0800

----------------------------------------------------------------------
 .../gms/mgr/GMSMembershipManager.java           | 190 +++++++++----------
 .../internal/DistributionManagerDUnitTest.java  |  72 +++----
 2 files changed, 122 insertions(+), 140 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/135bd77a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
index cf17025..050e201 100644
--- 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
+++ 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
@@ -128,7 +128,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Trick class to make the startup synch more visible in stack traces
-   * 
+   *
    * @see GMSMembershipManager#startupLock
    */
   static class EventProcessingLock {
@@ -143,7 +143,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
     /**
      * indicates whether the event is a departure, a surprise connect (i.e., 
before the view message
      * arrived), a view, or a regular message
-     * 
+     *
      * @see #SURPRISE_CONNECT
      * @see #VIEW
      * @see #MESSAGE
@@ -181,7 +181,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
     /**
      * Create a surprise connect event
-     * 
+     *
      * @param member the member connecting
      */
     StartupEvent(final InternalDistributedMember member) {
@@ -191,7 +191,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
     /**
      * Indicate if this is a surprise connect event
-     * 
+     *
      * @return true if this is a connect event
      */
     boolean isSurpriseConnect() {
@@ -200,7 +200,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
     /**
      * Create a view event
-     * 
+     *
      * @param v the new view
      */
     StartupEvent(NetView v) {
@@ -210,7 +210,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
     /**
      * Indicate if this is a view event
-     * 
+     *
      * @return true if this is a view event
      */
     boolean isGmsView() {
@@ -219,7 +219,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
     /**
      * Create a message event
-     * 
+     *
      * @param d the message
      */
     StartupEvent(DistributionMessage d) {
@@ -229,7 +229,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
     /**
      * Indicate if this is a message event
-     * 
+     *
      * @return true if this is a message event
      */
     boolean isDistributionMessage() {
@@ -248,14 +248,14 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * This is the latest view (ordered list of DistributedMembers) that has 
been installed
-   * 
+   *
    * All accesses to this object are protected via {@link #latestViewLock}
    */
   private NetView latestView = new NetView();
 
   /**
    * This is the lock for protecting access to latestView
-   * 
+   *
    * @see #latestView
    */
   private final ReadWriteLock latestViewLock = new ReentrantReadWriteLock();
@@ -290,11 +290,11 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
    * Members of the distributed system that we believe have shut down. Keys 
are instances of
    * {@link InternalDistributedMember}, values are Longs indicating the time 
this member was
    * shunned.
-   * 
+   *
    * Members are removed after {@link #SHUNNED_SUNSET} seconds have passed.
-   * 
+   *
    * Accesses to this list needs to be under the read or write lock of {@link 
#latestViewLock}
-   * 
+   *
    * @see System#currentTimeMillis()
    */
   // protected final Set shunnedMembers = Collections.synchronizedSet(new 
HashSet());
@@ -310,7 +310,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
    * per bug 39552, keep a list of members that have been shunned and for 
which a message is
    * printed. Contents of this list are cleared at the same time they are 
removed from
    * {@link #shunnedMembers}.
-   * 
+   *
    * Accesses to this list needs to be under the read or write lock of {@link 
#latestViewLock}
    */
   private final HashSet<DistributedMember> shunnedAndWarnedMembers = new 
HashSet<>();
@@ -326,7 +326,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
    * arrived, the member is removed from membership and member-left 
notification is performed.
    * <p>
    * > Accesses to this list needs to be under the read or write lock of 
{@link #latestViewLock}
-   * 
+   *
    * @see System#currentTimeMillis()
    */
   private final Map<InternalDistributedMember, Long> surpriseMembers = new 
ConcurrentHashMap<>();
@@ -344,7 +344,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Length of time, in seconds, that a member is retained in the zombie set
-   * 
+   *
    * @see #shunnedMembers
    */
   static private final int SHUNNED_SUNSET = Integer
@@ -368,7 +368,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
   /**
    * A list of messages received during channel startup that couldn't be 
processed yet. Additions or
    * removals of this list must be synchronized via {@link #startupLock}.
-   * 
+   *
    * @since GemFire 5.0
    */
   private final LinkedList<StartupEvent> startupMessages = new LinkedList<>();
@@ -381,8 +381,8 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
   /**
    * Insert our own MessageReceiver between us and the direct channel, in 
order to correctly filter
    * membership events.
-   * 
-   * 
+   *
+   *
    */
   class MyDCReceiver implements DirectChannelListener {
 
@@ -390,9 +390,9 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
     /**
      * Don't provide events until the caller has told us we are ready.
-     * 
+     *
      * Synchronization provided via GroupMembershipService.class.
-     * 
+     *
      * Note that in practice we only need to delay accepting the first client; 
we don't need to put
      * this check before every call...
      *
@@ -509,7 +509,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
           continue; // no additions processed after shutdown begins
         } else {
           boolean wasShunned = endShun(m); // bug #45158 - no longer shun a 
process that is now in
-                                           // view
+          // view
           if (wasShunned && logger.isDebugEnabled()) {
             logger.debug("No longer shunning {} as it is in the current 
membership view", m);
           }
@@ -576,7 +576,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
       // expire surprise members, add others to view
       long oldestAllowed = System.currentTimeMillis() - 
this.surpriseMemberTimeout;
       for (Iterator<Map.Entry<InternalDistributedMember, Long>> it =
-          surpriseMembers.entrySet().iterator(); it.hasNext();) {
+           surpriseMembers.entrySet().iterator(); it.hasNext();) {
         Map.Entry<InternalDistributedMember, Long> entry = it.next();
         Long birthtime = entry.getValue();
         if (birthtime.longValue() < oldestAllowed) {
@@ -608,7 +608,6 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
       }
       try {
         listener.viewInstalled(latestView);
-        startCleanupTimer();
       } catch (DistributedSystemDisconnectedException se) {
       }
     } finally {
@@ -616,9 +615,13 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
     }
   }
 
+  public boolean isCleanupTimerStarted() {
+    return this.cleanupTimer != null;
+  }
+
   /**
    * the timer used to perform periodic tasks
-   * 
+   *
    * Concurrency: protected by {@link #latestViewLock} ReentrantReadWriteLock
    */
   private SystemTimer cleanupTimer;
@@ -637,7 +640,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Joins the distributed system
-   * 
+   *
    * @throws GemFireConfigException - configuration error
    * @throws SystemConnectException - problem joining
    */
@@ -767,7 +770,9 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
   }
 
   @Override
-  public void started() {}
+  public void started() {
+    startCleanupTimer();
+  }
 
 
   /** this is invoked by JoinLeave when there is a loss of quorum in the 
membership system */
@@ -843,7 +848,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Process a surprise connect event, or place it on the startup queue.
-   * 
+   *
    * @param member the member
    */
   protected void handleOrDeferSurpriseConnect(InternalDistributedMember 
member) {
@@ -875,7 +880,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
    * <p>
    * Must be called with {@link #latestViewLock} held. Waits until there is a 
stable view. If the
    * member has already been added, simply returns; else adds the member.
-   * 
+   *
    * @param dm the member joining
    */
   public boolean addSurpriseMember(DistributedMember dm) {
@@ -942,12 +947,6 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
         surpriseMembers.remove(member);
       } else {
 
-        // Now that we're sure the member is new, add them.
-        // make sure the surprise-member cleanup task is running
-        if (this.cleanupTimer == null) {
-          startCleanupTimer();
-        } // cleanupTimer == null
-
         // Ensure that the member is accounted for in the view
         // Conjure up a new view including the new member. This is necessary
         // because we are about to tell the listener about a new member, so
@@ -978,40 +977,39 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /** starts periodic task to perform cleanup chores such as expire surprise 
members */
   private void startCleanupTimer() {
+    if (this.dcReceiver == null) {
+      // junit tests don't provide a direct-channel receiver
+      return;
+    }
+    DistributedSystem ds = this.dcReceiver.getDM().getSystem();
+    this.cleanupTimer = new SystemTimer(ds, true);
+    SystemTimer.SystemTimerTask st = new SystemTimer.SystemTimerTask() {
+      @Override
+      public void run2() {
+        cleanUpSurpriseMembers();
+      }
+    };
+    this.cleanupTimer.scheduleAtFixedRate(st, surpriseMemberTimeout, 
surpriseMemberTimeout / 3);
+  }
+
+  // invoked from the cleanupTimer task
+  private void cleanUpSurpriseMembers() {
     latestViewWriteLock.lock();
     try {
-      if (this.cleanupTimer != null) {
-        return;
+      long oldestAllowed = System.currentTimeMillis() - surpriseMemberTimeout;
+      for (Iterator it = surpriseMembers.entrySet().iterator(); it.hasNext();) 
{
+        Map.Entry entry = (Map.Entry) it.next();
+        Long birthtime = (Long) entry.getValue();
+        if (birthtime.longValue() < oldestAllowed) {
+          it.remove();
+          InternalDistributedMember m = (InternalDistributedMember) 
entry.getKey();
+          logger.info(LocalizedMessage.create(
+              
LocalizedStrings.GroupMembershipService_MEMBERSHIP_EXPIRING_MEMBERSHIP_OF_SURPRISE_MEMBER_0,
+              m));
+          removeWithViewLock(m, true,
+              "not seen in membership view in " + surpriseMemberTimeout + 
"ms");
+        }
       }
-      DistributedSystem ds = InternalDistributedSystem.getAnyInstance();
-      if (ds != null && ds.isConnected()) {
-        this.cleanupTimer = new SystemTimer(ds, true);
-        SystemTimer.SystemTimerTask st = new SystemTimer.SystemTimerTask() {
-          @Override
-          public void run2() {
-            latestViewWriteLock.lock();
-            try {
-              long oldestAllowed = System.currentTimeMillis() - 
surpriseMemberTimeout;
-              for (Iterator it = surpriseMembers.entrySet().iterator(); 
it.hasNext();) {
-                Map.Entry entry = (Map.Entry) it.next();
-                Long birthtime = (Long) entry.getValue();
-                if (birthtime.longValue() < oldestAllowed) {
-                  it.remove();
-                  InternalDistributedMember m = (InternalDistributedMember) 
entry.getKey();
-                  logger.info(LocalizedMessage.create(
-                      
LocalizedStrings.GroupMembershipService_MEMBERSHIP_EXPIRING_MEMBERSHIP_OF_SURPRISE_MEMBER_0,
-                      m));
-                  removeWithViewLock(m, true,
-                      "not seen in membership view in " + 
surpriseMemberTimeout + "ms");
-                }
-              }
-            } finally {
-              latestViewWriteLock.unlock();
-            }
-          }
-        };
-        this.cleanupTimer.scheduleAtFixedRate(st, surpriseMemberTimeout, 
surpriseMemberTimeout / 3);
-      } // ds != null && ds.isConnected()
     } finally {
       latestViewWriteLock.unlock();
     }
@@ -1019,7 +1017,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Dispatch the distribution message, or place it on the startup queue.
-   * 
+   *
    * @param msg the message to process
    */
   protected void handleOrDeferMessage(DistributionMessage msg) {
@@ -1068,7 +1066,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
    * <p>
    * It is possible to receive messages not consistent with our view. We 
handle this here, and
    * generate an uplevel event if necessary
-   * 
+   *
    * @param msg the message
    */
   private void dispatchMessage(DistributionMessage msg) {
@@ -1103,7 +1101,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
     }
 
     if (shunned) { // bug #41538 - shun notification must be outside 
synchronization to avoid
-                   // hanging
+      // hanging
       warnShun(m);
       if (logger.isTraceEnabled(LogMarker.DISTRIBUTION_VIEWS)) {
         logger.trace(LogMarker.DISTRIBUTION_VIEWS,
@@ -1118,7 +1116,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Process a new view object, or place on the startup queue
-   * 
+   *
    * @param viewArg the new view
    */
   protected void handleOrDeferViewEvent(NetView viewArg) {
@@ -1152,14 +1150,14 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   @Override
   public void memberSuspected(InternalDistributedMember initiator,
-      InternalDistributedMember suspect, String reason) {
+                              InternalDistributedMember suspect, String 
reason) {
     SuspectMember s = new SuspectMember(initiator, suspect, reason);
     handleOrDeferSuspect(s);
   }
 
   /**
    * Process a new view object, or place on the startup queue
-   * 
+   *
    * @param suspectInfo the suspectee and suspector
    */
   protected void handleOrDeferSuspect(SuspectMember suspectInfo) {
@@ -1196,7 +1194,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Dispatch routine for processing a single startup event
-   * 
+   *
    * @param o the startup event to handle
    */
   private void processStartupEvent(StartupEvent o) {
@@ -1350,7 +1348,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
    * <p>
    * If no members have partition detection enabled, there will be no lead 
member and this method
    * will return null.
-   * 
+   *
    * @return the lead member associated with the latest view
    */
   public DistributedMember getLeadMember() {
@@ -1368,7 +1366,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * test hook
-   * 
+   *
    * @return the current membership view coordinator
    */
   public DistributedMember getCoordinator() {
@@ -1415,7 +1413,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Ensure that the critical classes from components get loaded.
-   * 
+   *
    * @see SystemFailure#loadEmergencyClasses()
    */
   public static void loadEmergencyClasses() {
@@ -1430,7 +1428,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
   /**
    * Close the receiver, avoiding all potential deadlocks and eschewing any 
attempts at being
    * graceful.
-   * 
+   *
    * @see SystemFailure#emergencyClose()
    */
   public void emergencyClose() {
@@ -1652,7 +1650,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Perform the grossness associated with sending a message over a 
DirectChannel
-   * 
+   *
    * @param destinations the list of destinations
    * @param content the message
    * @param theStats the statistics object to update
@@ -1716,18 +1714,18 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
         return null;
 
       List<InternalDistributedMember> members = 
(List<InternalDistributedMember>) ex.getMembers(); // We
-                                                                               
                    // need
-                                                                               
                    // to
-                                                                               
                    // return
-                                                                               
                    // this
-                                                                               
                    // list
-                                                                               
                    // of
-                                                                               
                    // failures
+      // need
+      // to
+      // return
+      // this
+      // list
+      // of
+      // failures
 
       // SANITY CHECK: If we fail to send a message to an existing member
       // of the view, we have a serious error (bug36202).
       NetView view = services.getJoinLeave().getView(); // grab a recent view, 
excluding shunned
-                                                        // members
+      // members
 
       // Iterate through members and causes in tandem :-(
       Iterator it_mem = members.iterator();
@@ -1805,7 +1803,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
   }
 
   public Set<InternalDistributedMember> send(InternalDistributedMember[] 
destinations,
-      DistributionMessage msg, DMStats theStats) throws 
NotSerializableException {
+                                             DistributionMessage msg, DMStats 
theStats) throws NotSerializableException {
     Set<InternalDistributedMember> result;
     boolean allDestinations = msg.forAll();
 
@@ -1866,7 +1864,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
     }
 
     boolean sendViaMessenger = isForceUDPCommunications(); // enable when bug 
#46438 is fixed: ||
-                                                           // msg.sendViaUDP();
+    // msg.sendViaUDP();
 
     if (useMcast || tcpDisabled || sendViaMessenger) {
       checkAddressesForUUIDs(destinations);
@@ -1933,7 +1931,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Clean up and create consistent new view with member removed. No uplevel 
events are generated.
-   * 
+   *
    * Must be called with the {@link #latestViewLock} held.
    */
   private void destroyMember(final InternalDistributedMember member, final 
String reason) {
@@ -1996,12 +1994,12 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
 
   /**
    * Indicate whether the given member is in the zombie list (dead or dying)
-   * 
+   *
    * @param m the member in question
-   * 
+   *
    *        This also checks the time the given member was shunned, and has 
the side effect of
    *        removing the member from the list if it was shunned too far in the 
past.
-   * 
+   *
    *        Concurrency: protected by {@link #latestViewLock} 
ReentrantReadWriteLock
    *
    * @return true if the given member is a zombie
@@ -2049,9 +2047,9 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
    * during view processing.
    * <p>
    * Like isShunned, this method holds the view lock while executing
-   * 
+   *
    * Concurrency: protected by {@link #latestViewLock} ReentrantReadWriteLock
-   * 
+   *
    * @param m the member in question
    * @return true if the given member is a surprise member
    */
@@ -2072,7 +2070,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
   /**
    * for testing we need to be able to inject surprise members into the view 
to ensure that
    * sunsetting works properly
-   * 
+   *
    * @param m the member ID to add
    * @param birthTime the millisecond clock time that the member was first seen
    */
@@ -2106,7 +2104,7 @@ public class GMSMembershipManager implements 
MembershipManager, Manager {
    * really old.
    * <p>
    * Must be called with {@link #latestViewLock} held and the view stable.
-   * 
+   *
    * @param m the member to add
    */
   private void addShunnedMember(InternalDistributedMember m) {

http://git-wip-us.apache.org/repos/asf/geode/blob/135bd77a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
index ccce013..5e3cf3e 100644
--- 
a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
@@ -17,6 +17,7 @@ package org.apache.geode.distributed.internal;
 import static org.apache.geode.distributed.ConfigurationProperties.*;
 import static org.apache.geode.test.dunit.Assert.*;
 
+import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
 import org.awaitility.Awaitility;
 
 import java.net.InetAddress;
@@ -28,6 +29,7 @@ import org.apache.geode.test.junit.categories.MembershipTest;
 import org.apache.logging.log4j.Logger;
 import org.junit.Assert;
 import org.junit.Ignore;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -74,6 +76,9 @@ public class DistributionManagerDUnitTest extends 
JUnit4DistributedTestCase {
 
   public static DistributedSystem ds;
 
+  @Rule
+  public DistributedRestoreSystemProperties restoreSystemProperties = new 
DistributedRestoreSystemProperties();
+
   /**
    * Clears the exceptionInThread flag in the given distribution manager.
    */
@@ -137,18 +142,14 @@ public class DistributionManagerDUnitTest extends 
JUnit4DistributedTestCase {
     InternalDistributedMember idm = mgr.getLocalMember();
     // TODO GMS needs to have a system property allowing the bind-port to be 
set
     System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "jg-bind-port", "" 
+ idm.getPort());
-    try {
-      sys.disconnect();
-      sys = getSystem();
-      mgr = MembershipManagerHelper.getMembershipManager(sys);
-      sys.disconnect();
-      InternalDistributedMember idm2 = mgr.getLocalMember();
-      org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
-          .info("original ID=" + idm + " and after connecting=" + idm2);
-      assertTrue("should not have used a different udp port", idm.getPort() == 
idm2.getPort());
-    } finally {
-      System.getProperties().remove(DistributionConfig.GEMFIRE_PREFIX + 
"jg-bind-port");
-    }
+    sys.disconnect();
+    sys = getSystem();
+    mgr = MembershipManagerHelper.getMembershipManager(sys);
+    sys.disconnect();
+    InternalDistributedMember idm2 = mgr.getLocalMember();
+    org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
+        .info("original ID=" + idm + " and after connecting=" + idm2);
+    assertTrue("should not have used a different udp port", idm.getPort() == 
idm2.getPort());
   }
 
   /**
@@ -158,11 +159,12 @@ public class DistributionManagerDUnitTest extends 
JUnit4DistributedTestCase {
    * should be gone and force more view processing to have it scrubbed from 
the set.
    **/
   @Test
-  public void testSurpriseMemberHandling() {
-    VM vm0 = Host.getHost(0).getVM(0);
+  public void testSurpriseMemberHandling() throws Exception {
 
+    System.setProperty(DistributionConfig.GEMFIRE_PREFIX + 
"surprise-member-timeout", "3000");
     InternalDistributedSystem sys = getSystem();
     MembershipManager mgr = MembershipManagerHelper.getMembershipManager(sys);
+    assertTrue(((GMSMembershipManager) mgr).isCleanupTimerStarted());
 
     try {
       InternalDistributedMember mbr =
@@ -172,7 +174,8 @@ public class DistributionManagerDUnitTest extends 
JUnit4DistributedTestCase {
 
       // if the view number isn't being recorded correctly the test will pass 
but the
       // functionality is broken
-      Assert.assertTrue("expected view ID to be greater than zero", 
mgr.getView().getViewId() > 0);
+      Assert.assertTrue("expected view ID to be greater than zero",
+          mgr.getView().getViewId() > 0);
 
       int oldViewId = mbr.getVmViewId();
       mbr.setVmViewId((int) mgr.getView().getViewId() - 1);
@@ -180,19 +183,13 @@ public class DistributionManagerDUnitTest extends 
JUnit4DistributedTestCase {
           .info("current membership view is " + mgr.getView());
       org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
           .info("created ID " + mbr + " with view ID " + mbr.getVmViewId());
-      sys.getLogWriter()
-          .info("<ExpectedException action=add>attempt to add old 
member</ExpectedException>");
-      sys.getLogWriter()
-          .info("<ExpectedException action=add>Removing shunned GemFire 
node</ExpectedException>");
-      try {
-        boolean accepted = mgr.addSurpriseMember(mbr);
-        Assert.assertTrue("member with old ID was not rejected (bug #44566)", 
!accepted);
-      } finally {
-        sys.getLogWriter()
-            .info("<ExpectedException action=remove>attempt to add old 
member</ExpectedException>");
-        sys.getLogWriter().info(
-            "<ExpectedException action=remove>Removing shunned GemFire 
node</ExpectedException>");
-      }
+
+      IgnoredException.addIgnoredException("attempt to add old member");
+      IgnoredException.addIgnoredException("Removing shunned GemFire node");
+
+      boolean accepted = mgr.addSurpriseMember(mbr);
+      Assert.assertTrue("member with old ID was not rejected (bug #44566)", 
!accepted);
+
       mbr.setVmViewId(oldViewId);
 
       // now forcibly add it as a surprise member and show that it is reaped
@@ -203,28 +200,15 @@ public class DistributionManagerDUnitTest extends 
JUnit4DistributedTestCase {
       MembershipManagerHelper.addSurpriseMember(sys, mbr, birthTime);
       assertTrue("Member was not a surprise member", 
mgr.isSurpriseMember(mbr));
 
-      // force a real view change
-      SerializableRunnable connectDisconnect = new SerializableRunnable() {
-        public void run() {
-          getSystem().disconnect();
-        }
-      };
-      vm0.invoke(connectDisconnect);
-
       if (birthTime < (System.currentTimeMillis() - timeout)) {
         return; // machine is too busy and we didn't get enough CPU to perform 
more assertions
       }
       assertTrue("Member was incorrectly removed from surprise member set",
           mgr.isSurpriseMember(mbr));
 
-      try {
-        Thread.sleep(gracePeriod);
-      } catch (InterruptedException e) {
-        fail("test was interrupted", e);
-      }
-
-      vm0.invoke(connectDisconnect);
-      assertTrue("Member was not removed from surprise member set", 
!mgr.isSurpriseMember(mbr));
+      Awaitility.await("waiting for member to be removed")
+          .atMost((timeout / 3) + gracePeriod, TimeUnit.MILLISECONDS)
+          .until(() -> !mgr.isSurpriseMember(mbr));
 
     } finally {
       if (sys != null && sys.isConnected()) {

Reply via email to