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/21 23:26:38 UTC
[1/2] geode git commit: GEODE-2497 surprise members are never timed
out during startup
Repository: geode
Updated Branches:
refs/heads/feature/GEODE-2497 8d45ca227 -> 69e3523ec
GEODE-2497 surprise members are never timed out during startup
Moved the creation of the timer to GMSMembershipManager.started()
Removed write-lock in timer-creation method since it's only called from
one place now
Altered the way that the timer-creation method finds the
InternalDistributedSystem. The old way of using getAnyInstance() was
the primary source of the problem since it returns null until startup
is completed.
Altered the surprise-member unit test to ensure that it's using the
timer and not relying on installation of a new membership view to clean
things up.
Altered the surprise-member unit test to run faster. It now completes in
under 10 seconds.
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/a6dfa4ca
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/a6dfa4ca
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/a6dfa4ca
Branch: refs/heads/feature/GEODE-2497
Commit: a6dfa4ca630a82fcf92942a834f8255e86d2bfcb
Parents: dbea592
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Fri Feb 17 10:17:21 2017 -0800
Committer: Bruce Schuchardt <bs...@pivotal.io>
Committed: Tue Feb 21 15:07:00 2017 -0800
----------------------------------------------------------------------
.../gms/mgr/GMSMembershipManager.java | 76 ++++++++++----------
.../internal/DistributionManagerDUnitTest.java | 66 +++++++----------
2 files changed, 62 insertions(+), 80 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/a6dfa4ca/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
index cf17025..9652ad5 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
@@ -608,7 +608,6 @@ public class GMSMembershipManager implements MembershipManager, Manager {
}
try {
listener.viewInstalled(latestView);
- startCleanupTimer();
} catch (DistributedSystemDisconnectedException se) {
}
} finally {
@@ -616,6 +615,10 @@ public class GMSMembershipManager implements MembershipManager, Manager {
}
}
+ public boolean isCleanupTimerStarted() {
+ return this.cleanupTimer != null;
+ }
+
/**
* the timer used to perform periodic tasks
*
@@ -767,7 +770,9 @@ public class GMSMembershipManager implements MembershipManager, Manager {
}
@Override
- public void started() {}
+ public void started() {
+ startCleanupTimer();
+ }
/** this is invoked by JoinLeave when there is a loss of quorum in the membership system */
@@ -942,12 +947,6 @@ public class GMSMembershipManager implements MembershipManager, Manager {
surpriseMembers.remove(member);
} else {
- // Now that we're sure the member is new, add them.
- // make sure the surprise-member cleanup task is running
- if (this.cleanupTimer == null) {
- startCleanupTimer();
- } // cleanupTimer == null
-
// Ensure that the member is accounted for in the view
// Conjure up a new view including the new member. This is necessary
// because we are about to tell the listener about a new member, so
@@ -978,40 +977,39 @@ public class GMSMembershipManager implements MembershipManager, Manager {
/** starts periodic task to perform cleanup chores such as expire surprise members */
private void startCleanupTimer() {
+ if (this.dcReceiver == null) {
+ // junit tests don't provide a direct-channel receiver
+ return;
+ }
+ DistributedSystem ds = this.dcReceiver.getDM().getSystem();
+ this.cleanupTimer = new SystemTimer(ds, true);
+ SystemTimer.SystemTimerTask st = new SystemTimer.SystemTimerTask() {
+ @Override
+ public void run2() {
+ cleanUpSurpriseMembers();
+ }
+ };
+ this.cleanupTimer.scheduleAtFixedRate(st, surpriseMemberTimeout, surpriseMemberTimeout / 3);
+ }
+
+ // invoked from the cleanupTimer task
+ private void cleanUpSurpriseMembers() {
latestViewWriteLock.lock();
try {
- if (this.cleanupTimer != null) {
- return;
+ long oldestAllowed = System.currentTimeMillis() - surpriseMemberTimeout;
+ for (Iterator it = surpriseMembers.entrySet().iterator(); it.hasNext();) {
+ Map.Entry entry = (Map.Entry) it.next();
+ Long birthtime = (Long) entry.getValue();
+ if (birthtime.longValue() < oldestAllowed) {
+ it.remove();
+ InternalDistributedMember m = (InternalDistributedMember) entry.getKey();
+ logger.info(LocalizedMessage.create(
+ LocalizedStrings.GroupMembershipService_MEMBERSHIP_EXPIRING_MEMBERSHIP_OF_SURPRISE_MEMBER_0,
+ m));
+ removeWithViewLock(m, true,
+ "not seen in membership view in " + surpriseMemberTimeout + "ms");
+ }
}
- DistributedSystem ds = InternalDistributedSystem.getAnyInstance();
- if (ds != null && ds.isConnected()) {
- this.cleanupTimer = new SystemTimer(ds, true);
- SystemTimer.SystemTimerTask st = new SystemTimer.SystemTimerTask() {
- @Override
- public void run2() {
- latestViewWriteLock.lock();
- try {
- long oldestAllowed = System.currentTimeMillis() - surpriseMemberTimeout;
- for (Iterator it = surpriseMembers.entrySet().iterator(); it.hasNext();) {
- Map.Entry entry = (Map.Entry) it.next();
- Long birthtime = (Long) entry.getValue();
- if (birthtime.longValue() < oldestAllowed) {
- it.remove();
- InternalDistributedMember m = (InternalDistributedMember) entry.getKey();
- logger.info(LocalizedMessage.create(
- LocalizedStrings.GroupMembershipService_MEMBERSHIP_EXPIRING_MEMBERSHIP_OF_SURPRISE_MEMBER_0,
- m));
- removeWithViewLock(m, true,
- "not seen in membership view in " + surpriseMemberTimeout + "ms");
- }
- }
- } finally {
- latestViewWriteLock.unlock();
- }
- }
- };
- this.cleanupTimer.scheduleAtFixedRate(st, surpriseMemberTimeout, surpriseMemberTimeout / 3);
- } // ds != null && ds.isConnected()
} finally {
latestViewWriteLock.unlock();
}
http://git-wip-us.apache.org/repos/asf/geode/blob/a6dfa4ca/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
index ccce013..c795261 100644
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
@@ -17,6 +17,7 @@ package org.apache.geode.distributed.internal;
import static org.apache.geode.distributed.ConfigurationProperties.*;
import static org.apache.geode.test.dunit.Assert.*;
+import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
import org.awaitility.Awaitility;
import java.net.InetAddress;
@@ -28,6 +29,7 @@ import org.apache.geode.test.junit.categories.MembershipTest;
import org.apache.logging.log4j.Logger;
import org.junit.Assert;
import org.junit.Ignore;
+import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -74,6 +76,10 @@ public class DistributionManagerDUnitTest extends JUnit4DistributedTestCase {
public static DistributedSystem ds;
+ @Rule
+ public DistributedRestoreSystemProperties restoreSystemProperties =
+ new DistributedRestoreSystemProperties();
+
/**
* Clears the exceptionInThread flag in the given distribution manager.
*/
@@ -137,18 +143,14 @@ public class DistributionManagerDUnitTest extends JUnit4DistributedTestCase {
InternalDistributedMember idm = mgr.getLocalMember();
// TODO GMS needs to have a system property allowing the bind-port to be set
System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "jg-bind-port", "" + idm.getPort());
- try {
- sys.disconnect();
- sys = getSystem();
- mgr = MembershipManagerHelper.getMembershipManager(sys);
- sys.disconnect();
- InternalDistributedMember idm2 = mgr.getLocalMember();
- org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
- .info("original ID=" + idm + " and after connecting=" + idm2);
- assertTrue("should not have used a different udp port", idm.getPort() == idm2.getPort());
- } finally {
- System.getProperties().remove(DistributionConfig.GEMFIRE_PREFIX + "jg-bind-port");
- }
+ sys.disconnect();
+ sys = getSystem();
+ mgr = MembershipManagerHelper.getMembershipManager(sys);
+ sys.disconnect();
+ InternalDistributedMember idm2 = mgr.getLocalMember();
+ org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
+ .info("original ID=" + idm + " and after connecting=" + idm2);
+ assertTrue("should not have used a different udp port", idm.getPort() == idm2.getPort());
}
/**
@@ -158,11 +160,12 @@ public class DistributionManagerDUnitTest extends JUnit4DistributedTestCase {
* should be gone and force more view processing to have it scrubbed from the set.
**/
@Test
- public void testSurpriseMemberHandling() {
- VM vm0 = Host.getHost(0).getVM(0);
+ public void testSurpriseMemberHandling() throws Exception {
+ System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "surprise-member-timeout", "3000");
InternalDistributedSystem sys = getSystem();
MembershipManager mgr = MembershipManagerHelper.getMembershipManager(sys);
+ assertTrue(((GMSMembershipManager) mgr).isCleanupTimerStarted());
try {
InternalDistributedMember mbr =
@@ -180,19 +183,13 @@ public class DistributionManagerDUnitTest extends JUnit4DistributedTestCase {
.info("current membership view is " + mgr.getView());
org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
.info("created ID " + mbr + " with view ID " + mbr.getVmViewId());
- sys.getLogWriter()
- .info("<ExpectedException action=add>attempt to add old member</ExpectedException>");
- sys.getLogWriter()
- .info("<ExpectedException action=add>Removing shunned GemFire node</ExpectedException>");
- try {
- boolean accepted = mgr.addSurpriseMember(mbr);
- Assert.assertTrue("member with old ID was not rejected (bug #44566)", !accepted);
- } finally {
- sys.getLogWriter()
- .info("<ExpectedException action=remove>attempt to add old member</ExpectedException>");
- sys.getLogWriter().info(
- "<ExpectedException action=remove>Removing shunned GemFire node</ExpectedException>");
- }
+
+ IgnoredException.addIgnoredException("attempt to add old member");
+ IgnoredException.addIgnoredException("Removing shunned GemFire node");
+
+ boolean accepted = mgr.addSurpriseMember(mbr);
+ Assert.assertTrue("member with old ID was not rejected (bug #44566)", !accepted);
+
mbr.setVmViewId(oldViewId);
// now forcibly add it as a surprise member and show that it is reaped
@@ -203,27 +200,14 @@ public class DistributionManagerDUnitTest extends JUnit4DistributedTestCase {
MembershipManagerHelper.addSurpriseMember(sys, mbr, birthTime);
assertTrue("Member was not a surprise member", mgr.isSurpriseMember(mbr));
- // force a real view change
- SerializableRunnable connectDisconnect = new SerializableRunnable() {
- public void run() {
- getSystem().disconnect();
- }
- };
- vm0.invoke(connectDisconnect);
-
if (birthTime < (System.currentTimeMillis() - timeout)) {
return; // machine is too busy and we didn't get enough CPU to perform more assertions
}
assertTrue("Member was incorrectly removed from surprise member set",
mgr.isSurpriseMember(mbr));
- try {
- Thread.sleep(gracePeriod);
- } catch (InterruptedException e) {
- fail("test was interrupted", e);
- }
+ Thread.sleep((timeout / 3) + gracePeriod);
- vm0.invoke(connectDisconnect);
assertTrue("Member was not removed from surprise member set", !mgr.isSurpriseMember(mbr));
} finally {
[2/2] geode git commit: Merge branch 'feature/GEODE-2497' of
https://git-wip-us.apache.org/repos/asf/geode into feature/GEODE-2497
Posted by bs...@apache.org.
Merge branch 'feature/GEODE-2497' of https://git-wip-us.apache.org/repos/asf/geode into feature/GEODE-2497
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/69e3523e
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/69e3523e
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/69e3523e
Branch: refs/heads/feature/GEODE-2497
Commit: 69e3523ec25f49dc847b322ee59c156aed84a7f2
Parents: a6dfa4c 8d45ca2
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Tue Feb 21 15:15:41 2017 -0800
Committer: Bruce Schuchardt <bs...@pivotal.io>
Committed: Tue Feb 21 15:15:41 2017 -0800
----------------------------------------------------------------------
.../distributed/internal/DistributionManagerDUnitTest.java | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/69e3523e/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
----------------------------------------------------------------------
diff --cc geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
index c795261,2b405fa..3e6638b
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
@@@ -76,10 -74,6 +76,9 @@@ public class DistributionManagerDUnitTe
public static DistributedSystem ds;
+ @Rule
- public DistributedRestoreSystemProperties restoreSystemProperties =
- new DistributedRestoreSystemProperties();
-
++ public DistributedRestoreSystemProperties restoreSystemProperties = new DistributedRestoreSystemProperties();
++
/**
* Clears the exceptionInThread flag in the given distribution manager.
*/
@@@ -160,60 -158,75 +159,61 @@@
* should be gone and force more view processing to have it scrubbed from the set.
**/
@Test
- public void testSurpriseMemberHandling() {
+ public void testSurpriseMemberHandling() throws Exception {
System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "surprise-member-timeout", "3000");
+ InternalDistributedSystem sys = getSystem();
+ MembershipManager mgr = MembershipManagerHelper.getMembershipManager(sys);
+ assertTrue(((GMSMembershipManager) mgr).isCleanupTimerStarted());
+
try {
- InternalDistributedSystem sys = getSystem();
- MembershipManager mgr = MembershipManagerHelper.getMembershipManager(sys);
- assertTrue(((GMSMembershipManager) mgr).isCleanupTimerStarted());
-
- try {
- InternalDistributedMember mbr =
- new InternalDistributedMember(NetworkUtils.getIPLiteral(), 12345);
-
- // first make sure we can't add this as a surprise member (bug #44566)
-
- // if the view number isn't being recorded correctly the test will pass but the
- // functionality is broken
- Assert.assertTrue("expected view ID to be greater than zero",
- mgr.getView().getViewId() > 0);
-
- int oldViewId = mbr.getVmViewId();
- mbr.setVmViewId((int) mgr.getView().getViewId() - 1);
- org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
- .info("current membership view is " + mgr.getView());
- org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
- .info("created ID " + mbr + " with view ID " + mbr.getVmViewId());
- sys.getLogWriter()
- .info("<ExpectedException action=add>attempt to add old member</ExpectedException>");
- sys.getLogWriter().info(
- "<ExpectedException action=add>Removing shunned GemFire node</ExpectedException>");
- try {
- boolean accepted = mgr.addSurpriseMember(mbr);
- Assert.assertTrue("member with old ID was not rejected (bug #44566)", !accepted);
- } finally {
- sys.getLogWriter().info(
- "<ExpectedException action=remove>attempt to add old member</ExpectedException>");
- sys.getLogWriter().info(
- "<ExpectedException action=remove>Removing shunned GemFire node</ExpectedException>");
- }
- mbr.setVmViewId(oldViewId);
-
- // now forcibly add it as a surprise member and show that it is reaped
- long gracePeriod = 5000;
- long startTime = System.currentTimeMillis();
- long timeout = ((GMSMembershipManager) mgr).getSurpriseMemberTimeout();
- long birthTime = startTime - timeout + gracePeriod;
- MembershipManagerHelper.addSurpriseMember(sys, mbr, birthTime);
- assertTrue("Member was not a surprise member", mgr.isSurpriseMember(mbr));
-
- if (birthTime < (System.currentTimeMillis() - timeout)) {
- return; // machine is too busy and we didn't get enough CPU to perform more assertions
- }
- assertTrue("Member was incorrectly removed from surprise member set",
- mgr.isSurpriseMember(mbr));
-
- try {
- Thread.sleep((timeout / 3) + gracePeriod);
- } catch (InterruptedException e) {
- fail("test was interrupted", e);
- }
-
- assertTrue("Member was not removed from surprise member set", !mgr.isSurpriseMember(mbr));
-
- } finally {
- if (sys != null && sys.isConnected()) {
- sys.disconnect();
- }
+ InternalDistributedMember mbr =
+ new InternalDistributedMember(NetworkUtils.getIPLiteral(), 12345);
+
+ // first make sure we can't add this as a surprise member (bug #44566)
+
+ // if the view number isn't being recorded correctly the test will pass but the
+ // functionality is broken
- Assert.assertTrue("expected view ID to be greater than zero", mgr.getView().getViewId() > 0);
++ Assert.assertTrue("expected view ID to be greater than zero",
++ mgr.getView().getViewId() > 0);
+
+ int oldViewId = mbr.getVmViewId();
+ mbr.setVmViewId((int) mgr.getView().getViewId() - 1);
+ org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
+ .info("current membership view is " + mgr.getView());
+ org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
+ .info("created ID " + mbr + " with view ID " + mbr.getVmViewId());
+
+ IgnoredException.addIgnoredException("attempt to add old member");
+ IgnoredException.addIgnoredException("Removing shunned GemFire node");
+
+ boolean accepted = mgr.addSurpriseMember(mbr);
+ Assert.assertTrue("member with old ID was not rejected (bug #44566)", !accepted);
+
+ mbr.setVmViewId(oldViewId);
+
+ // now forcibly add it as a surprise member and show that it is reaped
+ long gracePeriod = 5000;
+ long startTime = System.currentTimeMillis();
+ long timeout = ((GMSMembershipManager) mgr).getSurpriseMemberTimeout();
+ long birthTime = startTime - timeout + gracePeriod;
+ MembershipManagerHelper.addSurpriseMember(sys, mbr, birthTime);
+ assertTrue("Member was not a surprise member", mgr.isSurpriseMember(mbr));
+
+ if (birthTime < (System.currentTimeMillis() - timeout)) {
+ return; // machine is too busy and we didn't get enough CPU to perform more assertions
}
+ assertTrue("Member was incorrectly removed from surprise member set",
+ mgr.isSurpriseMember(mbr));
+
+ Thread.sleep((timeout / 3) + gracePeriod);
+
+ assertTrue("Member was not removed from surprise member set", !mgr.isSurpriseMember(mbr));
+
} finally {
- System.getProperties().remove(DistributionConfig.GEMFIRE_PREFIX + "surprise-member-timeout");
+ if (sys != null && sys.isConnected()) {
+ sys.disconnect();
+ }
}
}