geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject incubator-geode git commit: GEODE-185: Fix races in testRegionIdleInvalidate
Date Thu, 06 Aug 2015 16:50:51 GMT
Repository: incubator-geode
Updated Branches:
  refs/heads/develop 82de565d2 -> 7d4ae09fc


GEODE-185: Fix races in testRegionIdleInvalidate

The test code is now careful to wait for the expiration clock to advance.
A different assertion will be triggered if the expiration clock goes back in time.
Fixed two places in the product expiration code that did not use the expiration clock.


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

Branch: refs/heads/develop
Commit: 7d4ae09fc92288788868739451254320617289ca
Parents: 82de565
Author: Darrel Schneider <dschneider@pivotal.io>
Authored: Wed Aug 5 14:17:19 2015 -0700
Committer: Darrel Schneider <dschneider@pivotal.io>
Committed: Thu Aug 6 09:46:12 2015 -0700

----------------------------------------------------------------------
 .../gemfire/internal/cache/ExpiryTask.java       |  2 +-
 .../internal/cache/RegionIdleExpiryTask.java     |  3 ++-
 .../gemstone/gemfire/cache30/RegionTestCase.java | 15 ++++++++++-----
 .../src/test/java/dunit/DistributedTestCase.java | 19 +++++++++++++++----
 4 files changed, 28 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/7d4ae09f/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/ExpiryTask.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/ExpiryTask.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/ExpiryTask.java
index 95cd3a8..d5dc5ee 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/ExpiryTask.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/ExpiryTask.java
@@ -437,7 +437,7 @@ public abstract class ExpiryTask extends SystemTimer.SystemTimerTask {
     return super.toString() + " for " + getLocalRegion()
       + ", ttl expiration time: " + expTtl
       + ", idle expiration time: " + expIdle +
-      ("[now:" + System.currentTimeMillis() + "]");
+      ("[now:" + calculateNow() + "]");
   }
 
   ////// Abstract methods ///////

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/7d4ae09f/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionIdleExpiryTask.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionIdleExpiryTask.java
b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionIdleExpiryTask.java
index fbcb12c..52975a2 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionIdleExpiryTask.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/RegionIdleExpiryTask.java
@@ -39,7 +39,8 @@ class RegionIdleExpiryTask extends RegionExpiryTask {
         if (!getLocalRegion().EXPIRY_UNITS_MS) {
           timeout *= 1000;
         }
-        return  timeout + System.currentTimeMillis();
+        // Expiration should always use the DSClock instead of the System clock.
+        return  timeout + getLocalRegion().cacheTimeMillis();
       }
     }
     // otherwise, expire at timeout plus last accessed time

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/7d4ae09f/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/RegionTestCase.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/RegionTestCase.java b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/RegionTestCase.java
index 69eeec0..c063a55 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/RegionTestCase.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/cache30/RegionTestCase.java
@@ -3853,29 +3853,34 @@ public abstract class RegionTestCase extends CacheTestCase {
         // expiration time to be extended.
         // For this phase we don't worry about actually expiring but just
         // making sure the expiration time gets extended.
-        region.getAttributesMutator().setRegionIdleTimeout(new ExpirationAttributes(9000/*ms*/,
ExpirationAction.INVALIDATE));
+        final int EXPIRATION_MS = 9000;
+        region.getAttributesMutator().setRegionIdleTimeout(new ExpirationAttributes(EXPIRATION_MS,
ExpirationAction.INVALIDATE));
         
         LocalRegion lr = (LocalRegion) region;
         {
           ExpiryTask expiryTask = lr.getRegionIdleExpiryTask();
           region.put(key, value);
           long createExpiry = expiryTask.getExpirationTime();
-          waitForExpiryClockToChange(lr);
+          long changeTime = waitForExpiryClockToChange(lr, createExpiry-EXPIRATION_MS);
           region.put(key, "VALUE2");
           long putExpiry = expiryTask.getExpirationTime();
+          assertTrue("CLOCK went back in time! Expected putBaseExpiry=" + (putExpiry-EXPIRATION_MS)
+ " to be >= than changeTime=" + changeTime, (putExpiry-EXPIRATION_MS - changeTime) >=
0);
           assertTrue("expected putExpiry=" + putExpiry + " to be > than createExpiry="
+ createExpiry, (putExpiry - createExpiry) > 0);
-          waitForExpiryClockToChange(lr);
+          changeTime = waitForExpiryClockToChange(lr, putExpiry-EXPIRATION_MS);
           region.get(key);
           long getExpiry = expiryTask.getExpirationTime();
+          assertTrue("CLOCK went back in time! Expected getBaseExpiry=" + (getExpiry-EXPIRATION_MS)
+ " to be >= than changeTime=" + changeTime, (getExpiry-EXPIRATION_MS - changeTime) >=
0);
           assertTrue("expected getExpiry=" + getExpiry + " to be > than putExpiry=" +
putExpiry, (getExpiry - putExpiry) > 0);
         
-          waitForExpiryClockToChange(lr);
+          changeTime = waitForExpiryClockToChange(lr, getExpiry-EXPIRATION_MS);
           sub.put(key, value);
           long subPutExpiry = expiryTask.getExpirationTime();
+          assertTrue("CLOCK went back in time! Expected subPutBaseExpiry=" + (subPutExpiry-EXPIRATION_MS)
+ " to be >= than changeTime=" + changeTime, (subPutExpiry-EXPIRATION_MS - changeTime)
>= 0);
           assertTrue("expected subPutExpiry=" + subPutExpiry + " to be > than getExpiry="
+ getExpiry, (subPutExpiry - getExpiry) > 0);
-          waitForExpiryClockToChange(lr);
+          changeTime = waitForExpiryClockToChange(lr, subPutExpiry-EXPIRATION_MS);
           sub.get(key);
           long subGetExpiry = expiryTask.getExpirationTime();
+          assertTrue("CLOCK went back in time! Expected subGetBaseExpiry=" + (subGetExpiry-EXPIRATION_MS)
+ " to be >= than changeTime=" + changeTime, (subGetExpiry-EXPIRATION_MS - changeTime)
>= 0);
           assertTrue("expected subGetExpiry=" + subGetExpiry + " to be > than subPutExpiry="
+ subPutExpiry, (subGetExpiry - subPutExpiry) > 0);
         }
       }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/7d4ae09f/gemfire-core/src/test/java/dunit/DistributedTestCase.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/dunit/DistributedTestCase.java b/gemfire-core/src/test/java/dunit/DistributedTestCase.java
index 564e7ef..8aa8b6d 100755
--- a/gemfire-core/src/test/java/dunit/DistributedTestCase.java
+++ b/gemfire-core/src/test/java/dunit/DistributedTestCase.java
@@ -976,13 +976,24 @@ public abstract class DistributedTestCase extends TestCase implements
java.io.Se
   }
   
   /**
-   * Blocks until the clock used for expiration on the given region changes.
+   * Blocks until the clock used for expiration moves forward.
+   * @return the last time stamp observed
    */
-  public static final void waitForExpiryClockToChange(LocalRegion lr) {
-    long startTime = lr.cacheTimeMillis();
+  public static final long waitForExpiryClockToChange(LocalRegion lr) {
+    return waitForExpiryClockToChange(lr, lr.cacheTimeMillis());
+  }
+  /**
+   * Blocks until the clock used for expiration moves forward.
+   * @param baseTime the timestamp that the clock must exceed
+   * @return the last time stamp observed
+   */
+  public static final long waitForExpiryClockToChange(LocalRegion lr, final long baseTime)
{
+    long nowTime;
     do {
       Thread.yield();
-    } while (startTime == lr.cacheTimeMillis());
+      nowTime = lr.cacheTimeMillis();
+    } while ((nowTime - baseTime) <= 0L);
+    return nowTime;
   }
   
   /** pause for specified ms interval


Mime
View raw message