GEODE-3052 Restarting 2 locators together causes potential locator split brain
There were four problems that new unit tests hit: 1. when recovering a view from disk we were treating it as a definitive (live) view. I've moved it to a new variable in GMSLocator and set its viewId to -1. At the same time I set the initial GMSJoinLeave SearchState.viewId to -100 so it will be overridden by the one returned by the locator. These changes allow GmsJoinLeave to know that the potential coordinator is from a recovered view. 2. when trying to join with a recovered view GMSJoinLeave.join() was giving up after the second ID in the view and becoming the coordinator. It needs to keep trying until the list is exhausted, and it shouldn't sleep between attempts. 3. GMSLocator wasn't returning registrants for use in findCoordinatorFromView(). This was causing it to choose itself as the coordinator instead of using registrant sort order and choosing a different registrant as the coordinator. 4. During concurrent startup GMSLocator didn't know when the decision was made to become coordinator. It is now notified of this decision and processRequest() uses this flag to have it override anything in the registrants set or in the recovered view. Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/3ee585c5 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/3ee585c5 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/3ee585c5 Branch: refs/heads/feature/GEODE-2804v3 Commit: 3ee585c5844607e5da9756afb089405a30ae3dd3 Parents: 6f60785 Author: Bruce Schuchardt <bschucha...@pivotal.io> Authored: Thu Jun 22 13:50:26 2017 -0700 Committer: Bruce Schuchardt <bschucha...@pivotal.io> Committed: Thu Jun 22 13:54:55 2017 -0700 ---------------------------------------------------------------------- .../internal/membership/NetView.java | 4 + .../internal/membership/gms/GMSUtil.java | 15 +- .../membership/gms/interfaces/Locator.java | 5 + .../gms/locator/FindCoordinatorResponse.java | 2 +- .../membership/gms/locator/GMSLocator.java | 52 +++- .../membership/gms/membership/GMSJoinLeave.java | 92 ++++-- .../membership/gms/messenger/GMSEncrypt.java | 14 +- .../gms/messenger/JGroupsMessenger.java | 2 +- .../geode/distributed/LocatorDUnitTest.java | 298 +++++++++++++------ .../LocatorUDPSecurityDUnitTest.java | 8 + .../gms/membership/GMSJoinLeaveTestHelper.java | 16 +- .../gms/messenger/GMSEncryptJUnitTest.java | 1 - 12 files changed, 360 insertions(+), 149 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java index 26b0327..c935074 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java @@ -165,6 +165,10 @@ public class NetView implements DataSerializableFixedID { } } + public void setViewId(int viewId) { + this.viewId = viewId; + } + public int[] getFailureDetectionPorts() { http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java index c6bef57..da295ab 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSUtil.java @@ -28,6 +28,13 @@ import org.apache.geode.internal.net.SocketCreator; public class GMSUtil { + /** + * parse locators & check that the resulting address is compatible with the given address + * + * @param locatorsString a DistributionConfig "locators" string + * @param bindAddress optional address to check for loopback compatibility + * @return addresses of locators + */ public static List<InetSocketAddress> parseLocators(String locatorsString, String bindAddress) { InetAddress addr = null; @@ -43,7 +50,13 @@ public class GMSUtil { return parseLocators(locatorsString, addr); } - + /** + * parse locators & check that the resulting address is compatible with the given address + * + * @param locatorsString a DistributionConfig "locators" string + * @param bindAddress optional address to check for loopback compatibility + * @return addresses of locators + */ public static List<InetSocketAddress> parseLocators(String locatorsString, InetAddress bindAddress) { List<InetSocketAddress> result = new ArrayList<>(2); http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java index c5fdf45..a365143 100755 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/interfaces/Locator.java @@ -26,4 +26,9 @@ public interface Locator { */ void installView(NetView v); + /** + * informs the locator that this process is becoming the membership coordinator + */ + void setIsCoordinator(boolean isCoordinator); + } http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/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 edfaf62..550d555 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 @@ -129,7 +129,7 @@ public class FindCoordinatorResponse extends HighPriorityDistributionMessage } else { return "FindCoordinatorResponse(coordinator=" + coordinator + ", fromView=" + fromView + ", viewId=" + (view == null ? "null" : view.getViewId()) + ", registrants=" - + (registrants == null ? 0 : registrants.size()) + ", senderId=" + senderId + + (registrants == null ? "none" : registrants) + ", senderId=" + senderId + ", network partition detection enabled=" + this.networkPartitionDetectionEnabled + ", locators preferred as coordinators=" + this.usePreferredCoordinators + ")"; } http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/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 93fa9da..aff1938 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 @@ -83,8 +83,12 @@ public class GMSLocator implements Locator, NetLocator { */ private transient NetView view; + private transient NetView recoveredView; + private File viewFile; + private volatile boolean isCoordinator; + /** * @param bindAddress network address that TcpServer will bind to * @param stateFile the file to persist state to/recover from @@ -122,6 +126,10 @@ public class GMSLocator implements Locator, NetLocator { NetView newView = services.getJoinLeave().getView(); if (newView != null) { this.view = newView; + } else if (localAddress != null) { + synchronized (this.registrants) { + this.registrants.add(localAddress); + } } this.notifyAll(); return true; @@ -158,9 +166,14 @@ public class GMSLocator implements Locator, NetLocator { } logger.info("Peer locator received new membership view: " + view); this.view = view; + this.recoveredView = null; saveView(view); } + @Override + public void setIsCoordinator(boolean isCoordinator) { + this.isCoordinator = isCoordinator; + } @Override public Object processRequest(Object request) throws IOException { @@ -218,6 +231,13 @@ public class GMSLocator implements Locator, NetLocator { boolean fromView = false; NetView v = this.view; + if (v == null) { + v = this.recoveredView; + } + + synchronized (registrants) { + registrants.add(findRequest.getMemberID()); + } if (v != null) { // if the ID of the requester matches an entry in the membership view then remove @@ -231,8 +251,8 @@ public class GMSLocator implements Locator, NetLocator { break; } } - int viewId = v.getViewId(); - if (viewId > findRequest.getLastViewId()) { + + if (v.getViewId() > findRequest.getLastViewId()) { // ignore the requests rejectedCoordinators if the view has changed coord = v.getCoordinator(Collections.emptyList()); } else { @@ -249,7 +269,6 @@ public class GMSLocator implements Locator, NetLocator { rejections = Collections.emptyList(); } synchronized (registrants) { - registrants.add(findRequest.getMemberID()); coord = services.getJoinLeave().getMemberID(); for (InternalDistributedMember mbr : registrants) { if (mbr != coord && (coord == null || mbr.compareTo(coord) < 0)) { @@ -264,14 +283,26 @@ public class GMSLocator implements Locator, NetLocator { } synchronized (registrants) { + if (isCoordinator) { + coord = localAddress; + InternalDistributedMember viewCoordinator = null; + if (v != null) { + viewCoordinator = v.getCoordinator(); + } + fromView = viewCoordinator != null && !viewCoordinator.equals(localAddress); + if (!fromView) { + logger.info("This member is becoming coordinator"); + v = null; + } + } byte[] coordPk = null; - if (view != null) { - coordPk = (byte[]) view.getPublicKey(coord); + if (v != null) { + coordPk = (byte[]) v.getPublicKey(coord); } if (coordPk == null) { coordPk = services.getMessenger().getPublicKey(coord); } - response = new FindCoordinatorResponse(coord, localAddress, fromView, view, + response = new FindCoordinatorResponse(coord, localAddress, fromView, v, new HashSet<InternalDistributedMember>(registrants), this.networkPartitionDetectionEnabled, this.usePreferredCoordinators, coordPk); } @@ -405,17 +436,18 @@ public class GMSLocator implements Locator, NetLocator { } Object o = DataSerializer.readObject(ois2); - this.view = (NetView) o; - List<InternalDistributedMember> members = new ArrayList<>(view.getMembers()); + recoveredView = (NetView) o; + recoveredView.setViewId(-1); // this is not a valid view so it shouldn't have a usable Id + List<InternalDistributedMember> members = new ArrayList<>(recoveredView.getMembers()); // GEODE-3052 - remove locators from the view. Since we couldn't recover from an existing // locator we know that all of the locators in the view are defunct for (InternalDistributedMember member : members) { if (member.getVmKind() == DistributionManager.LOCATOR_DM_TYPE) { - view.remove(member); + recoveredView.remove(member); } } - logger.info("Peer locator initial membership is " + view); + logger.info("Peer locator recovered membership is " + recoveredView); return true; } catch (Exception e) { http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java index 8abcc45..bc3d708 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java @@ -147,7 +147,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { /** * guarded by viewInstallationLock */ - private boolean isCoordinator; + private volatile boolean isCoordinator; /** * a synch object that guards view installation @@ -261,7 +261,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { Set<InternalDistributedMember> alreadyTried = new HashSet<>(); Set<InternalDistributedMember> registrants = new HashSet<>(); InternalDistributedMember possibleCoordinator; - int viewId = -1; + int viewId = -100; int locatorsContacted = 0; boolean hasContactedAJoinedLocator; NetView view; @@ -275,6 +275,17 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { responses.clear(); } } + + public String toString() { + StringBuffer sb = new StringBuffer(200); + sb.append("SearchState(locatorsContacted=").append(locatorsContacted) + .append("; alreadyTried=").append(alreadyTried).append("; registrants=") + .append(registrants).append("; possibleCoordinator=").append(possibleCoordinator) + .append("; viewId=").append(viewId).append("; hasContactedAJoinedLocator=") + .append(hasContactedAJoinedLocator).append("; view=").append(view).append("; responses=") + .append(responses).append(")"); + return sb.toString(); + } } Object getViewInstallationLock() { @@ -313,6 +324,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { for (int tries = 0; !this.isJoined && !this.isStopping; tries++) { logger.debug("searching for the membership coordinator"); boolean found = findCoordinator(); + logger.debug("state after looking for membership coordinator is {}", state); if (found) { logger.debug("found possible coordinator {}", state.possibleCoordinator); if (localAddress.getNetMember().preferredForCoordinator() @@ -341,7 +353,6 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { // break out of the loop and return false break; } - // reset the tries count and timer since we haven't actually tried to join yet tries = 0; giveupTime = now + timeout; } else if (now > giveupTime) { @@ -349,9 +360,19 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { } } try { - logger.debug("sleeping for {} before making another attempt to find the coordinator", - retrySleep); - Thread.sleep(retrySleep); + if (found && !state.hasContactedAJoinedLocator) { + // if locators are restarting they may be handing out IDs from a stale view that + // we should go through quickly. Otherwise we should sleep a bit to let failure + // detection select a new coordinator + if (state.possibleCoordinator.getVmViewId() < 0) { + logger.debug("sleeping for {} before making another attempt to find the coordinator", + retrySleep); + Thread.sleep(retrySleep); + } + // since we were given a coordinator that couldn't be used we should keep trying + tries = 0; + giveupTime = System.currentTimeMillis() + timeout; + } } catch (InterruptedException e) { logger.debug("retry sleep interrupted - giving up on joining the distributed system"); return false; @@ -681,19 +702,26 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { } private void recordViewRequest(DistributionMessage request) { - logger.debug("Recording the request to be processed in the next membership view"); - synchronized (viewRequests) { - viewRequests.add(request); - if (viewCreator != null && services.getMessenger().getClusterSecretKey() != null) { + try { + synchronized (viewRequests) { if (request instanceof JoinRequestMessage) { - JoinRequestMessage jreq = (JoinRequestMessage) request; - // this will inform about cluster-secret key, as we have authenticated at this point - JoinResponseMessage response = new JoinResponseMessage(jreq.getSender(), - services.getMessenger().getClusterSecretKey(), jreq.getRequestId()); - services.getMessenger().send(response); + if (isCoordinator + && !services.getConfig().getDistributionConfig().getSecurityUDPDHAlgo().isEmpty()) { + services.getMessenger().initClusterKey(); + JoinRequestMessage jreq = (JoinRequestMessage) request; + // this will inform about cluster-secret key, as we have authenticated at this point + JoinResponseMessage response = new JoinResponseMessage(jreq.getSender(), + services.getMessenger().getClusterSecretKey(), jreq.getRequestId()); + services.getMessenger().send(response); + } } + logger.debug("Recording the request to be processed in the next membership view"); + viewRequests.add(request); + viewRequests.notifyAll(); } - viewRequests.notifyAll(); + } catch (RuntimeException | Error t) { + logger.warn("unable to record a membership view request due to this exception", t); + throw t; } } @@ -744,6 +772,11 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { logger.info("This member is becoming the membership coordinator with address {}", localAddress); isCoordinator = true; + org.apache.geode.distributed.internal.membership.gms.interfaces.Locator locator = + services.getLocator(); + if (locator != null) { + locator.setIsCoordinator(true); + } if (currentView == null) { // create the initial membership view NetView newView = new NetView(this.localAddress); @@ -764,12 +797,12 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { private void createAndStartViewCreator(NetView newView) { if (viewCreator == null || viewCreator.isShutdown()) { + services.getMessenger().initClusterKey(); viewCreator = new ViewCreator("Geode Membership View Creator", Services.getThreadGroup()); if (newView != null) { viewCreator.setInitialView(newView, newView.getNewMembers(), newView.getShutdownMembers(), newView.getCrashedMembers()); } - services.getMessenger().initClusterKey(); viewCreator.setDaemon(true); logger.info("ViewCreator starting on:" + localAddress); viewCreator.start(); @@ -1041,7 +1074,8 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { // haven't joined the system (e.g., a collocated locator) // then jump to using the membership view to try to find // the coordinator - if (!state.hasContactedAJoinedLocator && state.view != null) { + if (!state.hasContactedAJoinedLocator && state.registrants.size() >= locators.size() + && state.view != null) { return findCoordinatorFromView(); } @@ -1075,6 +1109,9 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { } setCoordinatorPublicKey(response); state.locatorsContacted++; + if (response.getRegistrants() != null) { + state.registrants.addAll(response.getRegistrants()); + } if (!state.hasContactedAJoinedLocator && response.getSenderId() != null && response.getSenderId().getVmViewId() >= 0) { logger.debug("Locator's address indicates it is part of a distributed system " @@ -1089,9 +1126,6 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { state.viewId = viewId; state.view = v; state.registrants.clear(); - if (response.getRegistrants() != null) { - state.registrants.addAll(response.getRegistrants()); - } } if (viewId > -1) { coordinatorsWithView.add(response.getCoordinator()); @@ -1165,6 +1199,8 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { NetView v = state.view; List<InternalDistributedMember> recipients = new ArrayList<>(v.getMembers()); + logger.debug("searching for coordinator in findCoordinatorFromView"); + if (recipients.size() > MAX_DISCOVERY_NODES && MAX_DISCOVERY_NODES > 0) { recipients = recipients.subList(0, MAX_DISCOVERY_NODES); } @@ -1173,11 +1209,6 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { } recipients.remove(localAddress); - // FindCoordinatorRequest req = new FindCoordinatorRequest(localAddress, state.alreadyTried, - // state.viewId, services.getMessenger().getPublickey( - // localAddress), services.getMessenger().getRequestId()); - // req.setRecipients(v.getMembers()); - boolean testing = unitTesting.contains("findCoordinatorFromView"); synchronized (state.responses) { if (!testing) { @@ -1189,7 +1220,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { // Here we are sending message one-by-one to all recipients as we don't have cluster secret // key yet. // Usually this happens when locator re-joins the cluster and it has saved view. - for (InternalDistributedMember mbr : v.getMembers()) { + for (InternalDistributedMember mbr : recipients) { Set<InternalDistributedMember> r = new HashSet<>(); r.add(mbr); FindCoordinatorRequest req = new FindCoordinatorRequest(localAddress, state.alreadyTried, @@ -1203,7 +1234,7 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { FindCoordinatorRequest req = new FindCoordinatorRequest(localAddress, state.alreadyTried, state.viewId, services.getMessenger().getPublicKey(localAddress), services.getMessenger().getRequestId(), dhalgo); - req.setRecipients(v.getMembers()); + req.setRecipients(recipients); services.getMessenger().send(req, v); } @@ -1505,6 +1536,11 @@ public class GMSJoinLeave implements JoinLeave, MessageHandler { if (viewCreator != null && !viewCreator.isShutdown()) { logger.debug("Shutting down ViewCreator"); viewCreator.shutdown(); + org.apache.geode.distributed.internal.membership.gms.interfaces.Locator locator = + services.getLocator(); + if (locator != null) { + locator.setIsCoordinator(false); + } try { viewCreator.join(1000); } catch (InterruptedException e) { http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java index c7b1a26..f0461f2 100755 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncrypt.java @@ -30,6 +30,7 @@ import javax.crypto.spec.DHParameterSpec; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; +import org.apache.geode.GemFireConfigException; import org.apache.geode.distributed.internal.membership.InternalDistributedMember; import org.apache.geode.distributed.internal.membership.InternalDistributedMember.InternalDistributedMemberWrapper; import org.apache.geode.distributed.internal.membership.NetView; @@ -106,7 +107,6 @@ public class GMSEncrypt implements Cloneable { protected synchronized void initClusterSecretKey() throws Exception { if (this.clusterEncryptor == null) { this.clusterEncryptor = new ClusterEncryptor(this); - } } @@ -579,16 +579,12 @@ public class GMSEncrypt implements Cloneable { } private Cipher getEncryptCipher(String dhSKAlgo) throws Exception { - try { - if (encrypt == null) { - synchronized (this) { - if (encrypt == null) { - encrypt = GMSEncrypt.getEncryptCipher(dhSKAlgo, secretBytes); - } + if (encrypt == null) { + synchronized (this) { + if (encrypt == null) { + encrypt = GMSEncrypt.getEncryptCipher(dhSKAlgo, secretBytes); } } - } catch (Exception ex) { - throw ex; } return encrypt; } http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java index 390824e..122e3ac 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java @@ -1413,7 +1413,7 @@ public class JGroupsMessenger implements Messenger { public void initClusterKey() { if (encrypt != null) { try { - logger.debug("Initializing cluster key"); + logger.info("Initializing cluster key"); encrypt.initClusterSecretKey(); } catch (Exception e) { throw new RuntimeException("unable to create cluster key ", e); http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/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 7ecca61..cbe5938 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 @@ -21,6 +21,7 @@ import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS; import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL; import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT; import static org.apache.geode.distributed.ConfigurationProperties.MEMBER_TIMEOUT; +import static org.apache.geode.distributed.ConfigurationProperties.NAME; import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_PEER_AUTHENTICATOR; import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_PEER_AUTH_INIT; import static org.apache.geode.distributed.ConfigurationProperties.SSL_CIPHERS; @@ -81,6 +82,7 @@ import org.apache.geode.test.dunit.SerializableRunnable; import org.apache.geode.test.dunit.VM; import org.apache.geode.test.dunit.Wait; import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase; +import org.apache.geode.test.dunit.standalone.DUnitLauncher; import org.apache.geode.test.junit.categories.DistributedTest; import org.apache.geode.test.junit.categories.MembershipTest; import org.apache.geode.util.test.TestUtil; @@ -412,18 +414,18 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase { getBlackboard().initBlackboard(); AsyncInvocation<Boolean> async1 = loc1.invokeAsync("startLocator1", () -> { getBlackboard().signalGate("locator1"); - getBlackboard().waitForGate("go", 10, TimeUnit.SECONDS); + getBlackboard().waitForGate("go", 60, TimeUnit.SECONDS); return startLocatorWithPortAndProperties(port1, properties); }); AsyncInvocation<Boolean> async2 = loc2.invokeAsync("startLocator2", () -> { getBlackboard().signalGate("locator2"); - getBlackboard().waitForGate("go", 10, TimeUnit.SECONDS); + getBlackboard().waitForGate("go", 60, TimeUnit.SECONDS); return startLocatorWithPortAndProperties(port2, properties); }); - getBlackboard().waitForGate("locator1", 10, TimeUnit.SECONDS); - getBlackboard().waitForGate("locator2", 10, TimeUnit.SECONDS); + getBlackboard().waitForGate("locator1", 60, TimeUnit.SECONDS); + getBlackboard().waitForGate("locator2", 60, TimeUnit.SECONDS); getBlackboard().signalGate("go"); async1.await(); @@ -1644,120 +1646,225 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase { vm2.invoke(() -> startLocatorAsync(new Object[] {port3, dsProps})); try { - try { - vm3.invoke(() -> { - DistributedSystem.connect(dsProps); - return true; - }); - vm4.invoke(() -> { - DistributedSystem.connect(dsProps); - return true; - }); + vm3.invoke(() -> { + DistributedSystem.connect(dsProps); + return true; + }); + vm4.invoke(() -> { + DistributedSystem.connect(dsProps); + return true; + }); - system = (InternalDistributedSystem) DistributedSystem.connect(dsProps); + system = (InternalDistributedSystem) DistributedSystem.connect(dsProps); - Awaitility.waitAtMost(10000, TimeUnit.MILLISECONDS).pollInterval(200, TimeUnit.MILLISECONDS) - .until(() -> { - try { - return system.getDM().getViewMembers().size() == 6; - } catch (Exception e) { - e.printStackTrace(); - org.apache.geode.test.dunit.Assert.fail("unexpected exception", e); - } - return false; // NOTREACHED - }); + Awaitility.waitAtMost(10000, TimeUnit.MILLISECONDS).pollInterval(200, TimeUnit.MILLISECONDS) + .until(() -> { + try { + return system.getDM().getViewMembers().size() == 6; + } catch (Exception e) { + e.printStackTrace(); + org.apache.geode.test.dunit.Assert.fail("unexpected exception", e); + } + return false; // NOTREACHED + }); - // three applications plus - assertEquals(6, system.getDM().getViewMembers().size()); + // three applications plus + assertEquals(6, system.getDM().getViewMembers().size()); - vm0.invoke(() -> stopLocator()); - vm1.invoke(() -> stopLocator()); - vm2.invoke(() -> stopLocator()); + vm0.invoke(() -> stopLocator()); + vm1.invoke(() -> stopLocator()); + vm2.invoke(() -> stopLocator()); - Awaitility.waitAtMost(10000, TimeUnit.MILLISECONDS).pollInterval(200, TimeUnit.MILLISECONDS) - .until(() -> { - try { - return system.getDM().getMembershipManager().getView().size() <= 3; - } catch (Exception e) { - e.printStackTrace(); - org.apache.geode.test.dunit.Assert.fail("unexpected exception", e); + Awaitility.waitAtMost(10000, TimeUnit.MILLISECONDS).pollInterval(200, TimeUnit.MILLISECONDS) + .until(() -> { + try { + return system.getDM().getMembershipManager().getView().size() <= 3; + } catch (Exception e) { + e.printStackTrace(); + org.apache.geode.test.dunit.Assert.fail("unexpected exception", e); + } + return false; // NOTREACHED + }); + + final String newLocators = host0 + "[" + port2 + "]," + host0 + "[" + port3 + "]"; + dsProps.setProperty(LOCATORS, newLocators); + + final InternalDistributedMember currentCoordinator = + GMSJoinLeaveTestHelper.getCurrentCoordinator(); + DistributedMember vm3ID = vm3.invoke(() -> GMSJoinLeaveTestHelper + .getInternalDistributedSystem().getDM().getDistributionManagerId()); + assertTrue("View is " + system.getDM().getMembershipManager().getView() + " and vm3's ID is " + + vm3ID, vm3.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator())); + + vm1.invoke(() -> startLocatorAsync(new Object[] {port2, dsProps})); + vm2.invoke(() -> startLocatorAsync(new Object[] {port3, dsProps})); + + Awaitility.waitAtMost(30000, TimeUnit.MILLISECONDS).pollInterval(200, TimeUnit.MILLISECONDS) + .until(() -> { + try { + InternalDistributedMember c = GMSJoinLeaveTestHelper.getCurrentCoordinator(); + if (c.equals(currentCoordinator)) { + // now locator should be new coordinator + return false; } - return false; // NOTREACHED - }); + return system.getDM().getAllHostedLocators().size() == 2; + } catch (Exception e) { + e.printStackTrace(); + org.apache.geode.test.dunit.Assert.fail("unexpected exception", e); + } + return false; // NOTREACHED + }); - final String newLocators = host0 + "[" + port2 + "]," + host0 + "[" + port3 + "]"; - dsProps.setProperty(LOCATORS, newLocators); + vm1.invoke("waitUntilLocatorBecomesCoordinator", () -> waitUntilLocatorBecomesCoordinator()); + vm2.invoke("waitUntilLocatorBecomesCoordinator", () -> waitUntilLocatorBecomesCoordinator()); + vm3.invoke("waitUntilLocatorBecomesCoordinator", () -> waitUntilLocatorBecomesCoordinator()); + vm4.invoke("waitUntilLocatorBecomesCoordinator", () -> waitUntilLocatorBecomesCoordinator()); + + int netviewId = vm1.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.getViewId()); + assertEquals(netviewId, + (int) vm2.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId())); + assertEquals(netviewId, + (int) vm3.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId())); + assertEquals(netviewId, + (int) vm4.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId())); + assertFalse(vm4.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); + // Given the start up order of servers, this server is the elder server + assertFalse(vm3.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); + if (vm1.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator())) { + assertFalse( + vm2.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); + } else { + assertTrue( + vm2.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); + } - final InternalDistributedMember currentCoordinator = - GMSJoinLeaveTestHelper.getCurrentCoordinator(); - DistributedMember vm3ID = vm3.invoke(() -> GMSJoinLeaveTestHelper - .getInternalDistributedSystem().getDM().getDistributionManagerId()); - assertTrue("View is " + system.getDM().getMembershipManager().getView() - + " and vm3's ID is " + vm3ID, - vm3.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator())); + } finally { + system.disconnect(); + vm3.invoke(() -> disconnectDistributedSystem()); + vm4.invoke(() -> disconnectDistributedSystem()); + vm2.invoke(() -> stopLocator()); + vm1.invoke(() -> stopLocator()); + } + } - vm1.invoke(() -> startLocatorAsync(new Object[] {port2, dsProps})); - vm2.invoke(() -> startLocatorAsync(new Object[] {port3, dsProps})); + @Test + public void testMultipleLocatorsRestartingAtSameTimeWithMissingServers() throws Exception { + disconnectAllFromDS(); + IgnoredException.addIgnoredException("ForcedDisconnectException"); + IgnoredException.addIgnoredException("Possible loss of quorum"); + Host host = Host.getHost(0); + VM vm0 = host.getVM(0); + VM vm1 = host.getVM(1); + VM vm2 = host.getVM(2); + VM vm3 = host.getVM(3); + VM vm4 = host.getVM(4); - Awaitility.waitAtMost(30000, TimeUnit.MILLISECONDS).pollInterval(200, TimeUnit.MILLISECONDS) + int[] freeTCPPorts = AvailablePortHelper.getRandomAvailableTCPPorts(3); + this.port1 = freeTCPPorts[0]; + this.port2 = freeTCPPorts[1]; + int port3 = freeTCPPorts[2]; + DistributedTestUtils.deleteLocatorStateFile(port1, port2, port3); + final String host0 = NetworkUtils.getServerHostName(host); + final String locators = + host0 + "[" + port1 + "]," + host0 + "[" + port2 + "]," + host0 + "[" + port3 + "]"; + + final Properties dsProps = new Properties(); + dsProps.setProperty(LOCATORS, locators); + dsProps.setProperty(LOG_LEVEL, LogWriterUtils.getDUnitLogLevel()); + dsProps.setProperty(DISABLE_AUTO_RECONNECT, "true"); + dsProps.setProperty(MCAST_PORT, "0"); + + addDSProps(dsProps); + vm0.invoke(() -> startLocatorAsync(new Object[] {port1, dsProps})); + vm1.invoke(() -> startLocatorAsync(new Object[] {port2, dsProps})); + vm2.invoke(() -> startLocatorAsync(new Object[] {port3, dsProps})); + + try { + vm3.invoke(() -> { + DistributedSystem.connect(dsProps); + return true; + }); + vm4.invoke(() -> { + DistributedSystem.connect(dsProps); + + Awaitility.waitAtMost(10000, TimeUnit.MILLISECONDS).pollInterval(200, TimeUnit.MILLISECONDS) .until(() -> { try { - InternalDistributedMember c = GMSJoinLeaveTestHelper.getCurrentCoordinator(); - if (c.equals(currentCoordinator)) { - // now locator should be new coordinator - return false; - } - return system.getDM().getAllHostedLocators().size() == 2; + return InternalDistributedSystem.getConnectedInstance().getDM().getViewMembers() + .size() == 5; } catch (Exception e) { e.printStackTrace(); org.apache.geode.test.dunit.Assert.fail("unexpected exception", e); } return false; // NOTREACHED }); + return true; + }); - vm1.invoke("waitUntilLocatorBecomesCoordinator", - () -> waitUntilLocatorBecomesCoordinator()); - vm2.invoke("waitUntilLocatorBecomesCoordinator", - () -> waitUntilLocatorBecomesCoordinator()); - vm3.invoke("waitUntilLocatorBecomesCoordinator", - () -> waitUntilLocatorBecomesCoordinator()); - vm4.invoke("waitUntilLocatorBecomesCoordinator", - () -> waitUntilLocatorBecomesCoordinator()); - - int netviewId = - vm1.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.getViewId()); - assertEquals(netviewId, - (int) vm2.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId())); - assertEquals(netviewId, - (int) vm3.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId())); - assertEquals(netviewId, - (int) vm4.invoke("checking ViewID", () -> GMSJoinLeaveTestHelper.getViewId())); - assertFalse( - vm4.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); - // Given the start up order of servers, this server is the elder server - assertFalse( - vm3.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); - if (vm1.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator())) { - assertFalse( - vm2.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); - } else { - assertTrue( - vm2.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); + vm0.invoke(() -> forceDisconnect()); + vm1.invoke(() -> forceDisconnect()); + vm2.invoke(() -> forceDisconnect()); + + SerializableRunnable waitForDisconnect = new SerializableRunnable("waitForDisconnect") { + public void run() { + Awaitility.waitAtMost(10000, TimeUnit.MILLISECONDS) + .pollInterval(200, TimeUnit.MILLISECONDS).until(() -> { + try { + return InternalDistributedSystem.getConnectedInstance() == null; + } catch (Exception e) { + org.apache.geode.test.dunit.Assert.fail("unexpected exception", e); + } + return false; // NOTREACHED + }); } + }; + vm0.invoke(() -> waitForDisconnect); + vm1.invoke(() -> waitForDisconnect); + vm2.invoke(() -> waitForDisconnect); + disconnectAllFromDS(); - } finally { - system.disconnect(); - vm3.invoke(() -> disconnectDistributedSystem()); - vm4.invoke(() -> disconnectDistributedSystem()); - vm2.invoke(() -> stopLocator()); - vm1.invoke(() -> stopLocator()); + + final String newLocators = host0 + "[" + port2 + "]," + host0 + "[" + port3 + "]"; + dsProps.setProperty(LOCATORS, newLocators); + + getBlackboard().initBlackboard(); + AsyncInvocation async1 = vm1.invokeAsync(() -> { + getBlackboard().signalGate("vm1ready"); + getBlackboard().waitForGate("readyToConnect", 30, TimeUnit.SECONDS); + System.out.println("vm1 is ready to connect"); + startLocatorAsync(new Object[] {port2, dsProps}); + }); + AsyncInvocation async2 = vm2.invokeAsync(() -> { + getBlackboard().signalGate("vm2ready"); + getBlackboard().waitForGate("readyToConnect", 30, TimeUnit.SECONDS); + System.out.println("vm2 is ready to connect"); + startLocatorAsync(new Object[] {port3, dsProps}); + }); + getBlackboard().waitForGate("vm1ready", 30, TimeUnit.SECONDS); + getBlackboard().waitForGate("vm2ready", 30, TimeUnit.SECONDS); + getBlackboard().signalGate("readyToConnect"); + async1.join(); + async2.join(); + + vm1.invoke("waitUntilLocatorBecomesCoordinator", () -> waitUntilLocatorBecomesCoordinator()); + vm2.invoke("waitUntilLocatorBecomesCoordinator", () -> waitUntilLocatorBecomesCoordinator()); + + if (vm1.invoke(() -> GMSJoinLeaveTestHelper.isViewCreator())) { + assertFalse( + vm2.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); + } else { + assertTrue( + vm2.invoke("Checking ViewCreator", () -> GMSJoinLeaveTestHelper.isViewCreator())); } + } finally { + vm2.invoke(() -> stopLocator()); + vm1.invoke(() -> stopLocator()); } } private void waitUntilLocatorBecomesCoordinator() { - Awaitility.waitAtMost(15000, TimeUnit.MILLISECONDS).pollInterval(200, TimeUnit.MILLISECONDS) + Awaitility.waitAtMost(30000, TimeUnit.MILLISECONDS).pollInterval(1000, TimeUnit.MILLISECONDS) .until(() -> { try { InternalDistributedMember c = GMSJoinLeaveTestHelper.getCurrentCoordinator(); @@ -1781,8 +1888,10 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase { private void startLocatorAsync(Object[] args) { File logFile = new File(""); + Properties properties = (Properties) args[1]; + properties.put(NAME, "vm" + VM.getCurrentVMNum()); try { - Locator.startLocatorAndDS((int) args[0], logFile, (Properties) args[1]); + Locator.startLocatorAndDS((int) args[0], logFile, properties); } catch (IOException ex) { org.apache.geode.test.dunit.Assert.fail("While starting process on port " + args[0], ex); } @@ -1879,6 +1988,7 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase { p.setProperty(LOCATORS, Host.getHost(0).getHostName() + "[" + port1 + "]"); p.setProperty(MCAST_PORT, "0"); p.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false"); + p.setProperty(LOG_LEVEL, DUnitLauncher.logLevel); addDSProps(p); if (stateFile.exists()) { stateFile.delete(); @@ -1948,6 +2058,10 @@ public class LocatorDUnitTest extends JUnit4DistributedTestCase { } } + protected void forceDisconnect() { + DistributedTestUtils.crashDistributedSystem(InternalDistributedSystem.getConnectedInstance()); + } + private void startSBLocator(final int port) { File logFile = new File(""); try { http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java index df1d8d1..89d2507 100644 --- a/geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/LocatorUDPSecurityDUnitTest.java @@ -30,6 +30,7 @@ import org.apache.geode.test.junit.categories.DistributedTest; import org.apache.geode.test.junit.categories.FlakyTest; import org.apache.geode.test.junit.categories.MembershipTest; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -45,6 +46,13 @@ public class LocatorUDPSecurityDUnitTest extends LocatorDUnitTest { p.setProperty(SECURITY_UDP_DHALGO, "AES:128"); } + @Override + @Test + @Ignore // GEODE-3094 + public void testMultipleLocatorsRestartingAtSameTimeWithMissingServers() throws Exception { + super.testMultipleLocatorsRestartingAtSameTimeWithMissingServers(); + } + @Test public void testLocatorWithUDPSecurityButServer() throws Exception { disconnectAllFromDS(); http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java index 19cee06..8c953f7 100755 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveTestHelper.java @@ -34,12 +34,13 @@ public class GMSJoinLeaveTestHelper { public static boolean isViewCreator() { GMSJoinLeave gmsJoinLeave = getGmsJoinLeave(); if (gmsJoinLeave != null) { - GMSJoinLeave.ViewCreator viewCreator = gmsJoinLeave.getViewCreator(); - if (viewCreator != null && !viewCreator.isShutdown()) { - return true; - } else { - return false; - } + return gmsJoinLeave.getView().getCreator().equals(gmsJoinLeave.getMemberID()); + // GMSJoinLeave.ViewCreator viewCreator = gmsJoinLeave.getViewCreator(); + // if (viewCreator != null && !viewCreator.isShutdown()) { + // return true; + // } else { + // return false; + // } } throw new RuntimeException( "This should not have happened. There should be a JoinLeave for every DS"); @@ -88,6 +89,9 @@ public class GMSJoinLeaveTestHelper { InternalDistributedSystem distributedSystem = InternalDistributedSystem.getAnyInstance(); if (distributedSystem == null) { Locator locator = Locator.getLocator(); + if (locator == null) { + return null; + } return (InternalDistributedSystem) locator.getDistributedSystem(); } else { return distributedSystem; http://git-wip-us.apache.org/repos/asf/geode/blob/3ee585c5/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java index 7f64c67..78d508b 100755 --- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/messenger/GMSEncryptJUnitTest.java @@ -17,7 +17,6 @@ package org.apache.geode.distributed.internal.membership.gms.messenger; import static org.mockito.Mockito.*; import org.apache.geode.distributed.ConfigurationProperties; -import org.apache.geode.distributed.internal.DistributionConfig; import org.apache.geode.distributed.internal.DistributionConfigImpl; import org.apache.geode.distributed.internal.DistributionManager; import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
