geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kl...@apache.org
Subject [22/50] [abbrv] incubator-geode git commit: GEODE-706 Fixed race condition between expiry thread and put thread.
Date Mon, 31 Oct 2016 20:56:02 GMT
GEODE-706 Fixed race condition between expiry thread and put thread.

There was possibility that expiry thread destroying the entry and
another thread doing update on same key. In this case expiry thread
cancel's existing task and update thread adds expiry task. But this
tasks are refer by regionEntry, which is same for both the threads.
So in this case if expiry thread cancel's task after update thread
then that entry will never expire.


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

Branch: refs/heads/feature/GEODE-1930
Commit: a3bd256648e30b0bf04c565a8f21d00868c29806
Parents: f1df6fc
Author: Hitesh Khamesra <hkhamesra@pivotal.io>
Authored: Fri Oct 14 14:00:25 2016 -0700
Committer: Hitesh Khamesra <hkhamesra@pivotal.io>
Committed: Mon Oct 17 14:30:46 2016 -0700

----------------------------------------------------------------------
 .../geode/internal/cache/AbstractRegionMap.java  |  4 ++--
 .../geode/internal/cache/EntryEventImpl.java     |  9 +++++++++
 .../geode/internal/cache/EntryExpiryTask.java    |  3 ++-
 .../apache/geode/internal/cache/LocalRegion.java | 19 +++++++++++++++----
 4 files changed, 28 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/a3bd2566/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
index 5861e9a..e02c7e1 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
@@ -1513,9 +1513,9 @@ public abstract class AbstractRegionMap implements RegionMap {
           } finally {
             if (opCompleted) {
               if (re != null) {
-                owner.cancelExpiryTask(re);
+                owner.cancelExpiryTask(re, event.getExpiryTask());
               } else if (tombstone != null) {
-                owner.cancelExpiryTask(tombstone);
+                owner.cancelExpiryTask(tombstone, event.getExpiryTask());
               }
             }
           }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/a3bd2566/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
index 6a964c0..d059aab 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
@@ -73,6 +73,7 @@ public class EntryEventImpl
   // PACKAGE FIELDS //
   public transient LocalRegion region;
   private transient RegionEntry re;
+  private transient ExpiryTask expiryTask;
 
   protected KeyInfo keyInfo;
 
@@ -2853,4 +2854,12 @@ public class EntryEventImpl
   public boolean isOldValueOffHeap() {
     return isOffHeapReference(this.oldValue);
   }
+
+  public ExpiryTask getExpiryTask() {
+    return expiryTask;
+  }
+
+  public void setExpiryTask(ExpiryTask expiryTask) {
+    this.expiryTask = expiryTask;
+  }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/a3bd2566/geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
index 816f32f..0c20d32 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
@@ -117,6 +117,7 @@ public class EntryExpiryTask extends ExpiryTask {
     @Released EntryEventImpl event = EntryEventImpl.create(
         lr, Operation.EXPIRE_DESTROY, key, null,
         createExpireEntryCallback(lr, key), false, lr.getMyId());
+    event.setExpiryTask(this);
     try {
     event.setPendingSecondaryExpireDestroy(isPending);
     if (lr.generateEventID()) {
@@ -229,7 +230,7 @@ public class EntryExpiryTask extends ExpiryTask {
     // so the next call to addExpiryTaskIfAbsent will
     // add a new task instead of doing nothing, which would
     // erroneously cancel expiration for this key.
-    getLocalRegion().cancelExpiryTask(this.re);
+    getLocalRegion().cancelExpiryTask(this.re, null);
     getLocalRegion().performExpiryTimeout(this);
   }
   

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/a3bd2566/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
----------------------------------------------------------------------
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 a6951de..ac4c705 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
@@ -8717,14 +8717,25 @@ public class LocalRegion extends AbstractRegion
       }
     }
   }
+  
+  void cancelExpiryTask(RegionEntry re) {
+    cancelExpiryTask(re, null);
+  }
 
-  void cancelExpiryTask(RegionEntry re)
+  void cancelExpiryTask(RegionEntry re, ExpiryTask expiryTask)
   {
-    EntryExpiryTask oldTask = this.entryExpiryTasks.remove(re);
-    if (oldTask != null) {
-      if (oldTask.cancel()) {
+    if (expiryTask != null) {
+      this.entryExpiryTasks.remove(re, expiryTask);
+      if (expiryTask.cancel()) {
         this.cache.getExpirationScheduler().incCancels();
       }
+    } else {
+      EntryExpiryTask oldTask = this.entryExpiryTasks.remove(re);
+      if (oldTask != null) {
+        if (oldTask.cancel()) {
+          this.cache.getExpirationScheduler().incCancels();
+        }
+      }
     }
   }
 


Mime
View raw message