luissson commented on a change in pull request #5851:
URL: https://github.com/apache/geode/pull/5851#discussion_r553586928
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
##########
@@ -372,6 +372,8 @@ private void retryRemoveWithTombstone() {
} finally {
removeEntryOrLeaveTombstone();
}
+ } else if (regionEntry == null && !newRegionEntry.isTombstone()) {
Review comment:
I moved a bit fast here with the first solution, as it appears we fail
to pass the unit test
evictDestroyOfExistingTombstoneWithConcurrencyChecksReturnsFalse. Suggesting
that we don't properly handle the case where we're in an eviction and already
set the region entry as tombstone. If we break up the check on isEviction and
newRegionEntry.isTombstone we cause more unit tests to fail.
With the current issue, we're trying to catch the specific case where we
have concurrency checks enabled, we're in an eviction, and we've created a new
region entry marked as tombstone. I've restructured retryRemoveWithTombstone
some so eviction and non-evictions go down the same path, and avoids a clause
with a lone call to removeEntryOrLeaveTombstone that addresses this issue and
passes unit tests.
----------------------------------------------------------------
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]
