mhansonp commented on a change in pull request #6042:
URL: https://github.com/apache/geode/pull/6042#discussion_r580708414
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/versions/TombstoneDUnitTest.java
##########
@@ -144,6 +145,69 @@ public void testGetOldestTombstoneTimeReplicate() {
});
}
+ @Test
+ public void testRewriteBadOldestTombstoneTimeReplicateForTimestamp() {
+ VM server1 = VM.getVM(0);
+ VM server2 = VM.getVM(1);
+
+ final int count = 10;
+ server1.invoke(() -> {
+ createCacheAndRegion(RegionShortcut.REPLICATE_PERSISTENT);
+ for (int i = 0; i < count; i++) {
+ region.put("K" + i, "V" + i);
+ }
+ });
+
+ server2.invoke(() -> createCacheAndRegion(RegionShortcut.REPLICATE));
+
+ server1.invoke(() -> {
+ TombstoneService.TombstoneSweeper tombstoneSweeper =
+ ((InternalCache)
cache).getTombstoneService().getSweeper((LocalRegion) region);
+
+ RegionEntry regionEntry = ((LocalRegion) region).getRegionEntry("K0");
+ VersionTag<?> versionTag = regionEntry.getVersionStamp().asVersionTag();
+ versionTag.setVersionTimeStamp(System.currentTimeMillis() + 1000000);
+
+ TombstoneService.Tombstone modifiedTombstone =
+ new TombstoneService.Tombstone(regionEntry, (LocalRegion) region,
+ versionTag);
+ tombstoneSweeper.tombstones.add(modifiedTombstone);
+ tombstoneSweeper.checkOldestUnexpired(System.currentTimeMillis());
+ // Send tombstone gc message to vm1.
+ assertThat(tombstoneSweeper.getOldestTombstoneTime()).isEqualTo(0);
+ });
+ }
+
+
+ @Test
+ public void testGetOldestTombstoneTimeReplicateForTimestamp() {
Review comment:
It is a defensive approach to a problem. I agree that we should be
looking at the issue beyond this change, but the ramifications of having a bad
timestamp make using geode very challenging. I would also not like to see the
core bug closed, but I think it is safe to make the system run better. The way
this code is written (should be rewritten) is that it looks at the first entry
in the queue only. So if a timestamp is in the future it can be a long wait to
clear tombstones. It doesn't matter if others should be cleared in the queue
only the first...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
