This is an automated email from the ASF dual-hosted git repository.
boglesby pushed a commit to branch support/1.15
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.15 by this push:
new 5a32ec0 GEODE-9910: Stop embedded locator after failed start (#7393)
5a32ec0 is described below
commit 5a32ec00bdbd949bc473322c3643f3d96165d62d
Author: Barry Oglesby <[email protected]>
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)
---
...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 e8cecc1..d56a19b 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
@@ -652,9 +652,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) {
@@ -754,7 +756,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();
@@ -809,6 +811,9 @@ public class InternalDistributedSystem extends
DistributedSystem
// was created
InternalInstantiator.logInstantiators();
} catch (RuntimeException ex) {
+ if (startedLocator != null) {
+ stopLocator();
+ }
config.close();
throw ex;
}
@@ -906,9 +911,7 @@ public class InternalDistributedSystem extends
DistributedSystem
startedPeerLocation = true;
} finally {
if (!startedPeerLocation) {
- startedLocator.stop();
- startedLocator = null;
- membershipLocator = null;
+ stopLocator();
}
}
} catch (IOException e) {
@@ -937,6 +940,12 @@ public class InternalDistributedSystem extends
DistributedSystem
}
}
+ private void stopLocator() {
+ startedLocator.stop();
+ startedLocator = null;
+ membershipLocator = null;
+ }
+
/**
* Used by DistributionManager to fix bug 33362
*/
@@ -2973,6 +2982,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;
@@ -2982,6 +3007,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;
@@ -2998,6 +3026,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.
*/
@@ -3016,7 +3050,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;
