[GitHub] [geode] DonalEvans commented on a change in pull request #7124: GEODE-9815: Prefer to remove a redundant copy in the same zone

[GitBox <gi...@apache.org>]

DonalEvans commented on a change in pull request #7124:
URL: https://github.com/apache/geode/pull/7124#discussion_r763526274



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/rebalance/model/PartitionedRegionLoadModel.java
##########
@@ -435,10 +435,24 @@ private void initLowRedundancyBuckets() {
 
   private void initOverRedundancyBuckets() {
     this.overRedundancyBuckets = new TreeSet<>(REDUNDANCY_COMPARATOR);
+    Set<String> redundancyZones = new HashSet<>();
+
     for (BucketRollup b : this.buckets) {
-      if (b != null && b.getOnlineRedundancy() > this.requiredRedundancy) {
-        this.overRedundancyBuckets.add(b);
+      if (b != null) {
+        if (b.getOnlineRedundancy() > this.requiredRedundancy) {
+          this.overRedundancyBuckets.add(b);
+        } else {
+          for (Member member : b.getMembersHosting()) {
+            String redundancyZone = this.getRedundancyZone(member.getDistributedMember());
+            if (redundancyZones.contains(redundancyZone)) {

Review comment:
       This check is always false, because `redundancyZones` starts off empty and is only added to if this check returns true, so there's no way for anything to ever be added to it. Negating this check results in the tests failing, so it seems that this isn't just a typo, but with this check always being false, the logic in this method becomes identical to that in the unmodified method.
   
   In addition to this, reverting the changes in this file results in the added tests still passing, which would imply that the tests aren't testing the intended behaviour.




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

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