You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ag...@apache.org on 2018/11/13 00:50:57 UTC

[geode] branch develop updated: GEODE-6013: Made changes to use the expected initial image requester's rvv information (#2819)

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

agingade 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 b4befb9  GEODE-6013: Made changes to use the expected initial image requester's rvv information (#2819)
b4befb9 is described below

commit b4befb99626e85045379e5b6fb8804b9db2a3eb9
Author: agingade <ag...@pivotal.io>
AuthorDate: Mon Nov 12 16:50:47 2018 -0800

    GEODE-6013: Made changes to use the expected initial image requester's rvv information (#2819)
    
    * GEODE-6013: Made changes to use the expected initial image requester's rvv information instead of the image provider's local rvv while determining full or delta GII.
    
    There was logical error where it was using provider's local exception(rvv) instead of using requester's local exception(rvv). This could result in performing Delta GII instead of Full GII.
---
 .../cache/PersistentRegionRecoveryDUnitTest.java   |  86 +++++++++++++
 .../internal/cache/InitialImageOperation.java      |   1 -
 .../cache/versions/RegionVersionVector.java        |  27 ++--
 .../cache/versions/RegionVersionVectorTest.java    | 139 +++++++++++++++++++++
 4 files changed, 239 insertions(+), 14 deletions(-)

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PersistentRegionRecoveryDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PersistentRegionRecoveryDUnitTest.java
index 670622f..01d3f4b 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PersistentRegionRecoveryDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PersistentRegionRecoveryDUnitTest.java
@@ -29,6 +29,7 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
+import org.apache.geode.cache.DiskStore;
 import org.apache.geode.cache.DiskStoreFactory;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionFactory;
@@ -409,6 +410,91 @@ public class PersistentRegionRecoveryDUnitTest extends JUnit4DistributedTestCase
     });
   }
 
+  @Test
+  public void testRecoveryFromBackupAndRequestingDeltaGiiDoesFullGiiIfTombstoneGCVersionDiffers()
+      throws Exception {
+    getBlackboard().initBlackboard();
+    vm1.invoke(() -> cacheRule.createCache());
+
+    vm0.invoke(() -> createAsyncDiskRegion(true));
+    vm0.invoke(() -> {
+      Region<String, String> region = cacheRule.getCache().getRegion(regionName);
+      region.put("KEY-1", "VALUE-1");
+      region.put("KEY-2", "VALUE-2");
+      flushAsyncDiskRegion();
+    });
+
+    vm1.invoke(() -> createAsyncDiskRegion(true));
+    vm1.invoke(() -> {
+      Region<String, String> region = cacheRule.getCache().getRegion(regionName);
+      region.put("KEY-1", "VALUE-1");
+      region.put("KEY-2", "VALUE-2");
+      region.put("KEY-1", "VALUE-3");
+      region.put("KEY-2", "VALUE-4");
+      flushAsyncDiskRegion();
+    });
+
+    vm0.invoke(() -> {
+      Region<String, String> region = cacheRule.getCache().getRegion(regionName);
+      region.destroy("KEY-1");
+    });
+
+    vm0.bounceForcibly();
+
+    vm1.invoke(() -> flushAsyncDiskRegion());
+
+    vm1.invoke(() -> {
+      DistributionMessageObserver.setInstance(
+          new DistributionMessageObserver() {
+            @Override
+            public void beforeProcessMessage(ClusterDistributionManager dm,
+                DistributionMessage message) {
+              if (message instanceof InitialImageOperation.RequestImageMessage) {
+                InitialImageOperation.RequestImageMessage rim =
+                    (InitialImageOperation.RequestImageMessage) message;
+                if (rim.regionPath.contains(regionName)) {
+                  getBlackboard().signalGate("GotRegionIIRequest");
+                  await().until(() -> getBlackboard().isGateSignaled("TombstoneGCDone"));
+                }
+              }
+            }
+          });
+    });
+
+    AsyncInvocation vm0createRegion = vm0.invokeAsync(() -> createAsyncDiskRegion(true));
+
+    vm1.invoke(() -> {
+      await().until(() -> getBlackboard().isGateSignaled("GotRegionIIRequest"));
+      cacheRule.getCache().getTombstoneService().forceBatchExpirationForTests(1);
+      flushAsyncDiskRegion();
+      getBlackboard().signalGate("TombstoneGCDone");
+    });
+
+    vm0createRegion.await();
+
+    vm1.invoke(() -> {
+      Region<String, String> region = cacheRule.getCache().getRegion(regionName);
+      assertThat(region.get("KEY-1")).isEqualTo(null);
+      assertThat(region.get("KEY-2")).isEqualTo("VALUE-4");
+    });
+
+    vm0.invoke(() -> {
+      Region<String, String> region = cacheRule.getCache().getRegion(regionName);
+      assertThat(region.get("KEY-1")).isEqualTo(null);
+      assertThat(region.get("KEY-2")).isEqualTo("VALUE-4");
+
+      CachePerfStats stats = ((LocalRegion) region).getRegionPerfStats();
+      assertThat(stats.getDeltaGetInitialImagesCompleted()).isEqualTo(0);
+      assertThat(stats.getGetInitialImagesCompleted()).isEqualTo(1);
+    });
+  }
+
+  private void flushAsyncDiskRegion() {
+    for (DiskStore store : cacheRule.getCache().listDiskStoresIncludingRegionOwned()) {
+      ((DiskStoreImpl) store).forceFlush();
+    }
+  }
+
   private void createSyncDiskRegion() throws IOException {
     createDiskRegion(false, false, regionName);
   }
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 ba3f741..a3e892d 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
@@ -1596,7 +1596,6 @@ public class InitialImageOperation {
         }
         return true;
       }
-      // TODO GGG: verify GII after UpgradeDiskStore
       return false;
     }
 
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionVector.java b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionVector.java
index 7b03d48..ce10dfd 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionVector.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/RegionVersionVector.java
@@ -977,34 +977,35 @@ public abstract class RegionVersionVector<T extends VersionSource<?>>
     return false;
   }
 
-  private boolean isGCVersionDominatedByHolder(Long gcVersion, RegionVersionHolder<T> otherHolder) {
+  private boolean isGCVersionDominatedByOtherHolder(Long gcVersion,
+      RegionVersionHolder<T> otherHolder) {
     if (gcVersion == null || gcVersion.longValue() == 0) {
       return true;
     } else {
       RegionVersionHolder<T> holder = new RegionVersionHolder<T>(gcVersion.longValue());
-      return !holder.isNewerThanOrCanFillExceptionsFor(otherHolder);
+      return otherHolder.dominates(holder);
     }
   }
 
   /**
-   * Test to see if this vector's rvvgc has updates that has not seen.
+   * See if this vector's rvvgc has updates that has not seen.
    */
-  public synchronized boolean isRVVGCDominatedBy(RegionVersionVector<T> other) {
-    if (other.singleMember) {
+  public synchronized boolean isRVVGCDominatedBy(RegionVersionVector<T> requesterRVV) {
+    if (requesterRVV.singleMember) {
       // do the diff for only a single member. This is typically a member that
       // recently crashed.
       Map.Entry<T, RegionVersionHolder<T>> entry =
-          other.memberToVersion.entrySet().iterator().next();
+          requesterRVV.memberToVersion.entrySet().iterator().next();
 
       Long gcVersion = this.memberToGCVersion.get(entry.getKey());
-      return isGCVersionDominatedByHolder(gcVersion, entry.getValue());
+      return isGCVersionDominatedByOtherHolder(gcVersion, entry.getValue());
     }
 
     boolean isDominatedByRemote = true;
     long localgcversion = this.localGCVersion.get();
     if (localgcversion > 0) {
-      RegionVersionHolder<T> otherHolder = other.memberToVersion.get(this.myId);
-      isDominatedByRemote = isGCVersionDominatedByHolder(localgcversion, otherHolder);
+      RegionVersionHolder<T> otherHolder = requesterRVV.memberToVersion.get(this.myId);
+      isDominatedByRemote = isGCVersionDominatedByOtherHolder(localgcversion, otherHolder);
       if (isDominatedByRemote == false) {
         return false;
       }
@@ -1014,12 +1015,12 @@ public abstract class RegionVersionVector<T extends VersionSource<?>>
       T mbr = entry.getKey();
       Long gcVersion = entry.getValue();
       RegionVersionHolder<T> otherHolder = null;
-      if (mbr.equals(other.getOwnerId())) {
-        otherHolder = localExceptions;
+      if (mbr.equals(requesterRVV.getOwnerId())) {
+        otherHolder = requesterRVV.localExceptions;
       } else {
-        otherHolder = other.memberToVersion.get(mbr);
+        otherHolder = requesterRVV.memberToVersion.get(mbr);
       }
-      isDominatedByRemote = isGCVersionDominatedByHolder(gcVersion, otherHolder);
+      isDominatedByRemote = isGCVersionDominatedByOtherHolder(gcVersion, otherHolder);
       if (isDominatedByRemote == false) {
         return false;
       }
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionVectorTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionVectorTest.java
index deb7d4e..a018aaf 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionVectorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionVectorTest.java
@@ -661,6 +661,145 @@ public class RegionVersionVectorTest {
     assertThat(rvv.getVersionForMember(ownerId)).isEqualTo(newVersion);
   }
 
+  @Test
+  public void isRvvGcDominatedByRequesterRvvReturnsTrueIfRequesterRvvForLostMemberDominates()
+      throws Exception {
+    InternalDistributedMember lostMember = mock(InternalDistributedMember.class);
+    ConcurrentHashMap<InternalDistributedMember, Long> memberToGcVersion =
+        new ConcurrentHashMap<>();
+    memberToGcVersion.put(lostMember, new Long(1) /* lostMemberGcVersion */);
+    RegionVersionVector providerRvv = new VMRegionVersionVector(lostMember, null,
+        0, memberToGcVersion, 0, true, null);
+
+    ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
+        new ConcurrentHashMap<>();
+    RegionVersionHolder regionVersionHolder = new RegionVersionHolder(lostMember);
+    regionVersionHolder.setVersion(2);
+    memberToRegionVersionHolder.put(lostMember, regionVersionHolder);
+    RegionVersionVector requesterRvv =
+        new VMRegionVersionVector(lostMember, memberToRegionVersionHolder,
+            0, null, 0, true, null);
+
+    assertThat(providerRvv.isRVVGCDominatedBy(requesterRvv)).isTrue();
+  }
+
+  @Test
+  public void isRvvGcDominatedByRequesterRvvReturnsFalseIfRequesterRvvForLostMemberDominates()
+      throws Exception {
+    InternalDistributedMember lostMember = mock(InternalDistributedMember.class);
+    ConcurrentHashMap<InternalDistributedMember, Long> memberToGcVersion =
+        new ConcurrentHashMap<>();
+    memberToGcVersion.put(lostMember, new Long(1) /* lostMemberGcVersion */);
+    RegionVersionVector providerRvv = new VMRegionVersionVector(lostMember, null,
+        0, memberToGcVersion, 0, true, null);
+
+    ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
+        new ConcurrentHashMap<>();
+    RegionVersionHolder regionVersionHolder = new RegionVersionHolder(lostMember);
+    regionVersionHolder.setVersion(0);
+    memberToRegionVersionHolder.put(lostMember, regionVersionHolder);
+    RegionVersionVector requesterRvv =
+        new VMRegionVersionVector(lostMember, memberToRegionVersionHolder,
+            0, null, 0, true, null);
+
+    assertThat(providerRvv.isRVVGCDominatedBy(requesterRvv)).isFalse();
+  }
+
+  @Test
+  public void isRvvGcDominatedByRequesterRvvReturnsFalseIfRequesterRvvDominatesProvider()
+      throws Exception {
+    final String local = getIPLiteral();
+    InternalDistributedMember provider = new InternalDistributedMember(local, 101);
+    InternalDistributedMember requester = new InternalDistributedMember(local, 102);
+
+    RegionVersionVector providerRvv = new VMRegionVersionVector(provider, null,
+        1, null, 1, false, null);
+
+    ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
+        new ConcurrentHashMap<>();
+    RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
+    regionVersionHolder.setVersion(0);
+    memberToRegionVersionHolder.put(provider, regionVersionHolder);
+    RegionVersionVector requesterRvv =
+        new VMRegionVersionVector(requester, memberToRegionVersionHolder,
+            0, null, 0, false, null);
+
+    assertThat(providerRvv.isRVVGCDominatedBy(requesterRvv)).isFalse();
+  }
+
+  @Test
+  public void isRvvGcDominatedByRequesterRvvReturnsTrueIfRequesterRvvDominatesWithNoGcVersion()
+      throws Exception {
+    final String local = getIPLiteral();
+    InternalDistributedMember provider = new InternalDistributedMember(local, 101);
+    InternalDistributedMember requester = new InternalDistributedMember(local, 102);
+
+    ConcurrentHashMap<InternalDistributedMember, Long> memberToGcVersion =
+        new ConcurrentHashMap<>();
+    RegionVersionVector providerRvv = new VMRegionVersionVector(provider, null,
+        1, memberToGcVersion, 1, false, null);
+
+    ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
+        new ConcurrentHashMap<>();
+    RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
+    regionVersionHolder.setVersion(2);
+    memberToRegionVersionHolder.put(provider, regionVersionHolder);
+    RegionVersionVector requesterRvv =
+        new VMRegionVersionVector(requester, memberToRegionVersionHolder,
+            0, null, 0, false, null);
+
+    assertThat(providerRvv.isRVVGCDominatedBy(requesterRvv)).isTrue();
+  }
+
+  @Test
+  public void isRvvGcDominatedByRequesterRvvReturnsTrueIfRequesterRvvDominates() throws Exception {
+    final String local = getIPLiteral();
+    InternalDistributedMember provider = new InternalDistributedMember(local, 101);
+    InternalDistributedMember requester = new InternalDistributedMember(local, 102);
+    ConcurrentHashMap<InternalDistributedMember, Long> memberToGcVersion =
+        new ConcurrentHashMap<>();
+    memberToGcVersion.put(requester, new Long(1));
+    RegionVersionVector providerRvv = new VMRegionVersionVector(provider, null,
+        1, memberToGcVersion, 1, false, null);
+
+    ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
+        new ConcurrentHashMap<>();
+    RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
+    regionVersionHolder.setVersion(2);
+    memberToRegionVersionHolder.put(provider, regionVersionHolder);
+    RegionVersionVector requesterRvv =
+        new VMRegionVersionVector(requester, memberToRegionVersionHolder,
+            2, null, 0, false, regionVersionHolder);
+
+    assertThat(providerRvv.isRVVGCDominatedBy(requesterRvv)).isTrue();
+  }
+
+  @Test
+  public void isRvvGcDominatedByRequesterRvvReturnsFalseIfRequesterRvvDominates() throws Exception {
+    final String local = getIPLiteral();
+    InternalDistributedMember provider = new InternalDistributedMember(local, 101);
+    InternalDistributedMember requester = new InternalDistributedMember(local, 102);
+    ConcurrentHashMap<InternalDistributedMember, Long> memberToGcVersion =
+        new ConcurrentHashMap<>();
+    memberToGcVersion.put(requester, new Long(3));
+    RegionVersionHolder pRegionVersionHolder = new RegionVersionHolder(provider);
+    pRegionVersionHolder.setVersion(4);
+
+    RegionVersionVector providerRvv = new VMRegionVersionVector(provider, null,
+        1, memberToGcVersion, 1, false, pRegionVersionHolder);
+
+    ConcurrentHashMap<InternalDistributedMember, RegionVersionHolder<InternalDistributedMember>> memberToRegionVersionHolder =
+        new ConcurrentHashMap<>();
+    RegionVersionHolder regionVersionHolder = new RegionVersionHolder(provider);
+    regionVersionHolder.setVersion(2);
+    memberToRegionVersionHolder.put(provider, regionVersionHolder);
+    RegionVersionVector requesterRvv =
+        new VMRegionVersionVector(requester, memberToRegionVersionHolder,
+            2, null, 0, false, regionVersionHolder);
+
+    assertThat(providerRvv.isRVVGCDominatedBy(requesterRvv)).isFalse();
+  }
+
   private RegionVersionVector createRegionVersionVector(InternalDistributedMember ownerId,
       LocalRegion owner) {
     @SuppressWarnings({"unchecked", "rawtypes"})