You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2017/02/24 17:30:49 UTC
geode git commit: GEODE-2534 concurrently started locators fail to
create a unified system
Repository: geode
Updated Branches:
refs/heads/develop c35f442c2 -> 35f6d827d
GEODE-2534 concurrently started locators fail to create a unified system
GMS services were being installed in the locator before they were started,
causing the locator to not know its own member ID. This caused the
concurrent startup registry to not contain its member ID, allowing
FindCoordinatorResponses to be incorrect.
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/35f6d827
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/35f6d827
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/35f6d827
Branch: refs/heads/develop
Commit: 35f6d827d04ef2d898d780c921d841047bb7fa4a
Parents: c35f442
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Fri Feb 24 09:29:35 2017 -0800
Committer: Bruce Schuchardt <bs...@pivotal.io>
Committed: Fri Feb 24 09:30:39 2017 -0800
----------------------------------------------------------------------
.../org/apache/geode/distributed/Locator.java | 5 +++-
.../internal/membership/gms/Services.java | 12 ++++-----
.../gms/locator/FindCoordinatorResponse.java | 2 +-
.../membership/gms/locator/GMSLocator.java | 27 ++++++++++++--------
.../geode/distributed/LocatorDUnitTest.java | 26 ++++++++++++++++---
5 files changed, 50 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/35f6d827/geode-core/src/main/java/org/apache/geode/distributed/Locator.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/Locator.java b/geode-core/src/main/java/org/apache/geode/distributed/Locator.java
index 2c7c251..87cd243 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/Locator.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/Locator.java
@@ -391,7 +391,10 @@ public abstract class Locator {
}
}
StringBuilder locatorString = new StringBuilder(String.valueOf(ba));
- locatorString.append('[').append(this.getPort()).append(']');
+ Integer port = getPort();
+ if (port != null && port.intValue() > 0) {
+ locatorString.append('[').append(this.getPort()).append(']');
+ }
return locatorString.toString();
}
http://git-wip-us.apache.org/repos/asf/geode/blob/35f6d827/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
index 6d7c5a9..1404b3b 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/Services.java
@@ -135,12 +135,6 @@ public class Services {
this.manager.init(this);
this.joinLeave.init(this);
this.healthMon.init(this);
- InternalLocator l = (InternalLocator) org.apache.geode.distributed.Locator.getLocator();
- if (l != null && l.getLocatorHandler() != null) {
- if (l.getLocatorHandler().setMembershipManager((MembershipManager) this.manager)) {
- this.locator = (Locator) l.getLocatorHandler();
- }
- }
}
protected void start() {
@@ -176,6 +170,12 @@ public class Services {
this.joinLeave.started();
this.healthMon.started();
this.manager.started();
+ InternalLocator l = (InternalLocator) org.apache.geode.distributed.Locator.getLocator();
+ if (l != null && l.getLocatorHandler() != null) {
+ if (l.getLocatorHandler().setMembershipManager((MembershipManager) this.manager)) {
+ this.locator = (Locator) l.getLocatorHandler();
+ }
+ }
logger.debug("All membership services have been started");
try {
this.manager.joinDistributedSystem();
http://git-wip-us.apache.org/repos/asf/geode/blob/35f6d827/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
index 8d34007..edfaf62 100755
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/FindCoordinatorResponse.java
@@ -128,7 +128,7 @@ public class FindCoordinatorResponse extends HighPriorityDistributionMessage
return "FindCoordinatorResponse(coordinator=" + coordinator + ")";
} else {
return "FindCoordinatorResponse(coordinator=" + coordinator + ", fromView=" + fromView
- + ", viewId=" + (view == null ? "nul" : view.getViewId()) + ", registrants="
+ + ", viewId=" + (view == null ? "null" : view.getViewId()) + ", registrants="
+ (registrants == null ? 0 : registrants.size()) + ", senderId=" + senderId
+ ", network partition detection enabled=" + this.networkPartitionDetectionEnabled
+ ", locators preferred as coordinators=" + this.usePreferredCoordinators + ")";
http://git-wip-us.apache.org/repos/asf/geode/blob/35f6d827/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
index c05140d..e3635f2 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/locator/GMSLocator.java
@@ -112,9 +112,11 @@ public class GMSLocator implements Locator, NetLocator {
@Override
public synchronized boolean setMembershipManager(MembershipManager mgr) {
if (services == null || services.isStopped()) {
- logger.info("Peer locator is connecting to local membership services");
services = ((GMSMembershipManager) mgr).getServices();
localAddress = services.getMessenger().getMemberID();
+ assert localAddress != null : "member address should have been established";
+ logger.info("Peer locator is connecting to local membership services with ID {}",
+ localAddress);
services.setLocator(this);
NetView newView = services.getJoinLeave().getView();
if (newView != null) {
@@ -142,7 +144,7 @@ public class GMSLocator implements Locator, NetLocator {
}
if (services == null) {
try {
- wait(2000);
+ wait(10000);
} catch (InterruptedException e) {
}
}
@@ -177,12 +179,14 @@ public class GMSLocator implements Locator, NetLocator {
}
} else if (request instanceof FindCoordinatorRequest) {
findServices();
+
FindCoordinatorRequest findRequest = (FindCoordinatorRequest) request;
if (!findRequest.getDHAlgo().equals(securityUDPDHAlgo)) {
return new FindCoordinatorResponse(
"Rejecting findCoordinatorRequest, as member not configured same udp security("
+ findRequest.getDHAlgo() + " )as locator (" + securityUDPDHAlgo + ")");
}
+
if (services != null) {
services.getMessenger().setPublicKey(findRequest.getMyPublicKey(),
findRequest.getMemberID());
@@ -193,7 +197,11 @@ public class GMSLocator implements Locator, NetLocator {
registerMbrVsPK.put(new InternalDistributedMemberWrapper(findRequest.getMemberID()),
findRequest.getMyPublicKey());
}
+ logger.debug("Rejecting a request to find the coordinator - membership services are"
+ + " still initializing");
+ return null;
}
+
if (findRequest.getMemberID() != null) {
InternalDistributedMember coord = null;
@@ -201,6 +209,10 @@ public class GMSLocator implements Locator, NetLocator {
// which may be more up-to-date than the one known by the membership manager
if (view == null) {
findServices();
+ if (services == null) {
+ // we must know this process's identity in order to respond
+ return null;
+ }
}
boolean fromView = false;
@@ -237,9 +249,7 @@ public class GMSLocator implements Locator, NetLocator {
}
synchronized (registrants) {
registrants.add(findRequest.getMemberID());
- if (services != null) {
- coord = services.getJoinLeave().getMemberID();
- }
+ coord = services.getJoinLeave().getMemberID();
for (InternalDistributedMember mbr : registrants) {
if (mbr != coord && (coord == null || mbr.compareTo(coord) < 0)) {
if (!rejections.contains(mbr) && (mbr.getNetMember().preferredForCoordinator()
@@ -258,12 +268,7 @@ public class GMSLocator implements Locator, NetLocator {
coordPk = (byte[]) view.getPublicKey(coord);
}
if (coordPk == null) {
- if (services != null) {
- coordPk = services.getMessenger().getPublicKey(coord);
- } else {
- // coordPk = GMSEncrypt.getRegisteredPublicKey(coord);
- coordPk = registerMbrVsPK.get(new InternalDistributedMemberWrapper(coord));
- }
+ coordPk = services.getMessenger().getPublicKey(coord);
}
response = new FindCoordinatorResponse(coord, localAddress, fromView, view,
new HashSet<InternalDistributedMember>(registrants),
http://git-wip-us.apache.org/repos/asf/geode/blob/35f6d827/geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java
index da0daeb..47fef0b 100644
--- a/geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java
@@ -43,6 +43,8 @@ import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import org.apache.geode.distributed.internal.membership.gms.Services;
+import org.apache.geode.test.dunit.AsyncInvocation;
import org.awaitility.Awaitility;
import org.apache.geode.ForcedDisconnectException;
import org.apache.geode.GemFireConfigException;
@@ -408,10 +410,28 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase {
}
private void startVerifyAndStopLocator(VM loc1, VM loc2, int port1, int port2,
- Properties properties) {
+ Properties properties) throws Exception {
try {
- loc2.invoke("startLocator2", () -> startLocatorWithPortAndProperties(port2, properties));
- loc1.invoke("startLocator1", () -> startLocatorWithPortAndProperties(port1, properties));
+ getBlackboard().initBlackboard();
+ AsyncInvocation<Boolean> async1 = loc1.invokeAsync("startLocator1", () -> {
+ getBlackboard().signalGate("locator1");
+ getBlackboard().waitForGate("go", 10, TimeUnit.SECONDS);
+ return startLocatorWithPortAndProperties(port1, properties);
+ });
+
+ AsyncInvocation<Boolean> async2 = loc2.invokeAsync("startLocator2", () -> {
+ getBlackboard().signalGate("locator2");
+ getBlackboard().waitForGate("go", 10, TimeUnit.SECONDS);
+ return startLocatorWithPortAndProperties(port2, properties);
+ });
+
+ getBlackboard().waitForGate("locator1", 10, TimeUnit.SECONDS);
+ getBlackboard().waitForGate("locator2", 10, TimeUnit.SECONDS);
+ getBlackboard().signalGate("go");
+
+ async1.await();
+ async2.await();
+
} finally {
try {
// verify that they found each other