[jira] [Commented] (GEODE-8790) Evicted entries sometimes remain in region map

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GEODE-8790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17260066#comment-17260066 ] 

ASF GitHub Bot commented on GEODE-8790:
---------------------------------------

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:
users@infra.apache.org


> Evicted entries sometimes remain in region map
> ----------------------------------------------
>
>                 Key: GEODE-8790
>                 URL: https://issues.apache.org/jira/browse/GEODE-8790
>             Project: Geode
>          Issue Type: Bug
>          Components: eviction
>    Affects Versions: 1.1.0, 1.14.0
>            Reporter: Louis R. Jacome
>            Assignee: Louis R. Jacome
>            Priority: Major
>              Labels: pull-request-available
>
> Sometimes an entry set for eviction will remain in the region map after falling into an unhandled case in RegionMapDestroy.retryRemoveWithTombstone. 
> When testing the LRU capacity of a region, we create many entries beyond the LRU capacity, causing evictions. We sometimes run into a case where two threads compete to remove the same entry. One thread succeeds and removes it from the region map, while the other thread, in removal progress, will create a temporary entry in the map that does not get properly removed/handled.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)