geode-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bruce Schuchardt (JIRA)" <>
Subject [jira] [Created] (GEODE-707) Concurrent cache-load doesn't invoke loader multiple times if there is an exception in the first load
Date Mon, 21 Dec 2015 17:01:46 GMT
Bruce Schuchardt created GEODE-707:

             Summary: Concurrent cache-load doesn't invoke loader multiple times if there
is an exception in the first load
                 Key: GEODE-707
             Project: Geode
          Issue Type: Bug
          Components: core
            Reporter: Bruce Schuchardt

This is from an old Pivotal ticket that was fixed in bugfix branches but never made it to
develop.  The ticket text follows:

I see following behavior when we have a condition where 2 get() calls are sent concurrently
for the same key in which Loader is called,

    If the first call is sent to Loader we create a Future.
    For second call we wait on this Future.
    Before first call is finished we check second call its a cache miss here as well(which
we have checked before) we have isCreate=true.
    In case when isCreate is true in that flow we actually return null if we can't see the
value(which can be true in case we got an exception on the first Loader call). 

Now from customer point of view this is inconsistent, as they except GemFire? to try again
with the Loader and return exception as that is the true state here.

[Darrel's reply]
The first get creates a future and when it completes sets the future. If the first get fails
with an exception (like a CacheLoaderException?) it still sets the future with this code in
a finally block:

    VersionTag? tag = (clientEvent==null)? null : clientEvent.getVersionTag();
    thisFuture.set(new Object[]{result, tag});

In the case of the first get failing with an exception "result" will be null so it sets the
future to an array whose first element is null.

The second get is waiting on the future. It returns this array and sees the first element
as null which causes it to just return null. I think what it used to do was see Future.get
return null (instead of an array with null in it) and this caused it to fall through and attempt
to do the load itself which I think is what the customer also expects. You can see in the
code this was what was intended by this comment that still remains:

    if value == null, try our own search/load

Also if you look at the code in PartitionedRegion? that overrides this LocalRegion? code you
will see that it does exactly this; if an exception is thrown the future is set to null and
the second get see null returned from the future, falls through, and attempts the load itself.

This message was sent by Atlassian JIRA

View raw message