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

[geode] branch feature/GEODE-6246 created (now 6f352a9)

This is an automated email from the ASF dual-hosted git repository.

boglesby pushed a change to branch feature/GEODE-6246
in repository https://gitbox.apache.org/repos/asf/geode.git.


      at 6f352a9  GEODE-6246: Forced super.basicDestroy to be called during sender queue initialization

This branch includes the following new commits:

     new 6f352a9  GEODE-6246: Forced super.basicDestroy to be called during sender queue initialization

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[geode] 01/01: GEODE-6246: Forced super.basicDestroy to be called during sender queue initialization

Posted by bo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

boglesby pushed a commit to branch feature/GEODE-6246
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 6f352a9bbb5f2859cf27d649de092db52a39c437
Author: Barry Oglesby <bo...@pivotal.io>
AuthorDate: Thu Jan 3 16:02:22 2019 -0800

    GEODE-6246: Forced super.basicDestroy to be called during sender queue initialization
---
 .../internal/cache/AbstractBucketRegionQueue.java  | 48 +---------------------
 .../geode/internal/cache/BucketRegionQueue.java    | 20 ++++++---
 .../internal/cache/BucketRegionQueueJUnitTest.java |  4 +-
 3 files changed, 18 insertions(+), 54 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractBucketRegionQueue.java b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractBucketRegionQueue.java
index 7ac5669..2bf304f 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractBucketRegionQueue.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractBucketRegionQueue.java
@@ -27,7 +27,6 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.cache.CacheWriterException;
-import org.apache.geode.cache.EntryNotFoundException;
 import org.apache.geode.cache.Operation;
 import org.apache.geode.cache.RegionAttributes;
 import org.apache.geode.cache.RegionDestroyedException;
@@ -43,7 +42,6 @@ import org.apache.geode.internal.cache.wan.parallel.ConcurrentParallelGatewaySen
 import org.apache.geode.internal.concurrent.ConcurrentHashSet;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.offheap.OffHeapRegionEntryHelper;
-import org.apache.geode.internal.offheap.annotations.Released;
 
 public abstract class AbstractBucketRegionQueue extends BucketRegion {
   protected static final Logger logger = LogService.getLogger();
@@ -180,39 +178,7 @@ public abstract class AbstractBucketRegionQueue extends BucketRegion {
     return initializationLock;
   }
 
-  public void destroyKey(Object key) throws ForceReattemptException {
-    if (logger.isDebugEnabled()) {
-      logger.debug(" destroying primary key {}", key);
-    }
-    @Released
-    EntryEventImpl event = getPartitionedRegion().newDestroyEntryEvent(key, null);
-    event.setEventId(new EventID(cache.getInternalDistributedSystem()));
-    try {
-      event.setRegion(this);
-      basicDestroy(event, true, null);
-      checkReadiness();
-    } catch (EntryNotFoundException enf) {
-      if (getPartitionedRegion().isDestroyed()) {
-        getPartitionedRegion().checkReadiness();
-        if (isBucketDestroyed()) {
-          throw new ForceReattemptException("Bucket moved",
-              new RegionDestroyedException(
-                  "Region has been destroyed",
-                  getPartitionedRegion().getFullPath()));
-        }
-      }
-      throw enf;
-    } catch (RegionDestroyedException rde) {
-      getPartitionedRegion().checkReadiness();
-      if (isBucketDestroyed()) {
-        throw new ForceReattemptException("Bucket moved while destroying key " + key, rde);
-      }
-    } finally {
-      event.release();
-    }
-
-    this.notifyEntriesRemoved();
-  }
+  public abstract void destroyKey(Object key) throws ForceReattemptException;
 
   public void decQueueSize(int size) {
     this.gatewaySenderStats.decQueueSize(size);
@@ -343,18 +309,6 @@ public abstract class AbstractBucketRegionQueue extends BucketRegion {
 
   }
 
-  @Override
-  public void basicDestroy(final EntryEventImpl event, final boolean cacheWrite,
-      Object expectedOldValue)
-      throws EntryNotFoundException, CacheWriterException, TimeoutException {
-    try {
-      super.basicDestroy(event, cacheWrite, expectedOldValue);
-    } finally {
-      GatewaySenderEventImpl.release(event.getRawOldValue());
-    }
-  }
-
-
   /**
    * Return all of the user PR buckets for this bucket region queue.
    */
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
index 9d95672..af7b1ad 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
@@ -176,7 +176,14 @@ public class BucketRegionQueue extends AbstractBucketRegionQueue {
       }
       if (containsKey(key)) {
         try {
-          destroyKey(key);
+          // The destroyKey method is called with forceBasicDestroy set to true since containsKey
+          // can be true even though get on the key returns null. That happens when the
+          // ParallelQueueRemovalMessage destroys the entry first. In that case, when this method is
+          // invoked, the raw value is the DESTROYED token. This was causing the
+          // EntryNotFoundException to be thrown by basicDestroy. The forceBasicDestroy boolean set
+          // to true forces the super.basicDestroy call to be made instead of the
+          // EntryNotFoundException to be thrown.
+          destroyKey(key, true);
           if (isDebugEnabled) {
             logger.debug("Destroyed {} from bucket: ", key, getId());
           }
@@ -351,16 +358,15 @@ public class BucketRegionQueue extends AbstractBucketRegionQueue {
     return entryFound;
   }
 
-  @Override
   public void basicDestroy(final EntryEventImpl event, final boolean cacheWrite,
-      Object expectedOldValue)
+      Object expectedOldValue, boolean forceBasicDestroy)
       throws EntryNotFoundException, CacheWriterException, TimeoutException {
     boolean indexEntryFound = true;
     if (getPartitionedRegion().isConflationEnabled()) {
       indexEntryFound = containsKey(event.getKey()) && removeIndex((Long) event.getKey());
     }
     try {
-      if (indexEntryFound) {
+      if (indexEntryFound || forceBasicDestroy) {
         super.basicDestroy(event, cacheWrite, expectedOldValue);
       } else {
         throw new EntryNotFoundException(event.getKey().toString());
@@ -551,6 +557,10 @@ public class BucketRegionQueue extends AbstractBucketRegionQueue {
    * done during selective merge from r41425 from gemfire701X_maint.
    */
   public void destroyKey(Object key) throws ForceReattemptException {
+    destroyKey(key, false);
+  }
+
+  private void destroyKey(Object key, boolean forceBasicDestroy) throws ForceReattemptException {
     if (logger.isDebugEnabled()) {
       logger.debug(" destroying primary key {}", key);
     }
@@ -559,7 +569,7 @@ public class BucketRegionQueue extends AbstractBucketRegionQueue {
     try {
       event.setEventId(new EventID(cache.getInternalDistributedSystem()));
       event.setRegion(this);
-      basicDestroy(event, true, null);
+      basicDestroy(event, true, null, forceBasicDestroy);
       setLatestAcknowledgedKey((Long) key);
       checkReadiness();
     } catch (EntryNotFoundException enf) {
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/BucketRegionQueueJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/BucketRegionQueueJUnitTest.java
index db74bec..86f1b3e 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/BucketRegionQueueJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/BucketRegionQueueJUnitTest.java
@@ -98,7 +98,7 @@ public class BucketRegionQueueJUnitTest {
     doReturn(true).when(this.bucketRegionQueue).removeIndex(KEY);
 
     // Invoke basicDestroy
-    this.bucketRegionQueue.basicDestroy(event, true, null);
+    this.bucketRegionQueue.basicDestroy(event, true, null, false);
 
     // Verify mapDestroy is invoked
     verify(this.bucketRegionQueue).mapDestroy(event, true, false, null);
@@ -118,6 +118,6 @@ public class BucketRegionQueueJUnitTest {
     when(this.bucketRegionQueue.containsKey(KEY)).thenReturn(false);
 
     // Invoke basicDestroy
-    this.bucketRegionQueue.basicDestroy(event, true, null);
+    this.bucketRegionQueue.basicDestroy(event, true, null, false);
   }
 }