This is an automated email from the ASF dual-hosted git repository.
zhouxj pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.12 by this push:
new 1319ca0d66 GEODE-10229: GII image should fill disk region RVV's
exceptions. (#7602)
1319ca0d66 is described below
commit 1319ca0d6656eebf2f5909145a61745240b22bd7
Author: Xiaojian Zhou <[email protected]>
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 26a979c57a..2552bd1d61 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 d93f33e652..e4e56429be 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 542ee9eb4e..199a678914 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
@@ -476,8 +476,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);
}
}
}
@@ -508,8 +508,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 9562c96a79..75439ddcf3 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 8711d30b68..0c592330c9 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
@@ -889,7 +889,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);
@@ -914,8 +914,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);
