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-5276: fix entries stats from going negative (#2033)
Date Thu, 07 Jun 2018 17:47:52 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 3cef629  GEODE-5276: fix entries stats from going negative (#2033)
3cef629 is described below

commit 3cef62916627b8c87903b5044e93e0dcf8d1c465
Author: Darrel Schneider <dschneider@pivotal.io>
AuthorDate: Thu Jun 7 10:47:48 2018 -0700

    GEODE-5276: fix entries stats from going negative (#2033)
    
    Entries was going negative during a transaction because:
    * When txApplyDestroy was adding a new entry that would be a TOMBSTONE; it was setting
the value of the new entry initially to Token.DESTROYED. This was wrong. It should have waited
to set this until later when it knew it was in "tokenMode".
    * When makeTombstone was later called it did a check to see if the entry was new. It based
this in the value being Token.REMOVE_PHASE1. Since it was not (it was DESTROYED) this prevented
it from doing a corrective inc of the stat to prevent it from going negative.
    
    So the fix is the use REMOVE_PHASE1 during initialization and to only set the value of
the new entry to DESTROYED if we are in tokenMode.
---
 .../geode/internal/cache/AbstractRegionMap.java       |  7 +++----
 .../cache/AbstractRegionMapTxApplyDestroyTest.java    | 19 +++++++++++++++++--
 2 files changed, 20 insertions(+), 6 deletions(-)

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 3155d79..97f6ae4 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
@@ -1156,9 +1156,7 @@ public abstract class AbstractRegionMap
         // generate versions and Tombstones for destroys
         boolean dispatchListenerEvent = inTokenMode;
         boolean opCompleted = false;
-        // TODO: if inTokenMode then Token.DESTROYED is ok but what about !inTokenMode because
-        // owner.concurrencyChecksEnabled? In that case we do not want a DESTROYED token.
-        RegionEntry newRe = getEntryFactory().createEntry(owner, key, Token.DESTROYED);
+        RegionEntry newRe = getEntryFactory().createEntry(owner, key, Token.REMOVED_PHASE1);
         if (oqlIndexManager != null) {
           oqlIndexManager.waitForIndexInit();
         }
@@ -1236,7 +1234,6 @@ public abstract class AbstractRegionMap
               }
             }
             if (!opCompleted) {
-              // already has value set to Token.DESTROYED
               opCompleted = true;
               boolean invokeCallbacks = shouldInvokeCallbacks(owner, isRegionReady || inRI);
               callbackEvent = createCallbackEvent(owner, op, key, null, txId, txEvent, eventId,
@@ -1265,6 +1262,8 @@ public abstract class AbstractRegionMap
                   newRe.removePhase1(owner, false); // fix for bug 43063
                   newRe.removePhase2();
                   removeEntry(key, newRe, false);
+                } else {
+                  newRe.setValue(owner, Token.DESTROYED);
                 }
                 owner.txApplyDestroyPart2(newRe, newRe.getKey(), inTokenMode,
                     false /* clearConflict */, true);
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTxApplyDestroyTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTxApplyDestroyTest.java
index 1d649cd..376bec146 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTxApplyDestroyTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTxApplyDestroyTest.java
@@ -741,7 +741,8 @@ public class AbstractRegionMapTxApplyDestroyTest {
 
     doTxApplyDestroy();
 
-    verify(regionEntryFactory, times(1)).createEntry(same(owner), eq(key), eq(Token.DESTROYED));
+    verify(regionEntryFactory, times(1)).createEntry(same(owner), eq(key),
+        eq(Token.REMOVED_PHASE1));
   }
 
   @Test
@@ -752,7 +753,8 @@ public class AbstractRegionMapTxApplyDestroyTest {
 
     doTxApplyDestroy();
 
-    verify(regionEntryFactory, times(1)).createEntry(same(owner), eq(key), eq(Token.DESTROYED));
+    verify(regionEntryFactory, times(1)).createEntry(same(owner), eq(key),
+        eq(Token.REMOVED_PHASE1));
   }
 
   @Test
@@ -1036,6 +1038,19 @@ public class AbstractRegionMapTxApplyDestroyTest {
   }
 
   @Test
+  public void txApplyDestroyCallsSetValueWithDestroyedToken_givenFactoryRegionEntryWithoutConcurrencyChecksInTokenMode()
+      throws RegionClearedException {
+    givenLocalRegion();
+    givenFactoryRegionEntry();
+    givenNoConcurrencyChecks();
+    inTokenMode = true;
+
+    doTxApplyDestroy();
+
+    verify(factoryRegionEntry, times(1)).setValue(same(owner), eq(Token.DESTROYED));
+  }
+
+  @Test
   public void txApplyDestroyDoesNotCallSetVersionTag_givenFactoryRegionEntryWithPartitionedRegionButNoConcurrencyChecks()
{
     givenBucketRegion();
     givenFactoryRegionEntry();

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

Mime
View raw message