geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jensde...@apache.org
Subject [geode] branch develop updated: GEODE-6779: Create disk store command should only return when the MBeans are available (#3681)
Date Mon, 10 Jun 2019 15:03:11 GMT
This is an automated email from the ASF dual-hosted git repository.

jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new f8223f1  GEODE-6779: Create disk store command should only return when the MBeans
are available (#3681)
f8223f1 is described below

commit f8223f173a6d7dbc4c9c6f919f629528ab0b1a1e
Author: Jens Deppe <jdeppe@pivotal.io>
AuthorDate: Mon Jun 10 08:02:43 2019 -0700

    GEODE-6779: Create disk store command should only return when the MBeans are available
(#3681)
    
    
    Co-authored-by: Sarah Abbey <sabbey@pivotal.io>
    Co-authored-by: Jens Deppe <jdeppe@pivotal.io>
---
 ...ateRegionWithDiskstoreAndSecurityDUnitTest.java | 12 ++++++--
 .../cli/commands/AlterCompressorDUnitTest.java     |  1 -
 .../CreateAsyncEventQueueCommandDUnitTest.java     |  5 +--
 .../cli/commands/DiskStoreCommandsDUnitTest.java   | 12 +++++---
 .../ClusterConfigImportDUnitTest.java              |  4 +--
 .../cli/commands/CreateDiskStoreCommand.java       | 36 ++++++++++++++++++++++
 .../cli/commands/CreateDiskStoreCommandTest.java   |  2 ++
 .../cli/commands/AlterRegionCommandDUnitTest.java  |  4 ++-
 8 files changed, 62 insertions(+), 14 deletions(-)

diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/client/CreateRegionWithDiskstoreAndSecurityDUnitTest.java
b/geode-assembly/src/distributedTest/java/org/apache/geode/management/client/CreateRegionWithDiskstoreAndSecurityDUnitTest.java
index da2099a..54ee79a 100644
--- a/geode-assembly/src/distributedTest/java/org/apache/geode/management/client/CreateRegionWithDiskstoreAndSecurityDUnitTest.java
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/client/CreateRegionWithDiskstoreAndSecurityDUnitTest.java
@@ -69,7 +69,9 @@ public class CreateRegionWithDiskstoreAndSecurityDUnitTest {
   @Test
   public void createReplicateRegionWithDiskstoreWithoutDataManage() throws Exception {
     gfsh.executeAndAssertThat(String.format("create disk-store --name=DISKSTORE --dir=%s",
-        diskStoreDir.getAbsolutePath())).statusIsSuccess();
+        diskStoreDir.getAbsolutePath()))
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
 
     RegionConfig regionConfig = new RegionConfig();
     regionConfig.setName("REGION1");
@@ -92,7 +94,9 @@ public class CreateRegionWithDiskstoreAndSecurityDUnitTest {
   @Test
   public void createReplicateRegionWithDiskstoreWithoutClusterWrite() throws Exception {
     gfsh.executeAndAssertThat(String.format("create disk-store --name=DISKSTORE --dir=%s",
-        diskStoreDir.getAbsolutePath())).statusIsSuccess();
+        diskStoreDir.getAbsolutePath()))
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
 
     RegionConfig regionConfig = new RegionConfig();
     regionConfig.setName("REGION1");
@@ -115,7 +119,9 @@ public class CreateRegionWithDiskstoreAndSecurityDUnitTest {
   @Test
   public void createReplicateRegionWithDiskstoreSuccess() throws Exception {
     gfsh.executeAndAssertThat(String.format("create disk-store --name=DISKSTORE --dir=%s",
-        diskStoreDir.getAbsolutePath())).statusIsSuccess();
+        diskStoreDir.getAbsolutePath()))
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
 
     RegionConfig regionConfig = new RegionConfig();
     regionConfig.setName("REGION1");
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterCompressorDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterCompressorDUnitTest.java
index c99f54a..aafd0ef 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterCompressorDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterCompressorDUnitTest.java
@@ -69,7 +69,6 @@ public class AlterCompressorDUnitTest {
     gfsh.executeAndAssertThat(
         "create disk-store --name=diskStore --groups=dataStore --dir=diskStore").statusIsSuccess();
 
-    locator.waitUntilDiskStoreIsReadyOnExactlyThisManyServers("diskStore", 2);
     // create regions
     gfsh.executeAndAssertThat(
         "create region --name=testRegion --type=REPLICATE_PERSISTENT --group=dataStore --disk-store=diskStore")
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommandDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommandDUnitTest.java
index 20527e0..6e2d48d 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommandDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommandDUnitTest.java
@@ -92,8 +92,9 @@ public class CreateAsyncEventQueueCommandDUnitTest {
             " java.lang.IllegalStateException: A GatewaySender with id AsyncEventQueue_queue
is already defined in this cache.",
             " java.lang.IllegalStateException: A GatewaySender with id AsyncEventQueue_queue
is already defined in this cache.");
 
-    gfsh.executeAndAssertThat("create disk-store --name=diskStore2 --dir=diskstore");
-    locator.waitUntilDiskStoreIsReadyOnExactlyThisManyServers("diskStore2", 2);
+    gfsh.executeAndAssertThat("create disk-store --name=diskStore2 --dir=diskstore")
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
 
     // create another queue with different configuration
     gfsh.executeAndAssertThat(VALID_COMMAND + " --id=queue2 --group=group2 "
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
index ad4721f..2b3cf31 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java
@@ -87,14 +87,14 @@ public class DiskStoreCommandsDUnitTest implements Serializable {
   private void createDiskStore(MemberVM jmxManager, int serverCount, String group) {
     gfsh.executeAndAssertThat(String.format(
         "create disk-store --name=%s --dir=%s --group=%s --auto-compact=false --compaction-threshold=99
--max-oplog-size=1 --allow-force-compaction=true",
-        DISKSTORE, DISKSTORE, group)).statusIsSuccess();
+        DISKSTORE, DISKSTORE, group))
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
 
     List<String> diskStores =
         IntStream.rangeClosed(1, serverCount).mapToObj(x -> DISKSTORE).collect(Collectors.toList());
     gfsh.executeAndAssertThat("list disk-stores").statusIsSuccess()
         .tableHasColumnWithValuesContaining("Disk Store Name", diskStores.toArray(new String[0]));
-
-    jmxManager.waitUntilDiskStoreIsReadyOnExactlyThisManyServers(DISKSTORE, serverCount);
   }
 
   private static SerializableRunnableIF dataProducer() {
@@ -344,7 +344,8 @@ public class DiskStoreCommandsDUnitTest implements Serializable {
 
     gfsh.executeAndAssertThat(
         String.format("create disk-store --name=%s --dir=%s", DISKSTORE, DISKSTORE))
-        .statusIsSuccess();
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
 
     gfsh.executeAndAssertThat(String.format("destroy disk-store --name=%s --if-exists", DISKSTORE))
         .statusIsSuccess();
@@ -532,7 +533,8 @@ public class DiskStoreCommandsDUnitTest implements Serializable {
     // Create a disk store with the input disk-dir name
     gfsh.executeAndAssertThat(
         String.format("create disk-store --name=%s --dir=%s", DISKSTORE, diskDirectoryName))
-        .statusIsSuccess();
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
 
     // Verify the server defines the disk store with the disk-dir path
     server.invoke(() -> {
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
index 7cb6597..3e101d0 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
@@ -96,8 +96,8 @@ public class ClusterConfigImportDUnitTest extends ClusterConfigTestBase
{
   public void importFailWithExistingDiskStore() {
     lsRule.startServerVM(1, locatorVM.getPort());
     gfshConnector.executeAndAssertThat("create disk-store --name=diskStore1 --dir=testStore")
-        .statusIsSuccess();
-    locatorVM.waitUntilDiskStoreIsReadyOnExactlyThisManyServers("diskStore1", 1);
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
     gfshConnector
         .executeAndAssertThat(
             "import cluster-configuration --zip-file-name=" + clusterConfigZipPath)
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommand.java
index 329d149..e741902 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommand.java
@@ -20,8 +20,10 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.lang3.tuple.Pair;
+import org.apache.logging.log4j.Logger;
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
 
@@ -34,6 +36,8 @@ import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.internal.cache.DiskStoreAttributes;
 import org.apache.geode.internal.cache.execute.AbstractExecution;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.management.DistributedSystemMXBean;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.SingleGfshCommand;
@@ -47,6 +51,9 @@ import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.ResourcePermission;
 
 public class CreateDiskStoreCommand extends SingleGfshCommand {
+  private static final Logger logger = LogService.getLogger();
+  private static final int MBEAN_CREATION_WAIT_TIME = 10000;
+
   @CliCommand(value = CliStrings.CREATE_DISK_STORE, help = CliStrings.CREATE_DISK_STORE__HELP)
   @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE})
   @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
@@ -133,10 +140,39 @@ public class CreateDiskStoreCommand extends SingleGfshCommand {
     ResultModel result = ResultModel.createMemberStatusResult(functionResults);
     result.setConfigObject(createDiskStoreType(name, diskStoreAttributes));
 
+    if (!waitForDiskStoreMBeanCreation(name, targetMembers)) {
+      result.addInfo()
+          .addLine("Did not complete waiting for Disk Store MBean proxy creation");
+    }
+
     return result;
   }
 
   @VisibleForTesting
+  boolean waitForDiskStoreMBeanCreation(String diskStore,
+      Set<DistributedMember> membersToCreateDiskStoreOn) {
+    DistributedSystemMXBean dsMXBean = getManagementService().getDistributedSystemMXBean();
+
+    return poll(MBEAN_CREATION_WAIT_TIME, TimeUnit.MILLISECONDS,
+        () -> membersToCreateDiskStoreOn.stream()
+            .allMatch(m -> diskStoreBeanExists(dsMXBean, m.getName(), diskStore)));
+  }
+
+  private boolean diskStoreBeanExists(DistributedSystemMXBean dsMXBean, String memberName,
+      String diskStore) {
+    try {
+      dsMXBean.fetchDiskStoreObjectName(memberName, diskStore);
+      return true;
+    } catch (Exception e) {
+      if (!e.getMessage().toLowerCase().contains("not found")) {
+        logger.warn("Unable to retrieve Disk Store ObjectName for member: {}, diskstore:
{} - {}",
+            memberName, diskStore, e.getMessage());
+      }
+    }
+    return false;
+  }
+
+  @VisibleForTesting
   Pair<Boolean, String> validateDiskstoreAttributes(
       DiskStoreAttributes diskStoreAttributes,
       Set<DistributedMember> targetMembers) {
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommandTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommandTest.java
index a65340c..ac71f7a 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDiskStoreCommandTest.java
@@ -55,6 +55,7 @@ public class CreateDiskStoreCommandTest {
         .executeAndGetFunctionResult(any(), any(), any());
     doReturn(Pair.of(Boolean.TRUE, null)).when(command).validateDiskstoreAttributes(any(),
         any());
+    doReturn(true).when(command).waitForDiskStoreMBeanCreation(any(), any());
     ResultModel resultModel =
         gfsh.executeAndAssertThat(command, "create disk-store --name=ds1 --dir=./data/persist")
             .getResultModel();
@@ -72,6 +73,7 @@ public class CreateDiskStoreCommandTest {
         .executeAndGetFunctionResult(any(), any(), any());
     doReturn(Pair.of(Boolean.TRUE, null)).when(command).validateDiskstoreAttributes(any(),
         any());
+    doReturn(true).when(command).waitForDiskStoreMBeanCreation(any(), any());
     ResultModel resultModel =
         gfsh.executeAndAssertThat(command, "create disk-store --name=ds1 --dir=/data/persist")
             .getResultModel();
diff --git a/geode-wan/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterRegionCommandDUnitTest.java
b/geode-wan/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterRegionCommandDUnitTest.java
index 1f9a25d..eae2319 100644
--- a/geode-wan/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterRegionCommandDUnitTest.java
+++ b/geode-wan/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/AlterRegionCommandDUnitTest.java
@@ -57,7 +57,9 @@ public class AlterRegionCommandDUnitTest {
 
     gfsh.connectAndVerify(locator);
     gfsh.executeAndAssertThat(
-        "create disk-store --name=diskStore --dir=" + temporaryFolder.getRoot()).statusIsSuccess();
+        "create disk-store --name=diskStore --dir=" + temporaryFolder.getRoot())
+        .statusIsSuccess()
+        .doesNotContainOutput("Did not complete waiting");
   }
 
   @Test


Mime
View raw message