Return-Path: X-Original-To: apmail-geode-commits-archive@minotaur.apache.org Delivered-To: apmail-geode-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 19D7F18FB8 for ; Fri, 2 Oct 2015 23:14:09 +0000 (UTC) Received: (qmail 99307 invoked by uid 500); 2 Oct 2015 23:14:09 -0000 Delivered-To: apmail-geode-commits-archive@geode.apache.org Received: (qmail 99279 invoked by uid 500); 2 Oct 2015 23:14:08 -0000 Mailing-List: contact commits-help@geode.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@geode.incubator.apache.org Delivered-To: mailing list commits@geode.incubator.apache.org Received: (qmail 99270 invoked by uid 99); 2 Oct 2015 23:14:08 -0000 Received: from Unknown (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 02 Oct 2015 23:14:08 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 7DDEBC18DF for ; Fri, 2 Oct 2015 23:14:08 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.771 X-Spam-Level: * X-Spam-Status: No, score=1.771 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, T_RP_MATCHES_RCVD=-0.01, URIBL_BLOCKED=0.001] autolearn=disabled Received: from mx1-us-west.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id Lkze7s5CAS_u for ; Fri, 2 Oct 2015 23:14:01 +0000 (UTC) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx1-us-west.apache.org (ASF Mail Server at mx1-us-west.apache.org) with SMTP id 2BF2E203BD for ; Fri, 2 Oct 2015 23:14:01 +0000 (UTC) Received: (qmail 99212 invoked by uid 99); 2 Oct 2015 23:14:01 -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, 02 Oct 2015 23:14:01 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id F1AB2E00AA; Fri, 2 Oct 2015 23:14:00 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: bschuchardt@apache.org To: commits@geode.incubator.apache.org Message-Id: <2972a5ae88314745a07ba5703e453b91@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: incubator-geode git commit: GMSHealthMonitor was only accepting a health-check response to see if a member was alive or not. This change-set modifies it to allow any message from the suspect to prove that the suspect is alive. This reduced the amount o Date: Fri, 2 Oct 2015 23:14:00 +0000 (UTC) Repository: incubator-geode Updated Branches: refs/heads/feature/GEODE-77 b08159fbb -> b256262ef GMSHealthMonitor was only accepting a health-check response to see if a member was alive or not. This change-set modifies it to allow any message from the suspect to prove that the suspect is alive. This reduced the amount of suspect processing going on quite a bit, and made it less likely to incorrectly oust a busy member. GMSMembershipManager was not throwing an exception when distribution was attempted but the manager was shutting down. This caused large inconsistencies between clients and servers in client/server HA testing. TCPServer's message to determine the locator's version was using a 2-minute timeout, totally ignoring the timeout passed to it. It now pays attention to the requested timeout. A number of tests were using Java assertions instead of JUnit Assert. I changed all of these in our recently added tests to use the JUnit assertions in case the JUnit jvm doesn't have assertions enabled. Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/b256262e Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/b256262e Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/b256262e Branch: refs/heads/feature/GEODE-77 Commit: b256262ef2cb9800aa30cbcbf85639dbdc500f69 Parents: b08159f Author: Bruce Schuchardt Authored: Fri Oct 2 16:13:26 2015 -0700 Committer: Bruce Schuchardt Committed: Fri Oct 2 16:13:26 2015 -0700 ---------------------------------------------------------------------- build.gradle | 3 + .../membership/gms/fd/GMSHealthMonitor.java | 34 +++++----- .../membership/gms/locator/GMSLocator.java | 2 + .../membership/gms/membership/GMSJoinLeave.java | 9 ++- .../gms/mgr/GMSMembershipManager.java | 29 ++++++--- .../internal/tcpserver/TcpClient.java | 21 +++++-- .../membership/MembershipJUnitTest.java | 12 ++-- .../gms/membership/GMSJoinLeaveJUnitTest.java | 65 +++++++------------- .../gms/membership/StatRecorderJUnitTest.java | 35 +++++++---- 9 files changed, 119 insertions(+), 91 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/build.gradle ---------------------------------------------------------------------- diff --git a/build.gradle b/build.gradle index d7b4965..32d213f 100755 --- a/build.gradle +++ b/build.gradle @@ -341,6 +341,9 @@ subprojects { task distributedTest(type:Test) { include '**/*DUnitTest.class' + // maxParallelForks = 2 + // maxParallelForks = Runtime.runtime.availableProcessors() + // TODO add @Category(DistributedTest.class) to dunit tests // useJUnit { // excludeCategories 'com.gemstone.gemfire.test.junit.categories.UnitTest' http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java index 382850a..5f04ac4 100755 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -84,7 +85,7 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler { volatile long currentTimeStamp; - final private Map memberVsLastMsgTS = new ConcurrentHashMap<>(); + final ConcurrentMap memberVsLastMsgTS = new ConcurrentHashMap<>(); final private Map requestIdVsResponse = new ConcurrentHashMap<>(); final private ConcurrentHashMap suspectedMemberVsView = new ConcurrentHashMap<>(); final private Map> viewVsSuspectedMembers = new HashMap<>(); @@ -107,7 +108,7 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler { /** * to stop check scheduler */ - private ScheduledFuture monitorFuture; + private ScheduledFuture monitorFuture; /** test hook */ boolean playingDead = false; @@ -130,13 +131,10 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler { if (currentSuspects.remove(sender)) { logger.info("No longer suspecting {}", sender); } - CustomTimeStamp cTS = memberVsLastMsgTS.get(sender); + CustomTimeStamp cTS = new CustomTimeStamp(currentTimeStamp); + cTS = memberVsLastMsgTS.putIfAbsent(sender, cTS); if (cTS != null) { cTS.setTimeStamp(currentTimeStamp); - } else { - cTS = new CustomTimeStamp(); - cTS.setTimeStamp(currentTimeStamp); - memberVsLastMsgTS.put(sender, cTS); } } @@ -144,7 +142,11 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler { * this class is to avoid garbage */ private static class CustomTimeStamp { - volatile long timeStamp; + private volatile long timeStamp; + + CustomTimeStamp(long timeStamp) { + this.timeStamp = timeStamp; + } public long getTimeStamp() { return timeStamp; @@ -188,8 +190,7 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler { } if (nextNeighborTS == null) { - CustomTimeStamp customTS = new CustomTimeStamp(); - customTS.setTimeStamp(currentTime); + CustomTimeStamp customTS = new CustomTimeStamp(currentTime); memberVsLastMsgTS.put(neighbour, customTS); return; } @@ -445,8 +446,7 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler { long cts = System.currentTimeMillis(); for (InternalDistributedMember mbr: allMembers) { - CustomTimeStamp customTS = new CustomTimeStamp(); - customTS.setTimeStamp(cts); + CustomTimeStamp customTS = new CustomTimeStamp(cts); memberVsLastMsgTS.put(mbr, customTS); } } @@ -715,10 +715,17 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler { @Override public void run() { try { + long startTime = System.currentTimeMillis(); + CustomTimeStamp ts = new CustomTimeStamp(startTime); + memberVsLastMsgTS.put(mbr, ts); + logger.info("Membership: Doing final check for suspect member {}", mbr); boolean pinged = GMSHealthMonitor.this.doCheckMember(mbr); if (!pinged && !isStopping) { - GMSHealthMonitor.this.services.getJoinLeave().remove(mbr, reason); + ts = memberVsLastMsgTS.get(mbr); + if (ts == null || ts.getTimeStamp() <= startTime) { + services.getJoinLeave().remove(mbr, reason); + } } // whether it's alive or not, at this point we allow it to // be a suspect again @@ -846,7 +853,6 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler { suspectRequests.clear(); } } - NetView v = currentView; List recipients; // if (v.size() > 20) { // // TODO this needs some rethinking - we need the guys near the http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/locator/GMSLocator.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/locator/GMSLocator.java b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/locator/GMSLocator.java index 407ed9b..89f4d97 100755 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/locator/GMSLocator.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/locator/GMSLocator.java @@ -299,6 +299,7 @@ public class GMSLocator implements Locator, NetLocator { private boolean recover(InetSocketAddress other) { try { + logger.info("Peer locator attempting to recover from " + other); Object response = TcpClient.requestToServer(other.getAddress(), other.getPort(), new GetViewRequest(), 20000, true); if (response != null && (response instanceof GetViewResponse)) { @@ -309,6 +310,7 @@ public class GMSLocator implements Locator, NetLocator { } catch (IOException | ClassNotFoundException ignore) { logger.debug("Peer locator could not recover membership view from {}: {}", other, ignore.getMessage()); } + logger.info("Peer locator was unable to recover state from this locator"); return false; } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java index 4ea3a10..3fb86ee 100755 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java @@ -725,7 +725,12 @@ logger.debug("waiting for view responses"); FindCoordinatorRequest request = new FindCoordinatorRequest(this.localAddress, state.alreadyTried, state.viewId); Set coordinators = new HashSet(); - long giveUpTime = System.currentTimeMillis() + (services.getConfig().getLocatorWaitTime() * 1000L); + long waitTime = services.getConfig().getLocatorWaitTime() * 1000; + if (waitTime <= 0) { + waitTime = services.getConfig().getMemberTimeout() * 2; + } + long giveUpTime = System.currentTimeMillis() + waitTime; + int connectTimeout = (int)services.getConfig().getMemberTimeout(); boolean anyResponses = false; boolean flagsSet = false; @@ -735,7 +740,7 @@ logger.debug("waiting for view responses"); for (InetSocketAddress addr: locators) { try { Object o = TcpClient.requestToServer( - addr.getAddress(), addr.getPort(), request, services.getConfig().getJoinTimeout(), + addr.getAddress(), addr.getPort(), request, connectTimeout, true); FindCoordinatorResponse response = (o instanceof FindCoordinatorResponse) ? (FindCoordinatorResponse)o : null; if (response != null && response.getCoordinator() != null) { http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java index 159b1ca..0e3a0a6 100755 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java @@ -778,6 +778,7 @@ public class GMSMembershipManager implements MembershipManager, Manager */ private void join() { services.setShutdownCause(null); + services.getCancelCriterion().cancel(null); latestViewLock.writeLock().lock(); try { @@ -1944,8 +1945,15 @@ public class GMSMembershipManager implements MembershipManager, Manager this.services.getConfig().getDistributionConfig().getAckWaitThreshold(), this.services.getConfig().getDistributionConfig().getAckSevereAlertThreshold()); - if (theStats != null) + if (theStats != null) { theStats.incSentBytes(sentBytes); + } + + if (sentBytes == 0) { + if (services.getCancelCriterion().cancelInProgress() != null) { + throw new DistributedSystemDisconnectedException(); + } + } } catch (DistributedSystemDisconnectedException ex) { if (services.getShutdownCause() != null) { @@ -2070,10 +2078,12 @@ public class GMSMembershipManager implements MembershipManager, Manager Set result = null; boolean allDestinations = msg.forAll(); - if (!this.hasConnected) { - if (services.getCancelCriterion().cancelInProgress() != null) { - throw services.getCancelCriterion().generateCancelledException(null); - } + if (services.getCancelCriterion().cancelInProgress() != null) { + throw new DistributedSystemDisconnectedException("Distributed System is shutting down", + services.getCancelCriterion().generateCancelledException(null)); + } + + if (isJoining()) { // If we get here, we are starting up, so just report a failure. if (allDestinations) return null; @@ -2876,6 +2886,11 @@ public class GMSMembershipManager implements MembershipManager, Manager } final Exception shutdownCause = new ForcedDisconnectException(reason); + + // cache the exception so it can be appended to ShutdownExceptions + services.setShutdownCause(shutdownCause); + services.getCancelCriterion().cancel(reason); + if (!inhibitForceDisconnectLogging) { logger.fatal(LocalizedMessage.create( LocalizedStrings.GroupMembershipService_MEMBERSHIP_SERVICE_FAILURE_0, reason), shutdownCause); @@ -2887,10 +2902,6 @@ public class GMSMembershipManager implements MembershipManager, Manager AlertAppender.getInstance().shuttingDown(); - services.getCancelCriterion().cancel(reason); - // cache the exception so it can be appended to ShutdownExceptions - services.setShutdownCause(shutdownCause); - Thread reconnectThread = new Thread (new Runnable() { public void run() { // stop server locators immediately since they may not have correct http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java index 6720f64..8425ba8 100644 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/tcpserver/TcpClient.java @@ -91,8 +91,10 @@ public class TcpClient { logger.debug("TcpClient sending {} to {}", request, ipAddr); + long giveupTime = System.currentTimeMillis() + timeout; + // Get the GemFire version of the TcpServer first, before sending any other request. - short serverVersion = getServerVersion(ipAddr, REQUEST_TIMEOUT).shortValue(); + short serverVersion = getServerVersion(ipAddr, timeout/2).shortValue(); if (serverVersion > Version.CURRENT_ORDINAL) { serverVersion = Version.CURRENT_ORDINAL; @@ -105,10 +107,17 @@ public class TcpClient { gossipVersion = TcpServer.getOldGossipVersion(); } - Socket sock=SocketCreator.getDefaultInstance().connect(ipAddr.getAddress(), ipAddr.getPort(), timeout, null, false); - sock.setSoTimeout(timeout); + long newTimeout = giveupTime - System.currentTimeMillis(); + if (newTimeout <= 0) { + return null; + } + + Socket sock=SocketCreator.getDefaultInstance().connect(ipAddr.getAddress(), ipAddr.getPort(), (int)newTimeout, null, false); + sock.setSoTimeout((int)newTimeout); + DataOutputStream out = null; try { - DataOutputStream out=new DataOutputStream(sock.getOutputStream()); + out=new DataOutputStream(sock.getOutputStream()); + if (serverVersion < Version.CURRENT_ORDINAL) { out = new VersionedDataOutputStream(out, Version.fromOrdinalNoThrow(serverVersion, false)); } @@ -125,6 +134,7 @@ public class TcpClient { in = new VersionedDataInputStream(in, Version.fromOrdinal(serverVersion, false)); try { Object response = DataSerializer.readObject(in); + logger.debug("received response: {}", response); return response; } catch (EOFException ex) { throw new EOFException("Locator at " + ipAddr + " did not respond. This is normal if the locator was shutdown. If it wasn't check its log for exceptions."); @@ -141,6 +151,9 @@ public class TcpClient { } return null; } finally { + if (out != null) { + out.close(); + } try { sock.close(); } catch(Exception e) { http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/MembershipJUnitTest.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/MembershipJUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/MembershipJUnitTest.java index 8b68b18..e3a3688 100755 --- a/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/MembershipJUnitTest.java +++ b/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/MembershipJUnitTest.java @@ -201,10 +201,10 @@ public class MembershipJUnitTest extends TestCase { long giveUp = System.currentTimeMillis() + 15000; for (;;) { try { - assert jl2.getView().size() == 2 : "view = " + jl2.getView(); - assert jl1.getView().size() == 2 : "view = " + jl1.getView(); - assert jl1.getView().getCreator().equals(jl2.getView().getCreator()); - assert jl1.getView().getViewId() == jl2.getView().getViewId(); + assertTrue("view = " + jl2.getView(), jl2.getView().size() == 2); + assertTrue("view = " + jl1.getView(), jl1.getView().size() == 2); + assertTrue(jl1.getView().getCreator().equals(jl2.getView().getCreator())); + assertTrue(jl1.getView().getViewId() == jl2.getView().getViewId()); break; } catch (AssertionError e) { if (System.currentTimeMillis() > giveUp) { @@ -214,9 +214,9 @@ public class MembershipJUnitTest extends TestCase { } m2.shutdown(); - assert !m2.isConnected(); + assertTrue(!m2.isConnected()); - assert m1.getView().size() == 1; + assertTrue(m1.getView().size() == 1); } finally { http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java index 9ceff7c..d29a6dd 100644 --- a/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java +++ b/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java @@ -6,6 +6,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.net.UnknownHostException; @@ -43,8 +44,6 @@ import com.gemstone.gemfire.internal.Version; import com.gemstone.gemfire.security.AuthenticationFailedException; import com.gemstone.gemfire.test.junit.categories.UnitTest; -import dunit.standalone.DUnitLauncher; - @Category(UnitTest.class) public class GMSJoinLeaveJUnitTest { private Services services; @@ -149,8 +148,9 @@ public class GMSJoinLeaveJUnitTest { // now for the test boolean result = gmsJoinLeave.findCoordinatorFromView(); - assert result : "should have found coordinator " + mockMembers[2]; - assert state.possibleCoordinator == coordinator : "should have found " + coordinator + " but found " + state.possibleCoordinator; + assertTrue("should have found coordinator " + mockMembers[2], result); + assertTrue("should have found " + coordinator + " but found " + state.possibleCoordinator, + state.possibleCoordinator == coordinator); } @Test @@ -158,7 +158,7 @@ public class GMSJoinLeaveJUnitTest { initMocks(); gmsJoinLeave.processMessage(new JoinRequestMessage(mockOldMember, mockOldMember, null)); - Assert.assertTrue("JoinRequest should not have been added to view request", gmsJoinLeave.getViewRequests().size() == 0); + assertTrue("JoinRequest should not have been added to view request", gmsJoinLeave.getViewRequests().size() == 0); verify(messenger).send(any(JoinResponseMessage.class)); } @@ -170,7 +170,7 @@ public class GMSJoinLeaveJUnitTest { when(services.getMessenger()).thenReturn(messenger); gmsJoinLeave.processMessage(new JoinRequestMessage(mockMembers[0], mockMembers[0], credentials)); - Assert.assertTrue("JoinRequest should not have been added to view request", gmsJoinLeave.getViewRequests().size() == 0); + assertTrue("JoinRequest should not have been added to view request", gmsJoinLeave.getViewRequests().size() == 0); verify(messenger).send(any(JoinResponseMessage.class)); } @@ -182,27 +182,11 @@ public class GMSJoinLeaveJUnitTest { when(services.getMessenger()).thenReturn(messenger); gmsJoinLeave.processMessage(new JoinRequestMessage(mockMembers[0], mockMembers[0], null)); - Assert.assertTrue("JoinRequest should not have been added to view request", gmsJoinLeave.getViewRequests().size() == 0); + assertTrue("JoinRequest should not have been added to view request", gmsJoinLeave.getViewRequests().size() == 0); verify(messenger).send(any(JoinResponseMessage.class)); } - private void prepareView() throws IOException { - int viewId = 1; - List mbrs = new LinkedList<>(); - Set shutdowns = new HashSet<>(); - Set crashes = new HashSet<>(); - mbrs.add(mockMembers[0]); - - when(services.getMessenger()).thenReturn(messenger); - - //prepare the view - NetView netView = new NetView(mockMembers[0], viewId, mbrs, shutdowns, crashes); - InstallViewMessage installViewMessage = new InstallViewMessage(netView, credentials, true); - gmsJoinLeave.processMessage(installViewMessage); - verify(messenger).send(any(ViewAckMessage.class)); - } - private void prepareAndInstallView() throws IOException { int viewId = 1; List mbrs = new LinkedList<>(); @@ -232,7 +216,7 @@ public class GMSJoinLeaveJUnitTest { when(messenger.send(any(RemoveMemberMessage.class))).thenAnswer(removeMessageSent); gmsJoinLeave.remove(mockMembers[0], "removing for test"); Thread.sleep(GMSJoinLeave.MEMBER_REQUEST_COLLECTION_INTERVAL*2); - assert removeMessageSent.methodExecuted; + assertTrue(removeMessageSent.methodExecuted); } @@ -278,6 +262,7 @@ public class GMSJoinLeaveJUnitTest { verify(mockManager).forceDisconnect(any(String.class)); } + @SuppressWarnings("rawtypes") private class MethodExecuted implements Answer { private boolean methodExecuted = false; @Override @@ -286,10 +271,6 @@ public class GMSJoinLeaveJUnitTest { methodExecuted = true; return null; } - - public boolean isMethodExecuted() { - return methodExecuted; - } } @Test @@ -301,7 +282,7 @@ public class GMSJoinLeaveJUnitTest { RemoveMemberMessage msg = new RemoveMemberMessage(mockMembers[0], mockMembers[1], reason); msg.setSender(new InternalDistributedMember("localhost", 9000)); gmsJoinLeave.processMessage(msg); - Assert.assertTrue("RemoveMemberMessage should not have been added to view requests", gmsJoinLeave.getViewRequests().size() == 0); + assertTrue("RemoveMemberMessage should not have been added to view requests", gmsJoinLeave.getViewRequests().size() == 0); } @Test @@ -323,9 +304,9 @@ public class GMSJoinLeaveJUnitTest { waitForViewAndNoRequestsInProgress(7); NetView view = gmsJoinLeave.getView(); - Assert.assertTrue("expected member to be removed: " + mockMembers[0] + "; view: " + view, + assertTrue("expected member to be removed: " + mockMembers[0] + "; view: " + view, !view.contains(mockMembers[0])); - Assert.assertTrue("expected member to be in shutdownMembers collection: " + mockMembers[0] + "; view: " + view, + assertTrue("expected member to be in shutdownMembers collection: " + mockMembers[0] + "; view: " + view, view.getShutdownMembers().contains(mockMembers[0])); } @@ -348,9 +329,9 @@ public class GMSJoinLeaveJUnitTest { waitForViewAndNoRequestsInProgress(7); NetView view = gmsJoinLeave.getView(); - Assert.assertTrue("expected member to be removed: " + mockMembers[0] + "; view: " + view, + assertTrue("expected member to be removed: " + mockMembers[0] + "; view: " + view, !view.contains(mockMembers[0])); - Assert.assertTrue("expected member to be in crashedMembers collection: " + mockMembers[0] + "; view: " + view, + assertTrue("expected member to be in crashedMembers collection: " + mockMembers[0] + "; view: " + view, view.getCrashedMembers().contains(mockMembers[0])); } @@ -375,7 +356,7 @@ public class GMSJoinLeaveJUnitTest { waitForViewAndNoRequestsInProgress(7); NetView view = gmsJoinLeave.getView(); - Assert.assertTrue("expected member to be added: " + mockMembers[2] + "; view: " + view, + assertTrue("expected member to be added: " + mockMembers[2] + "; view: " + view, view.contains(mockMembers[2])); List members = view.getMembers(); int occurrences = 0; @@ -384,7 +365,7 @@ public class GMSJoinLeaveJUnitTest { occurrences += 1; } } - Assert.assertTrue("expected member to only be in the view once: " + mockMembers[2] + "; view: " + view, + assertTrue("expected member to only be in the view once: " + mockMembers[2] + "; view: " + view, occurrences == 1); verify(healthMonitor, times(5)).checkIfAvailable(any(InternalDistributedMember.class), any(String.class), any(Boolean.class)); @@ -439,7 +420,7 @@ public class GMSJoinLeaveJUnitTest { LeaveRequestMessage msg = new LeaveRequestMessage(gmsJoinLeave.getMemberID(), mockMembers[1], reason); msg.setSender(mockMembers[1]); gmsJoinLeave.processMessage(msg); - Assert.assertTrue("Expected leave request from non-member to be ignored", gmsJoinLeave.getViewRequests().isEmpty()); + assertTrue("Expected leave request from non-member to be ignored", gmsJoinLeave.getViewRequests().isEmpty()); } @Test @@ -453,7 +434,7 @@ public class GMSJoinLeaveJUnitTest { LeaveRequestMessage msg = new LeaveRequestMessage(creator, creator, reason); msg.setSender(creator); gmsJoinLeave.processMessage(msg); - Assert.assertTrue("Expected becomeCoordinator to be invoked", gmsJoinLeave.isCoordinator()); + assertTrue("Expected becomeCoordinator to be invoked", gmsJoinLeave.isCoordinator()); } @Test @@ -467,12 +448,11 @@ public class GMSJoinLeaveJUnitTest { RemoveMemberMessage msg = new RemoveMemberMessage(creator, creator, reason); msg.setSender(creator); gmsJoinLeave.processMessage(msg); - Assert.assertTrue("Expected becomeCoordinator to be invoked", gmsJoinLeave.isCoordinator()); + assertTrue("Expected becomeCoordinator to be invoked", gmsJoinLeave.isCoordinator()); } @Test public void testBecomeCoordinatorThroughViewChange() throws Exception { - String reason = "testing"; initMocks(); prepareAndInstallView(); NetView oldView = gmsJoinLeave.getView(); @@ -483,12 +463,11 @@ public class GMSJoinLeaveJUnitTest { InstallViewMessage msg = new InstallViewMessage(view, creator); msg.setSender(creator); gmsJoinLeave.processMessage(msg); - Assert.assertTrue("Expected it to become coordinator", gmsJoinLeave.isCoordinator()); + assertTrue("Expected it to become coordinator", gmsJoinLeave.isCoordinator()); } @Test public void testBecomeParticipantThroughViewChange() throws Exception { - String reason = "testing"; initMocks(); prepareAndInstallView(); NetView oldView = gmsJoinLeave.getView(); @@ -502,7 +481,7 @@ public class GMSJoinLeaveJUnitTest { InstallViewMessage msg = new InstallViewMessage(view, creator); msg.setSender(creator); gmsJoinLeave.processMessage(msg); - Assert.assertTrue("Expected it to stop being coordinator", !gmsJoinLeave.isCoordinator()); + assertTrue("Expected it to stop being coordinator", !gmsJoinLeave.isCoordinator()); } @Test @@ -591,7 +570,7 @@ public class GMSJoinLeaveJUnitTest { msg = new InstallViewMessage(alternateView, null, true); gmsJoinLeave.processMessage(msg); - Assert.assertTrue(gmsJoinLeave.getPreparedView().equals(newView)); + assertTrue(gmsJoinLeave.getPreparedView().equals(newView)); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/b256262e/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/StatRecorderJUnitTest.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/StatRecorderJUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/StatRecorderJUnitTest.java index 3801074..6d35818 100755 --- a/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/StatRecorderJUnitTest.java +++ b/gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/StatRecorderJUnitTest.java @@ -3,6 +3,8 @@ package com.gemstone.gemfire.distributed.internal.membership.gms.membership; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; import java.util.Properties; @@ -58,19 +60,22 @@ public class StatRecorderJUnitTest { Event evt = new Event(Event.MSG, msg); recorder.up(evt); - assert stats.ucastMessagesReceived == 1 : "stats.ucastMessagesReceived =" + stats.ucastMessagesReceived; - assert stats.ucastMessageBytesReceived == 150; + assertTrue("stats.ucastMessagesReceived =" + stats.ucastMessagesReceived, + stats.ucastMessagesReceived == 1); + assertEquals(stats.ucastMessageBytesReceived, 150); recorder.down(evt); - assert stats.ucastMessagesSent == 1 : "stats.ucastMessagesSent =" + stats.ucastMessagesSent; - assert stats.ucastMessageBytesSent == 150; + assertTrue("stats.ucastMessagesSent =" + stats.ucastMessagesSent, + stats.ucastMessagesSent == 1); + assertEquals(stats.ucastMessageBytesSent, 150); msg = mock(Message.class); when(msg.getHeader(any(Short.class))).thenReturn(Header.createXmitReqHeader()); when(msg.size()).thenReturn(150L); evt = new Event(Event.MSG, msg); recorder.down(evt); - assert stats.ucastRetransmits == 1 : "stats.ucastRetransmits =" + stats.ucastRetransmits; + assertTrue("stats.ucastRetransmits =" + stats.ucastRetransmits, + stats.ucastRetransmits == 1); } /** @@ -84,26 +89,30 @@ public class StatRecorderJUnitTest { Event evt = new Event(Event.MSG, msg); recorder.up(evt); - assert stats.mcastMessagesReceived == 1 : "mcastMessagesReceived = " + stats.mcastMessagesReceived; - assert stats.mcastMessageBytesReceived == 150; + assertTrue("mcastMessagesReceived = " + stats.mcastMessagesReceived, + stats.mcastMessagesReceived == 1); + assertEquals(stats.mcastMessageBytesReceived, 150); recorder.down(evt); - assert stats.mcastMessagesSent == 1 : "mcastMessagesSent = " + stats.mcastMessagesSent; - assert stats.mcastMessageBytesSent == 150; + assertTrue("mcastMessagesSent = " + stats.mcastMessagesSent, + stats.mcastMessagesSent == 1); + assertEquals(stats.mcastMessageBytesSent, 150); msg = mock(Message.class); when(msg.size()).thenReturn(150L); when(msg.getHeader(any(Short.class))).thenReturn(NakAckHeader2.createXmitRequestHeader(null)); evt = new Event(Event.MSG, msg); recorder.down(evt); - assert stats.mcastRetransmitRequests == 1 : "mcastRetransmitRequests = " + stats.mcastRetransmitRequests; + assertTrue("mcastRetransmitRequests = " + stats.mcastRetransmitRequests, + stats.mcastRetransmitRequests == 1); msg = mock(Message.class); when(msg.size()).thenReturn(150L); when(msg.getHeader(any(Short.class))).thenReturn(NakAckHeader2.createXmitResponseHeader()); evt = new Event(Event.MSG, msg); recorder.down(evt); - assert stats.mcastRetransmits == 1 : "mcastRetransmits = " + stats.mcastRetransmits; + assertTrue("mcastRetransmits = " + stats.mcastRetransmits, + stats.mcastRetransmits == 1); } @@ -132,7 +141,7 @@ public class StatRecorderJUnitTest { messenger.init(mockServices); String jgroupsConfig = messenger.getJGroupsStackConfig(); System.out.println(jgroupsConfig); - assert jgroupsConfig.contains("gms.messenger.StatRecorder"); + assertTrue(jgroupsConfig.contains("gms.messenger.StatRecorder")); // now test to see if the multicast stack has the recorder installed nonDefault.put(DistributionConfig.MCAST_PORT_NAME, "12345"); @@ -142,7 +151,7 @@ public class StatRecorderJUnitTest { when(mockConfig.getTransport()).thenReturn(transport); messenger = new JGroupsMessenger(); messenger.init(mockServices); - assert jgroupsConfig.contains("gms.messenger.StatRecorder"); + assertTrue(jgroupsConfig.contains("gms.messenger.StatRecorder")); } static class MyStats extends DummyDMStats {