mhansonp commented on a change in pull request #7124:
URL: https://github.com/apache/geode/pull/7124#discussion_r779956505
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/rebalance/model/PartitionedRegionLoadModel.java
##########
@@ -425,19 +425,58 @@ public void remoteOverRedundancyBucket(BucketRollup
bucket, Member targetMember)
}
private void initLowRedundancyBuckets() {
- this.lowRedundancyBuckets = new TreeSet<>(REDUNDANCY_COMPARATOR);
for (BucketRollup b : this.buckets) {
if (b != null && b.getRedundancy() >= 0 && b.getRedundancy() <
this.requiredRedundancy) {
this.lowRedundancyBuckets.add(b);
}
}
}
+
+ /**
+ * Original functionality if bucket's redundancy is greater than what is
necessary add it to the
+ * over redundancy bucket list, so it can be cleared
+ * <p>
+ * Newly added functionality is to make this so that we don't have a bucket
in the same redundancy
+ * zone twice if zones are in use.
+ */
private void initOverRedundancyBuckets() {
this.overRedundancyBuckets = new TreeSet<>(REDUNDANCY_COMPARATOR);
+ Set<String> redundancyZonesFound = new HashSet<>();
+
+ // For every bucket
for (BucketRollup b : this.buckets) {
- if (b != null && b.getOnlineRedundancy() > this.requiredRedundancy) {
- this.overRedundancyBuckets.add(b);
+ if (b != null) {
+ // check to see if the existing redundancy is greater than required
+ if (b.getOnlineRedundancy() > this.requiredRedundancy) {
+ // if so, add the bucket to the over redundancy list
+ this.overRedundancyBuckets.add(b);
+ } else {
+ // otherwise, for each member that is hosting the bucket
+ for (Member member : b.getMembersHosting()) {
+
+ // get the redundancy zone of the member
+ String redundancyZone =
this.getRedundancyZone(member.getDistributedMember());
+ if (redundancyZone != null) {
+ // if the redundancy zone is not already in the list
+ if (redundancyZonesFound.contains(redundancyZone)) {
+ // add the bucket to the over redundancy list because we have
more than one member
+ // with this bucket in the same zone. something we don't
prefer with multiple zones
+ this.overRedundancyBuckets.add(b);
+ if (b.getOnlineRedundancy() - 1 < b.getRedundancy()) {
+ this.lowRedundancyBuckets.add(b);
+ }
Review comment:
No. This check is absolutely necessary. Because in the case you have a
two servers in the same zone with a copy of the bucket and redundancy set to
one, as soon as you mark the bucket as being overredundant one of the copies
will go away, immediately leaving you with a low redundancy. Hence the reason,
we are adding to both. The method that clears overredundancy doesn't check for
lowredundancy.
--
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]
