[geode] branch develop updated: GEODE-8620: Do not include non-created buckets in redundancy calculation (#5642)

[donalevans]

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

donalevans 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 505eb3a  GEODE-8620: Do not include non-created buckets in redundancy 
calculation (#5642)
505eb3a is described below

commit 505eb3af1f61edd8dff9e199104c72987ef281ab
Author: Donal Evans <doev...@pivotal.io>
AuthorDate: Mon Oct 19 15:25:14 2020 -0700

    GEODE-8620: Do not include non-created buckets in redundancy calculation 
(#5642)
    
    Authored-by: Donal Evans <doev...@vmware.com>
---
 .../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 239c079..6d1877a 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 238646d..62a8a5d 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() {