CacheConfiguration reusage issues with EvictionPolicy enabled.

[Andrew Mashenkov]

Hi Igniters,

I'm working on a ticket IGNITE-6649 [1] and got stuck.
Currently, we allow user to set EvictionPolicy instance into configuration
instead of factory.
The leads to some isses when user tries to reuse EvictionPolicy instance,
e.g. it doesn't clean its queue on cache stop.

Seems, we should replace EvictionPolicy with its factory as policy objec is
a statefull object and current approach is error prone.

The issue I faced is that we make checks for EvicitonPolicy derives some
known class in several code places.
E.g. in HadoopModule you can find IgfsPerBlockLruEvictionPolicy usages.
What will be correct way to overwork this with using facrories?


Also, I've noticed that we check EvictionPolicy instances if they implement
LifecycleAware interface and no one policy available out-of-box really
implements it.
The second way here is to make these EvictionPolicies implements
LifecycleAware interface and gracefully clean their queues.
This will resolve the issue for case when user recreate cache with same
configuration instance,
but seem will not help in case when user share EvictionPolicy instance for
several caches.


Also`EvictionPolicies has non-transient "queue" field, so queue may leak to
another node.
I'm not sure it can happens, I'll check.

Thoughts?



[1] https://issues.apache.org/jira/browse/IGNITE-6649

--
Best regards,
Andrey V. Mashenkov

[Valentin Kulichenko]

Andrey,

Using factory instead of policy instance in configuration makes sense to
me, I think we should do this change.

As for Hadoop module issue, is it possible to do the mentioned validation
after creating the policy? I think that should fix the problem.

-Val

On Thu, Oct 19, 2017 at 7:50 AM Andrey Mashenkov <[hidden email]>
wrote:

> Hi Igniters,
>
> I'm working on a ticket IGNITE-6649 [1] and got stuck.
> Currently, we allow user to set EvictionPolicy instance into configuration
> instead of factory.
> The leads to some isses when user tries to reuse EvictionPolicy instance,
> e.g. it doesn't clean its queue on cache stop.
>
> Seems, we should replace EvictionPolicy with its factory as policy objec is
> a statefull object and current approach is error prone.
>
> The issue I faced is that we make checks for EvicitonPolicy derives some
> known class in several code places.
> E.g. in HadoopModule you can find IgfsPerBlockLruEvictionPolicy usages.
> What will be correct way to overwork this with using facrories?
>
>
> Also, I've noticed that we check EvictionPolicy instances if they implement
> LifecycleAware interface and no one policy available out-of-box really
> implements it.
> The second way here is to make these EvictionPolicies implements
> LifecycleAware interface and gracefully clean their queues.
> This will resolve the issue for case when user recreate cache with same
> configuration instance,
> but seem will not help in case when user share EvictionPolicy instance for
> several caches.
>
>
> Also`EvictionPolicies has non-transient "queue" field, so queue may leak to
> another node.
> I'm not sure it can happens, I'll check.
>
> Thoughts?
>
>
>
> [1] https://issues.apache.org/jira/browse/IGNITE-6649
>
> --
> Best regards,
> Andrey V. Mashenkov
>

[Andrew Mashenkov]

Val,

It doesn't look possible to do without refactoring.
I've make a PR#2896 [1] with a fix and want to someone look at it
as I'm not familiar with Hadoop module.

Will it be ok to left it "as is" for now and create a ticket for
refactoring?


On Fri, Oct 20, 2017 at 3:15 AM, Valentin Kulichenko <
[hidden email]> wrote:

> Andrey,
>
> Using factory instead of policy instance in configuration makes sense to
> me, I think we should do this change.
>
> As for Hadoop module issue, is it possible to do the mentioned validation
> after creating the policy? I think that should fix the problem.
>
> -Val
>
> On Thu, Oct 19, 2017 at 7:50 AM Andrey Mashenkov <
> [hidden email]>
> wrote:
>
> > Hi Igniters,
> >
> > I'm working on a ticket IGNITE-6649 [1] and got stuck.
> > Currently, we allow user to set EvictionPolicy instance into
> configuration
> > instead of factory.
> > The leads to some isses when user tries to reuse EvictionPolicy instance,
> > e.g. it doesn't clean its queue on cache stop.
> >
> > Seems, we should replace EvictionPolicy with its factory as policy objec
> is
> > a statefull object and current approach is error prone.
> >
> > The issue I faced is that we make checks for EvicitonPolicy derives some
> > known class in several code places.
> > E.g. in HadoopModule you can find IgfsPerBlockLruEvictionPolicy usages.
> > What will be correct way to overwork this with using facrories?
> >
> >
> > Also, I've noticed that we check EvictionPolicy instances if they
> implement
> > LifecycleAware interface and no one policy available out-of-box really
> > implements it.
> > The second way here is to make these EvictionPolicies implements
> > LifecycleAware interface and gracefully clean their queues.
> > This will resolve the issue for case when user recreate cache with same
> > configuration instance,
> > but seem will not help in case when user share EvictionPolicy instance
> for
> > several caches.
> >
> >
> > Also`EvictionPolicies has non-transient "queue" field, so queue may leak
> to
> > another node.
> > I'm not sure it can happens, I'll check.
> >
> > Thoughts?
> >
> >
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-6649
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
> >
>

--
Best regards,
Andrey V. Mashenkov

[Valentin Kulichenko]

I don't think it's a big problem. But it would be great if someone better
experienced in Hadoop stuff chimes in.

-Val

On Fri, Oct 20, 2017 at 8:50 AM, Andrey Mashenkov <
[hidden email]> wrote:

> Val,
>
> It doesn't look possible to do without refactoring.
> I've make a PR#2896 [1] with a fix and want to someone look at it
> as I'm not familiar with Hadoop module.
>
> Will it be ok to left it "as is" for now and create a ticket for
> refactoring?
>
>
> On Fri, Oct 20, 2017 at 3:15 AM, Valentin Kulichenko <
> [hidden email]> wrote:
>
> > Andrey,
> >
> > Using factory instead of policy instance in configuration makes sense to
> > me, I think we should do this change.
> >
> > As for Hadoop module issue, is it possible to do the mentioned validation
> > after creating the policy? I think that should fix the problem.
> >
> > -Val
> >
> > On Thu, Oct 19, 2017 at 7:50 AM Andrey Mashenkov <
> > [hidden email]>
> > wrote:
> >
> > > Hi Igniters,
> > >
> > > I'm working on a ticket IGNITE-6649 [1] and got stuck.
> > > Currently, we allow user to set EvictionPolicy instance into
> > configuration
> > > instead of factory.
> > > The leads to some isses when user tries to reuse EvictionPolicy
> instance,
> > > e.g. it doesn't clean its queue on cache stop.
> > >
> > > Seems, we should replace EvictionPolicy with its factory as policy
> objec
> > is
> > > a statefull object and current approach is error prone.
> > >
> > > The issue I faced is that we make checks for EvicitonPolicy derives
> some
> > > known class in several code places.
> > > E.g. in HadoopModule you can find IgfsPerBlockLruEvictionPolicy usages.
> > > What will be correct way to overwork this with using facrories?
> > >
> > >
> > > Also, I've noticed that we check EvictionPolicy instances if they
> > implement
> > > LifecycleAware interface and no one policy available out-of-box really
> > > implements it.
> > > The second way here is to make these EvictionPolicies implements
> > > LifecycleAware interface and gracefully clean their queues.
> > > This will resolve the issue for case when user recreate cache with same
> > > configuration instance,
> > > but seem will not help in case when user share EvictionPolicy instance
> > for
> > > several caches.
> > >
> > >
> > > Also`EvictionPolicies has non-transient "queue" field, so queue may
> leak
> > to
> > > another node.
> > > I'm not sure it can happens, I'll check.
> > >
> > > Thoughts?
> > >
> > >
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-6649
> > >
> > > --
> > > Best regards,
> > > Andrey V. Mashenkov
> > >
> >
>
>
>
> --
> Best regards,
> Andrey V. Mashenkov
>

[Andrew Mashenkov]

Up.
Would someone please review a PR#2896 for IGNITE-6649 [1]?


[1] https://issues.apache.org/jira/browse/IGNITE-6649


On Sat, Oct 21, 2017 at 1:51 AM, Valentin Kulichenko <
[hidden email]> wrote:

> I don't think it's a big problem. But it would be great if someone better
> experienced in Hadoop stuff chimes in.
>
> -Val
>
> On Fri, Oct 20, 2017 at 8:50 AM, Andrey Mashenkov <
> [hidden email]> wrote:
>
> > Val,
> >
> > It doesn't look possible to do without refactoring.
> > I've make a PR#2896 [1] with a fix and want to someone look at it
> > as I'm not familiar with Hadoop module.
> >
> > Will it be ok to left it "as is" for now and create a ticket for
> > refactoring?
> >
> >
> > On Fri, Oct 20, 2017 at 3:15 AM, Valentin Kulichenko <
> > [hidden email]> wrote:
> >
> > > Andrey,
> > >
> > > Using factory instead of policy instance in configuration makes sense
> to
> > > me, I think we should do this change.
> > >
> > > As for Hadoop module issue, is it possible to do the mentioned
> validation
> > > after creating the policy? I think that should fix the problem.
> > >
> > > -Val
> > >
> > > On Thu, Oct 19, 2017 at 7:50 AM Andrey Mashenkov <
> > > [hidden email]>
> > > wrote:
> > >
> > > > Hi Igniters,
> > > >
> > > > I'm working on a ticket IGNITE-6649 [1] and got stuck.
> > > > Currently, we allow user to set EvictionPolicy instance into
> > > configuration
> > > > instead of factory.
> > > > The leads to some isses when user tries to reuse EvictionPolicy
> > instance,
> > > > e.g. it doesn't clean its queue on cache stop.
> > > >
> > > > Seems, we should replace EvictionPolicy with its factory as policy
> > objec
> > > is
> > > > a statefull object and current approach is error prone.
> > > >
> > > > The issue I faced is that we make checks for EvicitonPolicy derives
> > some
> > > > known class in several code places.
> > > > E.g. in HadoopModule you can find IgfsPerBlockLruEvictionPolicy
> usages.
> > > > What will be correct way to overwork this with using facrories?
> > > >
> > > >
> > > > Also, I've noticed that we check EvictionPolicy instances if they
> > > implement
> > > > LifecycleAware interface and no one policy available out-of-box
> really
> > > > implements it.
> > > > The second way here is to make these EvictionPolicies implements
> > > > LifecycleAware interface and gracefully clean their queues.
> > > > This will resolve the issue for case when user recreate cache with
> same
> > > > configuration instance,
> > > > but seem will not help in case when user share EvictionPolicy
> instance
> > > for
> > > > several caches.
> > > >
> > > >
> > > > Also`EvictionPolicies has non-transient "queue" field, so queue may
> > leak
> > > to
> > > > another node.
> > > > I'm not sure it can happens, I'll check.
> > > >
> > > > Thoughts?
> > > >
> > > >
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-6649
> > > >
> > > > --
> > > > Best regards,
> > > > Andrey V. Mashenkov
> > > >
> > >
> >
> >
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
> >
>

--
Best regards,
Andrey V. Mashenkov