geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bschucha...@apache.org
Subject [2/3] geode git commit: GEODE-3043 surprise member added when the member is already in the cluster
Date Fri, 09 Jun 2017 00:10:44 GMT
GEODE-3043 surprise member added when the member is already in the cluster

Modified InternalDistributedMember to disregard the "name" field in compareTo
if partial IDs are being used in the comparison.

Modified GMSMembershipManager to attempt to replace partial IDs with the
full IDs from the membership view.  Typically these IDs will cause no
problems and methods in GMSMembershipManager that send messages to a
recipient will also attempt to replace the ID with a full ID from the
membership view, but it's good to try to keep IDs canonical.


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

Branch: refs/heads/develop
Commit: c585245cdc46393897a54602c009219c129d9daf
Parents: 699577d
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
Authored: Thu Jun 8 16:59:48 2017 -0700
Committer: Bruce Schuchardt <bschuchardt@pivotal.io>
Committed: Thu Jun 8 17:02:53 2017 -0700

----------------------------------------------------------------------
 .../membership/InternalDistributedMember.java   |  33 ++++--
 .../internal/membership/gms/GMSMember.java      |  24 +++++
 .../gms/mgr/GMSMembershipManager.java           |  24 +++++
 .../distributed/DistributedMemberDUnitTest.java | 107 ++++++++++++-------
 4 files changed, 141 insertions(+), 47 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/c585245c/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
index 6982d31..2060934 100755
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
@@ -163,6 +163,7 @@ public class InternalDistributedMember implements DistributedMember, Externaliza
       this.versionObj = Version.CURRENT;
     }
     cachedToString = null;
+    this.isPartial = true;
   }
 
   /**
@@ -558,16 +559,18 @@ public class InternalDistributedMember implements DistributedMember,
Externaliza
 
     String myName = getName();
     String otherName = other.getName();
-    if (myName == null && otherName == null) {
-      // do nothing
-    } else if (myName == null) {
-      return -1;
-    } else if (otherName == null) {
-      return 1;
-    } else {
-      int i = myName.compareTo(otherName);
-      if (i != 0) {
-        return i;
+    if (!(other.isPartial || this.isPartial)) {
+      if (myName == null && otherName == null) {
+        // do nothing
+      } else if (myName == null) {
+        return -1;
+      } else if (otherName == null) {
+        return 1;
+      } else {
+        int i = myName.compareTo(otherName);
+        if (i != 0) {
+          return i;
+        }
       }
     }
 
@@ -605,6 +608,16 @@ public class InternalDistributedMember implements DistributedMember,
Externaliza
     // @todo Add durableClientAttributes to compare
   }
 
+  /**
+   * An InternalDistributedMember created for a test or via readEssentialData will be a Partial
ID,
+   * possibly not having ancillary info like "name".
+   * 
+   * @return true if this is a partial ID
+   */
+  public boolean isPartial() {
+    return isPartial;
+  }
+
   @Override
   public boolean equals(Object obj) {
     if (this == obj) {

http://git-wip-us.apache.org/repos/asf/geode/blob/c585245c/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java
index c82d97e..e368c9a 100755
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMember.java
@@ -138,6 +138,30 @@ public class GMSMember implements NetMember, DataSerializableFixedID
{
     this.vmViewId = viewId;
   }
 
+
+  /**
+   * Clone a GMSMember
+   * 
+   * @param other the member to create a copy of
+   */
+  public GMSMember(GMSMember other) {
+    this.udpPort = other.udpPort;
+    this.preferredForCoordinator = other.preferredForCoordinator;
+    this.networkPartitionDetectionEnabled = other.networkPartitionDetectionEnabled;
+    this.memberWeight = other.memberWeight;
+    this.inetAddr = other.inetAddr;
+    this.processId = other.processId;
+    this.vmKind = other.vmKind;
+    this.vmViewId = other.vmViewId;
+    this.directPort = other.directPort;
+    this.name = other.name;
+    this.durableClientAttributes = other.durableClientAttributes;
+    this.groups = other.groups;
+    this.versionOrdinal = other.versionOrdinal;
+    this.uuidLSBs = other.uuidLSBs;
+    this.uuidMSBs = other.uuidMSBs;
+  }
+
   public int getPort() {
     return this.udpPort;
   }

http://git-wip-us.apache.org/repos/asf/geode/blob/c585245c/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 fe560d9..639c6a8 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
@@ -1077,6 +1077,12 @@ public class GMSMembershipManager implements MembershipManager, Manager
{
     InternalDistributedMember m = msg.getSender();
     boolean shunned = false;
 
+    // UDP messages received from surprise members will have partial IDs.
+    // Attempt to replace these with full IDs from the MembershipManager's view.
+    if (msg.getSender().isPartial()) {
+      replacePartialIdentifierInMessage(msg);
+    }
+
     // If this member is shunned or new, grab the latestViewWriteLock: update the appropriate
data
     // structure.
     // synchronized (latestViewLock) {
@@ -1117,6 +1123,24 @@ public class GMSMembershipManager implements MembershipManager, Manager
{
     listener.messageReceived(msg);
   }
 
+  /**
+   * If the message's sender ID is a partial ID, which can happen if it's received in a JGroups
+   * message, replace it with an ID from the current membership view.
+   *
+   * @param msg the message holding the sender ID
+   */
+  public void replacePartialIdentifierInMessage(DistributionMessage msg) {
+    InternalDistributedMember sender = msg.getSender();
+    sender = this.services.getJoinLeave().getMemberID(sender.getNetMember());
+    if (sender.isPartial()) {
+      // the DM's view also has surprise members, so let's check it as well
+      sender = this.dcReceiver.getDM().getCanonicalId(sender);
+    }
+    if (!sender.isPartial()) {
+      msg.setSender(sender);
+    }
+  }
+
 
   /**
    * Process a new view object, or place on the startup queue

http://git-wip-us.apache.org/repos/asf/geode/blob/c585245c/geode-core/src/test/java/org/apache/geode/distributed/DistributedMemberDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/DistributedMemberDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/distributed/DistributedMemberDUnitTest.java
index 0153ca6..eb0e658 100755
--- a/geode-core/src/test/java/org/apache/geode/distributed/DistributedMemberDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/DistributedMemberDUnitTest.java
@@ -17,20 +17,27 @@ package org.apache.geode.distributed;
 import org.apache.geode.IncompatibleSystemException;
 import org.apache.geode.distributed.internal.DM;
 import org.apache.geode.distributed.internal.DistributionConfig;
+import org.apache.geode.distributed.internal.HighPriorityAckedMessage;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.distributed.internal.membership.gms.GMSMember;
+import org.apache.geode.distributed.internal.membership.gms.MembershipManagerHelper;
+import org.apache.geode.distributed.internal.membership.gms.mgr.GMSMembershipManager;
 import org.apache.geode.test.dunit.Host;
 import org.apache.geode.test.dunit.SerializableCallable;
 import org.apache.geode.test.dunit.SerializableRunnable;
 import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
 import org.apache.geode.test.junit.categories.DistributedTest;
+import org.awaitility.Awaitility;
+import org.jgroups.protocols.pbcast.GMS;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.*;
+import java.util.concurrent.TimeUnit;
 
 import static org.apache.geode.distributed.ConfigurationProperties.*;
 import static org.apache.geode.test.dunit.Assert.*;
@@ -38,19 +45,10 @@ import static org.apache.geode.test.dunit.Assert.*;
 /**
  * Tests the functionality of the {@link DistributedMember} class.
  *
- * @since GemFire 5.0
  */
 @Category(DistributedTest.class)
 public class DistributedMemberDUnitTest extends JUnit4DistributedTestCase {
 
-  protected void sleep(long millis) { // TODO: replace with Awaitility
-    try {
-      Thread.sleep(millis);
-    } catch (InterruptedException e) {
-      fail("interrupted");
-    }
-  }
-
   /**
    * Tests default settings.
    */
@@ -209,20 +207,9 @@ public class DistributedMemberDUnitTest extends JUnit4DistributedTestCase
{
           Role myRole = (Role) myRoles.iterator().next();
           assertTrue(vmRoles[vm].equals(myRole.getName()));
 
-          Set members = null;
-          for (int i = 1; i <= 3; i++) {
-            try {
-              members = dm.getOtherNormalDistributionManagerIds();
-              assertEquals(3, members.size());
-              break;
-            } catch (AssertionError e) { // TODO: delete this
-              if (i < 3) {
-                sleep(200);
-              } else {
-                throw e;
-              }
-            }
-          }
+          Awaitility.await().atMost(10, TimeUnit.SECONDS)
+              .until(() -> dm.getOtherNormalDistributionManagerIds().size() == 3);
+          Set<DistributedMember> members = dm.getOtherNormalDistributionManagerIds();
 
           for (Iterator iterMembers = members.iterator(); iterMembers.hasNext();) {
             InternalDistributedMember member = (InternalDistributedMember) iterMembers.next();
@@ -246,6 +233,62 @@ public class DistributedMemberDUnitTest extends JUnit4DistributedTestCase
{
     }
   }
 
+
+  private InternalDistributedMember connectAndSetUpPartialID() throws Exception {
+    Properties properties = new Properties();
+    properties.put("name", "myName");
+    InternalDistributedSystem system = getSystem(properties);
+    assertTrue(system == basicGetSystem()); // senders will use basicGetSystem()
+    InternalDistributedMember internalDistributedMember = system.getDistributedMember();
+
+    GMSMember gmsMember = new GMSMember((GMSMember) internalDistributedMember.getNetMember());
+    assertTrue(gmsMember.equals(internalDistributedMember.getNetMember()));
+    gmsMember.setName(null);
+    InternalDistributedMember partialID = new InternalDistributedMember(gmsMember);
+    return partialID;
+  }
+
+  /**
+   * GEODE-3043 surprise member added when member is already in the cluster
+   *
+   * This test ensures that a partial ID (with no "name") is equal to its equivalent non-partial
ID.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testMemberNameIgnoredInPartialID() throws Exception {
+    InternalDistributedMember partialID = connectAndSetUpPartialID();
+
+    InternalDistributedMember internalDistributedMember = basicGetSystem().getDistributedMember();
+    assertTrue(partialID.isPartial());
+    assertFalse(internalDistributedMember.isPartial());
+
+    assertEquals(internalDistributedMember, partialID);
+    assertEquals(partialID, internalDistributedMember);
+  }
+
+  /**
+   * GEODE-3043 surprise member added when member is already in the cluster
+   *
+   * This test ensures that the membership manager can detect and replace a partial ID with
one that
+   * is not partial
+   * 
+   * @throws Exception
+   */
+  @Test
+  public void testPartialIDInMessageReplacedWithFullID() throws Exception {
+    InternalDistributedMember partialID = connectAndSetUpPartialID();
+
+    HighPriorityAckedMessage message = new HighPriorityAckedMessage();
+    message.setSender(partialID);
+
+    GMSMembershipManager manager =
+        (GMSMembershipManager) MembershipManagerHelper.getMembershipManager(basicGetSystem());
+    manager.replacePartialIdentifierInMessage(message);
+
+    assertFalse(message.getSender().isPartial());
+  }
+
   private static String makeOddEvenString(int vm) {
     return ((vm % 2) == 0) ? "EVENS" : "ODDS";
   }
@@ -291,20 +334,10 @@ public class DistributedMemberDUnitTest extends JUnit4DistributedTestCase
{
 
           assertEquals(Arrays.asList("" + vm, makeOddEvenString(vm)), myGroups);
 
-          Set<DistributedMember> members = null;
-          for (int i = 1; i <= 3; i++) {
-            try {
-              members = dm.getOtherNormalDistributionManagerIds();
-              assertEquals(3, members.size());
-              break;
-            } catch (AssertionError e) { // TODO: delete this
-              if (i < 3) {
-                sleep(200);
-              } else {
-                throw e;
-              }
-            }
-          }
+          Awaitility.await().atMost(10, TimeUnit.SECONDS)
+              .until(() -> dm.getOtherNormalDistributionManagerIds().size() == 3);
+          Set<DistributedMember> members = dm.getOtherNormalDistributionManagerIds();
+
           // Make sure getAllOtherMembers returns a set
           // containing our three peers plus an admin member.
           Set<DistributedMember> others = sys.getAllOtherMembers();


Mime
View raw message