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 to avoid 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]
