You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Kirk Lund <kl...@apache.org> on 2017/03/02 20:30:18 UTC
Functions need reference to Cache
I'm looking into a deadlock involving code under
org.apache.geode.management. Related to this, I'm also looking at all of
the Functions that are implemented as part of the management layer.
First thing every management Function appears to invoke is either
org.apache.geode.cache.CacheFactory.getAnyInstance() or
org.apache.geode.cache.internal.cache.GemFireCacheImpl getInstance() to get
a reference to the Cache.
This method on CacheFactory seems to be frequently involved in deadlocks in
general and I think we should find a way to deprecate it:
/**
* Gets an arbitrary open instance of {@link Cache} produced by an earlier
call to
* {@link #create()}.
*
* @throws CacheClosedException if a cache has not been created or the only
created one is
* {@link Cache#isClosed closed}
*/
public static synchronized Cache getAnyInstance() {
GemFireCacheImpl instance = GemFireCacheImpl.getInstance();
<snip>
In an effort to prevent deadlocks and move away from statics and
singletons, I'd like to find a good alternative to this for Functions. I
think adding this method to FunctionContext is the best solution:
/**
* Returns a reference to the Cache for which this function is executing.
*/
public Cache getCache();
Does this seem like a good solution? Is there any reason not to add this to
the API?
Re: Functions need reference to Cache
Posted by Jared Stewart <js...@pivotal.io>.
I agree this would be useful, and I think it already has an open GEODE
ticket:
https://issues.apache.org/jira/plugins/servlet/mobile#issue/GEODE-393
On Mar 2, 2017 12:30 PM, "Kirk Lund" <kl...@apache.org> wrote:
> I'm looking into a deadlock involving code under
> org.apache.geode.management. Related to this, I'm also looking at all of
> the Functions that are implemented as part of the management layer.
>
> First thing every management Function appears to invoke is either
> org.apache.geode.cache.CacheFactory.getAnyInstance() or
> org.apache.geode.cache.internal.cache.GemFireCacheImpl getInstance() to
> get
> a reference to the Cache.
>
> This method on CacheFactory seems to be frequently involved in deadlocks in
> general and I think we should find a way to deprecate it:
>
> /**
> * Gets an arbitrary open instance of {@link Cache} produced by an earlier
> call to
> * {@link #create()}.
> *
> * @throws CacheClosedException if a cache has not been created or the only
> created one is
> * {@link Cache#isClosed closed}
> */
> public static synchronized Cache getAnyInstance() {
> GemFireCacheImpl instance = GemFireCacheImpl.getInstance();
> <snip>
>
> In an effort to prevent deadlocks and move away from statics and
> singletons, I'd like to find a good alternative to this for Functions. I
> think adding this method to FunctionContext is the best solution:
>
> /**
> * Returns a reference to the Cache for which this function is executing.
> */
> public Cache getCache();
>
> Does this seem like a good solution? Is there any reason not to add this to
> the API?
>
Re: Functions need reference to Cache
Posted by Anthony Baker <ab...@pivotal.io>.
> On Mar 2, 2017, at 12:30 PM, Kirk Lund <kl...@apache.org> wrote:
>
>
> In an effort to prevent deadlocks and move away from statics and
> singletons, I'd like to find a good alternative to this for Functions. I
> think adding this method to FunctionContext is the best solution:
>
> /**
> * Returns a reference to the Cache for which this function is executing.
> */
> public Cache getCache();
>
> Does this seem like a good solution? Is there any reason not to add this to
> the API?
Someone else likes your idea too:
https://issues.apache.org/jira/browse/GEODE-393
Anthony
Re: Functions need reference to Cache
Posted by Darrel Schneider <ds...@pivotal.io>.
A GemFireCache will be either a ClientCache or a Cache. If Functions always
execute on a member of the cluster then it this new getCache method should
return Cache. But if it is possible to execution a function in a client
then getCache should return GemFireCache.
GemFireCache extends RegionService and the only other implementation of
RegionService that is not a GemFireCache is ProxyCache. I don't think we
would ever have a Function executing in a ProxyCache (ProxyCache instances
only exist in a client and are used to pass different security info to the
server). If I'm wrong about this then you would want this method to return
RegionService.
On Thu, Mar 2, 2017 at 1:46 PM, Dan Smith <ds...@pivotal.io> wrote:
> Yeah, there's even a PR for this - but it doesn't have tests. If anyone
> wants to pick that up and clean it up and add tests that would be great!
>
> https://github.com/apache/geode/pull/74
>
> -Dan
>
> On Thu, Mar 2, 2017 at 1:01 PM, John Blum <jb...@pivotal.io> wrote:
>
> > Also keep in my mind if you are given a RegionFunctionContext (in cases
> > where the *Function* is executed "on a *Region*") then you have indirect
> > access to the cache...
> >
> > regionFunctionContext.getDataSet()
> > <http://geode.apache.org/releases/latest/javadoc/org/
> > apache/geode/cache/execute/RegionFunctionContext.html#getDataSet-->
> > [1].getRegionService()
> > <http://geode.apache.org/releases/latest/javadoc/org/
> > apache/geode/cache/Region.html#getRegionService-->
> > [2]
> >
> > GemFireCache is the only (direct) implementation of RegionService
> > <http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/
> > RegionService.html>
> > [3]
> > that I am aware of.
> >
> > However, I generally agree that it would be useful to have direct access
> to
> > the GemFireCache from any FunctionContext.
> >
> > -j
> >
> > [1]
> > http://geode.apache.org/releases/latest/javadoc/org/
> > apache/geode/cache/execute/RegionFunctionContext.html#getDataSet--
> > [2]
> > http://geode.apache.org/releases/latest/javadoc/org/
> > apache/geode/cache/Region.html#getRegionService--
> > [3]
> > http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/
> > RegionService.html
> >
> >
> > On Thu, Mar 2, 2017 at 12:44 PM, Jinmei Liao <ji...@pivotal.io> wrote:
> >
> > > I think I even see comments in the code stating "TODO: use the cache in
> > the
> > > FunctionContext when it becomes available".
> > >
> > > On Thu, Mar 2, 2017 at 12:30 PM, Kirk Lund <kl...@apache.org> wrote:
> > >
> > > > I'm looking into a deadlock involving code under
> > > > org.apache.geode.management. Related to this, I'm also looking at all
> > of
> > > > the Functions that are implemented as part of the management layer.
> > > >
> > > > First thing every management Function appears to invoke is either
> > > > org.apache.geode.cache.CacheFactory.getAnyInstance() or
> > > > org.apache.geode.cache.internal.cache.GemFireCacheImpl getInstance()
> > to
> > > > get
> > > > a reference to the Cache.
> > > >
> > > > This method on CacheFactory seems to be frequently involved in
> > deadlocks
> > > in
> > > > general and I think we should find a way to deprecate it:
> > > >
> > > > /**
> > > > * Gets an arbitrary open instance of {@link Cache} produced by an
> > > earlier
> > > > call to
> > > > * {@link #create()}.
> > > > *
> > > > * @throws CacheClosedException if a cache has not been created or
> the
> > > only
> > > > created one is
> > > > * {@link Cache#isClosed closed}
> > > > */
> > > > public static synchronized Cache getAnyInstance() {
> > > > GemFireCacheImpl instance = GemFireCacheImpl.getInstance();
> > > > <snip>
> > > >
> > > > In an effort to prevent deadlocks and move away from statics and
> > > > singletons, I'd like to find a good alternative to this for
> Functions.
> > I
> > > > think adding this method to FunctionContext is the best solution:
> > > >
> > > > /**
> > > > * Returns a reference to the Cache for which this function is
> > executing.
> > > > */
> > > > public Cache getCache();
> > > >
> > > > Does this seem like a good solution? Is there any reason not to add
> > this
> > > to
> > > > the API?
> > > >
> > >
> > >
> > >
> > > --
> > > Cheers
> > >
> > > Jinmei
> > >
> >
> >
> >
> > --
> > -John
> > john.blum10101 (skype)
> >
>
Re: Functions need reference to Cache
Posted by Dan Smith <ds...@pivotal.io>.
Yeah, there's even a PR for this - but it doesn't have tests. If anyone
wants to pick that up and clean it up and add tests that would be great!
https://github.com/apache/geode/pull/74
-Dan
On Thu, Mar 2, 2017 at 1:01 PM, John Blum <jb...@pivotal.io> wrote:
> Also keep in my mind if you are given a RegionFunctionContext (in cases
> where the *Function* is executed "on a *Region*") then you have indirect
> access to the cache...
>
> regionFunctionContext.getDataSet()
> <http://geode.apache.org/releases/latest/javadoc/org/
> apache/geode/cache/execute/RegionFunctionContext.html#getDataSet-->
> [1].getRegionService()
> <http://geode.apache.org/releases/latest/javadoc/org/
> apache/geode/cache/Region.html#getRegionService-->
> [2]
>
> GemFireCache is the only (direct) implementation of RegionService
> <http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/
> RegionService.html>
> [3]
> that I am aware of.
>
> However, I generally agree that it would be useful to have direct access to
> the GemFireCache from any FunctionContext.
>
> -j
>
> [1]
> http://geode.apache.org/releases/latest/javadoc/org/
> apache/geode/cache/execute/RegionFunctionContext.html#getDataSet--
> [2]
> http://geode.apache.org/releases/latest/javadoc/org/
> apache/geode/cache/Region.html#getRegionService--
> [3]
> http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/
> RegionService.html
>
>
> On Thu, Mar 2, 2017 at 12:44 PM, Jinmei Liao <ji...@pivotal.io> wrote:
>
> > I think I even see comments in the code stating "TODO: use the cache in
> the
> > FunctionContext when it becomes available".
> >
> > On Thu, Mar 2, 2017 at 12:30 PM, Kirk Lund <kl...@apache.org> wrote:
> >
> > > I'm looking into a deadlock involving code under
> > > org.apache.geode.management. Related to this, I'm also looking at all
> of
> > > the Functions that are implemented as part of the management layer.
> > >
> > > First thing every management Function appears to invoke is either
> > > org.apache.geode.cache.CacheFactory.getAnyInstance() or
> > > org.apache.geode.cache.internal.cache.GemFireCacheImpl getInstance()
> to
> > > get
> > > a reference to the Cache.
> > >
> > > This method on CacheFactory seems to be frequently involved in
> deadlocks
> > in
> > > general and I think we should find a way to deprecate it:
> > >
> > > /**
> > > * Gets an arbitrary open instance of {@link Cache} produced by an
> > earlier
> > > call to
> > > * {@link #create()}.
> > > *
> > > * @throws CacheClosedException if a cache has not been created or the
> > only
> > > created one is
> > > * {@link Cache#isClosed closed}
> > > */
> > > public static synchronized Cache getAnyInstance() {
> > > GemFireCacheImpl instance = GemFireCacheImpl.getInstance();
> > > <snip>
> > >
> > > In an effort to prevent deadlocks and move away from statics and
> > > singletons, I'd like to find a good alternative to this for Functions.
> I
> > > think adding this method to FunctionContext is the best solution:
> > >
> > > /**
> > > * Returns a reference to the Cache for which this function is
> executing.
> > > */
> > > public Cache getCache();
> > >
> > > Does this seem like a good solution? Is there any reason not to add
> this
> > to
> > > the API?
> > >
> >
> >
> >
> > --
> > Cheers
> >
> > Jinmei
> >
>
>
>
> --
> -John
> john.blum10101 (skype)
>
Re: Functions need reference to Cache
Posted by John Blum <jb...@pivotal.io>.
Also keep in my mind if you are given a RegionFunctionContext (in cases
where the *Function* is executed "on a *Region*") then you have indirect
access to the cache...
regionFunctionContext.getDataSet()
<http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/execute/RegionFunctionContext.html#getDataSet-->
[1].getRegionService()
<http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/Region.html#getRegionService-->
[2]
GemFireCache is the only (direct) implementation of RegionService
<http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/RegionService.html>
[3]
that I am aware of.
However, I generally agree that it would be useful to have direct access to
the GemFireCache from any FunctionContext.
-j
[1]
http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/execute/RegionFunctionContext.html#getDataSet--
[2]
http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/Region.html#getRegionService--
[3]
http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/RegionService.html
On Thu, Mar 2, 2017 at 12:44 PM, Jinmei Liao <ji...@pivotal.io> wrote:
> I think I even see comments in the code stating "TODO: use the cache in the
> FunctionContext when it becomes available".
>
> On Thu, Mar 2, 2017 at 12:30 PM, Kirk Lund <kl...@apache.org> wrote:
>
> > I'm looking into a deadlock involving code under
> > org.apache.geode.management. Related to this, I'm also looking at all of
> > the Functions that are implemented as part of the management layer.
> >
> > First thing every management Function appears to invoke is either
> > org.apache.geode.cache.CacheFactory.getAnyInstance() or
> > org.apache.geode.cache.internal.cache.GemFireCacheImpl getInstance() to
> > get
> > a reference to the Cache.
> >
> > This method on CacheFactory seems to be frequently involved in deadlocks
> in
> > general and I think we should find a way to deprecate it:
> >
> > /**
> > * Gets an arbitrary open instance of {@link Cache} produced by an
> earlier
> > call to
> > * {@link #create()}.
> > *
> > * @throws CacheClosedException if a cache has not been created or the
> only
> > created one is
> > * {@link Cache#isClosed closed}
> > */
> > public static synchronized Cache getAnyInstance() {
> > GemFireCacheImpl instance = GemFireCacheImpl.getInstance();
> > <snip>
> >
> > In an effort to prevent deadlocks and move away from statics and
> > singletons, I'd like to find a good alternative to this for Functions. I
> > think adding this method to FunctionContext is the best solution:
> >
> > /**
> > * Returns a reference to the Cache for which this function is executing.
> > */
> > public Cache getCache();
> >
> > Does this seem like a good solution? Is there any reason not to add this
> to
> > the API?
> >
>
>
>
> --
> Cheers
>
> Jinmei
>
--
-John
john.blum10101 (skype)
Re: Functions need reference to Cache
Posted by Jinmei Liao <ji...@pivotal.io>.
I think I even see comments in the code stating "TODO: use the cache in the
FunctionContext when it becomes available".
On Thu, Mar 2, 2017 at 12:30 PM, Kirk Lund <kl...@apache.org> wrote:
> I'm looking into a deadlock involving code under
> org.apache.geode.management. Related to this, I'm also looking at all of
> the Functions that are implemented as part of the management layer.
>
> First thing every management Function appears to invoke is either
> org.apache.geode.cache.CacheFactory.getAnyInstance() or
> org.apache.geode.cache.internal.cache.GemFireCacheImpl getInstance() to
> get
> a reference to the Cache.
>
> This method on CacheFactory seems to be frequently involved in deadlocks in
> general and I think we should find a way to deprecate it:
>
> /**
> * Gets an arbitrary open instance of {@link Cache} produced by an earlier
> call to
> * {@link #create()}.
> *
> * @throws CacheClosedException if a cache has not been created or the only
> created one is
> * {@link Cache#isClosed closed}
> */
> public static synchronized Cache getAnyInstance() {
> GemFireCacheImpl instance = GemFireCacheImpl.getInstance();
> <snip>
>
> In an effort to prevent deadlocks and move away from statics and
> singletons, I'd like to find a good alternative to this for Functions. I
> think adding this method to FunctionContext is the best solution:
>
> /**
> * Returns a reference to the Cache for which this function is executing.
> */
> public Cache getCache();
>
> Does this seem like a good solution? Is there any reason not to add this to
> the API?
>
--
Cheers
Jinmei