This is an automated email from the ASF dual-hosted git repository.
boglesby pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.12 by this push:
new f435ec9 GEODE-9910: Stop embedded locator after failed start (#7393)
f435ec9 is described below
commit f435ec93ab3b8796bed39d3b67729449cb9a8f47
Author: Barry Oglesby <bogle...@users.noreply.github.com>
AuthorDate: Mon Mar 7 08:50:19 2022 -1000
GEODE-9910: Stop embedded locator after failed start (#7393)
The start-locator property causes a locator to be started when the
InternalDistributedSystem is initialized. The initialize method creates
the locator and then creates a ClusterDistributionManager. If the
creation of the ClusterDistributionManager failed, the started locator
was not stopped. This change addresses that by stopping the locator if
an exception occurs.
(cherry picked from commit 72665b1ec5c6a6b91d0d6c57e997c23033578c58)
(cherry picked from commit 5a32ec00bdbd949bc473322c3643f3d96165d62d)
(cherry picked from commit 53e2b2e07a6312fa5307265d7e418e65667671cd)
(cherry picked from commit 490900535b1fc1582be63a4eed266e42cb170213)
---
...nalDistributedSystemBuilderIntegrationTest.java | 44 +++++++++++++++++++-
.../internal/InternalDistributedSystem.java | 48 ++++++++++++++++++----
2 files changed, 84 insertions(+), 8 deletions(-)
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalDistributedSystemBuilderIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalDistributedSystemBuilderIntegrationTest.java
index 5bb5ebe..971859a 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalDistributedSystemBuilderIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/InternalDistributedSystemBuilderIntegrationTest.java
@@ -14,8 +14,11 @@
*/
package org.apache.geode.distributed.internal;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.NAME;
+import static
org.apache.geode.distributed.ConfigurationProperties.START_LOCATOR;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -26,6 +29,11 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import org.apache.geode.SystemConnectException;
+import org.apache.geode.distributed.Locator;
+import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.distributed.internal.membership.api.MembershipLocator;
+import org.apache.geode.internal.AvailablePortHelper;
import org.apache.geode.metrics.internal.MetricsService;
import org.apache.geode.security.PostProcessor;
import org.apache.geode.security.SecurityManager;
@@ -43,7 +51,9 @@ public class InternalDistributedSystemBuilderIntegrationTest {
@After
public void tearDown() {
- system.disconnect();
+ if (system != null) {
+ system.disconnect();
+ }
}
@Test
@@ -76,4 +86,36 @@ public class InternalDistributedSystemBuilderIntegrationTest
{
assertThat(system.getSecurityService().getPostProcessor())
.isSameAs(thePostProcessor);
}
+
+ @Test
+ public void buildThatStartsLocatorAndFailsThenStopsLocator() {
+ // Create properties the cause the locator to be started
+ int locatorPort = AvailablePortHelper.getRandomAvailableTCPPort();
+ Properties configProperties = new Properties();
+ configProperties.setProperty(MCAST_PORT, "0");
+ configProperties.setProperty(START_LOCATOR, "localhost[" + locatorPort +
"]");
+
+ // Create a Builder with the TestClusterDistributionManagerConstructor
+ InternalDistributedSystem.Builder builder =
+ new InternalDistributedSystem.Builder(configProperties,
metricsSessionBuilder)
+ .setClusterDistributionManagerConstructor(
+ new TestClusterDistributionManagerConstructor());
+
+ // Assert that attempting to build the InternalDistributedSystem throws the
+ // SystemConnectException
+
assertThatExceptionOfType(SystemConnectException.class).isThrownBy(builder::build);
+
+ // Assert that no locator exists after the build attempt
+ assertThat(Locator.getLocator()).isNull();
+ }
+
+ static class TestClusterDistributionManagerConstructor implements
+ InternalDistributedSystem.ClusterDistributionManagerConstructor {
+
+ @Override
+ public ClusterDistributionManager create(InternalDistributedSystem system,
+ MembershipLocator<InternalDistributedMember> membershipLocator) {
+ throw new SystemConnectException("Problem starting up membership
services");
+ }
+ }
}
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
index ddf99fe..da1e92a 100644
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
@@ -651,9 +651,11 @@ public class InternalDistributedSystem extends
DistributedSystem
/**
* Initializes this connection to a distributed system with the current
configuration state.
*/
- private void initialize(SecurityManager securityManager, PostProcessor
postProcessor,
+ @VisibleForTesting
+ void initialize(SecurityManager securityManager, PostProcessor postProcessor,
MetricsService.Builder metricsServiceBuilder,
- final MembershipLocator<InternalDistributedMember> membershipLocatorArg)
{
+ final MembershipLocator<InternalDistributedMember> membershipLocatorArg,
+ ClusterDistributionManagerConstructor
clusterDistributionManagerConstructor) {
if (originalConfig.getLocators().equals("")) {
if (originalConfig.getMcastPort() != 0) {
@@ -753,7 +755,7 @@ public class InternalDistributedSystem extends
DistributedSystem
if (!isLoner) {
try {
- dm = ClusterDistributionManager.create(this, membershipLocator);
+ dm = clusterDistributionManagerConstructor.create(this,
membershipLocator);
// fix bug #46324
if (InternalLocator.hasLocator()) {
InternalLocator internalLocator = InternalLocator.getLocator();
@@ -808,6 +810,9 @@ public class InternalDistributedSystem extends
DistributedSystem
// was created
InternalInstantiator.logInstantiators();
} catch (RuntimeException ex) {
+ if (startedLocator != null) {
+ stopLocator();
+ }
config.close();
throw ex;
}
@@ -907,9 +912,7 @@ public class InternalDistributedSystem extends
DistributedSystem
startedPeerLocation = true;
} finally {
if (!startedPeerLocation) {
- startedLocator.stop();
- startedLocator = null;
- membershipLocator = null;
+ stopLocator();
}
}
} catch (IOException e) {
@@ -938,6 +941,12 @@ public class InternalDistributedSystem extends
DistributedSystem
}
}
+ private void stopLocator() {
+ startedLocator.stop();
+ startedLocator = null;
+ membershipLocator = null;
+ }
+
/**
* Used by DistributionManager to fix bug 33362
*/
@@ -2968,6 +2977,22 @@ public class InternalDistributedSystem extends
DistributedSystem
};
}
+ @FunctionalInterface
+ @VisibleForTesting
+ interface ClusterDistributionManagerConstructor {
+ ClusterDistributionManager create(InternalDistributedSystem system,
+ final MembershipLocator<InternalDistributedMember> membershipLocator);
+ }
+
+ private static class DefaultClusterDistributionManagerConstructor
+ implements ClusterDistributionManagerConstructor {
+ @Override
+ public ClusterDistributionManager create(InternalDistributedSystem system,
+ final MembershipLocator<InternalDistributedMember> membershipLocator) {
+ return ClusterDistributionManager.create(system, membershipLocator);
+ }
+ }
+
public static class Builder {
private final Properties configProperties;
@@ -2977,6 +3002,9 @@ public class InternalDistributedSystem extends
DistributedSystem
private MembershipLocator<InternalDistributedMember> locator;
+ private ClusterDistributionManagerConstructor
clusterDistributionManagerConstructor =
+ new DefaultClusterDistributionManagerConstructor();
+
public Builder(Properties configProperties, MetricsService.Builder
metricsServiceBuilder) {
this.configProperties = configProperties;
this.metricsServiceBuilder = metricsServiceBuilder;
@@ -2993,6 +3021,12 @@ public class InternalDistributedSystem extends
DistributedSystem
return this;
}
+ public Builder setClusterDistributionManagerConstructor(
+ ClusterDistributionManagerConstructor
clusterDistributionManagerConstructor) {
+ this.clusterDistributionManagerConstructor =
clusterDistributionManagerConstructor;
+ return this;
+ }
+
/**
* Builds and initializes new instance of InternalDistributedSystem.
*/
@@ -3011,7 +3045,7 @@ public class InternalDistributedSystem extends
DistributedSystem
FunctionStatsManager::new);
newSystem
.initialize(securityConfig.getSecurityManager(),
securityConfig.getPostProcessor(),
- metricsServiceBuilder, locator);
+ metricsServiceBuilder, locator,
clusterDistributionManagerConstructor);
notifyConnectListeners(newSystem);
stopThreads = false;
return newSystem;
