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/20 21:35:59 UTC
[geode] branch develop 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 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 28dbac1a86 GEODE-10229: GII image should fill disk region RVV's exceptions. (#7602)
28dbac1a86 is described below
commit 28dbac1a86dd40ca0c3ceba004263824f6de4653
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.
---
.../geode/internal/cache/GIIDeltaDUnitTest.java | 39 ++++++++++++++++--
.../geode/internal/cache/AbstractRegionMap.java | 6 ---
.../internal/cache/InitialImageOperation.java | 8 ++--
.../cache/versions/RegionVersionHolder.java | 2 +-
.../internal/cache/AbstractRegionMapTest.java | 6 +--
.../versions/RegionVersionHolderJUnitTest.java | 48 ++++++++++++++++++++++
6 files changed, 91 insertions(+), 18 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..20e247b718 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 fadf308736..e148b15dda 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
@@ -787,12 +787,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 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/AbstractRegionMapTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionMapTest.java
index 2777409091..e983282ecf 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);