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 <[email protected]>
AuthorDate: Thu Jul 26 14:33:45 2018 -0700
GEODE-5478: Simplified low redundancy calculation
Co-authored-by: Darrel Schneider <[email protected]>
---
...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();
