8:37 a.m.
While adding changes for cache methods (entrySet, keySet, values, size) to include
passivated entries it had been pointed out to conditionally do these operations if flags
such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does anyone have any
feedback on if both flags should apply or just say SKIP_CACHE_STORE? Currently
SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand do not inherit from
FlagAffectedCommand which are used for commands that behavior is dependent upon flags.
The problem is that FlagAffectedCommand currently extends VisitableCommand,
TopologyAffectedCommand, MetadataAwareCommand interfaces. These interfaces do not really
apply to the commands I am working on as they are local only. I was planning on removing
the extended interfaces from the FlagAffectedCommand interface and update all the commands
that implement this interface currently to make sure they still properly implement the
additional interfaces. Searching for instanceof FlagAffectedCommand it also appears that
all current references should be alright after refactoring. Once these interfaces are
decoupled I can add the FlagAffectedCommand interface to the SizeCommand, EntrySetCommand,
ValuesCommand and KeySetCommand and subsequently to the CommandFactory and my changes
should be good.
Any opinions or concerns?
Thanks,
- Will
9:11 a.m.
On 31 May 2013 14:37, William Burns <wburns(a)redhat.com> wrote:
While adding changes for cache methods (entrySet, keySet, values,
size) to include passivated entries it had been pointed out to conditionally do these
operations if flags such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does
anyone have any feedback on if both flags should apply or just say SKIP_CACHE_STORE?
Right, that could definitely be improved. I would expect both to be
needed as one mentions "store", the other "load", but in practice it
seems that SKIP_CACHE_STORE skips both operations. I think this should
at least be clarified in the javadocs.
Currently SizeCommand, EntrySetCommand, ValuesCommand and
KeySetCommand do not inherit from FlagAffectedCommand which are used for commands that
behavior is dependent upon flags.
You just listed the most evil operations you can use in Infinispan.
All of these are inherently broken as we need to constantly clarify to
users that they only apply to the local datastore, which makes it even
trickier to test something on a single node and then expect it to work
on multiple nodes as well..
I know, we have warnings on javadoc but since it implements
ConcurrentMap and Map, these warnings are easily overseen.
So my doubt actually is, since these operations are fundamentally
unreliable, why should they include CacheStore s as well? IMHO they
should all throw a runtime exception to flag they are not supported.
I guess we don't throw such exceptions as they could be useful for
some metrics, but still I think it would be more appropriate to move
this responsibility to a statistics/monitoring API.
Sanne
The problem is that FlagAffectedCommand currently extends
VisitableCommand, TopologyAffectedCommand, MetadataAwareCommand interfaces. These
interfaces do not really apply to the commands I am working on as they are local only. I
was planning on removing the extended interfaces from the FlagAffectedCommand interface
and update all the commands that implement this interface currently to make sure they
still properly implement the additional interfaces. Searching for instanceof
FlagAffectedCommand it also appears that all current references should be alright after
refactoring. Once these interfaces are decoupled I can add the FlagAffectedCommand
interface to the SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand and
subsequently to the CommandFactory and my changes should be good.
Any opinions or concerns?
Thanks,
- Will
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
10:53 a.m.
New subject: FlagAffectedCommand interface hierarchy (ISPN-761)
Unfortunately the requirement that was desired was specifically for methods such as
entrySet which wouldn't work with statistics, but I understand your point. I for one
didn't realize the operations were local only until I looked at them recently.
Although one thing this makes me wonder about is shared cache loaders. In this case each
node would pull every passivated entry from the cache store where as a non shared would
only pull the nodes that are local to the given node - I wonder if that is a concern or
not.
Also looking closer at separating out the interfaces I didn't realize how much
FlagAffectedCommand was referenced and the scope of the change is a lot larger than I
would have hoped changing many other interfaces. This worries me as I am not familiar
with which interfaces are called by external code and could cause compile time errors for
some references or extending classes. I will have to see what I can work up though if it
is acceptable or not.
- Will
----- Original Message -----
From: "Sanne Grinovero" <sanne(a)infinispan.org>
To: "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org>
Sent: Friday, May 31, 2013 10:11:57 AM
Subject: Re: [infinispan-dev] FlagAffectedCommand interface hierarchy (ISPN-761)
On 31 May 2013 14:37, William Burns <wburns(a)redhat.com> wrote:
While adding changes for cache methods (entrySet, keySet, values,
size) to include passivated entries it had been pointed out to conditionally do these
operations if flags such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does
anyone have any feedback on if both flags should apply or just say SKIP_CACHE_STORE?
Right, that could definitely be improved. I would expect both to be
needed as one mentions "store", the other "load", but in practice it
seems that SKIP_CACHE_STORE skips both operations. I think this should
at least be clarified in the javadocs.
Currently SizeCommand, EntrySetCommand, ValuesCommand and
KeySetCommand do not inherit from FlagAffectedCommand which are used for commands that
behavior is dependent upon flags.
You just listed the most evil operations you can use in Infinispan.
All of these are inherently broken as we need to constantly clarify to
users that they only apply to the local datastore, which makes it even
trickier to test something on a single node and then expect it to work
on multiple nodes as well..
I know, we have warnings on javadoc but since it implements
ConcurrentMap and Map, these warnings are easily overseen.
So my doubt actually is, since these operations are fundamentally
unreliable, why should they include CacheStore s as well? IMHO they
should all throw a runtime exception to flag they are not supported.
I guess we don't throw such exceptions as they could be useful for
some metrics, but still I think it would be more appropriate to move
this responsibility to a statistics/monitoring API.
Sanne
The problem is that FlagAffectedCommand currently extends
VisitableCommand, TopologyAffectedCommand, MetadataAwareCommand interfaces. These
interfaces do not really apply to the commands I am working on as they are local only. I
was planning on removing the extended interfaces from the FlagAffectedCommand interface
and update all the commands that implement this interface currently to make sure they
still properly implement the additional interfaces. Searching for instanceof
FlagAffectedCommand it also appears that all current references should be alright after
refactoring. Once these interfaces are decoupled I can add the FlagAffectedCommand
interface to the SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand and
subsequently to the CommandFactory and my changes should be good.
Any opinions or concerns?
Thanks,
- Will
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
8:27 a.m.
On May 31, 2013, at 4:11 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
On 31 May 2013 14:37, William Burns <wburns(a)redhat.com> wrote:
> While adding changes for cache methods (entrySet, keySet, values, size) to include
passivated entries it had been pointed out to conditionally do these operations if flags
such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does anyone have any
feedback on if both flags should apply or just say SKIP_CACHE_STORE?
Right, that could definitely be improved. I would expect both to be
needed as one mentions "store", the other "load", but in practice it
seems that SKIP_CACHE_STORE skips both operations. I think this should
at least be clarified in the javadocs.
> Currently SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand do not
inherit from FlagAffectedCommand which are used for commands that behavior is dependent
upon flags.
You just listed the most evil operations you can use in Infinispan.
All of these are inherently broken as we need to constantly clarify to
users that they only apply to the local datastore, which makes it even
trickier to test something on a single node and then expect it to work
on multiple nodes as well..
… which is why they're not included in JCache/JSR-107. All you have there is an
iterator()...
I know, we have warnings on javadoc but since it implements
ConcurrentMap and Map, these warnings are easily overseen.
So my doubt actually is, since these operations are fundamentally
unreliable, why should they include CacheStore s as well? IMHO they
should all throw a runtime exception to flag they are not supported.
I guess we don't throw such exceptions as they could be useful for
some metrics, but still I think it would be more appropriate to move
this responsibility to a statistics/monitoring API.
Indeed, they should be removed at some point and align ourselves closer to JCache on this
occasion. +1 on being something about statistics/monitoring, which is done at the local
level (tools can be used to aggregate results, or even map/reduce?)
Sanne
> The problem is that FlagAffectedCommand currently extends VisitableCommand,
TopologyAffectedCommand, MetadataAwareCommand interfaces. These interfaces do not really
apply to the commands I am working on as they are local only. I was planning on removing
the extended interfaces from the FlagAffectedCommand interface and update all the commands
that implement this interface currently to make sure they still properly implement the
additional interfaces. Searching for instanceof FlagAffectedCommand it also appears that
all current references should be alright after refactoring. Once these interfaces are
decoupled I can add the FlagAffectedCommand interface to the SizeCommand, EntrySetCommand,
ValuesCommand and KeySetCommand and subsequently to the CommandFactory and my changes
should be good.
>
> Any opinions or concerns?
>
> Thanks,
>
> - Will
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
11:21 a.m.
On 3 Jun 2013, at 14:27, Galder Zamarreño <galder(a)redhat.com> wrote:
On May 31, 2013, at 4:11 PM, Sanne Grinovero
<sanne(a)infinispan.org> wrote:
> On 31 May 2013 14:37, William Burns <wburns(a)redhat.com> wrote:
>> While adding changes for cache methods (entrySet, keySet, values, size) to
include passivated entries it had been pointed out to conditionally do these operations if
flags such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does anyone have
any feedback on if both flags should apply or just say SKIP_CACHE_STORE?
>
> Right, that could definitely be improved. I would expect both to be
> needed as one mentions "store", the other "load", but in practice
it
> seems that SKIP_CACHE_STORE skips both operations. I think this should
> at least be clarified in the javadocs.
>
>> Currently SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand do not
inherit from FlagAffectedCommand which are used for commands that behavior is dependent
upon flags.
>
> You just listed the most evil operations you can use in Infinispan.
> All of these are inherently broken as we need to constantly clarify to
> users that they only apply to the local datastore, which makes it even
> trickier to test something on a single node and then expect it to work
> on multiple nodes as well..
… which is why they're not included in JCache/JSR-107. All you have there is an
iterator()...
> I know, we have warnings on javadoc but since it implements
> ConcurrentMap and Map, these warnings are easily overseen.
>
> So my doubt actually is, since these operations are fundamentally
> unreliable, why should they include CacheStore s as well? IMHO they
> should all throw a runtime exception to flag they are not supported.
> I guess we don't throw such exceptions as they could be useful for
> some metrics, but still I think it would be more appropriate to move
> this responsibility to a statistics/monitoring API.
Indeed, they should be removed at some point and align ourselves closer to JCache on this
occasion. +1 on being something about statistics/monitoring, which is done at the local
level (tools can be used to aggregate results, or even map/reduce?)
Agreed. Infinispan 6.0 is quite loaded as it is, let's do it in next major.
Cheers,
--
Mircea Markus
Infinispan lead (www.infinispan.org)
10:44 a.m.
On 31 May 2013, at 15:11, Sanne Grinovero <sanne(a)infinispan.org> wrote:
On 31 May 2013 14:37, William Burns <wburns(a)redhat.com> wrote:
> While adding changes for cache methods (entrySet, keySet, values, size) to include
passivated entries it had been pointed out to conditionally do these operations if flags
such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does anyone have any
feedback on if both flags should apply or just say SKIP_CACHE_STORE?
Right, that could definitely be improved. I would expect both to be
needed as one mentions "store", the other "load", but in practice it
seems that SKIP_CACHE_STORE skips both operations. I think this should
at least be clarified in the javadocs.
> Currently SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand do not
inherit from FlagAffectedCommand which are used for commands that behavior is dependent
upon flags.
You just listed the most evil operations you can use in Infinispan.
All of these are inherently broken as we need to constantly clarify to
users that they only apply to the local datastore, which makes it even
trickier to test something on a single node and then expect it to work
on multiple nodes as well..
I know, we have warnings on javadoc but since it implements
ConcurrentMap and Map, these warnings are easily overseen.
So my doubt actually is, since these operations are fundamentally
unreliable, why should they include CacheStore s as well? IMHO they
should all throw a runtime exception to flag they are not supported.
I guess we don't throw such exceptions as they could be useful for
some metrics, but still I think it would be more appropriate to move
this responsibility to a statistics/monitoring API.
Indeed these are very costly operations and using them should be done with care. I think
they make sense at times, especially with local cache. Having them partially implemented
creates confusion through the users.
Cheers,
--
Mircea Markus
Infinispan lead (www.infinispan.org)
8:24 a.m.
On May 31, 2013, at 3:37 PM, William Burns <wburns(a)redhat.com> wrote:
While adding changes for cache methods (entrySet, keySet, values,
size) to include passivated entries it had been pointed out to conditionally do these
operations if flags such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does
anyone have any feedback on if both flags should apply or just say SKIP_CACHE_STORE?
Currently SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand do not inherit
from FlagAffectedCommand which are used for commands that behavior is dependent upon
flags.
The problem is that FlagAffectedCommand currently extends VisitableCommand,
TopologyAffectedCommand, MetadataAwareCommand interfaces. These interfaces do not really
apply to the commands I am working on as they are local only.
^ This is a new scenario really. Normally all flagged operations where clustered. In fact,
FlagAffectedCommand was added to ship flags around, which we didn't used to do at the
beginning ;).
I was planning on removing the extended interfaces from the
FlagAffectedCommand interface and update all the commands that implement this interface
currently to make sure they still properly implement the additional interfaces. Searching
for instanceof FlagAffectedCommand it also appears that all current references should be
alright after refactoring. Once these interfaces are decoupled I can add the
FlagAffectedCommand interface to the SizeCommand, EntrySetCommand, ValuesCommand and
KeySetCommand and subsequently to the CommandFactory and my changes should be good.
Any opinions or concerns?
Hmmm, since now all commands (local or clustered) can carry flags, what about making
VisitableCommand extend FlagAffectedCommand? That might be easier way to solve this
issue...
Thanks,
- Will
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
10:13 a.m.
New subject: FlagAffectedCommand interface hierarchy (ISPN-761)
Yeah I was thinking of changing it to be the inverse as you were thinking. There are a
few places such as EntryFactory, CacheNotifier, and a few interceptors that reference the
FlagAffectedCommand that will have to be changed around. Hopefully these class
definitions are fine to change as well, I don't believe they are usually referenced by
external code. I will look into it more later this week or next week though since this
isn't going to make 5.3.
Thanks for the input guys.
----- Original Message -----
From: "Galder Zamarreño" <galder(a)redhat.com>
To: "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org>
Sent: Monday, June 3, 2013 9:24:46 AM
Subject: Re: [infinispan-dev] FlagAffectedCommand interface hierarchy (ISPN-761)
On May 31, 2013, at 3:37 PM, William Burns <wburns(a)redhat.com> wrote:
While adding changes for cache methods (entrySet, keySet, values,
size) to include passivated entries it had been pointed out to conditionally do these
operations if flags such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does
anyone have any feedback on if both flags should apply or just say SKIP_CACHE_STORE?
Currently SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand do not inherit
from FlagAffectedCommand which are used for commands that behavior is dependent upon
flags.
The problem is that FlagAffectedCommand currently extends VisitableCommand,
TopologyAffectedCommand, MetadataAwareCommand interfaces. These interfaces do not really
apply to the commands I am working on as they are local only.
^ This is a new scenario really. Normally all flagged operations where clustered. In fact,
FlagAffectedCommand was added to ship flags around, which we didn't used to do at the
beginning ;).
I was planning on removing the extended interfaces from the
FlagAffectedCommand interface and update all the commands that implement this interface
currently to make sure they still properly implement the additional interfaces. Searching
for instanceof FlagAffectedCommand it also appears that all current references should be
alright after refactoring. Once these interfaces are decoupled I can add the
FlagAffectedCommand interface to the SizeCommand, EntrySetCommand, ValuesCommand and
KeySetCommand and subsequently to the CommandFactory and my changes should be good.
Any opinions or concerns?
Hmmm, since now all commands (local or clustered) can carry flags, what about making
VisitableCommand extend FlagAffectedCommand? That might be easier way to solve this
issue...
Thanks,
- Will
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
10:39 a.m.
On 31 May 2013, at 14:37, William Burns <wburns(a)redhat.com> wrote:
While adding changes for cache methods (entrySet, keySet, values,
size) to include passivated entries it had been pointed out to conditionally do these
operations if flags such as SKIP_CACHE_STORE and SKIP_CACHE_LOAD were provided. Also does
anyone have any feedback on if both flags should apply or just say SKIP_CACHE_STORE?
Currently SizeCommand, EntrySetCommand, ValuesCommand and KeySetCommand do not inherit
from FlagAffectedCommand which are used for commands that behavior is dependent upon
flags.
The problem is that FlagAffectedCommand currently extends VisitableCommand,
TopologyAffectedCommand, MetadataAwareCommand interfaces. These interfaces do not really
apply to the commands I am working on as they are local only. I was planning on removing
the extended interfaces from the FlagAffectedCommand interface and update all the commands
that implement this interface currently to make sure they still properly implement the
additional interfaces. Searching for instanceof FlagAffectedCommand it also appears that
all current references should be alright after refactoring. Once these interfaces are
decoupled I can add the FlagAffectedCommand interface to the SizeCommand, EntrySetCommand,
ValuesCommand and KeySetCommand and subsequently to the CommandFactory and my changes
should be good.
Any opinions or concerns?
That sounds good to me.
Thanks,
- Will
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Cheers,
--
Mircea Markus
Infinispan lead (www.infinispan.org)
4005
days inactive
4008
days old
infinispan-dev@lists.jboss.org
8 comments
4 participants
participants (4)
-
Galder Zamarreño -
Mircea Markus -
Sanne Grinovero -
William Burns