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 2015/08/05 23:24:46 UTC
incubator-geode git commit: fix races in testRegionIdleInvalidate
Repository: incubator-geode
Updated Branches:
refs/heads/feature/GEODE-185 [created] e300012b6
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/e300012b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/e300012b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/e300012b
Branch: refs/heads/feature/GEODE-185
Commit: e300012b60f206ba791e010711fc2fc03ce156ab
Parents: fa9bd37
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Wed Aug 5 14:17:19 2015 -0700
Committer: Darrel Schneider <ds...@pivotal.io>
Committed: Wed Aug 5 14:17:19 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/e300012b/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/e300012b/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/e300012b/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/e300012b/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