Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 2F486200CB0 for ; Fri, 9 Jun 2017 02:10:48 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 2DD9C160BE5; Fri, 9 Jun 2017 00:10:48 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 2396D160BD5 for ; Fri, 9 Jun 2017 02:10:46 +0200 (CEST) Received: (qmail 91690 invoked by uid 500); 9 Jun 2017 00:10:46 -0000 Mailing-List: contact commits-help@geode.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@geode.apache.org Delivered-To: mailing list commits@geode.apache.org Received: (qmail 91671 invoked by uid 99); 9 Jun 2017 00:10:46 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 09 Jun 2017 00:10:46 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0E5EADF9FD; Fri, 9 Jun 2017 00:10:43 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: bschuchardt@apache.org To: commits@geode.apache.org Date: Fri, 09 Jun 2017 00:10:44 -0000 Message-Id: <498a4cdc8b6d417a8b6a1c31874531ef@git.apache.org> In-Reply-To: <2584ad08e2ee4b21883856f40e7c6bf5@git.apache.org> References: <2584ad08e2ee4b21883856f40e7c6bf5@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [2/3] geode git commit: GEODE-3043 surprise member added when the member is already in the cluster archived-at: Fri, 09 Jun 2017 00:10:48 -0000 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 Authored: Thu Jun 8 16:59:48 2017 -0700 Committer: Bruce Schuchardt 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 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 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 members = dm.getOtherNormalDistributionManagerIds(); + // Make sure getAllOtherMembers returns a set // containing our three peers plus an admin member. Set others = sys.getAllOtherMembers();