You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by zh...@apache.org on 2022/04/17 22:53:13 UTC

[geode] 01/01: GEODE-10229: GII image should fill disk region RVV's exceptions. There're 2 issues: Tombstone in GII image should save version tag into disk region RVV even the tombstone is the same as local one. Disk region RVV holder did not use bitset. Only bitset based holder can fill special exception. Should do it for non-bitset holder too.

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

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

commit 7d3623991dc0cb82a57052a9ff622276fcbe4892
Author: zhouxh <gz...@pivotal.io>
AuthorDate: Sun Apr 17 15:41:35 2022 -0700

    GEODE-10229: GII image should fill disk region RVV's exceptions.
        There're 2 issues: Tombstone in GII image should save version tag into disk region RVV
        even the tombstone is the same as local one. Disk region RVV holder did not use bitset.
        Only bitset based holder can fill special exception. Should do it for non-bitset holder too.
---
 .../geode/internal/cache/GIIDeltaDUnitTest.java    | 19 ++++++--
 .../geode/internal/cache/AbstractRegionMap.java    |  7 +++
 .../internal/cache/InitialImageOperation.java      |  8 ++--
 .../cache/versions/RegionVersionHolder.java        |  2 +-
 .../versions/RegionVersionHolderJUnitTest.java     | 52 ++++++++++++++++++++++
 5 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/GIIDeltaDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/GIIDeltaDUnitTest.java
index 7bed8f01b8..8e4de72136 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/GIIDeltaDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/GIIDeltaDUnitTest.java
@@ -980,8 +980,8 @@ public class GIIDeltaDUnitTest extends JUnit4CacheTestCase {
     AsyncInvocation async3 = createDistributedRegionAsync(R);
     waitForCallbackStarted(R, GIITestHookType.BeforeSavedReceivedRVV);
 
-    doOneDestroy(P, 4, "key2");
-    doOnePut(P, 5, "key1");
+    doOnePut(P, 4, "key1");
+    doOneDestroy(P, 5, "key2");
     R.invoke(
         () -> InitialImageOperation.resetGIITestHook(GIITestHookType.BeforeSavedReceivedRVV, true));
     waitForCallbackStarted(R, GIITestHookType.AfterSavedReceivedRVV);
@@ -992,6 +992,12 @@ public class GIIDeltaDUnitTest extends JUnit4CacheTestCase {
     changeForceFullGII(R, true, true);
     changeForceFullGII(P, false, true);
     verifyDeltaSizeFromStats(R, 2, 0);
+
+    // Now restart R again to re-do GII
+    closeCache(R);
+    createDistributedRegion(R);
+    waitForToVerifyRVV(R, memberP, 5, null, 0); // P's rvv=r5, gc=0
+    verifyTombstoneExist(R, "key2", true, false);
   }
 
   /**
@@ -1038,8 +1044,8 @@ public class GIIDeltaDUnitTest extends JUnit4CacheTestCase {
     AsyncInvocation async3 = createDistributedRegionAsync(R);
     waitForCallbackStarted(R, GIITestHookType.AfterCalculatedUnfinishedOps);
 
-    doOneDestroy(P, 4, "key2");
-    doOnePut(P, 5, "key1");
+    doOnePut(P, 4, "key1");
+    doOneDestroy(P, 5, "key2");
     R.invoke(() -> InitialImageOperation
         .resetGIITestHook(GIITestHookType.AfterCalculatedUnfinishedOps, true));
     waitForCallbackStarted(R, GIITestHookType.AfterSavedReceivedRVV);
@@ -1050,6 +1056,11 @@ public class GIIDeltaDUnitTest extends JUnit4CacheTestCase {
     verifyDeltaSizeFromStats(R, 2, 1);
     changeForceFullGII(R, false, true);
     changeForceFullGII(P, false, true);
+    // Now restart R again to re-do GII
+    closeCache(R);
+    createDistributedRegion(R);
+    waitForToVerifyRVV(R, memberP, 5, null, 0); // P's rvv=r5, gc=0
+    verifyTombstoneExist(R, "key2", true, false);
   }
 
   /*
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 fadf308736..6f01e0eb3a 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
@@ -791,6 +791,13 @@ public abstract class AbstractRegionMap extends BaseRegionMap
                   boolean isSameTombstone = oldRe.isTombstone() && isTombstone
                       && oldRe.getVersionStamp().asVersionTag().equals(entryVersion);
                   if (isSameTombstone) {
+                    if (owner.getDiskRegion() != null
+                        && owner.getDiskRegion().getRegionVersionVector() != null) {
+                      // it's possible the region without persistence has RVV
+                      RegionVersionVector drRVV = owner.getDiskRegion().getRegionVersionVector();
+                      drRVV.recordVersion(entryVersion.getMemberID(),
+                          entryVersion.getRegionVersion());
+                    }
                     return true;
                   }
                   processVersionTagForGII(oldRe, owner, entryVersion, isTombstone, sender,
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java
index 4b0d230d35..3c90a4ef14 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java
@@ -471,8 +471,8 @@ public class InitialImageOperation {
               m.unfinishedKeys = keysOfUnfinishedOps;
               if (isDebugEnabled) {
                 logger.debug(
-                    "Region {} recovered with EndGII flag, rvv is {}. recovered rvv is {}. Do delta GII",
-                    region.getFullPath(), m.versionVector, recoveredRVV);
+                    "Region {} recovered with EndGII flag, rvv is {}. Received rvv is {}. Do delta GII",
+                    region.getFullPath(), m.versionVector, received_rvv);
               }
             }
           }
@@ -503,8 +503,8 @@ public class InitialImageOperation {
         }
 
         // do not remove the following log statement
-        logger.info("Region {} requesting initial image from {}",
-            new Object[] {region.getName(), recipient});
+        logger.info("Region {} requesting initial image from {}, recovered RVV is {}",
+            new Object[] {region.getName(), recipient, recoveredRVV});
 
         dm.putOutgoing(m);
         region.cache.getCancelCriterion().checkCancelInProgress(null);
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionHolder.java b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionHolder.java
index f8215f2d76..5910f79f53 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionHolder.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionHolder.java
@@ -338,7 +338,7 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
       this.version = version;
       return;
     }
-    if (this.version > version) {
+    if (this.version >= version) {
       addOlderVersion(version);
     }
     this.version = Math.max(this.version, version);
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java
index 0d1e385b1d..30c7074282 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java
@@ -53,6 +53,58 @@ public class RegionVersionHolderJUnitTest {
     return member;
   }
 
+  @Test
+  public void fillSpecialExceptionForRVHWithBitSet() {
+    fillSpecialExceptionForRVH(true);
+  }
+
+  @Test
+  public void fillSpecialExceptionForRVHWithoutBitSet() {
+    fillSpecialExceptionForRVH(false);
+  }
+
+  private void fillSpecialExceptionForRVH(boolean useBitSet) {
+    RegionVersionHolder vh1 = null;
+    RegionVersionHolder vh2 = null;
+    if (useBitSet) {
+      vh1 = new RegionVersionHolder(member);
+      vh2 = new RegionVersionHolder(member);
+    } else {
+      vh1 = new RegionVersionHolder(0);
+      vh2 = new RegionVersionHolder(0);
+    }
+    for (int i = 1; i <= 3; i++) {
+      vh1.recordVersion(i);
+    }
+
+    for (int i = 1; i <= 5; i++) {
+      vh2.recordVersion(i);
+    }
+
+    // create special exception 5(n=6,p=3)
+    vh2.initializeFrom(vh1);
+    System.out.println("vh2=" + vh2);
+    List<RVVException> exceptionList = vh2.getExceptionForTest();
+    assertEquals(1, exceptionList.size());
+    RVVException exception = exceptionList.get(0);
+    assertEquals(3, exception.previousVersion);
+    assertEquals(6, exception.nextVersion);
+    System.out.println("vh2=" + vh2);
+
+    vh2.recordVersion(5);
+    exceptionList = vh2.getExceptionForTest();
+    assertEquals(1, exceptionList.size());
+    exception = exceptionList.get(0);
+    assertEquals(3, exception.previousVersion);
+    assertEquals(5, exception.nextVersion);
+    System.out.println("vh2=" + vh2);
+
+    vh2.recordVersion(4);
+    exceptionList = vh2.getExceptionForTest();
+    assertEquals(0, exceptionList.size());
+    System.out.println("vh2=" + vh2);
+  }
+
   @Test
   public void test48066_1() {
     RegionVersionHolder vh1 = new RegionVersionHolder(member);