[geode] branch support/1.13 updated: GEODE-8620: Do not include non-created buckets in redundancy calculation

[do...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

donalevans 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 34f2f2f  GEODE-8620: Do not include non-created buckets in redundancy calculation
34f2f2f is described below

commit 34f2f2f408aedf1902a37ca487c20e97a21cd94d
Author: Donal Evans <do...@vmware.com>
AuthorDate: Mon Oct 19 09:57:53 2020 -0700

    GEODE-8620: Do not include non-created buckets in redundancy calculation
    
    Authored-by: Donal Evans <do...@vmware.com>
    (cherry picked from commit 87db2c4e4dbcfb232b293ae427829e9a808ff08d)
---
 .../SerializableRegionRedundancyStatusImpl.java    |  4 +++-
 .../control/RegionRedundancyStatusImplTest.java    | 19 ++++++++++++++++++-
 .../RestoreRedundancyCommandDUnitTest.java         | 22 ++++++++++++++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/control/SerializableRegionRedundancyStatusImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/control/SerializableRegionRedundancyStatusImpl.java
index a3ab5f5..e9af580 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/control/SerializableRegionRedundancyStatusImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/control/SerializableRegionRedundancyStatusImpl.java
@@ -56,7 +56,9 @@ public class SerializableRegionRedundancyStatusImpl extends
     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/geode-core/src/test/java/org/apache/geode/internal/cache/control/RegionRedundancyStatusImplTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/control/RegionRedundancyStatusImplTest.java
index 39eec88..927b488 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/control/RegionRedundancyStatusImplTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/control/RegionRedundancyStatusImplTest.java
@@ -56,7 +56,7 @@ public class RegionRedundancyStatusImplTest {
   @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);
 
@@ -69,6 +69,23 @@ public class RegionRedundancyStatusImplTest {
   }
 
   @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);
     // Have only the bucket with ID = 1 report being under redundancy
diff --git a/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/RestoreRedundancyCommandDUnitTest.java b/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/RestoreRedundancyCommandDUnitTest.java
index 980d06e..417bb0a 100644
--- a/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/RestoreRedundancyCommandDUnitTest.java
+++ b/geode-gfsh/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/RestoreRedundancyCommandDUnitTest.java
@@ -381,6 +381,28 @@ public class RestoreRedundancyCommandDUnitTest {
     });
   }
 
+  @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() {