dschneider-pivotal commented on a change in pull request #5851:
URL: https://github.com/apache/geode/pull/5851#discussion_r552972357
##########
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 think it would be better to change it so the only the we don't do if
we are evicting is handleEntryNotFound(). When evicting we do not want an
exception if the entry is already gone.
So I would change line 369 to just be an "else" and then put a "if
(!isEviction)" on handleEntryNotFound (even better might be to change
handleEntryNotFound to test isEviction and return since this is the only place
it is called).
Then you can get rid of your new code on 375 and 376.
I do think it is probably correct to call removeEntryOrLeaveTombstone for an
eviction just to make sure it follows the normal rules of scheduling a
tombstone when concurrency checks are enabled. This method is already a noop if
evicting without concurrency checks.
----------------------------------------------------------------
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]
