This is an automated email from the ASF dual-hosted git repository. boglesby pushed a commit to branch feature/GEODE-5478 in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-5478 by this push: new 18b9fd4 GEODE-5478: Simplified low redundancy calculation 18b9fd4 is described below commit 18b9fd47fcd21ab9eb3bf0d2d325f9871e5453a8 Author: Barry Oglesby <bogle...@pivotal.io> AuthorDate: Thu Jul 26 14:33:45 2018 -0700 GEODE-5478: Simplified low redundancy calculation Co-authored-by: Darrel Schneider <dschnei...@pivotal.io> --- ...edRegionLowBucketRedundancyDistributedTest.java | 12 ++++ .../internal/cache/BucketRedundancyTracker.java | 69 +++++++++++++--------- .../cache/BucketRedundancyTrackerTest.java | 9 +-- 3 files changed, 55 insertions(+), 35 deletions(-) diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionLowBucketRedundancyDistributedTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionLowBucketRedundancyDistributedTest.java index 979937b..5a27b15 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionLowBucketRedundancyDistributedTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionLowBucketRedundancyDistributedTest.java @@ -58,6 +58,9 @@ public class PartitionedRegionLowBucketRedundancyDistributedTest implements Seri // Start server1 and create region MemberVM server1 = startServerAndCreateRegion(1, locatorPort, PARTITION, 1); + // Verify lowBucketRedundancyCount == 0 in server1 + server1.getVM().invoke(() -> waitForLowBucketRedundancyCount(0)); + // Do puts in server1 server1.getVM().invoke(() -> doPuts(500)); @@ -87,6 +90,9 @@ public class PartitionedRegionLowBucketRedundancyDistributedTest implements Seri // Start server1 and create region MemberVM server1 = startServerAndCreateRegion(1, locatorPort, PARTITION, 2); + // Verify lowBucketRedundancyCount == 0 in server1 + server1.getVM().invoke(() -> waitForLowBucketRedundancyCount(0)); + // Do puts in server1 server1.getVM().invoke(() -> doPuts(500)); @@ -121,6 +127,12 @@ public class PartitionedRegionLowBucketRedundancyDistributedTest implements Seri MemberVM server3 = startServerAndCreateRegion(3, locatorPort, PARTITION_PERSISTENT, 1); MemberVM server4 = startServerAndCreateRegion(4, locatorPort, PARTITION_PERSISTENT, 1); + // Verify lowBucketRedundancyCount == 0 in all servers + server1.getVM().invoke(() -> waitForLowBucketRedundancyCount(0)); + server2.getVM().invoke(() -> waitForLowBucketRedundancyCount(0)); + server3.getVM().invoke(() -> waitForLowBucketRedundancyCount(0)); + server4.getVM().invoke(() -> waitForLowBucketRedundancyCount(0)); + // Do puts in server1 server1.getVM().invoke(() -> doPuts(500)); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java index 20d2f9e..f7340a0 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java @@ -19,10 +19,11 @@ package org.apache.geode.internal.cache; * {@link PartitionedRegionRedundancyTracker} of the bucket's status for the region. */ class BucketRedundancyTracker { - private boolean redundancySatisfied = false; - private boolean hasAnyCopies = false; + // if true decrement allowed; if false increment allowed + private boolean noCopiesDecrementOkay = false; + // if true decrement allowed; if false increment allowed + private boolean lowRedundancyDecrementOkay = false; private boolean redundancyEverSatisfied = false; - private boolean hasEverHadCopies = false; private volatile int currentRedundancy = -1; private final int targetRedundancy; private final PartitionedRegionRedundancyTracker regionRedundancyTracker; @@ -45,14 +46,8 @@ class BucketRedundancyTracker { * Adjust statistics based on closing a bucket */ synchronized void closeBucket() { - if (!redundancySatisfied) { - regionRedundancyTracker.decrementLowRedundancyBucketCount(); - redundancySatisfied = true; - } - if (hasEverHadCopies && !hasAnyCopies) { - regionRedundancyTracker.decrementNoCopiesBucketCount(); - hasAnyCopies = true; - } + decrementLowRedundancy(); + decrementNoCopies(); } /** @@ -76,37 +71,53 @@ class BucketRedundancyTracker { } private void updateNoCopiesStatistics(int currentBucketHosts) { - if (hasAnyCopies && currentBucketHosts == 0) { - hasAnyCopies = false; + if (currentBucketHosts == 0) { + incrementNoCopies(); + } else if (currentBucketHosts > 0) { + decrementNoCopies(); + } + } + + private void decrementNoCopies() { + if (noCopiesDecrementOkay) { + noCopiesDecrementOkay = false; + regionRedundancyTracker.decrementNoCopiesBucketCount(); + } + } + + private void incrementNoCopies() { + if (!noCopiesDecrementOkay) { + noCopiesDecrementOkay = true; regionRedundancyTracker.incrementNoCopiesBucketCount(); - } else if (!hasAnyCopies && currentBucketHosts > 0) { - if (hasEverHadCopies) { - regionRedundancyTracker.decrementNoCopiesBucketCount(); - } - hasEverHadCopies = true; - hasAnyCopies = true; } } private void updateRedundancyStatistics(int updatedBucketHosts) { int updatedRedundancy = updatedBucketHosts - 1; updateCurrentRedundancy(updatedRedundancy); - if (updatedRedundancy < targetRedundancy) { reportUpdatedBucketCount(updatedBucketHosts); - if (redundancySatisfied) { - regionRedundancyTracker.incrementLowRedundancyBucketCount(); - redundancySatisfied = false; - } else if (!hasAnyCopies && !hasEverHadCopies && updatedRedundancy >= 0) { - regionRedundancyTracker.incrementLowRedundancyBucketCount(); - } - } else if (!redundancySatisfied && updatedRedundancy == targetRedundancy) { - regionRedundancyTracker.decrementLowRedundancyBucketCount(); - redundancySatisfied = true; + incrementLowRedundancy(); + } else if (updatedRedundancy == targetRedundancy) { + decrementLowRedundancy(); redundancyEverSatisfied = true; } } + private void decrementLowRedundancy() { + if (lowRedundancyDecrementOkay) { + lowRedundancyDecrementOkay = false; + regionRedundancyTracker.decrementLowRedundancyBucketCount(); + } + } + + private void incrementLowRedundancy() { + if (!lowRedundancyDecrementOkay) { + lowRedundancyDecrementOkay = true; + regionRedundancyTracker.incrementLowRedundancyBucketCount(); + } + } + private void updateCurrentRedundancy(int updatedRedundancy) { if (updatedRedundancy != currentRedundancy) { regionRedundancyTracker.setActualRedundancy(updatedRedundancy); diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/BucketRedundancyTrackerTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/BucketRedundancyTrackerTest.java index f9410bf..d48d9c1 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/cache/BucketRedundancyTrackerTest.java +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/BucketRedundancyTrackerTest.java @@ -24,7 +24,6 @@ import static org.mockito.Mockito.verify; import org.junit.Before; import org.junit.Test; - public class BucketRedundancyTrackerTest { private static final int TARGET_COPIES = 2; @@ -57,7 +56,7 @@ public class BucketRedundancyTrackerTest { bucketRedundancyTracker.updateStatistics(TARGET_COPIES); bucketRedundancyTracker.updateStatistics(TARGET_COPIES - 1); bucketRedundancyTracker.updateStatistics(TARGET_COPIES); - verify(regionRedundancyTracker, times(2)).decrementLowRedundancyBucketCount(); + verify(regionRedundancyTracker, times(1)).decrementLowRedundancyBucketCount(); assertEquals(TARGET_COPIES - 1, bucketRedundancyTracker.getCurrentRedundancy()); } @@ -66,7 +65,7 @@ public class BucketRedundancyTrackerTest { bucketRedundancyTracker.updateStatistics(TARGET_COPIES); bucketRedundancyTracker.updateStatistics(TARGET_COPIES - 1); bucketRedundancyTracker.closeBucket(); - verify(regionRedundancyTracker, times(2)).decrementLowRedundancyBucketCount(); + verify(regionRedundancyTracker, times(1)).decrementLowRedundancyBucketCount(); assertEquals(0, bucketRedundancyTracker.getCurrentRedundancy()); } @@ -76,7 +75,7 @@ public class BucketRedundancyTrackerTest { bucketRedundancyTracker.updateStatistics(TARGET_COPIES - 1); bucketRedundancyTracker.updateStatistics(0); bucketRedundancyTracker.closeBucket(); - verify(regionRedundancyTracker, times(2)).decrementLowRedundancyBucketCount(); + verify(regionRedundancyTracker, times(1)).decrementLowRedundancyBucketCount(); assertEquals(-1, bucketRedundancyTracker.getCurrentRedundancy()); } @@ -85,8 +84,6 @@ public class BucketRedundancyTrackerTest { bucketRedundancyTracker = new BucketRedundancyTracker(2, regionRedundancyTracker); bucketRedundancyTracker.updateStatistics(3); - // Verify decrementLowRedundancyBucketCount is invoked. Note: It won't decrement below 0. - verify(regionRedundancyTracker, times(1)).decrementLowRedundancyBucketCount(); bucketRedundancyTracker.updateStatistics(2); // Verify incrementLowRedundancyBucketCount is invoked. verify(regionRedundancyTracker, times(1)).incrementLowRedundancyBucketCount();