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 2015/12/01 17:35:31 UTC
[2/2] incubator-geode git commit: corrections for GMSHealthMonitor
from an impromptu code review
corrections for GMSHealthMonitor from an impromptu code review
Engineers working on the GEODE-77 effort reviewed some of the final-check
code in GMSHealthMonitor and came up with these corrections.
This commit also includes some test clean-up that I hope will help with
GEODE-589.
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/253549d5
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/253549d5
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/253549d5
Branch: refs/heads/develop
Commit: 253549d555abc5cb8bb12595a9c4ac09052ad94d
Parents: e4448ad
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Tue Dec 1 08:35:15 2015 -0800
Committer: Bruce Schuchardt <bs...@pivotal.io>
Committed: Tue Dec 1 08:35:15 2015 -0800
----------------------------------------------------------------------
.../membership/gms/fd/GMSHealthMonitor.java | 13 ++-----------
.../gms/messenger/JGroupsMessenger.java | 8 +++++++-
.../internal/cache/xmlcache/CacheCreation.java | 7 ++++++-
.../cache30/ClientMembershipDUnitTest.java | 12 +++++++++---
.../gemfire/cache30/ClientServerCCEDUnitTest.java | 2 +-
...istributedAckPersistentRegionCCEDUnitTest.java | 18 +++++++++++++++---
.../cache30/DistributedAckRegionDUnitTest.java | 2 +-
.../DistributedNoAckRegionCCEDUnitTest.java | 11 +++++++++--
.../gemfire/cache30/MultiVMRegionTestCase.java | 4 ++--
.../gemfire/cache30/TXDistributedDUnitTest.java | 2 +-
.../cache/locks/TXLockServiceDUnitTest.java | 2 +-
.../src/test/java/dunit/DistributedTestCase.java | 9 +++++++++
.../java/dunit/standalone/ProcessManager.java | 8 --------
13 files changed, 63 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/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 1c005cc..bd60236 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
@@ -441,13 +441,9 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler {
}
TimeStamp ts = memberTimeStamps.get(member);
if (pingResp.getResponseMsg() == null) {
- // double check the activity map
if (isStopping) {
return true;
}
- if (checkRecentActivity(member)) {
- return true;
- }
logger.trace("no heartbeat response received from {} and no recent activity", member);
return false;
} else {
@@ -486,10 +482,6 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler {
*/
private boolean doTCPCheckMember(InternalDistributedMember suspectMember, int port) {
Socket clientSocket = null;
- // first check for a recent timestamp
- if (checkRecentActivity(suspectMember)) {
- return true;
- }
try {
logger.debug("Checking member {} with TCP socket connection {}:{}.", suspectMember, suspectMember.getInetAddress(), port);
clientSocket = SocketCreator.getDefaultInstance().connect(suspectMember.getInetAddress(), port,
@@ -516,10 +508,10 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler {
return true;
} else {
//received ERROR
- return checkRecentActivity(suspectMember);
+ return false;
}
} else {// cannot establish TCP connection with suspect member
- return checkRecentActivity(suspectMember);
+ return false;
}
} catch (SocketTimeoutException e) {
logger.debug("tcp/ip connection timed out");
@@ -1192,7 +1184,6 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler {
}
if (!failed) {
logger.info("Final check passed");
- contactedBy(mbr);
}
// whether it's alive or not, at this point we allow it to
// be watched again
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
index b37c930..91c32ae 100755
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
@@ -551,7 +551,13 @@ public class JGroupsMessenger implements Messenger {
}
filterOutgoingMessage(msg);
-
+
+ // JGroupsMessenger does not support direct-replies, so register
+ // the message's processor if necessary
+ if ((msg instanceof DirectReplyMessage) && msg.isDirectAck() && msg.getProcessorId() <= 0) {
+ ((DirectReplyMessage)msg).registerProcessor();
+ }
+
InternalDistributedMember[] destinations = msg.getRecipients();
boolean allDestinations = msg.forAll();
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java
index ed67ca4..f7063bc 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java
@@ -226,6 +226,11 @@ public class CacheCreation implements InternalCache {
public CacheCreation() {
this(false);
}
+
+ /** clear thread locals that may have been set by previous uses of CacheCreation */
+ public static void clearThreadLocals() {
+ createInProgress = new ThreadLocal<>();
+ }
/**
* @param forParsing if true then this creation is used for parsing xml;
@@ -357,7 +362,7 @@ public class CacheCreation implements InternalCache {
* Used from PoolManager to defer to CacheCreation as a manager of pools.
* @since 5.7
*/
- private static final ThreadLocal createInProgress = new ThreadLocal();
+ private static ThreadLocal createInProgress = new ThreadLocal();
/**
* Returns null if the current thread is not doing a CacheCreation create.
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientMembershipDUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientMembershipDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientMembershipDUnitTest.java
index d9e9f4c..76b5b75 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientMembershipDUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientMembershipDUnitTest.java
@@ -1328,12 +1328,16 @@ public class ClientMembershipDUnitTest extends ClientServerTestCase {
Properties config = new Properties();
config.setProperty(DistributionConfig.MCAST_PORT_NAME, "0");
config.setProperty(DistributionConfig.LOCATORS_NAME, "");
+ // 11/30/2015 this test is periodically failing during distributedTest runs
+ // so we are setting the log-level to fine to figure out what's going on
+ config.setProperty(DistributionConfig.LOG_LEVEL_NAME, "fine");
getSystem(config);
AttributesFactory factory = new AttributesFactory();
factory.setScope(Scope.LOCAL);
- ClientServerTestCase.configureConnectionPool(factory, getServerHostName(host), ports, true, -1, -1, null);
+ Pool p = ClientServerTestCase.configureConnectionPool(factory, getServerHostName(host), ports, true, -1, -1, null);
createRegion(name, factory.create());
assertNotNull(getRootRegion().getSubregion(name));
+ assertTrue(p.getServers().size() > 0);
}
};
@@ -1376,8 +1380,10 @@ public class ClientMembershipDUnitTest extends ClientServerTestCase {
String connectedClient = (String)iter.next();
getLogWriter().info("[testGetConnectedClients] checking for client " + connectedClient);
assertTrue(clientMemberIds.contains(connectedClient));
- getLogWriter().info("[testGetConnectedClients] count for connectedClient: " +
- connectedClients.get(connectedClient));
+ Object[] result = (Object[])connectedClients.get(connectedClient);
+ getLogWriter().info("[testGetConnectedClients] result: " +
+ (result==null? "none"
+ : String.valueOf(result[0])+"; connections="+result[1]));
}
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientServerCCEDUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientServerCCEDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientServerCCEDUnitTest.java
index 9847d7b..784d0c8 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientServerCCEDUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/ClientServerCCEDUnitTest.java
@@ -613,7 +613,7 @@ public class ClientServerCCEDUnitTest extends CacheTestCase {
// bug #50683 - secondary durable queue retains all GC messages
cf.set("durable-client-id", ""+vm.getPid());
cf.set("durable-client-timeout", "" + 200);
- cf.set("log-level", "fine");
+ cf.set("log-level", getDUnitLogLevel());
ClientCache cache = getClientCache(cf);
ClientRegionFactory crf = cache.createClientRegionFactory(ClientRegionShortcut.CACHING_PROXY);
crf.setConcurrencyChecksEnabled(ccEnabled);
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckPersistentRegionCCEDUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckPersistentRegionCCEDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckPersistentRegionCCEDUnitTest.java
index 3994b49..b17c3c3 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckPersistentRegionCCEDUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckPersistentRegionCCEDUnitTest.java
@@ -36,17 +36,17 @@ import com.gemstone.gemfire.internal.cache.LocalRegion;
import com.gemstone.gemfire.internal.cache.tier.sockets.Part;
import com.gemstone.gemfire.internal.cache.versions.VersionTag;
import com.gemstone.gemfire.internal.i18n.LocalizedStrings;
-
+
import dunit.AsyncInvocation;
import dunit.Host;
import dunit.SerializableCallable;
import dunit.SerializableRunnable;
import dunit.VM;
import dunit.DistributedTestCase.WaitCriterion;
-
+
import java.io.IOException;
import java.util.Map;
-
+
import junit.framework.Assert;
/**
@@ -73,6 +73,18 @@ public class DistributedAckPersistentRegionCCEDUnitTest extends DistributedAckRe
return factory.create();
}
+// public void testClearWithConcurrentEventsAsync() throws Exception {
+// int end = 100;
+// for (int i=0; i<end; i++) {
+// System.out.println("Starting run #" + i);
+// super.testClearWithConcurrentEventsAsync();
+// if (i<(end-1)) {
+// tearDown();
+// setUp();
+// }
+// }
+// }
+
public void testClearOnNonReplicateWithConcurrentEvents() {}
public void testConcurrentEventsOnNonReplicatedRegion() {}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionDUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionDUnitTest.java
index d3ef4ac..cf258b4 100755
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionDUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedAckRegionDUnitTest.java
@@ -74,7 +74,7 @@ public class DistributedAckRegionDUnitTest extends MultiVMRegionTestCase {
public Properties getDistributedSystemProperties() {
Properties p = new Properties();
p.put(DistributionConfig.STATISTIC_SAMPLING_ENABLED_NAME, "true");
- p.put(DistributionConfig.LOG_LEVEL_NAME, "config");
+ p.put(DistributionConfig.LOG_LEVEL_NAME, getDUnitLogLevel());
return p;
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedNoAckRegionCCEDUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedNoAckRegionCCEDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedNoAckRegionCCEDUnitTest.java
index 791341e..4c77562 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedNoAckRegionCCEDUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/DistributedNoAckRegionCCEDUnitTest.java
@@ -266,8 +266,15 @@ public class DistributedNoAckRegionCCEDUnitTest extends
}
- public void testTombstones() {
- versionTestTombstones();
+ public void testTombstones() throws Exception {
+// for (int i=0; i<1000; i++) {
+// System.out.println("starting run #"+i);
+ versionTestTombstones();
+// if (i < 999) {
+// tearDown();
+// setUp();
+// }
+// }
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
index b67b1f9..b995494 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
@@ -8229,12 +8229,12 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
return !o1.equals(o2);
}
- protected AsyncInvocation performOps4ClearWithConcurrentEvents(VM vm, final int opNum) {
+ protected AsyncInvocation performOps4ClearWithConcurrentEvents(VM vm, final int msToRun) {
SerializableRunnable performOps = new SerializableRunnable("perform concurrent ops") {
public void run() {
try {
boolean includeClear = true;
- doOpsLoop(opNum, includeClear);
+ doOpsLoop(msToRun, includeClear);
} catch (CacheException e) {
fail("while performing concurrent operations", e);
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TXDistributedDUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TXDistributedDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TXDistributedDUnitTest.java
index b318841..c0034cd 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TXDistributedDUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/TXDistributedDUnitTest.java
@@ -522,7 +522,7 @@ public class TXDistributedDUnitTest extends CacheTestCase {
@Override
public Properties getDistributedSystemProperties() {
Properties p = super.getDistributedSystemProperties();
- p.put("log-level", "fine");
+ p.put("log-level", getDUnitLogLevel());
return p;
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/locks/TXLockServiceDUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/locks/TXLockServiceDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/locks/TXLockServiceDUnitTest.java
index 002854e..bc6fad1 100755
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/locks/TXLockServiceDUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/locks/TXLockServiceDUnitTest.java
@@ -661,7 +661,7 @@ public class TXLockServiceDUnitTest extends DistributedTestCase {
public Properties getDistributedSystemProperties() {
Properties props = super.getDistributedSystemProperties();
- props.setProperty("log-level", "fine");
+ props.setProperty("log-level", getDUnitLogLevel());
return props;
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/dunit/DistributedTestCase.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/dunit/DistributedTestCase.java b/gemfire-core/src/test/java/dunit/DistributedTestCase.java
index 7db4033..a3d4785 100755
--- a/gemfire-core/src/test/java/dunit/DistributedTestCase.java
+++ b/gemfire-core/src/test/java/dunit/DistributedTestCase.java
@@ -49,6 +49,7 @@ import com.gemstone.gemfire.cache.hdfs.internal.HDFSStoreImpl;
import com.gemstone.gemfire.cache.hdfs.internal.hoplog.HoplogConfig;
import com.gemstone.gemfire.cache.query.QueryTestUtils;
import com.gemstone.gemfire.cache.query.internal.QueryObserverHolder;
+import com.gemstone.gemfire.cache30.ClientServerTestCase;
import com.gemstone.gemfire.cache30.GlobalLockingDUnitTest;
import com.gemstone.gemfire.cache30.MultiVMRegionTestCase;
import com.gemstone.gemfire.cache30.RegionTestCase;
@@ -76,6 +77,7 @@ import com.gemstone.gemfire.internal.cache.tier.InternalClientMembership;
import com.gemstone.gemfire.internal.cache.tier.sockets.CacheServerTestUtil;
import com.gemstone.gemfire.internal.cache.tier.sockets.ClientProxyMembershipID;
import com.gemstone.gemfire.internal.cache.tier.sockets.DataSerializerPropogationDUnitTest;
+import com.gemstone.gemfire.internal.cache.xmlcache.CacheCreation;
import com.gemstone.gemfire.internal.logging.InternalLogWriter;
import com.gemstone.gemfire.internal.logging.LocalLogWriter;
import com.gemstone.gemfire.internal.logging.LogService;
@@ -756,6 +758,9 @@ public abstract class DistributedTestCase extends TestCase implements java.io.Se
closeCache();
SocketCreator.resolve_dns = true;
+ CacheCreation.clearThreadLocals();
+ System.getProperties().remove("gemfire.log-level");
+ System.getProperties().remove("jgroups.resolve_dns");
InitialImageOperation.slowImageProcessing = 0;
DistributionMessageObserver.setInstance(null);
QueryTestUtils.setCache(null);
@@ -767,10 +772,14 @@ public abstract class DistributedTestCase extends TestCase implements java.io.Se
MultiVMRegionTestCase.CCRegion = null;
InternalClientMembership.unregisterAllListeners();
ClientStatsManager.cleanupForTests();
+ ClientServerTestCase.AUTO_LOAD_BALANCE = false;
unregisterInstantiatorsInThisVM();
DistributionMessageObserver.setInstance(null);
QueryObserverHolder.reset();
DiskStoreObserver.setInstance(null);
+ System.getProperties().remove("gemfire.log-level");
+ System.getProperties().remove("jgroups.resolve_dns");
+
if (InternalDistributedSystem.systemAttemptingReconnect != null) {
InternalDistributedSystem.systemAttemptingReconnect.stopReconnecting();
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/253549d5/gemfire-core/src/test/java/dunit/standalone/ProcessManager.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/dunit/standalone/ProcessManager.java b/gemfire-core/src/test/java/dunit/standalone/ProcessManager.java
index b6a4493..60ac04d 100644
--- a/gemfire-core/src/test/java/dunit/standalone/ProcessManager.java
+++ b/gemfire-core/src/test/java/dunit/standalone/ProcessManager.java
@@ -34,11 +34,8 @@ import java.util.Map;
import org.apache.commons.io.FileUtils;
-import com.gemstone.gemfire.distributed.internal.DistributionConfig;
import com.gemstone.gemfire.internal.FileUtil;
import com.gemstone.gemfire.internal.logging.LogService;
-import com.gemstone.gemfire.internal.sequencelog.EntryLogger;
-import com.gemstone.gemfire.internal.sequencelog.SequenceLoggerImpl;
import dunit.RemoteDUnitVMIF;
@@ -47,8 +44,6 @@ import dunit.RemoteDUnitVMIF;
*
*/
public class ProcessManager {
- public static final boolean IS_WINDOWS = System.getProperty("os.name").contains("Windows");
-
private int namingPort;
private Map<Integer, ProcessHolder> processes = new HashMap<Integer, ProcessHolder>();
private File log4jConfig;
@@ -179,9 +174,6 @@ public class ProcessManager {
"-Dgemfire.DEFAULT_MAX_OPLOG_SIZE=10",
"-Dgemfire.disallowMcastDefaults=true",
"-ea",
- // use IPv4 on Windows
- // see https://github.com/belaban/JGroups/wiki/FAQ
-// (IS_WINDOWS?"-Djava.net.preferIPv4Stack=true":""),
agent,
"dunit.standalone.ChildVM"
};