GEODE-8620: Do not include non-created buckets in redundancy calculat…

…ion (#5642) Authored-by: Donal Evans <doevans@vmware.com>
apache · Oct 19, 2020 · 505eb3a · 505eb3a
1 parent 02ffbce
commit 505eb3a
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 2 deletions.
diff --git a/.../java/org/apache/geode/internal/cache/control/SerializableRegionRedundancyStatusImpl.java b/.../java/org/apache/geode/internal/cache/control/SerializableRegionRedundancyStatusImpl.java
@@ -56,7 +56,9 @@ private int calculateLowestRedundancy(PartitionedRegion region) {
     int minRedundancy = Integer.MAX_VALUE;
     for (int i = 0; i < numBuckets; i++) {
       int bucketRedundancy = region.getRegionAdvisor().getBucketRedundancy(i);
-      if (bucketRedundancy < minRedundancy) {
+      // Only consider redundancy for buckets that exist. Buckets that have not been created yet
+      // have a redundancy value of -1
+      if (bucketRedundancy != -1 && bucketRedundancy < minRedundancy) {
         minRedundancy = bucketRedundancy;
       }
     }

diff --git a/...src/test/java/org/apache/geode/internal/cache/control/RegionRedundancyStatusImplTest.java b/...src/test/java/org/apache/geode/internal/cache/control/RegionRedundancyStatusImplTest.java
@@ -56,7 +56,7 @@ public void setUp() {
   @Parameters(method = "getActualRedundancyAndExpectedStatusAndMessage")
   @TestCaseName("[{index}] {method} (Desired redundancy:" + desiredRedundancy
       + "; Actual redundancy:{0}; Expected status:{1})")
-  public void constructorPopulatesValuesCorrectly(int actualRedundancy,
+  public void constructorPopulatesValuesCorrectlyWhenAllBucketsExist(int actualRedundancy,
       RegionRedundancyStatus.RedundancyStatus expectedStatus) {
     when(mockRegion.getRegionAdvisor().getBucketRedundancy(anyInt())).thenReturn(actualRedundancy);
 
@@ -68,6 +68,23 @@ public void constructorPopulatesValuesCorrectly(int actualRedundancy,
     assertThat(result.toString(), containsString(expectedStatus.name()));
   }
 
+  @Test
+  @Parameters(method = "getActualRedundancyAndExpectedStatusAndMessage")
+  @TestCaseName("[{index}] {method} (Desired redundancy:" + desiredRedundancy
+      + "; Actual redundancy:{0}; Expected status:{1})")
+  public void constructorPopulatesValuesCorrectlyWhenNotAllBucketsExist(int actualRedundancy,
+      RegionRedundancyStatus.RedundancyStatus expectedStatus) {
+    when(mockRegion.getRegionAdvisor().getBucketRedundancy(anyInt())).thenReturn(-1)
+        .thenReturn(actualRedundancy);
+
+    RegionRedundancyStatus result = new SerializableRegionRedundancyStatusImpl(mockRegion);
+
+    assertThat(result.getConfiguredRedundancy(), is(desiredRedundancy));
+    assertThat(result.getActualRedundancy(), is(actualRedundancy));
+    assertThat(result.getStatus(), is(expectedStatus));
+    assertThat(result.toString(), containsString(expectedStatus.name()));
+  }
+
   @Test
   public void constructorPopulatesValuesCorrectlyWhenNotAllBucketsReturnTheSameRedundancy() {
     when(mockRegion.getRegionAdvisor().getBucketRedundancy(anyInt())).thenReturn(desiredRedundancy);

diff --git a/.../org/apache/geode/management/internal/cli/commands/RestoreRedundancyCommandDUnitTest.java b/.../org/apache/geode/management/internal/cli/commands/RestoreRedundancyCommandDUnitTest.java
@@ -381,6 +381,28 @@ public void restoreRedundancyDoesNotBalancePrimariesWhenOptionIsUsed() {
     });
   }
 
+  @Test
+  public void restoreRedundancyReturnsCorrectStatusWhenNotAllBucketsHaveBeenCreated() {
+    servers.forEach(s -> s.invoke(() -> {
+      createLowRedundancyRegion();
+      // Put a single entry into the region so that only one bucket gets created
+      Objects.requireNonNull(ClusterStartupRule.getCache())
+          .getRegion(LOW_REDUNDANCY_REGION_NAME)
+          .put("key", "value");
+    }));
+
+    int numberOfServers = servers.size();
+    locator.waitUntilRegionIsReadyOnExactlyThisManyServers(SEPARATOR + LOW_REDUNDANCY_REGION_NAME,
+        numberOfServers);
+
+    String command = new CommandStringBuilder(RESTORE_REDUNDANCY).getCommandString();
+
+    CommandResultAssert commandResult = gfsh.executeAndAssertThat(command).statusIsSuccess();
+
+    verifyGfshOutput(commandResult, new ArrayList<>(), new ArrayList<>(),
+        Collections.singletonList(LOW_REDUNDANCY_REGION_NAME));
+  }
+
   // Helper methods
 
   private List<String> getAllRegionNames() {