GEODE-5994: Cleanup from some additional review comments (apache#6852)

10088 · Sep 10, 2021 · 7b924b5 · 7b924b5
1 parent 151d8c1
commit 7b924b5
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 32 deletions.
diff --git a/...ava/org/apache/geode/internal/cache/control/RebalanceOperationComplexDistributedTest.java b/...ava/org/apache/geode/internal/cache/control/RebalanceOperationComplexDistributedTest.java
@@ -89,7 +89,7 @@ public class RebalanceOperationComplexDistributedTest implements Serializable {
   public ClusterStartupRule clusterStartupRule = new ClusterStartupRule(8);
 
   @Before
-  public void setup() {
+  public void setup() throws Exception {
     // Start the locator
     MemberVM locatorVM = clusterStartupRule.startLocatorVM(0);
     locatorPort = locatorVM.getPort();
@@ -98,6 +98,15 @@ public void setup() {
 
     runID.incrementAndGet();
     cleanOutServerDirectories();
+
+    // Startup the servers
+    for (Map.Entry<Integer, String> entry : SERVER_ZONE_MAP.entrySet()) {
+      startServerInRedundancyZone(entry.getKey(), entry.getValue());
+    }
+
+    // Put data in the server regions
+    clientPopulateServers();
+
   }
 
   @After
@@ -115,16 +124,7 @@ public void after() {
   @Test
   @Parameters({"1,2", "1,4", "4,1", "5,6"})
   public void testEnforceZoneWithSixServersAndTwoZones(int rebalanceServer,
-      int serverToBeShutdownAndRestarted)
-      throws Exception {
-
-    // Startup the servers
-    for (Map.Entry<Integer, String> entry : SERVER_ZONE_MAP.entrySet()) {
-      startServerInRedundancyZone(entry.getKey(), entry.getValue());
-    }
-
-    // Put data in the server regions
-    clientPopulateServers();
+      int serverToBeShutdownAndRestarted) {
 
     // Rebalance Server VM will initiate all of the rebalances in this test
     VM rebalanceServerVM = clusterStartupRule.getVM(rebalanceServer);

diff --git a/...ore/src/main/java/org/apache/geode/internal/cache/partitioned/rebalance/model/Member.java b/...ore/src/main/java/org/apache/geode/internal/cache/partitioned/rebalance/model/Member.java
@@ -20,7 +20,6 @@
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.annotations.VisibleForTesting;
-import org.apache.geode.distributed.internal.ClusterDistributionManager;
 import org.apache.geode.distributed.internal.DistributionManager;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
 import org.apache.geode.logging.internal.log4j.api.LogService;
@@ -67,16 +66,9 @@ public Member(AddressComparor addressComparor, InternalDistributedMember memberI
    * @param bucket -- bucket to be deleted from the member
    * @param distributionManager -- used to check members of redundancy zones
    */
-
   public RefusalReason canDelete(Bucket bucket, DistributionManager distributionManager) {
     // This code only applies to Clusters.
-    if (!(distributionManager instanceof ClusterDistributionManager)) {
-      return RefusalReason.NONE;
-    }
-
-    ClusterDistributionManager clstrDistrMgr = (ClusterDistributionManager) distributionManager;
-    String myRedundancyZone = clstrDistrMgr.getRedundancyZone(memberId);
-    boolean lastMemberOfZone = true;
+    String myRedundancyZone = distributionManager.getRedundancyZone(memberId);
 
     if (myRedundancyZone == null) {
       // Not using redundancy zones, so...
@@ -89,7 +81,7 @@ public RefusalReason canDelete(Bucket bucket, DistributionManager distributionMa
         continue;
       }
 
-      String memberRedundancyZone = clstrDistrMgr.getRedundancyZone(member.memberId);
+      String memberRedundancyZone = distributionManager.getRedundancyZone(member.memberId);
       if (memberRedundancyZone == null) {
         // Not using redundancy zones, so...
         continue;
@@ -98,15 +90,11 @@ public RefusalReason canDelete(Bucket bucket, DistributionManager distributionMa
       // Does the member redundancy zone match my redundancy zone?
       // if so we are not the last in the redundancy zone.
       if (memberRedundancyZone.equals(myRedundancyZone)) {
-        lastMemberOfZone = false;
+        return RefusalReason.NONE;
       }
     }
 
-    if (lastMemberOfZone) {
-      return RefusalReason.LAST_MEMBER_IN_ZONE;
-    }
-
-    return RefusalReason.NONE;
+    return RefusalReason.LAST_MEMBER_IN_ZONE;
   }
 
 

diff --git a/...pache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java b/...pache/geode/internal/cache/partitioned/rebalance/PartitionedRegionLoadModelJUnitTest.java
@@ -43,7 +43,6 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentMatchers;
-import org.mockito.Mockito;
 
 import org.apache.geode.DataSerializer;
 import org.apache.geode.cache.partition.PartitionMemberInfo;
@@ -58,7 +57,6 @@
 import org.apache.geode.internal.cache.partitioned.rebalance.model.AddressComparor;
 import org.apache.geode.internal.cache.partitioned.rebalance.model.Bucket;
 import org.apache.geode.internal.cache.partitioned.rebalance.model.Member;
-import org.apache.geode.internal.cache.partitioned.rebalance.model.Move;
 import org.apache.geode.internal.cache.partitioned.rebalance.model.PartitionedRegionLoadModel;
 import org.apache.geode.internal.cache.persistence.PersistentMemberID;
 import org.apache.geode.internal.inet.LocalHostUtil;
@@ -1545,10 +1543,10 @@ public void testFindBestRemoveRemovesFromMoreLoadedMember() {
 
     when(bucket.getMembersHosting()).thenReturn(membersHostingBucket);
 
-    Mockito.when(clusterDistributionManager.getRedundancyZone(memberId)).thenReturn("zoneA");
-    Mockito.when(clusterDistributionManager.getRedundancyZone(otherMemberId)).thenReturn("zoneA");
+    when(clusterDistributionManager.getRedundancyZone(memberId)).thenReturn("zoneA");
+    when(clusterDistributionManager.getRedundancyZone(otherMemberId)).thenReturn("zoneA");
 
-    Mockito.when(bucket.getMembersHosting()).thenReturn(membersHostingBucket);
+    when(bucket.getMembersHosting()).thenReturn(membersHostingBucket);
 
     org.apache.geode.internal.cache.partitioned.rebalance.model.Move bestRemove =
         model.findBestRemove(bucket);