ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yakov Zhdanov <yzhda...@apache.org>
Subject Re: IGNITE-1355 Potential NPE in CacheAffinityProxy
Date Thu, 26 Nov 2015 07:48:37 GMT
I agree with proposed changes.

--Yakov

2015-11-25 16:35 GMT+03:00 Artem Shutak <ashutak@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 <yzhdanov@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 <ashutak@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 --
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message