You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Artem Shutak <as...@gridgain.com> on 2015/11/12 14:01:54 UTC
IGNITE-1355 Potential NPE in CacheAffinityProxy
Igniters,
I'm working on https://issues.apache.org/jira/browse/IGNITE-1355.
I want to hear a community opinion what should do Ignite in case when user
uses Affinity for nonexistent cache? Current implementation returns
AffinityProxy on a call of Ignite.affinity("nonexistent_cache") and
AffinityProxy throws NPE on a call of any method if cache has not been
created yet.
I see next possible decisions:
- Ignite.affinity("nonexistent_cache") can return 'null' instead of
AffinityProxy like Ignite does it for Ignite.cache("nonexistent_cache").
But it breaks backward compatibility.
- AffinityProxy methods can return a special value like 0 for
'partitions()' method and empty array for 'primaryPartitions(ClusterNode)'
method.
- AffinityProxy methods can throw Exception that cache does not exist.
I vote for the second one.
Thoughts?
Thanks,
-- Artem --
Re: IGNITE-1355 Potential NPE in CacheAffinityProxy
Posted by Artem Shutak <as...@gridgain.com>.
I need to add that in the next scenario AffinityImpl will be used (not
AffinityProxy) and the code throws NPE too.
ignite.getOrCreateCache(new CacheConfiguration("myCache"));
Affinity<Object> affinity = ignite.affinity("myCache");
affinity.mapPartitionToNode(0); // All ok.
ignite2.cache("myCache").destroy();
affinity.mapPartitionToNode(0); // NPE.
This case looks like an exception for me, so I need to change my opinion
and I vote for throwing a special exception if cache does not exists.
-- Artem --
On Thu, Nov 12, 2015 at 4:01 PM, Artem Shutak <as...@gridgain.com> wrote:
> Igniters,
>
> I'm working on https://issues.apache.org/jira/browse/IGNITE-1355.
>
> I want to hear a community opinion what should do Ignite in case when user
> uses Affinity for nonexistent cache? Current implementation returns
> AffinityProxy on a call of Ignite.affinity("nonexistent_cache") and
> AffinityProxy throws NPE on a call of any method if cache has not been
> created yet.
>
> I see next possible decisions:
> - Ignite.affinity("nonexistent_cache") can return 'null' instead of
> AffinityProxy like Ignite does it for Ignite.cache("nonexistent_cache").
> But it breaks backward compatibility.
> - AffinityProxy methods can return a special value like 0 for
> 'partitions()' method and empty array for 'primaryPartitions(ClusterNode)'
> method.
> - AffinityProxy methods can throw Exception that cache does not exist.
>
> I vote for the second one.
>
> Thoughts?
>
> Thanks,
> -- Artem --
>
Re: IGNITE-1355 Potential NPE in CacheAffinityProxy
Posted by Yakov Zhdanov <yz...@apache.org>.
I agree with proposed changes.
--Yakov
2015-11-25 16:35 GMT+03:00 Artem Shutak <as...@gridgain.com>:
> Igniters,
>
> I've fixed the issue according to the last discussed approach [1].
>
> I've faced with the following API issues:
> - IgniteCluster and Affinity interfaces have the same methods:
> mapKeysToNodes and mapKeyToNode, which use the same implementation.
> - The both methods in both interfaces say that they return special value in
> case when cache doesn't exist (empty map for 'mapKeysToNodes' and null for
> 'mapKeyToNode').
>
> I want do the following (already did in my PR [1]):
> 1. Deprecate the methods in IgniteCluster interface.
> 2. Change javadoc of the methods on Affinity interface. Javadoc will say
> that an exception will be thrown if cache does not exist.
>
> The problem is the point 2 breaks backward compatibility. But as I see from
> the methods implementation, they can throw IgniteException in some cases
> now, so a user should already have processing of the exception. So, I think
> we can do these changes.
>
> Thoughts?
>
> [1] https://github.com/apache/ignite/pull/263
>
> Thanks,
> -- Artem --
>
> On Thu, Nov 12, 2015 at 4:10 PM, Yakov Zhdanov <yz...@apache.org>
> wrote:
>
> > Any cache can be destroyed after proxy is created, so any cache may
> become
> > unexistent at any moment. So, I would still allow proxy creation for
> > unexistent caches, but throw an exception if cache does not exist at the
> > moment of proxy method call.
> >
> > --Yakov
> >
> > 2015-11-12 16:01 GMT+03:00 Artem Shutak <as...@gridgain.com>:
> >
> > > Igniters,
> > >
> > > I'm working on https://issues.apache.org/jira/browse/IGNITE-1355.
> > >
> > > I want to hear a community opinion what should do Ignite in case when
> > user
> > > uses Affinity for nonexistent cache? Current implementation returns
> > > AffinityProxy on a call of Ignite.affinity("nonexistent_cache") and
> > > AffinityProxy throws NPE on a call of any method if cache has not been
> > > created yet.
> > >
> > > I see next possible decisions:
> > > - Ignite.affinity("nonexistent_cache") can return 'null' instead of
> > > AffinityProxy like Ignite does it for
> Ignite.cache("nonexistent_cache").
> > > But it breaks backward compatibility.
> > > - AffinityProxy methods can return a special value like 0 for
> > > 'partitions()' method and empty array for
> > 'primaryPartitions(ClusterNode)'
> > > method.
> > > - AffinityProxy methods can throw Exception that cache does not exist.
> > >
> > > I vote for the second one.
> > >
> > > Thoughts?
> > >
> > > Thanks,
> > > -- Artem --
> > >
> >
>
Re: IGNITE-1355 Potential NPE in CacheAffinityProxy
Posted by Artem Shutak <as...@gridgain.com>.
Igniters,
I've fixed the issue according to the last discussed approach [1].
I've faced with the following API issues:
- IgniteCluster and Affinity interfaces have the same methods:
mapKeysToNodes and mapKeyToNode, which use the same implementation.
- The both methods in both interfaces say that they return special value in
case when cache doesn't exist (empty map for 'mapKeysToNodes' and null for
'mapKeyToNode').
I want do the following (already did in my PR [1]):
1. Deprecate the methods in IgniteCluster interface.
2. Change javadoc of the methods on Affinity interface. Javadoc will say
that an exception will be thrown if cache does not exist.
The problem is the point 2 breaks backward compatibility. But as I see from
the methods implementation, they can throw IgniteException in some cases
now, so a user should already have processing of the exception. So, I think
we can do these changes.
Thoughts?
[1] https://github.com/apache/ignite/pull/263
Thanks,
-- Artem --
On Thu, Nov 12, 2015 at 4:10 PM, Yakov Zhdanov <yz...@apache.org> wrote:
> Any cache can be destroyed after proxy is created, so any cache may become
> unexistent at any moment. So, I would still allow proxy creation for
> unexistent caches, but throw an exception if cache does not exist at the
> moment of proxy method call.
>
> --Yakov
>
> 2015-11-12 16:01 GMT+03:00 Artem Shutak <as...@gridgain.com>:
>
> > Igniters,
> >
> > I'm working on https://issues.apache.org/jira/browse/IGNITE-1355.
> >
> > I want to hear a community opinion what should do Ignite in case when
> user
> > uses Affinity for nonexistent cache? Current implementation returns
> > AffinityProxy on a call of Ignite.affinity("nonexistent_cache") and
> > AffinityProxy throws NPE on a call of any method if cache has not been
> > created yet.
> >
> > I see next possible decisions:
> > - Ignite.affinity("nonexistent_cache") can return 'null' instead of
> > AffinityProxy like Ignite does it for Ignite.cache("nonexistent_cache").
> > But it breaks backward compatibility.
> > - AffinityProxy methods can return a special value like 0 for
> > 'partitions()' method and empty array for
> 'primaryPartitions(ClusterNode)'
> > method.
> > - AffinityProxy methods can throw Exception that cache does not exist.
> >
> > I vote for the second one.
> >
> > Thoughts?
> >
> > Thanks,
> > -- Artem --
> >
>
Re: IGNITE-1355 Potential NPE in CacheAffinityProxy
Posted by Yakov Zhdanov <yz...@apache.org>.
Any cache can be destroyed after proxy is created, so any cache may become
unexistent at any moment. So, I would still allow proxy creation for
unexistent caches, but throw an exception if cache does not exist at the
moment of proxy method call.
--Yakov
2015-11-12 16:01 GMT+03:00 Artem Shutak <as...@gridgain.com>:
> Igniters,
>
> I'm working on https://issues.apache.org/jira/browse/IGNITE-1355.
>
> I want to hear a community opinion what should do Ignite in case when user
> uses Affinity for nonexistent cache? Current implementation returns
> AffinityProxy on a call of Ignite.affinity("nonexistent_cache") and
> AffinityProxy throws NPE on a call of any method if cache has not been
> created yet.
>
> I see next possible decisions:
> - Ignite.affinity("nonexistent_cache") can return 'null' instead of
> AffinityProxy like Ignite does it for Ignite.cache("nonexistent_cache").
> But it breaks backward compatibility.
> - AffinityProxy methods can return a special value like 0 for
> 'partitions()' method and empty array for 'primaryPartitions(ClusterNode)'
> method.
> - AffinityProxy methods can throw Exception that cache does not exist.
>
> I vote for the second one.
>
> Thoughts?
>
> Thanks,
> -- Artem --
>