[infinispan-dev] ClusteredGet and direct operations on a cacheloader

[Mircea Markus]

Manik Surtani wrote:
> Mircea
>
> Regarding direct loading from a cache loader in the 
> ClusteredGetCommand, this is an issue since 

> a) it does not acquire any locks to put this value back in the data 
> container, and 
cache loaders are read only and stores use bucket locking, it's a 
different locking level though...
> b) probably as a consequence of a), doesn't bother to put any looked 
> up value in the data container at all.
>
> This is inefficient since multiple CGCs on the same key (coming from 
> different cache instances in a cluster) would cause the owning cache 
> to load this several times repeatedly from a loader.
At the same time, unique calls for different keys would increase the 
memory footprint... though I agree with your point of view overall: 
after all remote cache can be considered just another cache user.
>
> I see this problem with DIST since I use CGC to load an entry from a 
> remote cache and if L1 caching is disabled on the requesting cache, 
> multiple uses of an entry will result in this entry being read off a 
> loader repeatedly.  Which is no good.  :-)
>
> So I think the CGC's perform() method should actually be doing a 
> cache.get() - or something very similar - where the entire interceptor 
> chain is invoked and locks are acquired, entries moved to the data 
> container if loaded, etc., and metrics on cache hits/misses properly 
> updated.  The challenges here are, of course:
>
> 1.  The ping-pong effect.  So we need to make sure any clustered cache 
> loader or the DistributionInterceptor do not try and load this from 
> elsewhere in the cluster.  Easily solved with the isOriginLocal flag 
> on the context.
yes, +1
> 2.   Making sure we get an InternalCacheEntry and not just a value to 
> return.  This is trickier since ICEs are not exposed via public APIs 
> at all.  Perhaps we need a separate, non-replicable Command for this 
> purpose - maybe a subclass of GetKeyValueCommand 
> (GetCacheEntryCommand?) which (a) cannot be replicated and (b) will 
> return the ICE.
I see, it gets a bit complicated now... TBH, I think the 'problem' is in 
the fact that our interceptors are a bit too 'fat' - with this 
interceptor approach we "spread" the ClusterGet logic in too many places:
- in the ClusterGet command, that does the call
- in the DistributionInterceptor, where you have to add the additional 
if, to check that the call is local
- (other interceptors will have to do that as well).

Having thinner interceptors would move the logic in one place only (e.g. 
LockingInterceptor is thin, as all locking logic is in LockManager, and 
all the calls are delegated to it, no 'fat' logic in interceptor itself) 
would make the code easier to follow and change (and solve the above 
issue, with not having access to InternalCacheEntry).
So, the code in ClusterGetCommand might look something like this:

perform() {
    lockManager.lock(key);
    if  (dataContainer.get(key) != null) return...;
    if (loaderManager != null) return loaderManager.get(key);
    lockManager.release(key);
}

I think the code is much slef-explanatory than a plain cache.get(), that 
does lots of things not imediately visible. Also if he wants to add 
another lookup from somewhere else, it's easier.
There is no code duplication as well, as all the work is delegated to 
the managers.
> So, CGC would create a GCEC and pass it up the interceptor chain, and 
> retrieve the ICE from the GCEC after the call returns?
>
> WDYT?
> -- 
> Manik Surtani
> manik at jboss.org
> Lead, Infinispan
> Lead, JBoss Cache
> http://www.infinispan.org
> http://www.jbosscache.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev