You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2016/07/07 00:01:06 UTC

incubator-geode git commit: renamed to removeIf; test now sleeps longer to be safe

Repository: incubator-geode
Updated Branches:
  refs/heads/feature/GEODE-1420 2f99f0be4 -> e2c8e2326


renamed to removeIf; test now sleeps longer to be safe


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/e2c8e232
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/e2c8e232
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/e2c8e232

Branch: refs/heads/feature/GEODE-1420
Commit: e2c8e232614d4f45a059e0ccde179520660b2b3f
Parents: 2f99f0b
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Wed Jul 6 17:00:24 2016 -0700
Committer: Darrel Schneider <ds...@pivotal.io>
Committed: Wed Jul 6 17:00:24 2016 -0700

----------------------------------------------------------------------
 .../internal/cache/TombstoneService.java        | 49 +++++++++-----------
 .../gemfire/cache30/MultiVMRegionTestCase.java  | 15 +++---
 2 files changed, 30 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e2c8e232/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java
index 6ef6a56..dca792f 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TombstoneService.java
@@ -211,7 +211,7 @@ public class TombstoneService {
     final VersionSource myId = r.getVersionMember();
     final TombstoneSweeper sweeper = getSweeper(r);
     final List<Tombstone> removals = new ArrayList<Tombstone>();
-    sweeper.scanUnexpired(t -> {
+    sweeper.removeUnexpiredIf(t -> {
       if (t.region == r) {
         VersionSource destroyingMember = t.getMemberID();
         if (destroyingMember == null) {
@@ -275,7 +275,7 @@ public class TombstoneService {
     }
     final TombstoneSweeper sweeper = this.getSweeper(r);
     final List<Tombstone> removals = new ArrayList<Tombstone>(tombstoneKeys.size());
-    sweeper.scanUnexpired(t -> {
+    sweeper.removeUnexpiredIf(t -> {
       if (t.region == r) {
         if (tombstoneKeys.contains(t.entry.getKey())) {
           removals.add(t);
@@ -300,7 +300,7 @@ public class TombstoneService {
    * @return true if the expiration occurred 
    */
   public boolean forceBatchExpirationForTests(int count) throws InterruptedException {
-    return this.replicatedTombstoneSweeper.forceBatchExpirationForTests(count);
+    return this.replicatedTombstoneSweeper.testHook_forceExpiredTombstoneGC(count);
   }
 
   @Override
@@ -349,7 +349,7 @@ public class TombstoneService {
     }
 
     @Override
-    protected boolean scanExpired(Predicate<Tombstone> predicate) {
+    protected boolean removeExpiredIf(Predicate<Tombstone> predicate) {
       return false;
     }
     @Override protected void updateStatistics() {
@@ -366,13 +366,13 @@ public class TombstoneService {
       tombstone.region.getRegionMap().removeTombstone(tombstone.entry, tombstone, false, true);
     }
     @Override
-    protected void checkExpiredTombstones() {
+    protected void checkExpiredTombstoneGC() {
     }
     @Override
     protected void handleNoUnexpiredTombstones() {
     }
     @Override
-    boolean forceBatchExpirationForTests(int count) throws InterruptedException {
+    boolean testHook_forceExpiredTombstoneGC(int count) throws InterruptedException {
       return true;
     }
     @Override
@@ -448,7 +448,7 @@ public class TombstoneService {
     }
 
     @Override
-    protected boolean scanExpired(Predicate<Tombstone> predicate) {
+    protected boolean removeExpiredIf(Predicate<Tombstone> predicate) {
       boolean result = false;
       long removalSize = 0;
       synchronized(getBlockGCLock()) {
@@ -521,7 +521,7 @@ public class TombstoneService {
         }
 
         //Remove the tombstones from the in memory region map.
-        scanExpired(t -> {
+        removeExpiredIf(t -> {
           // for PR buckets we have to keep track of the keys removed because clients have
           // them all lumped in a single non-PR region
           DistributedRegion tr = (DistributedRegion) t.region;
@@ -565,7 +565,7 @@ public class TombstoneService {
       } // sync on deltaGIILock
     }
     @Override
-    protected void checkExpiredTombstones() {
+    protected void checkExpiredTombstoneGC() {
       if (shouldCallExpireBatch()) {
         this.forceBatchExpiration = false;
         expireBatch();
@@ -645,7 +645,7 @@ public class TombstoneService {
     }
 
     @Override
-    boolean forceBatchExpirationForTests(int count) throws InterruptedException {
+    boolean testHook_forceExpiredTombstoneGC(int count) throws InterruptedException {
       // sync on blockGCLock since expireBatch syncs on it
       synchronized(getBlockGCLock()) {
         testHook_forceBatchExpireCall = new CountDownLatch(1);
@@ -739,7 +739,7 @@ public class TombstoneService {
     }
 
     public void unscheduleTombstones(final LocalRegion r) {
-      this.scanAll(t -> {
+      this.removeIf(t -> {
         if (t.region == r) {
           return true;
         }
@@ -753,7 +753,7 @@ public class TombstoneService {
      * and update the memory estimate.
      * @return true if predicate ever returned true
      */
-    private boolean scanUnexpired(Predicate<Tombstone> predicate) {
+    private boolean removeUnexpiredIf(Predicate<Tombstone> predicate) {
       boolean result = false;
       long removalSize = 0;
       lockQueueHead();
@@ -779,8 +779,8 @@ public class TombstoneService {
      * and update the memory estimate.
      * @return true if predicate ever returned true
      */
-    private boolean scanAll(Predicate<Tombstone> predicate) {
-      return scanUnexpired(predicate) || scanExpired(predicate);
+    private boolean removeIf(Predicate<Tombstone> predicate) {
+      return removeUnexpiredIf(predicate) || removeExpiredIf(predicate);
     }
 
     synchronized void start() {
@@ -824,23 +824,16 @@ public class TombstoneService {
       updateMemoryEstimate(ts.getSize());
     }
     
-    /**
-     * The run loop picks a tombstone off of the expiration queue and waits
-     * for it to expire.  It also periodically scans for resurrected tombstones
-     * and handles batch expiration.  Batch expiration works by tossing the
-     * expired tombstones into a set and delaying the removal of those tombstones
-     * from the Region until scheduled points in the calendar.  
-     */
     public void run() {
       if (logger.isTraceEnabled(LogMarker.TOMBSTONE)) {
-        logger.trace(LogMarker.TOMBSTONE, "Destroyed entries sweeper starting with default sleep interval={}", this.EXPIRY_TIME);
+        logger.trace(LogMarker.TOMBSTONE, "Destroyed entries sweeper starting with sleep interval of {} milliseconds", EXPIRY_TIME);
       }
       while (!isStopped && cancelCriterion.cancelInProgress() == null) {
         try {
           updateStatistics();
           SystemFailure.checkFailure();
           final long now = getNow();
-          checkExpiredTombstones();
+          checkExpiredTombstoneGC();
           checkOldestUnexpired(now);
           purgeObsoleteTombstones(now);
           doSleep();
@@ -898,7 +891,7 @@ public class TombstoneService {
       lastPurgeTimestamp = now;
       long start = now;
       // see if any have been superseded
-      boolean removedObsoleteTombstone = scanAll(tombstone -> {
+      boolean removedObsoleteTombstone = removeIf(tombstone -> {
         if (tombstone.region.getRegionMap().isTombstoneNotNeeded(tombstone.entry, tombstone.getEntryVersion())) {
           if (logger.isTraceEnabled(LogMarker.TOMBSTONE)) {
             logger.trace(LogMarker.TOMBSTONE, "removing obsolete tombstone: {}", tombstone);
@@ -911,7 +904,7 @@ public class TombstoneService {
         sleepTime = 0;
       } else {
         long elapsed = getNow() - start;
-        sleepTime = sleepTime - elapsed;
+        sleepTime -= elapsed;
         if (sleepTime <= 0) {
           minimumPurgeTime = elapsed;
         }
@@ -971,9 +964,9 @@ public class TombstoneService {
      * <p>Some sweepers batch up the expired tombstones to gc them later.
      * @return true if predicate ever returned true
      */
-    protected abstract boolean scanExpired(Predicate<Tombstone> predicate);
+    protected abstract boolean removeExpiredIf(Predicate<Tombstone> predicate);
     /** see if the already expired tombstones should be processed */
-    protected abstract void checkExpiredTombstones();
+    protected abstract void checkExpiredTombstoneGC();
     protected abstract void handleNoUnexpiredTombstones();
     protected abstract boolean hasExpired(long msTillTombstoneExpires);
     protected abstract void expireTombstone(Tombstone tombstone);
@@ -982,6 +975,6 @@ public class TombstoneService {
      * Do anything needed before the sweeper sleeps.
      */
     protected abstract void beforeSleepChecks();
-    abstract boolean forceBatchExpirationForTests(int count) throws InterruptedException;
+    abstract boolean testHook_forceExpiredTombstoneGC(int count) throws InterruptedException;
   } // class TombstoneSweeper
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e2c8e232/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java b/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
index bf68610..9ad8e0e 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/cache30/MultiVMRegionTestCase.java
@@ -8564,15 +8564,17 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
     final int oldExpiredTombstoneLimit = TombstoneService.EXPIRED_TOMBSTONE_LIMIT;
     final boolean oldIdleExpiration = TombstoneService.IDLE_EXPIRATION;
     final double oldLimit = TombstoneService.GC_MEMORY_THRESHOLD;
+    final long oldMaxSleepTime = TombstoneService.MAX_SLEEP_TIME;
     try {
       SerializableRunnable setTimeout = new SerializableRunnable() {
         @Override
         public void run() {
-          TombstoneService.REPLICATE_TOMBSTONE_TIMEOUT = 2000;
-          TombstoneService.NON_REPLICATE_TOMBSTONE_TIMEOUT = 1900;
+          TombstoneService.REPLICATE_TOMBSTONE_TIMEOUT = 1000;
+          TombstoneService.NON_REPLICATE_TOMBSTONE_TIMEOUT = 900;
           TombstoneService.EXPIRED_TOMBSTONE_LIMIT = numEntries;
           TombstoneService.IDLE_EXPIRATION = true;
           TombstoneService.GC_MEMORY_THRESHOLD = 0;  // turn this off so heap profile won't cause test to fail
+          TombstoneService.MAX_SLEEP_TIME = 500;
         }
       };
       vm0.invoke(setTimeout);
@@ -8635,7 +8637,7 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
             }
           };
           try {
-            Wait.waitForCriterion(waitForExpiration, TombstoneService.REPLICATE_TOMBSTONE_TIMEOUT+10000, 100, true);
+            Wait.waitForCriterion(waitForExpiration, TombstoneService.REPLICATE_TOMBSTONE_TIMEOUT+(TombstoneService.MAX_SLEEP_TIME*9), 100, true);
           } catch (AssertionError e) {
             CCRegion.dumpBackingMap();
             com.gemstone.gemfire.test.dunit.LogWriterUtils.getLogWriter().info("tombstone service state: " + CCRegion.getCache().getTombstoneService());
@@ -8660,7 +8662,7 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
                   + " tombstones left out of " + origCount + " initial tombstones. " + CCRegion.getCache().getTombstoneService();
               }
             };
-            Wait.waitForCriterion(waitForExpiration, TombstoneService.REPLICATE_TOMBSTONE_TIMEOUT+10000, 100, true);
+            Wait.waitForCriterion(waitForExpiration, TombstoneService.REPLICATE_TOMBSTONE_TIMEOUT+(TombstoneService.MAX_SLEEP_TIME*9), 100, true);
             logger.debug("creating tombstones.  current count={}", CCRegion.getTombstoneCount());
             for (int i=0; i<numEntries; i++) {
               CCRegion.create("cckey" + i, i);
@@ -8733,7 +8735,7 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
                   + " tombstones left out of " + numEntries + " initial tombstones. " + CCRegion.getCache().getTombstoneService();
               }
             };
-            Wait.waitForCriterion(waitForExpiration, 10000, 100, true);
+            Wait.waitForCriterion(waitForExpiration, TombstoneService.REPLICATE_TOMBSTONE_TIMEOUT*5, 100, true);
           } catch (CacheException e) {
             fail("while performing create operations", e);
           }
@@ -8757,7 +8759,7 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
                 + " tombstones left out of " + numEntries + " initial tombstones. " + CCRegion.getCache().getTombstoneService();
             }
           };
-          Wait.waitForCriterion(waitForExpiration, 10000, 100, true);
+          Wait.waitForCriterion(waitForExpiration, TombstoneService.REPLICATE_TOMBSTONE_TIMEOUT*5, 100, true);
         }
       });
 
@@ -8770,6 +8772,7 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
           TombstoneService.EXPIRED_TOMBSTONE_LIMIT = oldExpiredTombstoneLimit;
           TombstoneService.IDLE_EXPIRATION = oldIdleExpiration;
           TombstoneService.GC_MEMORY_THRESHOLD = oldLimit;
+          TombstoneService.MAX_SLEEP_TIME = oldMaxSleepTime;
         }
       };
       vm0.invoke(resetTimeout);