geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bogle...@apache.org
Subject [geode] branch feature/GEODE-5478 updated: GEODE-5478: Simplified low redundancy calculation
Date Thu, 26 Jul 2018 21:41:21 GMT
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 <boglesby@pivotal.io>
AuthorDate: Thu Jul 26 14:33:45 2018 -0700

    GEODE-5478: Simplified low redundancy calculation
    
    Co-authored-by: Darrel Schneider <dschneider@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();


Mime
View raw message