GEODE-8670: ReconnectDUnitTest is hiding a NullPointerException (#5744)
* GEODE-8670: ReconnectDUnitTest is hiding a NullPointerException
1. Fixed all of the IgnorableException problems in ReconnectDUnitTest
2. Modified InternalLocator to avoid throwing a NullPointerException and
added a test for this change
* fixing invalid test - had to move it to a new class to use SystemOutRule
the rule wasn't working for some reason when I added it to
InternalLocatorIntegrationTest
* removed use of SystemOutRule, which wasn't working in Stress tests
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/cache30/ReconnectDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/cache30/ReconnectDUnitTest.java
index 2bfb534..315a700 100755
--- a/geode-core/src/distributedTest/java/org/apache/geode/cache30/ReconnectDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/cache30/ReconnectDUnitTest.java
@@ -134,7 +134,8 @@
@Override
public final void postSetUp() throws Exception {
- IgnoredException.addIgnoredException("ForcedDisconnectException||Possible loss of quorum");
+ IgnoredException.addIgnoredException("ForcedDisconnectException");
+ IgnoredException.addIgnoredException("Possible loss of quorum");
locatorPort = AvailablePort.getRandomAvailablePort(AvailablePort.SOCKET);
final int locPort = locatorPort;
Host.getHost(0).getVM(locatorVMNumber).invoke(new SerializableRunnable("start locator") {
@@ -264,8 +265,8 @@
props.put(MAX_NUM_RECONNECT_TRIES, "2");
// props.put("log-file", "autoReconnectVM"+VM.getCurrentVMNum()+"_"+getPID()+".log");
cache = (InternalCache) new CacheFactory(props).create();
- IgnoredException.addIgnoredException(
- "org.apache.geode.ForcedDisconnectException||Possible loss of quorum");
+ IgnoredException.addIgnoredException("org.apache.geode.ForcedDisconnectException");
+ IgnoredException.addIgnoredException("Possible loss of quorum");
Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
myRegion.put("MyKey1", "MyValue1");
@@ -678,7 +679,8 @@
@Test
public void testReconnectWithRoleLoss() throws TimeoutException, RegionExistsException {
-
+ IgnoredException.addIgnoredException(CacheClosedException.class);
+ IgnoredException.addIgnoredException(DistributedSystemDisconnectedException.class);
final String rr1 = "RoleA";
final String rr2 = "RoleB";
final String[] requiredRoles = {rr1, rr2};
@@ -809,6 +811,7 @@
// for the 2014 8.0 release.
@Test
public void testReconnectWithRequiredRoleRegained() throws Throwable {
+ IgnoredException.addIgnoredException(DistributedSystemDisconnectedException.class);
final String rr1 = "RoleA";
// final String rr2 = "RoleB";
@@ -1115,6 +1118,9 @@
*/
@Test
public void testReconnectFailsDueToBadCacheXML() throws Exception {
+ IgnoredException.addIgnoredException(DistributedSystemDisconnectedException.class);
+ IgnoredException.addIgnoredException("Cause parsing to fail");
+ IgnoredException.addIgnoredException("Exception while initializing an instance");
Host host = getHost(0);
VM vm0 = host.getVM(0);
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalLocatorIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalLocatorIntegrationTest.java
index 09efe36..e712508 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalLocatorIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalLocatorIntegrationTest.java
@@ -22,14 +22,17 @@
import java.io.File;
import java.io.IOException;
import java.net.InetAddress;
+import java.nio.charset.Charset;
import java.nio.file.Path;
import java.util.Properties;
+import org.apache.commons.io.FileUtils;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
import org.junit.rules.TemporaryFolder;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
@@ -37,13 +40,16 @@
import org.mockito.quality.Strictness;
import org.apache.geode.distributed.Locator;
-import org.apache.geode.internal.logging.InternalLogWriter;
+import org.apache.geode.distributed.internal.membership.gms.membership.GMSJoinLeave;
import org.apache.geode.internal.security.SecurableCommunicationChannel;
import org.apache.geode.logging.internal.LoggingSession;
public class InternalLocatorIntegrationTest {
@Rule
+ public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
+
+ @Rule
public MockitoRule mockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);
@Rule
@@ -52,12 +58,7 @@
private int port;
@Mock
private LoggingSession loggingSession;
- @Mock
private File logFile;
- @Mock
- private InternalLogWriter logWriter;
- @Mock
- private InternalLogWriter securityLogWriter;
private InetAddress bindAddress;
private String hostnameForClients;
@Mock
@@ -69,6 +70,8 @@
@Before
public void setUp() throws IOException {
+ // set a property to tell membership to create a new cluster
+ System.setProperty(GMSJoinLeave.BYPASS_DISCOVERY_PROPERTY, "true");
port = 0;
hostnameForClients = "";
bindAddress = null;
@@ -89,6 +92,24 @@
}
@Test
+ public void startedLocatorDoesNotAttemptReconnect() throws IOException, InterruptedException {
+ // start a locator that's not part of a cluster
+ final boolean joinCluster = false;
+ internalLocator = InternalLocator.startLocator(port, logFile, null,
+ null, bindAddress, joinCluster,
+ distributedSystemProperties, hostnameForClients, workingDirectory);
+ port = internalLocator.getPort();
+ // the locator shouldn't attempt a reconnect because it's not part of a cluster
+ internalLocator.stoppedForReconnect = true;
+ assertThat(internalLocator.attemptReconnect()).isFalse();
+ String output = FileUtils.readFileToString(logFile, Charset.defaultCharset());
+ assertThat(output).isNotEmpty();
+ assertThat(output).contains(InternalLocator.IGNORING_RECONNECT_REQUEST);
+ }
+
+
+
+ @Test
public void constructs() {
when(distributionConfig.getLogFile()).thenReturn(logFile);
when(distributionConfig.getLocators()).thenReturn("");
@@ -97,7 +118,7 @@
assertThatCode(() -> {
internalLocator =
- new InternalLocator(port, loggingSession, logFile, logWriter, securityLogWriter,
+ new InternalLocator(port, loggingSession, logFile, null, null,
bindAddress, hostnameForClients, distributedSystemProperties, distributionConfig,
workingDirectory);
}).doesNotThrowAnyException();
@@ -105,8 +126,8 @@
@Test
public void restartingClusterConfigurationDoesNotThrowException() throws IOException {
- internalLocator = InternalLocator.startLocator(port, logFile, logWriter,
- securityLogWriter, bindAddress, true,
+ internalLocator = InternalLocator.startLocator(port, logFile, null,
+ null, bindAddress, true,
distributedSystemProperties, hostnameForClients, workingDirectory);
port = internalLocator.getPort();
internalLocator.stop(true, true, false);
@@ -119,8 +140,8 @@
@Test
public void startedLocatorIsRunning() throws IOException {
- internalLocator = InternalLocator.startLocator(port, logFile, logWriter,
- securityLogWriter, bindAddress, true,
+ internalLocator = InternalLocator.startLocator(port, logFile, null,
+ null, bindAddress, true,
distributedSystemProperties, hostnameForClients, workingDirectory);
port = internalLocator.getPort();
@@ -129,8 +150,8 @@
@Test
public void startedLocatorHasLocator() throws IOException {
- internalLocator = InternalLocator.startLocator(port, logFile, logWriter,
- securityLogWriter, bindAddress, true,
+ internalLocator = InternalLocator.startLocator(port, logFile, null,
+ null, bindAddress, true,
distributedSystemProperties, hostnameForClients, workingDirectory);
port = internalLocator.getPort();
@@ -139,8 +160,8 @@
@Test
public void stoppedLocatorIsStopped() throws IOException {
- internalLocator = InternalLocator.startLocator(port, logFile, logWriter,
- securityLogWriter, bindAddress, true,
+ internalLocator = InternalLocator.startLocator(port, null, null,
+ null, bindAddress, true,
distributedSystemProperties, hostnameForClients, workingDirectory);
port = internalLocator.getPort();
@@ -151,8 +172,8 @@
@Test
public void stoppedLocatorDoesNotHaveLocator() throws IOException {
- internalLocator = InternalLocator.startLocator(port, logFile, logWriter,
- securityLogWriter, bindAddress, true,
+ internalLocator = InternalLocator.startLocator(port, null, null,
+ null, bindAddress, true,
distributedSystemProperties, hostnameForClients, workingDirectory);
port = internalLocator.getPort();
@@ -170,8 +191,8 @@
// so this would demonstrate that we would throw the exception when we encounter an error when
// calling InternalLocator.startConfigurationPersistenceService
properties.put("load-cluster-configuration-from-dir", "true");
- assertThatThrownBy(() -> InternalLocator.startLocator(port, logFile, logWriter,
- securityLogWriter, bindAddress, true,
+ assertThatThrownBy(() -> InternalLocator.startLocator(port, null, null,
+ null, bindAddress, true,
properties, hostnameForClients, workingDirectory)).isInstanceOf(RuntimeException.class);
assertThat(InternalLocator.hasLocator()).isFalse();
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
index f497776..e011936 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
@@ -156,6 +156,8 @@
*/
public static final String LOCATORS_PREFERRED_AS_COORDINATORS =
GEMFIRE_PREFIX + "disable-floating-coordinator";
+ static final String IGNORING_RECONNECT_REQUEST =
+ "ignoring request to reconnect to the cluster - there is no DistributedSystem available to reconnect.";
/**
* the locator hosted by this JVM. As of 7.0 it is a singleton.
@@ -178,7 +180,8 @@
/**
* whether the locator was stopped during forced-disconnect processing but a reconnect will occur
*/
- private volatile boolean stoppedForReconnect;
+ @VisibleForTesting
+ volatile boolean stoppedForReconnect;
private volatile boolean reconnected;
/**
@@ -1099,12 +1102,18 @@
*
* @return true if able to reconnect the locator to the new distributed system
*/
- private boolean attemptReconnect() throws InterruptedException, IOException {
+ @VisibleForTesting
+ boolean attemptReconnect() throws InterruptedException, IOException {
boolean restarted = false;
if (stoppedForReconnect) {
- logger.info("attempting to restart locator");
boolean tcpServerStarted = false;
InternalDistributedSystem system = internalDistributedSystem;
+ if (system == null) {
+ logger.info(
+ IGNORING_RECONNECT_REQUEST);
+ return false;
+ }
+ logger.info("attempting to restart locator");
long waitTime = system.getConfig().getMaxWaitTimeForReconnect() / 2;
QuorumChecker quorumChecker = null;
