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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
