This is an automated email from the ASF dual-hosted git repository.
zhouxj pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.13 by this push:
new 03a882cfa8 GEODE-10229: GII image should fill disk region RVV's
exceptions. (#7602)
03a882cfa8 is described below
commit 03a882cfa8cab51f15450a724a5edd1859ac47f6
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 0ec9ceb677..a86c7c02bc 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 132b0bff0e..b5cffca1b4 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
@@ -477,8 +477,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);
}
}
}
@@ -509,8 +509,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 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);
