You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@geode.apache.org by "Darrel Schneider (JIRA)" <ji...@apache.org> on 2018/04/03 23:13:00 UTC

[jira] [Commented] (GEODE-4957) The key used in a putIfAbsent call that returns null may not be the one in the RegionEntry

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

Darrel Schneider commented on GEODE-4957:
-----------------------------------------

I think this is what is happening:

Thread 1 does putIfAbsent(k1, v1) and adds a new RegionEntry to the map with k1 and value REMOVE_PHASE1

Thread 2 does putIfAbsent(k2, v2) and finds the RegionEntry that Thread 1 added because k1.equals(k2). It syncs this RegionEntry and sets the value to v2. It returns null because the old value was REMOVE_PHASE1.

Thread 1 then syncs the RegionEntry and finds the value is v2 so putIfAbsent does not put but returns v2.

So when this happens the key will be k1 even though Thread 2 is the one that did the putIfAbsent.

I think we can fix this by having AbstractRegionMap.basicPut when it is doing a create (ifNew==true) check to see if the key has the same identity (==) as the one on the region entry. If it is not identical then it can change the key on the RegionEntry. Since the key has the same hash I think this is safe.

 Another solution is to treat REMOVE_PHASE1 with a non-identical key like we do REMOVE_PHASE2. In that case we remove the old RegionEntry and then create a new one and do a retry. The problem with this is two or more threads could keep doing this to each other, spinning around possibly forever.

The best solution would be to refactor getOrCreateRegionEntry so that when it adds a new RegionEntry that is REMOVE_PHASE1 it is already synced. The code used to be this way but was changed when the getOrCreateRegionEntry method was introduced. If we could do it like this then this bug goes away because no other thread can change the RegionEntry we add with REMOVE_PHASE1 so it will already have our key in it.

 

> The key used in a putIfAbsent call that returns null may not be the one in the RegionEntry
> ------------------------------------------------------------------------------------------
>
>                 Key: GEODE-4957
>                 URL: https://issues.apache.org/jira/browse/GEODE-4957
>             Project: Geode
>          Issue Type: Bug
>          Components: regions
>            Reporter: Barry Oglesby
>            Priority: Major
>
> With simultaneous putIfAbsent calls, sometimes the thread that returns null is not the thread that actually creates the RegionEntry.
> Below is some logging that shows this behavior.
> Thread-77 returns null from the putIfAbsent call (which means there was no previous value). 1 ms before Thread-77's putEntryIfAbsent call, Thread-9 creates the RegionEntry. You can see that because not only is oldRe=null for Thread-9, but also the event's key (eventKey) and entry's key (reKey) are identical. But Thread-9 returns a non-null old value.
> {noformat}
> Thread-77 at 1522187267493: AbstractRegionMap.putEntryIfAbsent regionEntry=VersionedStatsDiskLRURegionEntryHeapObjectKey@6a8119a0 (key=ComplexKey[identity=1682369152; key=key]; rawValue=REMOVED_PHASE1; version=\{v0; rv0; ds=0; time=0};member=null); oldRe=VersionedStatsDiskLRURegionEntryHeapObjectKey@1aac7604 (key=ComplexKey[identity=342592289; key=key]; rawValue=REMOVED_PHASE1; version=\{v0; rv0; ds=0; time=0};member=null)
> Thread-77 at 1522187267493: AbstractRegionMap.basicPut eventKey=ComplexKey[identity=1682369152; key=key]; reKey=ComplexKey[identity=342592289; key=key]
> Thread-77 at 1522187267500: LocalRegion.putIfAbsent returning null
> {noformat}
> {noformat}
> Thread-9 at 1522187267492: AbstractRegionMap.putEntryIfAbsent regionEntry=VersionedStatsDiskLRURegionEntryHeapObjectKey@1aac7604 (key=ComplexKey[identity=342592289; key=key]; rawValue=REMOVED_PHASE1; version=\{v0; rv0; ds=0; time=0};member=null); oldRe=null
> Thread-9 at 1522187267495: AbstractRegionMap.basicPut eventKey=ComplexKey[identity=342592289; key=key]; reKey=ComplexKey[identity=342592289; key=key]
> Thread-9 at 1522187267504: LocalRegion.putIfAbsent returning v1
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)