You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by nn...@apache.org on 2018/03/14 00:12:51 UTC

[geode] branch develop updated: GEODE-4823: OQL index not updated for tombstone (#1603)

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

nnag 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 140118d  GEODE-4823: OQL index not updated for tombstone (#1603)
140118d is described below

commit 140118d066dc5da914d23eb79313148c3462721e
Author: Nabarun Nag <na...@users.noreply.github.com>
AuthorDate: Tue Mar 13 17:12:48 2018 -0700

    GEODE-4823: OQL index not updated for tombstone (#1603)
    
    * If the region entry is a tombstone then OQL indexes are not updated
    	* The operation extracted to a external method.
---
 .../cache/entries/AbstractRegionEntry.java         | 51 ++++++++++++----------
 .../cache/entries/AbstractRegionEntryTest.java     | 23 +++++++++-
 2 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
index 33d6e34..761cb43 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/AbstractRegionEntry.java
@@ -14,7 +14,8 @@
  */
 package org.apache.geode.internal.cache.entries;
 
-import static org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.*;
+import static org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.ABSTRACT_REGION_ENTRY_FILL_IN_VALUE;
+import static org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.ABSTRACT_REGION_ENTRY_PREPARE_VALUE_FOR_CACHE;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -49,14 +50,12 @@ import org.apache.geode.internal.cache.CachedDeserializableFactory;
 import org.apache.geode.internal.cache.DistributedRegion;
 import org.apache.geode.internal.cache.EntryEventImpl;
 import org.apache.geode.internal.cache.FilterProfile;
-import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.ImageState;
 import org.apache.geode.internal.cache.InitialImageOperation.Entry;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.InternalCacheEvent;
 import org.apache.geode.internal.cache.InternalRegion;
 import org.apache.geode.internal.cache.RegionClearedException;
-import org.apache.geode.internal.cache.RegionEntry;
 import org.apache.geode.internal.cache.RegionEntryContext;
 import org.apache.geode.internal.cache.RegionQueue;
 import org.apache.geode.internal.cache.TXManagerImpl;
@@ -868,27 +867,7 @@ public abstract class AbstractRegionEntry implements HashRegionEntry<Object, Obj
       region.recordEvent(event);
       // don't do index maintenance on a destroy if the value in the
       // RegionEntry (the old value) is invalid
-      if (!region.isProxy() && !isInvalid()) {
-        IndexManager indexManager = region.getIndexManager();
-        if (indexManager != null) {
-          try {
-            if (isValueNull()) {
-              @Released
-              Object value = getValueOffHeapOrDiskWithoutFaultIn(region);
-              try {
-                Object preparedValue = prepareValueForCache(region, value, false);
-                _setValue(preparedValue);
-                releaseOffHeapRefIfRegionBeingClosedOrDestroyed(region, preparedValue);
-              } finally {
-                OffHeapHelper.release(value);
-              }
-            }
-            indexManager.updateIndexes(this, IndexManager.REMOVE_ENTRY, IndexProtocol.OTHER_OP);
-          } catch (QueryException e) {
-            throw new IndexMaintenanceException(e);
-          }
-        }
-      }
+      updateIndexOnDestroyOperation(region);
 
       boolean removeEntry = false;
       VersionTag v = event.getVersionTag();
@@ -931,6 +910,30 @@ public abstract class AbstractRegionEntry implements HashRegionEntry<Object, Obj
     }
   }
 
+  protected void updateIndexOnDestroyOperation(InternalRegion region) {
+    if (!isTombstone() && !region.isProxy() && !isInvalid()) {
+      IndexManager indexManager = region.getIndexManager();
+      if (indexManager != null) {
+        try {
+          if (isValueNull()) {
+            @Released
+            Object value = getValueOffHeapOrDiskWithoutFaultIn(region);
+            try {
+              Object preparedValue = prepareValueForCache(region, value, false);
+              _setValue(preparedValue);
+              releaseOffHeapRefIfRegionBeingClosedOrDestroyed(region, preparedValue);
+            } finally {
+              OffHeapHelper.release(value);
+            }
+          }
+          indexManager.updateIndexes(this, IndexManager.REMOVE_ENTRY, IndexProtocol.OTHER_OP);
+        } catch (QueryException e) {
+          throw new IndexMaintenanceException(e);
+        }
+      }
+    }
+  }
+
   private static boolean destroyShouldProceedBasedOnCurrentValue(Object curValue) {
     if (curValue == null) {
       return false;
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/entries/AbstractRegionEntryTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/entries/AbstractRegionEntryTest.java
index 3c1c5e1..f95b023 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/entries/AbstractRegionEntryTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/entries/AbstractRegionEntryTest.java
@@ -16,7 +16,11 @@ package org.apache.geode.internal.cache.entries;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.io.ByteArrayInputStream;
@@ -29,13 +33,15 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import org.apache.geode.DataSerializer;
+import org.apache.geode.cache.query.QueryException;
+import org.apache.geode.cache.query.internal.index.IndexManager;
+import org.apache.geode.cache.query.internal.index.IndexProtocol;
 import org.apache.geode.internal.cache.EntryEventImpl;
+import org.apache.geode.internal.cache.InternalRegion;
 import org.apache.geode.internal.cache.LocalRegion;
 import org.apache.geode.internal.cache.RegionClearedException;
 import org.apache.geode.internal.cache.RegionEntryContext;
 import org.apache.geode.internal.cache.Token;
-import org.apache.geode.internal.cache.entries.AbstractRegionEntry;
-import org.apache.geode.internal.cache.entries.OffHeapRegionEntry;
 import org.apache.geode.internal.cache.versions.RegionVersionVector;
 import org.apache.geode.internal.cache.versions.VersionTag;
 import org.apache.geode.internal.offheap.MemoryAllocatorImpl;
@@ -66,6 +72,19 @@ public class AbstractRegionEntryTest {
   }
 
 
+  @Test
+  public void whenRegionEntryIsTombstoneThenIndexShouldNotBeUpdated() throws QueryException {
+    InternalRegion region = mock(InternalRegion.class);
+    IndexManager indexManager = mock(IndexManager.class);
+    when(region.getIndexManager()).thenReturn(indexManager);
+    AbstractRegionEntry abstractRegionEntry = mock(AbstractRegionEntry.class);
+    when(abstractRegionEntry.isTombstone()).thenReturn(true);
+    abstractRegionEntry.updateIndexOnDestroyOperation(region);
+    verify(indexManager, never()).updateIndexes(any(AbstractRegionEntry.class),
+        eq(IndexManager.REMOVE_ENTRY), eq(IndexProtocol.OTHER_OP));
+
+  }
+
 
   @Test
   public void whenPrepareValueForCacheCalledWithOffHeapEntryHasNewCachedSerializedValue()

-- 
To stop receiving notification emails like this one, please contact
nnag@apache.org.