GEODE-8099: add dlock around cms create/delete operations.

apache · Jun 1, 2020 · 1344f6c · 1344f6c
1 parent 57cc3c7
commit 1344f6c
Show file tree

Hide file tree

Showing 5 changed files with 207 additions and 155 deletions.
diff --git a/...he/geode/distributed/internal/InternalLocatorClusterManagementServiceIntegrationTest.java b/...he/geode/distributed/internal/InternalLocatorClusterManagementServiceIntegrationTest.java
@@ -28,6 +28,7 @@
 import java.util.Optional;
 
 import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 
 import org.apache.geode.cache.RegionShortcut;
@@ -43,11 +44,11 @@
 public class InternalLocatorClusterManagementServiceIntegrationTest {
 
   private InternalLocator internalLocator; // the instance under test
-  private DistributionConfigImpl distributionConfig = mock(DistributionConfigImpl.class);
-  private InternalCacheForClientAccess cache = mock(InternalCacheForClientAccess.class);
-  private BaseManagementService managementService = mock(BaseManagementService.class);
-  private AgentUtil agentUtil = mock(AgentUtil.class);
-  private HttpService httpService = mock(HttpService.class);
+  private DistributionConfigImpl distributionConfig;
+  private InternalCacheForClientAccess cache;
+  private BaseManagementService managementService;
+  private AgentUtil agentUtil;
+  private HttpService httpService;
 
   @After
   public void tearDown() {
@@ -56,13 +57,38 @@ public void tearDown() {
     }
   }
 
+  @Before
+  public void setup() throws URISyntaxException {
+    distributionConfig = mock(DistributionConfigImpl.class);
+    cache = mock(InternalCacheForClientAccess.class);
+    managementService = mock(BaseManagementService.class);
+    agentUtil = mock(AgentUtil.class);
+    httpService = mock(HttpService.class);
+
+    LoggingSession loggingSession = mock(LoggingSession.class);
+    when(distributionConfig.getJmxManager()).thenReturn(true);
+    when(distributionConfig.getJmxManagerPort())
+        .thenReturn(AvailablePortHelper.getRandomAvailableTCPPort());
+    when(distributionConfig.getLocators()).thenReturn("");
+    when(distributionConfig.getSecurableCommunicationChannels())
+        .thenReturn(new SecurableCommunicationChannel[] {});
+    when(distributionConfig.getSecurityAuthTokenEnabledComponents()).thenReturn(new String[] {});
+    internalLocator = new InternalLocator(0, loggingSession, null, null, null, null,
+        null, null, distributionConfig, null);
+    InternalRegionFactory regionFactory = mock(InternalRegionFactory.class);
+    when(cache.createInternalRegionFactory(RegionShortcut.REPLICATE)).thenReturn(regionFactory);
+    when(cache.getOptionalService(HttpService.class))
+        .thenReturn(Optional.of(httpService));
+    when(cache.getCacheForProcessingClientRequests()).thenReturn(cache);
+    BaseManagementService.setManagementService(cache, managementService);
+    URI uri = new URI("file", "/management.war", null);
+    when(agentUtil.findWarLocation("geode-web-management")).thenReturn(uri);
+  }
+
   @Test
   public void startClusterManagementServiceWithRestServiceEnabledInvokesStartManager()
       throws Exception {
-    createInternalLocator();
-    setupForStartClusterManagementService();
     when(distributionConfig.getEnableManagementRestService()).thenReturn(true);
-
     internalLocator.startClusterManagementService(cache, agentUtil);
 
     verify(managementService).startManager();
@@ -72,8 +98,6 @@ public void startClusterManagementServiceWithRestServiceEnabledInvokesStartManag
   @Test
   public void startClusterManagementServiceWithRunningManagerNeverInvokesStartManager()
       throws Exception {
-    createInternalLocator();
-    setupForStartClusterManagementService();
     when(distributionConfig.getEnableManagementRestService()).thenReturn(true);
     when(managementService.isManager()).thenReturn(true);
 
@@ -87,10 +111,7 @@ public void startClusterManagementServiceWithRunningManagerNeverInvokesStartMana
   @Test
   public void startClusterManagementServiceWithRestServiceDisabledNeverInvokesStartManager()
       throws Exception {
-    createInternalLocator();
-    setupForStartClusterManagementService();
     when(distributionConfig.getEnableManagementRestService()).thenReturn(false);
-
     internalLocator.startClusterManagementService(cache, agentUtil);
 
     verify(distributionConfig).getEnableManagementRestService();
@@ -101,8 +122,6 @@ public void startClusterManagementServiceWithRestServiceDisabledNeverInvokesStar
   @Test
   public void startClusterManagementServiceWithRestServiceEnabledDoesNotThrowWhenStartManagerThrows()
       throws Exception {
-    createInternalLocator();
-    setupForStartClusterManagementService();
     when(distributionConfig.getEnableManagementRestService()).thenReturn(true);
     RuntimeException startManagerEx = new RuntimeException("startManager failed");
     doThrow(startManagerEx).when(managementService).startManager();
@@ -113,28 +132,4 @@ public void startClusterManagementServiceWithRestServiceEnabledDoesNotThrowWhenS
     verify(httpService, never()).addWebApplication(eq("/management"), any(), any());
   }
 
-  private void createInternalLocator() {
-    LoggingSession loggingSession = mock(LoggingSession.class);
-    when(distributionConfig.getJmxManager()).thenReturn(true);
-    when(distributionConfig.getJmxManagerPort())
-        .thenReturn(AvailablePortHelper.getRandomAvailableTCPPort());
-    when(distributionConfig.getLocators()).thenReturn("");
-    when(distributionConfig.getSecurableCommunicationChannels())
-        .thenReturn(new SecurableCommunicationChannel[] {});
-    when(distributionConfig.getSecurityAuthTokenEnabledComponents()).thenReturn(new String[] {});
-    internalLocator = new InternalLocator(0, loggingSession, null, null, null, null,
-        null, null, distributionConfig, null);
-  }
-
-  private void setupForStartClusterManagementService() throws URISyntaxException {
-    InternalRegionFactory regionFactory = mock(InternalRegionFactory.class);
-    when(cache.createInternalRegionFactory(RegionShortcut.REPLICATE)).thenReturn(regionFactory);
-    when(cache.getOptionalService(HttpService.class))
-        .thenReturn(Optional.of(httpService));
-    when(cache.getCacheForProcessingClientRequests()).thenReturn(cache);
-    BaseManagementService.setManagementService(cache, managementService);
-    URI uri = new URI("file", "/management.war", null);
-    when(agentUtil.findWarLocation("geode-web-management")).thenReturn(uri);
-  }
-
 }
diff --git a/...n/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java b/...n/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java
@@ -66,7 +66,6 @@
 import org.apache.geode.distributed.ConfigurationPersistenceService;
 import org.apache.geode.distributed.DistributedLockService;
 import org.apache.geode.distributed.DistributedMember;
-import org.apache.geode.distributed.DistributedSystem;
 import org.apache.geode.distributed.internal.locks.DLockService;
 import org.apache.geode.internal.JarDeployer;
 import org.apache.geode.internal.cache.ClusterConfigurationLoader;
@@ -129,7 +128,8 @@ public class InternalConfigurationPersistenceService implements ConfigurationPer
   public InternalConfigurationPersistenceService(InternalCache cache, Path workingDirectory,
       JAXBService jaxbService) {
     this(cache,
-        sharedConfigLockService(cache.getDistributedSystem()),
+        DLockService.getOrCreateService(SHARED_CONFIG_LOCK_SERVICE_NAME,
+            cache.getInternalDistributedSystem()),
         jaxbService,
         workingDirectory.resolve(CLUSTER_CONFIG_ARTIFACTS_DIR_NAME),
         workingDirectory
@@ -153,24 +153,6 @@ public InternalConfigurationPersistenceService(JAXBService jaxbService) {
     this.jaxbService = jaxbService;
   }
 
-
-  /**
-   * Gets or creates (if not created) shared configuration lock service
-   */
-  private static DistributedLockService sharedConfigLockService(DistributedSystem ds) {
-    DistributedLockService sharedConfigDls =
-        DLockService.getServiceNamed(SHARED_CONFIG_LOCK_SERVICE_NAME);
-    try {
-      if (sharedConfigDls == null) {
-        sharedConfigDls = DLockService.create(SHARED_CONFIG_LOCK_SERVICE_NAME,
-            (InternalDistributedSystem) ds, true, true);
-      }
-    } catch (IllegalArgumentException ignore) {
-      return DLockService.getServiceNamed(SHARED_CONFIG_LOCK_SERVICE_NAME);
-    }
-    return sharedConfigDls;
-  }
-
   public JAXBService getJaxbService() {
     return jaxbService;
   }

diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
@@ -2663,6 +2663,18 @@ public int getSerialNumber() {
   // -------------------------------------------------------------------------
   // External API methods
   // -------------------------------------------------------------------------
+  public static DistributedLockService getOrCreateService(String serviceName,
+      InternalDistributedSystem ds) {
+    DistributedLockService cmsLockService = DLockService.getServiceNamed(serviceName);
+    try {
+      if (cmsLockService == null) {
+        cmsLockService = DLockService.create(serviceName, ds, true, true);
+      }
+    } catch (IllegalArgumentException ignore) {
+      return DLockService.getServiceNamed(serviceName);
+    }
+    return cmsLockService;
+  }
 
   /**
    * @see org.apache.geode.distributed.DistributedLockService#getServiceNamed(String)