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/22 07:12:09 UTC
[geode] branch support/1.14 updated: GEODE-10229: GII image should fill disk region RVV's exceptions. (#7602)
This is an automated email from the ASF dual-hosted git repository.
zhouxj pushed a commit to branch support/1.14
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.14 by this push:
new d6d96d465a GEODE-10229: GII image should fill disk region RVV's exceptions. (#7602)
d6d96d465a is described below
commit d6d96d465a62d009836552a1d0dabadb1637c9c4
Author: Xiaojian Zhou <ge...@users.noreply.github.com>
AuthorDate: Wed Apr 20 14:35:54 2022 -0700
GEODE-10229: GII image should fill disk region RVV's exceptions. (#7602)
* 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.
(cherry picked from commit 28dbac1a86dd40ca0c3ceba004263824f6de4653)
---
.../geode/internal/cache/GIIDeltaDUnitTest.java | 39 ++++++++++++++++--
.../geode/internal/cache/AbstractRegionMap.java | 6 ---
.../internal/cache/InitialImageOperation.java | 8 ++--
.../cache/versions/RegionVersionHolder.java | 4 +-
.../internal/cache/AbstractRegionMapTest.java | 6 +--
.../versions/RegionVersionHolderJUnitTest.java | 48 ++++++++++++++++++++++
6 files changed, 92 insertions(+), 19 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 1f3fcc1e66..305794fff9 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
@@ -938,6 +938,15 @@ public class GIIDeltaDUnitTest extends JUnit4CacheTestCase {
}
}
+ private void verifyDiskRegionRVV() {
+ DiskStoreID P_ID = getMemberID(P);
+ R.invoke(() -> {
+ LocalRegion lr = (LocalRegion) getCache().getRegion(REGION_NAME);
+ RegionVersionVector drRVV = lr.getDiskRegion().getRegionVersionVector();
+ assertEquals(0, drRVV.getExceptionCount(P_ID));
+ });
+ }
+
/**
* P1, P2, P3 R does GII but wait at BeforeSavedReceivedRVV, so R's RVV=P3R0 P4, P5 R goes on to
* save received RVV. R's new RVV=P5(3-6)R0
@@ -980,8 +989,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 +1001,17 @@ public class GIIDeltaDUnitTest extends JUnit4CacheTestCase {
changeForceFullGII(R, true, true);
changeForceFullGII(P, false, true);
verifyDeltaSizeFromStats(R, 2, 0);
+ verifyDiskRegionRVV();
+
+ // Close cache P then restart R to make sure R will recover from its own
+ // then restart P to re-do GII
+ closeCache(P);
+ closeCache(R);
+ createDistributedRegion(R);
+ createDistributedRegion(P);
+ waitForToVerifyRVV(P, memberP, 5, null, 0); // P's rvv=r5, gc=0
+ verifyTombstoneExist(P, "key2", true, false);
+ verifyTombstoneExist(R, "key2", true, false);
}
/**
@@ -1038,8 +1058,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 +1070,17 @@ public class GIIDeltaDUnitTest extends JUnit4CacheTestCase {
verifyDeltaSizeFromStats(R, 2, 1);
changeForceFullGII(R, false, true);
changeForceFullGII(P, false, true);
+ verifyDiskRegionRVV();
+
+ // Close cache P then restart R to make sure R will recover from its own
+ // then restart P to re-do GII
+ closeCache(P);
+ closeCache(R);
+ createDistributedRegion(R);
+ createDistributedRegion(P);
+ waitForToVerifyRVV(P, memberP, 5, null, 0); // P's rvv=r5, gc=0
+ verifyTombstoneExist(P, "key2", true, false);
+ 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 f21ef7da0d..5883a4f7df 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
@@ -810,12 +810,6 @@ public abstract class AbstractRegionMap extends BaseRegionMap
Assert.assertTrue(entryVersion.getMemberID() != null,
"GII entry versions must have identifiers");
boolean isTombstone = (newValue == Token.TOMBSTONE);
- // don't reschedule the tombstone if it hasn't changed
- boolean isSameTombstone = oldRe.isTombstone() && isTombstone
- && oldRe.getVersionStamp().asVersionTag().equals(entryVersion);
- if (isSameTombstone) {
- return true;
- }
processVersionTagForGII(oldRe, owner, entryVersion, isTombstone, sender,
!wasRecovered || isSynchronizing);
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 dbf185e497..32e5995a68 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
@@ -479,8 +479,8 @@ public class InitialImageOperation {
m.unfinishedKeys = keysOfUnfinishedOps;
if (isDebugEnabled) {
logger.debug(
- "Region {} recovered with EndGII flag, rvv is {}. recovered rvv is {}. Do delta GII",
- this.region.getFullPath(), m.versionVector, recoveredRVV);
+ "Region {} recovered with EndGII flag, rvv is {}. Received rvv is {}. Do delta GII",
+ region.getFullPath(), m.versionVector, received_rvv);
}
}
}
@@ -511,8 +511,8 @@ public class InitialImageOperation {
}
// do not remove the following log statement
- logger.info("Region {} requesting initial image from {}",
- new Object[] {this.region.getName(), recipient});
+ logger.info("Region {} requesting initial image from {}, recovered RVV is {}",
+ new Object[] {region.getName(), recipient, recoveredRVV});
dm.putOutgoing(m);
this.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 64ed6cb12a..6e22f8116e 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,8 +338,8 @@ public class RegionVersionHolder<T> implements Cloneable, DataSerializable {
this.version = version;
return;
}
- if (this.version > version) {
- this.addOlderVersion(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/AbstractRegionMapTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
index 47904bf214..a1c85e8853 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
@@ -886,7 +886,7 @@ public class AbstractRegionMapTest {
}
@Test
- public void initialImagePut_givenPutIfAbsentReturningRegionEntryAndSameTombstoneWillAttemptToRemoveREAndInvokeNothingElse()
+ public void initialImagePut_givenPutIfAbsentReturningRegionEntryAndSameTombstoneWillReprocessTheTombstone()
throws RegionClearedException {
ConcurrentMapWithReusableEntries map = mock(ConcurrentMapWithReusableEntries.class);
RegionEntry entry = mock(RegionEntry.class);
@@ -911,8 +911,8 @@ public class AbstractRegionMapTest {
arm.initialImagePut(KEY, 0, Token.TOMBSTONE, false, false, versionTag, null, false);
- verify(map, times(1)).remove(eq(KEY), eq(createdEntry));
- verify(entry, never()).initialImagePut(any(), anyLong(), any(), anyBoolean(), anyBoolean());
+ verify(map, times(0)).remove(eq(KEY), eq(createdEntry));
+ verify(entry, times(1)).initialImagePut(any(), anyLong(), any(), anyBoolean(), anyBoolean());
}
@Test
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..48bc8ba9ab 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
@@ -14,6 +14,7 @@
*/
package org.apache.geode.internal.cache.versions;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -53,6 +54,53 @@ public class RegionVersionHolderJUnitTest {
return member;
}
+ @Test
+ public void fillSpecialExceptionForRVHWithBitSet() {
+ fillSpecialExceptionForRVH(true);
+ }
+
+ @Test
+ public void fillSpecialExceptionForRVHWithoutBitSet() {
+ fillSpecialExceptionForRVH(false);
+ }
+
+ private void fillSpecialExceptionForRVH(boolean useBitSet) {
+ RegionVersionHolder<InternalDistributedMember> vh1;
+ RegionVersionHolder<InternalDistributedMember> vh2;
+ 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);
+ List<RVVException> exceptionList = vh2.getExceptionForTest();
+ assertThat(exceptionList.size()).isEqualTo(1);
+ RVVException exception = exceptionList.get(0);
+ assertThat(exception.previousVersion).isEqualTo(3);
+ assertThat(exception.nextVersion).isEqualTo(6);
+
+ vh2.recordVersion(5);
+ exceptionList = vh2.getExceptionForTest();
+ assertThat(exceptionList.size()).isEqualTo(1);
+ exception = exceptionList.get(0);
+ assertThat(exception.previousVersion).isEqualTo(3);
+ assertThat(exception.nextVersion).isEqualTo(5);
+
+ vh2.recordVersion(4);
+ exceptionList = vh2.getExceptionForTest();
+ assertThat(exceptionList.size()).isEqualTo(0);
+ }
+
@Test
public void test48066_1() {
RegionVersionHolder vh1 = new RegionVersionHolder(member);