geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject [geode] branch develop updated: GEODE-3543: refactor ExpiryTask to not call GemFireCacheImpl.getInstance
Date Mon, 11 Sep 2017 20:27:19 GMT
This is an automated email from the ASF dual-hosted git repository.

dschneider pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 83f27a7  GEODE-3543: refactor ExpiryTask to not call GemFireCacheImpl.getInstance
83f27a7 is described below

commit 83f27a7db3d2a226d9458292e734ae35abe4b332
Author: Darrel Schneider <dschneider@pivotal.io>
AuthorDate: Wed Aug 30 09:34:04 2017 -0700

    GEODE-3543: refactor ExpiryTask to not call GemFireCacheImpl.getInstance
---
 .../apache/geode/internal/cache/ExpiryTask.java    | 54 +++++++++++-----------
 .../apache/geode/internal/cache/LocalRegion.java   | 12 ++---
 .../geode/internal/cache/RegionExpiryTask.java     |  2 +-
 .../geode/cache30/MultiVMRegionTestCase.java       |  8 ++--
 .../org/apache/geode/cache30/RegionTestCase.java   |  2 +-
 5 files changed, 37 insertions(+), 41 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ExpiryTask.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ExpiryTask.java
index 3048e52..9e21115 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/ExpiryTask.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ExpiryTask.java
@@ -144,7 +144,6 @@ public abstract class ExpiryTask extends SystemTimer.SystemTimerTask {
       return extm;
   }
 
-
   /**
    * Return true if current task could have expired. Return false if expiration is impossible.
    */
@@ -457,41 +456,25 @@ public abstract class ExpiryTask extends SystemTimer.SystemTimerTask
{
   private static final ThreadLocal<Long> now = new ThreadLocal<Long>();
 
   /**
-   * To reduce the number of times we need to call System.currentTimeMillis you can call
this method
-   * to set a thread local. Make sure and call {@link #clearNow()} in a finally block after
calling
-   * this method.
+   * To reduce the number of times we need to call calculateNow, you can call this method
to set now
+   * in a thread local. When the run returns the thread local is cleared.
    */
-  public static void setNow() {
-    now.set(calculateNow());
-  }
-
-  private static long calculateNow() {
-    InternalCache cache = GemFireCacheImpl.getInstance();
-    if (cache != null) {
-      // Use cache.cacheTimeMillis here. See bug 52267.
-      InternalDistributedSystem ids = cache.getInternalDistributedSystem();
-      if (ids != null) {
-        return ids.getClock().cacheTimeMillis();
-      }
+  static void doWithNowSet(LocalRegion lr, Runnable runnable) {
+    now.set(calculateNow(lr.getCache()));
+    try {
+      runnable.run();
+    } finally {
+      now.remove();
     }
-    return 0L;
-  }
-
-  /**
-   * Call this method after a thread has called {@link #setNow()} once you are done calling
code
-   * that may call {@link #getNow()}.
-   */
-  public static void clearNow() {
-    now.remove();
   }
 
   /**
-   * Returns the current time in milliseconds. If the current thread has called {@link #setNow()}
-   * then that time is return.
+   * Returns the current time in milliseconds. If the current thread has set the now thread
local
+   * then that time is return. Otherwise now is calculated and returned.
    * 
    * @return the current time in milliseconds
    */
-  public static long getNow() {
+  protected long getNow() {
     long result;
     Long tl = now.get();
     if (tl != null) {
@@ -502,6 +485,21 @@ public abstract class ExpiryTask extends SystemTimer.SystemTimerTask
{
     return result;
   }
 
+  public long calculateNow() {
+    return calculateNow(getLocalRegion().getCache());
+  }
+
+  public static long calculateNow(InternalCache cache) {
+    if (cache != null) {
+      // Use cache.cacheTimeMillis here. See bug 52267.
+      InternalDistributedSystem ids = cache.getInternalDistributedSystem();
+      if (ids != null) {
+        return ids.getClock().cacheTimeMillis();
+      }
+    }
+    return 0L;
+  }
+
   // Should only be set by unit tests
   public static ExpiryTaskListener expiryTaskListener;
 
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 6bee770..47ce7e0 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -7746,19 +7746,17 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
     if (!isInitialized()) {
       return; // don't schedule expiration until region is initialized (bug
     }
+    if (!isEntryExpiryPossible()) {
+      return;
+    }
     // OK to ignore transaction since Expiry only done non-tran
     Iterator<RegionEntry> it = this.entries.regionEntries().iterator();
     if (it.hasNext()) {
-      try {
-        if (isEntryExpiryPossible()) {
-          ExpiryTask.setNow();
-        }
+      ExpiryTask.doWithNowSet(this, () -> {
         while (it.hasNext()) {
           addExpiryTask(it.next());
         }
-      } finally {
-        ExpiryTask.clearNow();
-      }
+      });
     }
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionExpiryTask.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionExpiryTask.java
index 065d966..06e2c99 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionExpiryTask.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionExpiryTask.java
@@ -141,6 +141,6 @@ abstract class RegionExpiryTask extends ExpiryTask {
       SystemFailure.checkFailure();
     }
     return super.toString() + " for " + getLocalRegion().getFullPath() + ", expiration time:
"
-        + expireTime + " [now: " + getNow() + "]";
+        + expireTime + " [now: " + calculateNow() + "]";
   }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/cache30/MultiVMRegionTestCase.java
b/geode-core/src/test/java/org/apache/geode/cache30/MultiVMRegionTestCase.java
index 34cf1ca..e7e6ee1 100644
--- a/geode-core/src/test/java/org/apache/geode/cache30/MultiVMRegionTestCase.java
+++ b/geode-core/src/test/java/org/apache/geode/cache30/MultiVMRegionTestCase.java
@@ -3933,8 +3933,8 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
                       ((InternalDistributedSystem) (region.getCache().getDistributedSystem()))
                           .getClock().getStopTime();
                   logger.info("DEBUG: waiting for expire destroy expirationTime= "
-                      + eet.getExpirationTime() + " now=" + eet.getNow() + " stopTime=" +
stopTime
-                      + " currentTimeMillis=" + System.currentTimeMillis());
+                      + eet.getExpirationTime() + " now=" + eet.calculateNow() + " stopTime="
+                      + stopTime + " currentTimeMillis=" + System.currentTimeMillis());
                 } else {
                   logger.info("DEBUG: waiting for expire destroy but expiry task is null");
                 }
@@ -3948,8 +3948,8 @@ public abstract class MultiVMRegionTestCase extends RegionTestCase {
               try {
                 EntryExpiryTask eet = getEntryExpiryTask(region, key);
                 if (eet != null) {
-                  expiryInfo = "expirationTime= " + eet.getExpirationTime() + " now=" + eet.getNow()
-                      + " currentTimeMillis=" + System.currentTimeMillis();
+                  expiryInfo = "expirationTime= " + eet.getExpirationTime() + " now="
+                      + eet.calculateNow() + " currentTimeMillis=" + System.currentTimeMillis();
                 }
               } catch (EntryNotFoundException ex) {
                 expiryInfo = "EntryNotFoundException when getting expiry task";
diff --git a/geode-core/src/test/java/org/apache/geode/cache30/RegionTestCase.java b/geode-core/src/test/java/org/apache/geode/cache30/RegionTestCase.java
index 3b56748..f086125 100644
--- a/geode-core/src/test/java/org/apache/geode/cache30/RegionTestCase.java
+++ b/geode-core/src/test/java/org/apache/geode/cache30/RegionTestCase.java
@@ -3480,7 +3480,7 @@ public abstract class RegionTestCase extends JUnit4CacheTestCase {
         // ignore
       }
       Date ttlTime = new Date(et.getTTLExpirationTime());
-      Date getNow = new Date(et.getNow());
+      Date getNow = new Date(et.calculateNow());
       Date scheduleETime = new Date(et.scheduledExecutionTime());
       getCache().getLogger()
           .info(callback + " now: " + getCurrentTimeStamp(now) + " ttl:" + getCurrentTimeStamp(ttl)

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <commits@geode.apache.org>'].

Mime
View raw message