[ 
https://issues.apache.org/jira/browse/GEODE-8547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17225718#comment-17225718
 ] 

ASF GitHub Bot commented on GEODE-8547:
---------------------------------------

jinmeiliao commented on a change in pull request #5567:
URL: https://github.com/apache/geode/pull/5567#discussion_r516987265



##########
File path: 
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoreCommand.java
##########
@@ -53,16 +53,19 @@ public ResultModel showMissingDiskStore() {
     Set<DistributedMember> dataMembers =
         DiskStoreCommandsUtils.getNormalMembers((InternalCache) getCache());
 
-    if (dataMembers.isEmpty()) {
-      return 
ResultModel.createInfo(CliStrings.NO_CACHING_MEMBERS_FOUND_MESSAGE);
+    List<ColocatedRegionDetails> missingRegions = null;
+    boolean cacheMemberExist = false;
+
+    if (!dataMembers.isEmpty()) {
+      cacheMemberExist = true;

Review comment:
       I believe you don't need this boolean. If no servers, `missingRegions` 
will be null, otherwise, it's empty or has data

##########
File path: 
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoreCommandDUnitTest.java
##########
@@ -156,6 +160,71 @@ public void 
stoppingAndRestartingMemberDoesNotResultInMissingDiskStore() {
     assertThat(missingDiskStoreIds).isNull();
   }
 
+
+  @Test
+  public void stopAllMembersAndStart2ndLocator() throws Exception {
+    
IgnoredException.addIgnoredException(DistributedSystemDisconnectedException.class);
+
+    MemberVM locator1 = lsRule.startLocatorVM(1, locator.getPort());
+
+    MemberVM server1 = lsRule.startServerVM(2, locator.getPort(), 
locator1.getPort());
+    @SuppressWarnings("unused")

Review comment:
       instead of suppress the warning, just don't assign it to a variable

##########
File path: 
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoreCommandDUnitTest.java
##########
@@ -156,6 +160,71 @@ public void 
stoppingAndRestartingMemberDoesNotResultInMissingDiskStore() {
     assertThat(missingDiskStoreIds).isNull();
   }
 
+
+  @Test
+  public void stopAllMembersAndStart2ndLocator() throws Exception {
+    
IgnoredException.addIgnoredException(DistributedSystemDisconnectedException.class);
+
+    MemberVM locator1 = lsRule.startLocatorVM(1, locator.getPort());
+
+    MemberVM server1 = lsRule.startServerVM(2, locator.getPort(), 
locator1.getPort());
+    @SuppressWarnings("unused")
+    MemberVM server2 = lsRule.startServerVM(3, locator.getPort(), 
locator1.getPort());
+
+    final String testRegionName = "regionA";
+    CommandStringBuilder csb;
+    csb = new CommandStringBuilder(CliStrings.CREATE_DISK_STORE)
+        .addOption(CliStrings.CREATE_DISK_STORE__NAME, "diskStore")
+        .addOption(CliStrings.CREATE_DISK_STORE__DIRECTORY_AND_SIZE, 
"diskStoreDir");
+    
gfshConnector.executeAndAssertThat(csb.getCommandString()).statusIsSuccess();
+
+    CommandStringBuilder createRegion = new 
CommandStringBuilder(CliStrings.CREATE_REGION)
+        .addOption(CliStrings.CREATE_REGION__REGION, testRegionName)
+        .addOption(CliStrings.CREATE_REGION__DISKSTORE, "diskStore")
+        .addOption(CliStrings.CREATE_REGION__REGIONSHORTCUT,
+            RegionShortcut.PARTITION_REDUNDANT_PERSISTENT.toString());
+    await().untilAsserted(() -> 
gfshConnector.executeAndAssertThat(createRegion.getCommandString())
+        .statusIsSuccess());
+
+
+    lsRule.stop(1, false);
+
+    // Add data to the region
+    addData(server1, testRegionName);

Review comment:
       you also don't need to add data and do rebalance

##########
File path: 
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoreCommand.java
##########
@@ -113,7 +116,9 @@ private ResultModel toMissingDiskStoresTabularResult(
     }
 
     TabularResultModel missingRegionsSection = 
result.addTable(MISSING_COLOCATED_REGIONS_SECTION);
-    if (hasMissingColocatedRegions) {
+    if (!cacheMemberExist) {

Review comment:
       here you can do something like:
   ```
   if (missingColocatedRegions == null) {
     // set the header as "no caching members"
   }
   else if (missingColocatedRegions.isEmpty()) {
   ....
   }
   else{
   ....
   }

##########
File path: 
geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoreCommandDUnitTest.java
##########
@@ -156,6 +160,71 @@ public void 
stoppingAndRestartingMemberDoesNotResultInMissingDiskStore() {
     assertThat(missingDiskStoreIds).isNull();
   }
 
+
+  @Test
+  public void stopAllMembersAndStart2ndLocator() throws Exception {
+    
IgnoredException.addIgnoredException(DistributedSystemDisconnectedException.class);
+
+    MemberVM locator1 = lsRule.startLocatorVM(1, locator.getPort());
+
+    MemberVM server1 = lsRule.startServerVM(2, locator.getPort(), 
locator1.getPort());
+    @SuppressWarnings("unused")
+    MemberVM server2 = lsRule.startServerVM(3, locator.getPort(), 
locator1.getPort());
+
+    final String testRegionName = "regionA";

Review comment:
       I believe you can just create a region without the diskstore, the same 
problem would still manifest. Make the test simpler




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Command "show missing-disk-stores" not working, when all servers are down
> -------------------------------------------------------------------------
>
>                 Key: GEODE-8547
>                 URL: https://issues.apache.org/jira/browse/GEODE-8547
>             Project: Geode
>          Issue Type: Bug
>          Components: gfsh
>            Reporter: Mario Ivanac
>            Assignee: Mario Ivanac
>            Priority: Major
>              Labels: needs-review, pull-request-available
>
> If cluster with 2 locators and 2 servers was ungracefully shutdown it can 
> happen that locators that are able to start up are not having most recent 
> data to bring up Cluster Configuration Service.
> If we excute command "show missing-disk-stores" it will not work, since all 
> servers are down,
> so we are stuck in this situation.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to