mhansonp commented on a change in pull request #7124:
URL: https://github.com/apache/geode/pull/7124#discussion_r769162677
##########
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:
So part of this change set is to roll back some changes that will
perform poorly in cases not currently tested. So, it is not surprising that
reverting all the changes would yield passing tests. With regards to the chunk
of code you were referencing, you are correct there needed to be an else with
the zone being added to redundancyZones there.
Lastly, there is also a challenge of the tests in
RebalanceOperationComplexPart2DistributedTests.java not really behaving
properly. The failure relies the partitioned region load model doing the wrong
thing which it only does periodically depending on how it allocates buckets. I
am reviewing the fact that I cannot get them to fail in the way I expect at
this point.
This also leads me to think that a unit test might yield a more consistent
result than a dunit. I will look at it.
--
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]
