This is an automated email from the ASF dual-hosted git repository. boglesby pushed a commit to branch support/1.13 in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.13 by this push: new 732b735 GEODE-9910: Stop embedded locator after failed start (#7393) 732b735 is described below commit 732b735c1229ffda1e9d30f9cd23ccf6db34a999 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) --- ...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 0807f8b..2209843 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; } @@ -904,9 +909,7 @@ public class InternalDistributedSystem extends DistributedSystem startedPeerLocation = true; } finally { if (!startedPeerLocation) { - startedLocator.stop(); - startedLocator = null; - membershipLocator = null; + stopLocator(); } } } catch (IOException e) { @@ -935,6 +938,12 @@ public class InternalDistributedSystem extends DistributedSystem } } + private void stopLocator() { + startedLocator.stop(); + startedLocator = null; + membershipLocator = null; + } + /** * Used by DistributionManager to fix bug 33362 */ @@ -2965,6 +2974,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; @@ -2974,6 +2999,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; @@ -2990,6 +3018,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. */ @@ -3008,7 +3042,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;