Quantcast

[infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Eduardo Martins
Hi all, just completed first iteration of Mobicents Cluster Framework
2.x, which includes impl using first alpha release of Infinispan 5. We
have everything we had in jboss cache working, it's a dumb down but
higher level framework, with stuff like Fault Tolerant timers, which
is then reuse in whole Mobicents platform to provide cluster related
features. I believe we now already use a lot of stuff from Infinispan
rich feature set, so I guess it's good timing to give some feedback,
report some minor issues, and clear some doubts.

1) Marshallers

Generally I'm "just OK" with the current API, the fact it
Externalizers depends on Annotations makes impossible for a higher
level framework to allow it's clients to plug their own kind of
Externalizers, without exposing Infinispan classes. In concrete, our
framework exposes its own Externalizer model, but to use them we need
to wrap the instances of classes these handle, in classes bound to
Infinispan Externalizers, which when invoked by Infinispan then lookup
our "own" Externalizers, that is, 2 Externalizers lookups per call. If
there was no annotations required we could wrap instead our clients
Externalizers and plug them into Infinispan, this would mean a single
Externalizer lookup. By the way, since the Externalizer now uses
generics the Annotations values look bad, specially since it's
possible to introduce errors that can compile.

Another issue, in this case I consider it of minor importance due to
low level of Externalizers concept, is the lack of inheritance of
Externalizers, for instance we extend DefaultConsistentHash, instead
of extending its Externalizer I had to make an Externalizer from
scratch, and copy of DefaultConsistenHash's Externalizer code. This is
messy to manage, if the code on Infinispan change we will always have
to check if the code we copied is still valid.

2) CacheContainer vs CacheManager

The API is now deprecating the CacheManager, replacing with
CacheContainer, but some functionality is missing, for instance
CacheContainer is not a Listenable, thus no option to register
Listeners, unless unsafe casting of the instance exposed by the Cache
or a ref is available to the original CacheManager. Related matter,
IMHO a Cache listener should be able to listen to CacheManager events,
becoming global listeners.

3) Configuration stuff

Generally I think the Configuration and GlobalConfiguration could be
simplified a bit, I found myself several times looking at the impl
code to understand how to achieve some configurations. Better
documentation wouldn't hurt too, it's great to have a complete
reference, but the configuration samples are not ok, one is basically
empty, the other has all possible stuff, very unrealistic, would be
better to have reusable examples for each mode, with then
recommendations on how to improve these.

Infinispan 5 introduces a new global configuration setter to provide
an instance, with same method name as the one to provide the class
name. I believe one is enough, and to be more friendly with
Microcontainer and similar frameworks I would choose the one to set
the instance.

4) AtomicMap doubt

I read in the Infinispan blog that AtomicMap provides colocation of
all entries, is that idea outdated? If not we may need a way to turn
that off :) For instance would not that mean the Tree API does not
works well with distribution mode? I apologize in advance if I'm
missing something, but if AtomicMap defines colocation, AtomicMap is
good for the node's data map, but not for the node's childs fqns.
Shouldn't each child fqn be freely distributed, being colocated
instead with the related node cache entry and data (atomic)map? Our
impl is kind of an "hybrid" of the Tree API, allows cache entries
references (similar to childs) but no data map, and the storage of
references through AtomicMap in same way as Tree API worries me.
Please clarify.

5) Minor issues found

See these a lot, forgotten info logging?

03:39:06,603 INFO  [TransactionLoggerImpl] Starting transaction logging
03:39:06,623 INFO  [TransactionLoggerImpl] Stopping transaction logging

MBean registration tries twice the same MBean, the second time fails
and prints log (no harm besides that, the process continues without
failures):

03:39:06,395 INFO  [ComponentsJmxRegistration] Could not register
object with name:
org.infinispan:type=Cache,name="___defaultcache(dist_sync)",manager="DefaultCacheManager",component=Cache

6) Final thoughts

Kind of feel bad to provide all these negative stuff in a single mail,
took me an hour to write it, but don't get me wrong, I really enjoy
Infinispan, it's a great improvement. I'm really excited to have it
plugged in AS7 (any plan on this?) and then migrate our platform to
this new cluster framework. I expect a big performance improvement, on
something already pretty fast, and much less memory usage, our
Infinispan impl "feels" very fine grained and optimized in all points
of view. Of course, the distribution mode is the cherry on top of the
cake, hello true scalability.

I hope to find time to contribute back more, and in better ways, like
concrete enhancements or issues with test cases as happened with jboss
cache, but right now that's the best I could. By the way, I'm using
the nick mart1ns in the infinispan freenode irc channel, feel free to
ping me there.

Regards,

-- Eduardo
..............................................
http://emmartins.blogspot.com
http://redhat.com/solutions/telco
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Galder Zamarreño
See below my comments:

On Dec 30, 2010, at 7:05 AM, Eduardo Martins wrote:

> Hi all, just completed first iteration of Mobicents Cluster Framework
> 2.x, which includes impl using first alpha release of Infinispan 5. We
> have everything we had in jboss cache working, it's a dumb down but
> higher level framework, with stuff like Fault Tolerant timers, which
> is then reuse in whole Mobicents platform to provide cluster related
> features. I believe we now already use a lot of stuff from Infinispan
> rich feature set, so I guess it's good timing to give some feedback,
> report some minor issues, and clear some doubts.
>
> 1) Marshallers
>
> Generally I'm "just OK" with the current API, the fact it
> Externalizers depends on Annotations makes impossible for a higher
> level framework to allow it's clients to plug their own kind of
> Externalizers, without exposing Infinispan classes. In concrete, our
> framework exposes its own Externalizer model, but to use them we need
> to wrap the instances of classes these handle, in classes bound to
> Infinispan Externalizers, which when invoked by Infinispan then lookup
> our "own" Externalizers, that is, 2 Externalizers lookups per call. If
> there was no annotations required we could wrap instead our clients
> Externalizers and plug them into Infinispan, this would mean a single
> Externalizer lookup. By the way, since the Externalizer now uses
> generics the Annotations values look bad, specially since it's
> possible to introduce errors that can compile.

First of all, the current API that you use in 5.0.0.ALPHA1 is not yet set in stone, and so feedback like this is invaluable to get it right for ALPHA2 or BETA.

Externalizer is a concept that was borrowed from JBoss Marshalling that was squashed to the bare minimum needed by Infinispan. Unless you plan to provide alternative Externalizer implementations based in another framework (I don't see this really doable since the way they're instantiated and used is very particular), I don't see the need to abstract this as hugely important, although I can understand your leak worries.

Personally I think annotations fit perfectly for defining the classes it externalizers and optionally the ids. However, a different avenue could be taken here which is adding getTypeClasses()/getTypeClassNames()/getId() methods to Externalizer interface, but I'm not keen on doing that since IMO pollutes a very clean Externalizer interface. Another possibility could be to provide a sub-interface of Externalizer where those methods are added and give people the freedom to decide how to feed in this metadata, either via API or annotations. However, this could be adding complexity that might confuse Externalizer providers.

Not sure I understand your 2 lookup call problem though. Maybe you can clarify this further with the proposal I make above and how it would vary?

And I don't understand the errors that compile either. Please provide an example to understand this better.

>
> Another issue, in this case I consider it of minor importance due to
> low level of Externalizers concept, is the lack of inheritance of
> Externalizers, for instance we extend DefaultConsistentHash, instead
> of extending its Externalizer I had to make an Externalizer from
> scratch, and copy of DefaultConsistenHash's Externalizer code. This is
> messy to manage, if the code on Infinispan change we will always have
> to check if the code we copied is still valid.

Hmmm, can't you use delegation instead of inheritance? Delegation is generally considered to be cleaner than inheritance. For example, see org.infinispan.transaction.xa.DldGlobalTransaction and org.infinispan.transaction.xa.GlobalTransaction. DldGlobalTransaction extends GlobalTransaction but DldGlobalTransaction's Externalizer does not extend GlobalTransaction's. Instead, DldGlobalTransaction's Externalizer takes GlobalTransaction's Externalizer in the constructor and then delegates accordingly. This avoids any code duplication.

Btw, out of curiosity, why finding yourself needing to extend the DefaultConsistentHash externalizer?

>
> 2) CacheContainer vs CacheManager
>
> The API is now deprecating the CacheManager, replacing with
> CacheContainer, but some functionality is missing, for instance
> CacheContainer is not a Listenable, thus no option to register
> Listeners, unless unsafe casting of the instance exposed by the Cache
> or a ref is available to the original CacheManager. Related matter,
> IMHO a Cache listener should be able to listen to CacheManager events,
> becoming global listeners.

Did you look at EmbeddedCacheManager? That's what you should be coding against that allows you to add/remove listeners. You should not use CacheManager at all.

Wrt listener, what are you trying to say? That if a listener is registered to a Cache, then it should receive CacheManager notifications if it has methods annotated accordingly, i.e. @CacheStopped?

>
> 3) Configuration stuff
>
> Generally I think the Configuration and GlobalConfiguration could be
> simplified a bit, I found myself several times looking at the impl
> code to understand how to achieve some configurations.

Configurations such as?

> Better
> documentation wouldn't hurt too, it's great to have a complete
> reference, but the configuration samples are not ok, one is basically
> empty, the other has all possible stuff, very unrealistic, would be
> better to have reusable examples for each mode, with then
> recommendations on how to improve these.

Which configuration samples do you refer to? The ones in the testsuite?

I suppose the reusable examples that you look for might be partially available via TestCacheManagerFactory except that these are oriented at testing (i.e. make sure each testsuite thread uses a different mcast address). Maybe we should extract these out and make more easily available.

>
> Infinispan 5 introduces a new global configuration setter to provide
> an instance, with same method name as the one to provide the class
> name. I believe one is enough, and to be more friendly with
> Microcontainer and similar frameworks I would choose the one to set
> the instance.

Which configuration setter are you referring to? It's worth mentioning that GlobalConfiguration mixes both JAXB method requirements and programmatic configuration needs, so maybe that's what's confusing things.

>
> 4) AtomicMap doubt
>
> I read in the Infinispan blog that AtomicMap provides colocation of
> all entries, is that idea outdated? If not we may need a way to turn
> that off :) For instance would not that mean the Tree API does not
> works well with distribution mode? I apologize in advance if I'm
> missing something, but if AtomicMap defines colocation, AtomicMap is
> good for the node's data map, but not for the node's childs fqns.
> Shouldn't each child fqn be freely distributed, being colocated
> instead with the related node cache entry and data (atomic)map? Our
> impl is kind of an "hybrid" of the Tree API, allows cache entries
> references (similar to childs) but no data map, and the storage of
> references through AtomicMap in same way as Tree API worries me.
> Please clarify.

Each FQN has underneath an AtomicMap, so each you won't find yourself finding k,v pairs belonging to a particular Fqn in different nodes.

We make no guarantees wrt child fqn nodes. So, just cos FQN B is child of FQN A, it does not mean that the atomic map of B will be in same node as atomic map of B.

>
> 5) Minor issues found
>
> See these a lot, forgotten info logging?
>
> 03:39:06,603 INFO  [TransactionLoggerImpl] Starting transaction logging
> 03:39:06,623 INFO  [TransactionLoggerImpl] Stopping transaction logging

Most likely: https://issues.jboss.org/browse/ISPN-852

>
> MBean registration tries twice the same MBean, the second time fails
> and prints log (no harm besides that, the process continues without
> failures):
>
> 03:39:06,395 INFO  [ComponentsJmxRegistration] Could not register
> object with name:
> org.infinispan:type=Cache,name="___defaultcache(dist_sync)",manager="DefaultCacheManager",component=Cache

That should not happen: https://issues.jboss.org/browse/ISPN-853

>
> 6) Final thoughts
>
> Kind of feel bad to provide all these negative stuff in a single mail,
> took me an hour to write it, but don't get me wrong, I really enjoy
> Infinispan, it's a great improvement. I'm really excited to have it
> plugged in AS7 (any plan on this?) and then migrate our platform to
> this new cluster framework. I expect a big performance improvement, on
> something already pretty fast, and much less memory usage, our
> Infinispan impl "feels" very fine grained and optimized in all points
> of view. Of course, the distribution mode is the cherry on top of the
> cake, hello true scalability.

I don't think it's negative stuff at all. On the contrary, I think it's really valuable feedback from another Infinispan user :)

AS7 will of course have Infinispan but in due course.

>
> I hope to find time to contribute back more, and in better ways, like
> concrete enhancements or issues with test cases as happened with jboss
> cache, but right now that's the best I could. By the way, I'm using
> the nick mart1ns in the infinispan freenode irc channel, feel free to
> ping me there.
>
> Regards,
>
> -- Eduardo
> ..............................................
> http://emmartins.blogspot.com
> http://redhat.com/solutions/telco
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Eduardo Martins
See inline.

On Mon, Jan 3, 2011 at 10:38 AM, Galder Zamarreño <[hidden email]> wrote:

> See below my comments:
>
> On Dec 30, 2010, at 7:05 AM, Eduardo Martins wrote:
>
>> Hi all, just completed first iteration of Mobicents Cluster Framework
>> 2.x, which includes impl using first alpha release of Infinispan 5. We
>> have everything we had in jboss cache working, it's a dumb down but
>> higher level framework, with stuff like Fault Tolerant timers, which
>> is then reuse in whole Mobicents platform to provide cluster related
>> features. I believe we now already use a lot of stuff from Infinispan
>> rich feature set, so I guess it's good timing to give some feedback,
>> report some minor issues, and clear some doubts.
>>
>> 1) Marshallers
>>
>> Generally I'm "just OK" with the current API, the fact it
>> Externalizers depends on Annotations makes impossible for a higher
>> level framework to allow it's clients to plug their own kind of
>> Externalizers, without exposing Infinispan classes. In concrete, our
>> framework exposes its own Externalizer model, but to use them we need
>> to wrap the instances of classes these handle, in classes bound to
>> Infinispan Externalizers, which when invoked by Infinispan then lookup
>> our "own" Externalizers, that is, 2 Externalizers lookups per call. If
>> there was no annotations required we could wrap instead our clients
>> Externalizers and plug them into Infinispan, this would mean a single
>> Externalizer lookup. By the way, since the Externalizer now uses
>> generics the Annotations values look bad, specially since it's
>> possible to introduce errors that can compile.
>
> First of all, the current API that you use in 5.0.0.ALPHA1 is not yet set in stone, and so feedback like this is invaluable to get it right for ALPHA2 or BETA.
>
> Externalizer is a concept that was borrowed from JBoss Marshalling that was squashed to the bare minimum needed by Infinispan. Unless you plan to provide alternative Externalizer implementations based in another framework (I don't see this really doable since the way they're instantiated and used is very particular), I don't see the need to abstract this as hugely important, although I can understand your leak worries.
>

We have the need to keep our framework fully independent of what is
below, for instance in 1.x we had a few bounds to JBoss Cache API,
such as FQN, now we had to redo a lot in the framework. The framework
is used in different projects of the Mobicents platform, being
independent of the underlying impl allows the "client" code to don't
need any update, that is, we just update the impl and everything keeps
working (ideally).

> Personally I think annotations fit perfectly for defining the classes it externalizers and optionally the ids. However, a different avenue could be taken here which is adding getTypeClasses()/getTypeClassNames()/getId() methods to Externalizer interface, but I'm not keen on doing that since IMO pollutes a very clean Externalizer interface. Another possibility could be to provide a sub-interface of Externalizer where those methods are added and give people the freedom to decide how to feed in this metadata, either via API or annotations. However, this could be adding complexity that might confuse Externalizer providers.
>

We are talking about 3 methods, don't think it would be that bad, the
implementation of these methods would possibly be as simple as setting
the annotations. IMHO this would be a very good move.

> Not sure I understand your 2 lookup call problem though. Maybe you can clarify this further with the proposal I make above and how it would vary?

Our framework does not limits what the "clients" store in the cache,
it can be objects of any class, which means that if we want
serialization to be as fast as possible we need to expose
Externalizers concept. Our current solution is to wrap the data going
into the Cache in objects that have Externalizers, the double lookup
is the Infinispan lookup for the wrapper externalizer, and then the
wrapper's Externalizer lookup process for the concrete "client"
externalizer code.

A quick example, lets say the "client" wants to store X and provided
our framework a class XExternalizer that has the methods to read and
write X to I/O. When storing an X instance in Infinispan we wrap it in
XWrapper, being XWrapper bound to a XWrapperExternalizer installed in
Infinispan. So for instance on reading an X instance from Input,
Infinispan will lookup XWrapperExternalizer and invoke it, then
XWrapperExternalizer will lookup XExternalizer to do the actual read.

> And I don't understand the errors that compile either. Please provide an example to understand this better.

@Marshalls(typeClasses = Y.class, id = ExternalizerIds.XExternalizer)
public class XExternalizer implements
                Externalizer<X> {

Is there any validation that Y is related to X?

>>
>> Another issue, in this case I consider it of minor importance due to
>> low level of Externalizers concept, is the lack of inheritance of
>> Externalizers, for instance we extend DefaultConsistentHash, instead
>> of extending its Externalizer I had to make an Externalizer from
>> scratch, and copy of DefaultConsistenHash's Externalizer code. This is
>> messy to manage, if the code on Infinispan change we will always have
>> to check if the code we copied is still valid.
>
> Hmmm, can't you use delegation instead of inheritance? Delegation is generally considered to be cleaner than inheritance. For example, see org.infinispan.transaction.xa.DldGlobalTransaction and org.infinispan.transaction.xa.GlobalTransaction. DldGlobalTransaction extends GlobalTransaction but DldGlobalTransaction's Externalizer does not extend GlobalTransaction's. Instead, DldGlobalTransaction's Externalizer takes GlobalTransaction's Externalizer in the constructor and then delegates accordingly. This avoids any code duplication.
>

The problem in such model is that you need a way to construct the
externalizer to delegate, so unless you have a factory somewhere in
Infinispan you will always rely on its code. That or require that
externalizers have a specific constructor.

> Btw, out of curiosity, why finding yourself needing to extend the DefaultConsistentHash externalizer?

I actually don't know, it was not made due to an execution error. I
saw there an Externalizer, I assume it's going to be "externalized",
which means our DefaultConsistentHashExt that replaces it will also
be.

Out of curiosity, here is the code, it forces colocation of certain data:

http://mobicents.googlecode.com/svn/branches/servers/cluster/2.x/infinispan/src/main/java/org/mobicents/cluster/infinispan/distribution/DefaultConsistentHashExt.java

>>
>> 2) CacheContainer vs CacheManager
>>
>> The API is now deprecating the CacheManager, replacing with
>> CacheContainer, but some functionality is missing, for instance
>> CacheContainer is not a Listenable, thus no option to register
>> Listeners, unless unsafe casting of the instance exposed by the Cache
>> or a ref is available to the original CacheManager. Related matter,
>> IMHO a Cache listener should be able to listen to CacheManager events,
>> becoming global listeners.
>
> Did you look at EmbeddedCacheManager? That's what you should be coding against that allows you to add/remove listeners. You should not use CacheManager at all.
>
Yes, I looked, the concrete issue here is not being able to register a
CacheManager listener starting with a Cache instance, that is, not
without un unsafe cast, did I miss anything?

> Wrt listener, what are you trying to say? That if a listener is registered to a Cache, then it should receive CacheManager notifications if it has methods annotated accordingly, i.e. @CacheStopped?

Yes.

>>
>> 3) Configuration stuff
>>
>> Generally I think the Configuration and GlobalConfiguration could be
>> simplified a bit, I found myself several times looking at the impl
>> code to understand how to achieve some configurations.
>
> Configurations such as?

Example below, but this is a generic remark, more like a call to try
to simplify as much as possible, I can say that it took me much more
time to settle on configurations than to code.

>
>> Better
>> documentation wouldn't hurt too, it's great to have a complete
>> reference, but the configuration samples are not ok, one is basically
>> empty, the other has all possible stuff, very unrealistic, would be
>> better to have reusable examples for each mode, with then
>> recommendations on how to improve these.
>
> Which configuration samples do you refer to? The ones in the testsuite?

The ones bundled in etc/config-samples.

>
> I suppose the reusable examples that you look for might be partially available via TestCacheManagerFactory except that these are oriented at testing (i.e. make sure each testsuite thread uses a different mcast address). Maybe we should extract these out and make more easily available.
>
>>
>> Infinispan 5 introduces a new global configuration setter to provide
>> an instance, with same method name as the one to provide the class
>> name. I believe one is enough, and to be more friendly with
>> Microcontainer and similar frameworks I would choose the one to set
>> the instance.
>
> Which configuration setter are you referring to?

setMBeanServerLookup(...)

> It's worth mentioning that GlobalConfiguration mixes both JAXB method requirements and programmatic configuration needs, so maybe that's what's confusing things.

Perhaps.

>>
>> 4) AtomicMap doubt
>>
>> I read in the Infinispan blog that AtomicMap provides colocation of
>> all entries, is that idea outdated? If not we may need a way to turn
>> that off :) For instance would not that mean the Tree API does not
>> works well with distribution mode? I apologize in advance if I'm
>> missing something, but if AtomicMap defines colocation, AtomicMap is
>> good for the node's data map, but not for the node's childs fqns.
>> Shouldn't each child fqn be freely distributed, being colocated
>> instead with the related node cache entry and data (atomic)map? Our
>> impl is kind of an "hybrid" of the Tree API, allows cache entries
>> references (similar to childs) but no data map, and the storage of
>> references through AtomicMap in same way as Tree API worries me.
>> Please clarify.
>
> Each FQN has underneath an AtomicMap, so each you won't find yourself finding k,v pairs belonging to a particular Fqn in different nodes.
>
> We make no guarantees wrt child fqn nodes. So, just cos FQN B is child of FQN A, it does not mean that the atomic map of B will be in same node as atomic map of B.

Before I proceed with my reasoning, please clarify, the colocation
within AtomicMap is real? If I store data there, all data will be
colocated?

>
>>
>> 5) Minor issues found
>>
>> See these a lot, forgotten info logging?
>>
>> 03:39:06,603 INFO  [TransactionLoggerImpl] Starting transaction logging
>> 03:39:06,623 INFO  [TransactionLoggerImpl] Stopping transaction logging
>
> Most likely: https://issues.jboss.org/browse/ISPN-852
>
>>
>> MBean registration tries twice the same MBean, the second time fails
>> and prints log (no harm besides that, the process continues without
>> failures):
>>
>> 03:39:06,395 INFO  [ComponentsJmxRegistration] Could not register
>> object with name:
>> org.infinispan:type=Cache,name="___defaultcache(dist_sync)",manager="DefaultCacheManager",component=Cache
>
> That should not happen: https://issues.jboss.org/browse/ISPN-853
>
>>
>> 6) Final thoughts
>>
>> Kind of feel bad to provide all these negative stuff in a single mail,
>> took me an hour to write it, but don't get me wrong, I really enjoy
>> Infinispan, it's a great improvement. I'm really excited to have it
>> plugged in AS7 (any plan on this?) and then migrate our platform to
>> this new cluster framework. I expect a big performance improvement, on
>> something already pretty fast, and much less memory usage, our
>> Infinispan impl "feels" very fine grained and optimized in all points
>> of view. Of course, the distribution mode is the cherry on top of the
>> cake, hello true scalability.
>
> I don't think it's negative stuff at all. On the contrary, I think it's really valuable feedback from another Infinispan user :)

I don't think it's negative also, but some could interpret like that,
you know, too many complaints, whatever ;-)

>
> AS7 will of course have Infinispan but in due course.
>

So right now, there is no work being done to plug Infinispan in AS7?

-- Eduardo
..............................................
http://emmartins.blogspot.com
http://redhat.com/solutions/telco

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Galder Zamarreño
See comments inline:

On Jan 3, 2011, at 3:44 PM, Eduardo Martins wrote:

> See inline.
>
> On Mon, Jan 3, 2011 at 10:38 AM, Galder Zamarreño <[hidden email]> wrote:
>> See below my comments:
>>
>> On Dec 30, 2010, at 7:05 AM, Eduardo Martins wrote:
>>
>>> Hi all, just completed first iteration of Mobicents Cluster Framework
>>> 2.x, which includes impl using first alpha release of Infinispan 5. We
>>> have everything we had in jboss cache working, it's a dumb down but
>>> higher level framework, with stuff like Fault Tolerant timers, which
>>> is then reuse in whole Mobicents platform to provide cluster related
>>> features. I believe we now already use a lot of stuff from Infinispan
>>> rich feature set, so I guess it's good timing to give some feedback,
>>> report some minor issues, and clear some doubts.
>>>
>>> 1) Marshallers
>>>
>>> Generally I'm "just OK" with the current API, the fact it
>>> Externalizers depends on Annotations makes impossible for a higher
>>> level framework to allow it's clients to plug their own kind of
>>> Externalizers, without exposing Infinispan classes. In concrete, our
>>> framework exposes its own Externalizer model, but to use them we need
>>> to wrap the instances of classes these handle, in classes bound to
>>> Infinispan Externalizers, which when invoked by Infinispan then lookup
>>> our "own" Externalizers, that is, 2 Externalizers lookups per call. If
>>> there was no annotations required we could wrap instead our clients
>>> Externalizers and plug them into Infinispan, this would mean a single
>>> Externalizer lookup. By the way, since the Externalizer now uses
>>> generics the Annotations values look bad, specially since it's
>>> possible to introduce errors that can compile.
>>
>> First of all, the current API that you use in 5.0.0.ALPHA1 is not yet set in stone, and so feedback like this is invaluable to get it right for ALPHA2 or BETA.
>>
>> Externalizer is a concept that was borrowed from JBoss Marshalling that was squashed to the bare minimum needed by Infinispan. Unless you plan to provide alternative Externalizer implementations based in another framework (I don't see this really doable since the way they're instantiated and used is very particular), I don't see the need to abstract this as hugely important, although I can understand your leak worries.
>>
>
> We have the need to keep our framework fully independent of what is
> below, for instance in 1.x we had a few bounds to JBoss Cache API,
> such as FQN, now we had to redo a lot in the framework. The framework
> is used in different projects of the Mobicents platform, being
> independent of the underlying impl allows the "client" code to don't
> need any update, that is, we just update the impl and everything keeps
> working (ideally).
>
>> Personally I think annotations fit perfectly for defining the classes it externalizers and optionally the ids. However, a different avenue could be taken here which is adding getTypeClasses()/getTypeClassNames()/getId() methods to Externalizer interface, but I'm not keen on doing that since IMO pollutes a very clean Externalizer interface. Another possibility could be to provide a sub-interface of Externalizer where those methods are added and give people the freedom to decide how to feed in this metadata, either via API or annotations. However, this could be adding complexity that might confuse Externalizer providers.
>>
>
> We are talking about 3 methods, don't think it would be that bad, the
> implementation of these methods would possibly be as simple as setting
> the annotations. IMHO this would be a very good move.
>
>> Not sure I understand your 2 lookup call problem though. Maybe you can clarify this further with the proposal I make above and how it would vary?
>
> Our framework does not limits what the "clients" store in the cache,
> it can be objects of any class, which means that if we want
> serialization to be as fast as possible we need to expose
> Externalizers concept. Our current solution is to wrap the data going
> into the Cache in objects that have Externalizers, the double lookup
> is the Infinispan lookup for the wrapper externalizer, and then the
> wrapper's Externalizer lookup process for the concrete "client"
> externalizer code.
>
> A quick example, lets say the "client" wants to store X and provided
> our framework a class XExternalizer that has the methods to read and
> write X to I/O. When storing an X instance in Infinispan we wrap it in
> XWrapper, being XWrapper bound to a XWrapperExternalizer installed in
> Infinispan. So for instance on reading an X instance from Input,
> Infinispan will lookup XWrapperExternalizer and invoke it, then
> XWrapperExternalizer will lookup XExternalizer to do the actual read.

Right, so your XWrapperExternalizer would be the bridge that would be annotated, hence hiding away the annotation from the user, correct?

>
>> And I don't understand the errors that compile either. Please provide an example to understand this better.
>
> @Marshalls(typeClasses = Y.class, id = ExternalizerIds.XExternalizer)
> public class XExternalizer implements
> Externalizer<X> {
>
> Is there any validation that Y is related to X?

Good point Eduardo. No, there's no such validation and it's not possible to do cos generics are not available at runtime and there's no link between the annotation and the Externalizer. I think this is a powerful reason to integrate the get* methods referred above in the Externalizer interface to allow us to tie types, i.e.

List<Class<? extends T>> getTypeClasses();

Bounded wildcards in this case are Ok cos the consumer of this method would be the internal framework and not any client code. Also, using collections rather than arrays makes it easier to code.

This is being tracked by https://issues.jboss.org/browse/ISPN-857

>
>>>
>>> Another issue, in this case I consider it of minor importance due to
>>> low level of Externalizers concept, is the lack of inheritance of
>>> Externalizers, for instance we extend DefaultConsistentHash, instead
>>> of extending its Externalizer I had to make an Externalizer from
>>> scratch, and copy of DefaultConsistenHash's Externalizer code. This is
>>> messy to manage, if the code on Infinispan change we will always have
>>> to check if the code we copied is still valid.
>>
>> Hmmm, can't you use delegation instead of inheritance? Delegation is generally considered to be cleaner than inheritance. For example, see org.infinispan.transaction.xa.DldGlobalTransaction and org.infinispan.transaction.xa.GlobalTransaction. DldGlobalTransaction extends GlobalTransaction but DldGlobalTransaction's Externalizer does not extend GlobalTransaction's. Instead, DldGlobalTransaction's Externalizer takes GlobalTransaction's Externalizer in the constructor and then delegates accordingly. This avoids any code duplication.
>>
>
> The problem in such model is that you need a way to construct the
> externalizer to delegate, so unless you have a factory somewhere in
> Infinispan you will always rely on its code. That or require that
> externalizers have a specific constructor.

Hmmm, not necessarily. Normally Externalizer instances are singletons, without any state that actually changes, so this kind of code would be quite typical:

private final GlobalTransaction.Externalizer delegate = new GlobalTransaction.Externalizer(new GlobalTransactionFactory(true));

Where DldGlobalTransaction's Externalizer has the delegate as a private final constant.

>
>> Btw, out of curiosity, why finding yourself needing to extend the DefaultConsistentHash externalizer?
>
> I actually don't know, it was not made due to an execution error. I
> saw there an Externalizer, I assume it's going to be "externalized",
> which means our DefaultConsistentHashExt that replaces it will also
> be.
>
> Out of curiosity, here is the code, it forces colocation of certain data:
>
> http://mobicents.googlecode.com/svn/branches/servers/cluster/2.x/infinispan/src/main/java/org/mobicents/cluster/infinispan/distribution/DefaultConsistentHashExt.java

That code is a bit strange. If you need to make sure that the hash from InfinispanClusterDataKey is derived from its key's hash depending on whether the key is DependentClusterDataKey or something else, why not just adjust  InfinispanClusterDataKey.hashCode() accordingly? i.e.

InfinispanClusterDataKey.hashCode() {
  if (getKey() instanceof DependentClusterDataKey) {
    return ((DependentClusterDataKey) clusterDataKey).dependsOn().hashCode();
  } else {
    return getKey().hashCode()
  }
}

I don't think you need a different consistent hash implementation.

>
>>>
>>> 2) CacheContainer vs CacheManager
>>>
>>> The API is now deprecating the CacheManager, replacing with
>>> CacheContainer, but some functionality is missing, for instance
>>> CacheContainer is not a Listenable, thus no option to register
>>> Listeners, unless unsafe casting of the instance exposed by the Cache
>>> or a ref is available to the original CacheManager. Related matter,
>>> IMHO a Cache listener should be able to listen to CacheManager events,
>>> becoming global listeners.
>>
>> Did you look at EmbeddedCacheManager? That's what you should be coding against that allows you to add/remove listeners. You should not use CacheManager at all.
>>
> Yes, I looked, the concrete issue here is not being able to register a
> CacheManager listener starting with a Cache instance, that is, not
> without un unsafe cast, did I miss anything?

Oh right, I see what you mean. You're saying that getCacheManager() in Cache returns a CacheContainer rather than an EmbeddedCacheManager. There's a reason for this:

CacheContainer is the common demoninator for operations that are accessible by remote and local caches. EmbeddedCacheManager extends CacheContainer to add operations that only make sense on a local basis, whereas RemoteCacheManager does the same for remotely accessed Infinispan instances. For the moment, you cannot add listeners to remote cache/cachemanager instances, hence why those methods are only available in EmbeddedCacheManager.

It might make sense to have an EmbeddedCache interface that ties up this up via covariant returns, i.e.:

EmbeddedCacheManager getCacheManager();


>
>> Wrt listener, what are you trying to say? That if a listener is registered to a Cache, then it should receive CacheManager notifications if it has methods annotated accordingly, i.e. @CacheStopped?
>
> Yes.

I think this could be tricky cos if you promote something like this and you have @CacheStarted and @ViewChanged annotations, you're likely to miss them at least for the Cache in which you've added the cache manager listener, and you'd have maybe lost the first view change. This could be partially solved by adding the capability to get a cache without starting it, but you'd still have the issue of view change miss.

To make sure you don't miss a notification, the recommended thing is add cache manager listeners after creating the cache manager and before starting it.

>
>>>
>>> 3) Configuration stuff
>>>
>>> Generally I think the Configuration and GlobalConfiguration could be
>>> simplified a bit, I found myself several times looking at the impl
>>> code to understand how to achieve some configurations.
>>
>> Configurations such as?
>
> Example below, but this is a generic remark, more like a call to try
> to simplify as much as possible, I can say that it took me much more
> time to settle on configurations than to code.
>
>>
>>> Better
>>> documentation wouldn't hurt too, it's great to have a complete
>>> reference, but the configuration samples are not ok, one is basically
>>> empty, the other has all possible stuff, very unrealistic, would be
>>> better to have reusable examples for each mode, with then
>>> recommendations on how to improve these.
>>
>> Which configuration samples do you refer to? The ones in the testsuite?
>
> The ones bundled in etc/config-samples.

Precisely for 5.0.0.ALPHA1 and 4.2.0.FINAL we removed all.xml and added sample.xml that has more meaningful configurations in there with several of them commented and ready for use.

>
>>
>> I suppose the reusable examples that you look for might be partially available via TestCacheManagerFactory except that these are oriented at testing (i.e. make sure each testsuite thread uses a different mcast address). Maybe we should extract these out and make more easily available.
>>
>>>
>>> Infinispan 5 introduces a new global configuration setter to provide
>>> an instance, with same method name as the one to provide the class
>>> name. I believe one is enough, and to be more friendly with
>>> Microcontainer and similar frameworks I would choose the one to set
>>> the instance.
>>
>> Which configuration setter are you referring to?
>
> setMBeanServerLookup(...)

Right. The one that takes a String is used by JAXB code whereas the one taking the instance is used by programmatic configuration.

I think the latter should be renamed to setMBeanServerLookupInstance at least.

On a more global level, GlobalConfiguration might need separating into a superclass for programmatic configuration and a subclass for JAXB or something similar, in order to avoid confusion between the different methods. Vladimir WDYT?

>
>> It's worth mentioning that GlobalConfiguration mixes both JAXB method requirements and programmatic configuration needs, so maybe that's what's confusing things.
>
> Perhaps.
>
>>>
>>> 4) AtomicMap doubt
>>>
>>> I read in the Infinispan blog that AtomicMap provides colocation of
>>> all entries, is that idea outdated? If not we may need a way to turn
>>> that off :) For instance would not that mean the Tree API does not
>>> works well with distribution mode? I apologize in advance if I'm
>>> missing something, but if AtomicMap defines colocation, AtomicMap is
>>> good for the node's data map, but not for the node's childs fqns.
>>> Shouldn't each child fqn be freely distributed, being colocated
>>> instead with the related node cache entry and data (atomic)map? Our
>>> impl is kind of an "hybrid" of the Tree API, allows cache entries
>>> references (similar to childs) but no data map, and the storage of
>>> references through AtomicMap in same way as Tree API worries me.
>>> Please clarify.
>>
>> Each FQN has underneath an AtomicMap, so each you won't find yourself finding k,v pairs belonging to a particular Fqn in different nodes.
>>
>> We make no guarantees wrt child fqn nodes. So, just cos FQN B is child of FQN A, it does not mean that the atomic map of B will be in same node as atomic map of B.
>
> Before I proceed with my reasoning, please clarify, the colocation
> within AtomicMap is real? If I store data there, all data will be
> colocated?

Yup, all *data* stored in the AtomicMap will be located in the same node. It's treated as a single entity.

>
>>
>>>
>>> 5) Minor issues found
>>>
>>> See these a lot, forgotten info logging?
>>>
>>> 03:39:06,603 INFO  [TransactionLoggerImpl] Starting transaction logging
>>> 03:39:06,623 INFO  [TransactionLoggerImpl] Stopping transaction logging
>>
>> Most likely: https://issues.jboss.org/browse/ISPN-852
>>
>>>
>>> MBean registration tries twice the same MBean, the second time fails
>>> and prints log (no harm besides that, the process continues without
>>> failures):
>>>
>>> 03:39:06,395 INFO  [ComponentsJmxRegistration] Could not register
>>> object with name:
>>> org.infinispan:type=Cache,name="___defaultcache(dist_sync)",manager="DefaultCacheManager",component=Cache
>>
>> That should not happen: https://issues.jboss.org/browse/ISPN-853
>>
>>>
>>> 6) Final thoughts
>>>
>>> Kind of feel bad to provide all these negative stuff in a single mail,
>>> took me an hour to write it, but don't get me wrong, I really enjoy
>>> Infinispan, it's a great improvement. I'm really excited to have it
>>> plugged in AS7 (any plan on this?) and then migrate our platform to
>>> this new cluster framework. I expect a big performance improvement, on
>>> something already pretty fast, and much less memory usage, our
>>> Infinispan impl "feels" very fine grained and optimized in all points
>>> of view. Of course, the distribution mode is the cherry on top of the
>>> cake, hello true scalability.
>>
>> I don't think it's negative stuff at all. On the contrary, I think it's really valuable feedback from another Infinispan user :)
>
> I don't think it's negative also, but some could interpret like that,
> you know, too many complaints, whatever ;-)
>
>>
>> AS7 will of course have Infinispan but in due course.
>>
>
> So right now, there is no work being done to plug Infinispan in AS7?

I can't say 100% for sure, Paul Ferraro or Scott Marlow will be able to clarify this further, but AFAIK, this is not being done yet.  

>
> -- Eduardo
> ..............................................
> http://emmartins.blogspot.com
> http://redhat.com/solutions/telco
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Eduardo Martins
Hi Galder, more comments inline:

On Tue, Jan 4, 2011 at 10:30 AM, Galder Zamarreño <[hidden email]> wrote:

> See comments inline:
>
> On Jan 3, 2011, at 3:44 PM, Eduardo Martins wrote:
>
>> See inline.
>>
>> On Mon, Jan 3, 2011 at 10:38 AM, Galder Zamarreño <[hidden email]> wrote:
>>> See below my comments:
>>>
>>> On Dec 30, 2010, at 7:05 AM, Eduardo Martins wrote:
>>>
>>>> Hi all, just completed first iteration of Mobicents Cluster Framework
>>>> 2.x, which includes impl using first alpha release of Infinispan 5. We
>>>> have everything we had in jboss cache working, it's a dumb down but
>>>> higher level framework, with stuff like Fault Tolerant timers, which
>>>> is then reuse in whole Mobicents platform to provide cluster related
>>>> features. I believe we now already use a lot of stuff from Infinispan
>>>> rich feature set, so I guess it's good timing to give some feedback,
>>>> report some minor issues, and clear some doubts.
>>>>
>>>> 1) Marshallers
>>>>
>>>> Generally I'm "just OK" with the current API, the fact it
>>>> Externalizers depends on Annotations makes impossible for a higher
>>>> level framework to allow it's clients to plug their own kind of
>>>> Externalizers, without exposing Infinispan classes. In concrete, our
>>>> framework exposes its own Externalizer model, but to use them we need
>>>> to wrap the instances of classes these handle, in classes bound to
>>>> Infinispan Externalizers, which when invoked by Infinispan then lookup
>>>> our "own" Externalizers, that is, 2 Externalizers lookups per call. If
>>>> there was no annotations required we could wrap instead our clients
>>>> Externalizers and plug them into Infinispan, this would mean a single
>>>> Externalizer lookup. By the way, since the Externalizer now uses
>>>> generics the Annotations values look bad, specially since it's
>>>> possible to introduce errors that can compile.
>>>
>>> First of all, the current API that you use in 5.0.0.ALPHA1 is not yet set in stone, and so feedback like this is invaluable to get it right for ALPHA2 or BETA.
>>>
>>> Externalizer is a concept that was borrowed from JBoss Marshalling that was squashed to the bare minimum needed by Infinispan. Unless you plan to provide alternative Externalizer implementations based in another framework (I don't see this really doable since the way they're instantiated and used is very particular), I don't see the need to abstract this as hugely important, although I can understand your leak worries.
>>>
>>
>> We have the need to keep our framework fully independent of what is
>> below, for instance in 1.x we had a few bounds to JBoss Cache API,
>> such as FQN, now we had to redo a lot in the framework. The framework
>> is used in different projects of the Mobicents platform, being
>> independent of the underlying impl allows the "client" code to don't
>> need any update, that is, we just update the impl and everything keeps
>> working (ideally).
>>
>>> Personally I think annotations fit perfectly for defining the classes it externalizers and optionally the ids. However, a different avenue could be taken here which is adding getTypeClasses()/getTypeClassNames()/getId() methods to Externalizer interface, but I'm not keen on doing that since IMO pollutes a very clean Externalizer interface. Another possibility could be to provide a sub-interface of Externalizer where those methods are added and give people the freedom to decide how to feed in this metadata, either via API or annotations. However, this could be adding complexity that might confuse Externalizer providers.
>>>
>>
>> We are talking about 3 methods, don't think it would be that bad, the
>> implementation of these methods would possibly be as simple as setting
>> the annotations. IMHO this would be a very good move.
>>
>>> Not sure I understand your 2 lookup call problem though. Maybe you can clarify this further with the proposal I make above and how it would vary?
>>
>> Our framework does not limits what the "clients" store in the cache,
>> it can be objects of any class, which means that if we want
>> serialization to be as fast as possible we need to expose
>> Externalizers concept. Our current solution is to wrap the data going
>> into the Cache in objects that have Externalizers, the double lookup
>> is the Infinispan lookup for the wrapper externalizer, and then the
>> wrapper's Externalizer lookup process for the concrete "client"
>> externalizer code.
>>
>> A quick example, lets say the "client" wants to store X and provided
>> our framework a class XExternalizer that has the methods to read and
>> write X to I/O. When storing an X instance in Infinispan we wrap it in
>> XWrapper, being XWrapper bound to a XWrapperExternalizer installed in
>> Infinispan. So for instance on reading an X instance from Input,
>> Infinispan will lookup XWrapperExternalizer and invoke it, then
>> XWrapperExternalizer will lookup XExternalizer to do the actual read.
>
> Right, so your XWrapperExternalizer would be the bridge that would be annotated, hence hiding away the annotation from the user, correct?

Yes, now if there was no annotation needed, we could instead wrap the
user's externalizer in an Infinispan externalizer, thus only one
lookup.

>
>>
>>> And I don't understand the errors that compile either. Please provide an example to understand this better.
>>
>> @Marshalls(typeClasses = Y.class, id = ExternalizerIds.XExternalizer)
>> public class XExternalizer implements
>>               Externalizer<X> {
>>
>> Is there any validation that Y is related to X?
>
> Good point Eduardo. No, there's no such validation and it's not possible to do cos generics are not available at runtime and there's no link between the annotation and the Externalizer. I think this is a powerful reason to integrate the get* methods referred above in the Externalizer interface to allow us to tie types, i.e.
>
> List<Class<? extends T>> getTypeClasses();
>
> Bounded wildcards in this case are Ok cos the consumer of this method would be the internal framework and not any client code. Also, using collections rather than arrays makes it easier to code.
>
> This is being tracked by https://issues.jboss.org/browse/ISPN-857
>

Wouldn't make more sense to use a Set?

>>
>>>>
>>>> Another issue, in this case I consider it of minor importance due to
>>>> low level of Externalizers concept, is the lack of inheritance of
>>>> Externalizers, for instance we extend DefaultConsistentHash, instead
>>>> of extending its Externalizer I had to make an Externalizer from
>>>> scratch, and copy of DefaultConsistenHash's Externalizer code. This is
>>>> messy to manage, if the code on Infinispan change we will always have
>>>> to check if the code we copied is still valid.
>>>
>>> Hmmm, can't you use delegation instead of inheritance? Delegation is generally considered to be cleaner than inheritance. For example, see org.infinispan.transaction.xa.DldGlobalTransaction and org.infinispan.transaction.xa.GlobalTransaction. DldGlobalTransaction extends GlobalTransaction but DldGlobalTransaction's Externalizer does not extend GlobalTransaction's. Instead, DldGlobalTransaction's Externalizer takes GlobalTransaction's Externalizer in the constructor and then delegates accordingly. This avoids any code duplication.
>>>
>>
>> The problem in such model is that you need a way to construct the
>> externalizer to delegate, so unless you have a factory somewhere in
>> Infinispan you will always rely on its code. That or require that
>> externalizers have a specific constructor.
>
> Hmmm, not necessarily. Normally Externalizer instances are singletons, without any state that actually changes, so this kind of code would be quite typical:
>
> private final GlobalTransaction.Externalizer delegate = new GlobalTransaction.Externalizer(new GlobalTransactionFactory(true));
>
> Where DldGlobalTransaction's Externalizer has the delegate as a private final constant.

Still bounded to the concrete implementation code of the Externalizer
to delegate, anyway I'm not sure there is much to do here, adding
externalizer factories would complicate, default constructor would not
avoid a shift of custom code needed in initialization to additional
setters...

>
>>
>>> Btw, out of curiosity, why finding yourself needing to extend the DefaultConsistentHash externalizer?
>>
>> I actually don't know, it was not made due to an execution error. I
>> saw there an Externalizer, I assume it's going to be "externalized",
>> which means our DefaultConsistentHashExt that replaces it will also
>> be.
>>
>> Out of curiosity, here is the code, it forces colocation of certain data:
>>
>> http://mobicents.googlecode.com/svn/branches/servers/cluster/2.x/infinispan/src/main/java/org/mobicents/cluster/infinispan/distribution/DefaultConsistentHashExt.java
>
> That code is a bit strange. If you need to make sure that the hash from InfinispanClusterDataKey is derived from its key's hash depending on whether the key is DependentClusterDataKey or something else, why not just adjust  InfinispanClusterDataKey.hashCode() accordingly? i.e.
>
> InfinispanClusterDataKey.hashCode() {
>  if (getKey() instanceof DependentClusterDataKey) {
>    return ((DependentClusterDataKey) clusterDataKey).dependsOn().hashCode();
>  } else {
>    return getKey().hashCode()
>  }
> }
>
> I don't think you need a different consistent hash implementation.
>

hashCode() is not used only by the hash impl, it should be as unique
as possible elsewhere.

>>
>>>>
>>>> 2) CacheContainer vs CacheManager
>>>>
>>>> The API is now deprecating the CacheManager, replacing with
>>>> CacheContainer, but some functionality is missing, for instance
>>>> CacheContainer is not a Listenable, thus no option to register
>>>> Listeners, unless unsafe casting of the instance exposed by the Cache
>>>> or a ref is available to the original CacheManager. Related matter,
>>>> IMHO a Cache listener should be able to listen to CacheManager events,
>>>> becoming global listeners.
>>>
>>> Did you look at EmbeddedCacheManager? That's what you should be coding against that allows you to add/remove listeners. You should not use CacheManager at all.
>>>
>> Yes, I looked, the concrete issue here is not being able to register a
>> CacheManager listener starting with a Cache instance, that is, not
>> without un unsafe cast, did I miss anything?
>
> Oh right, I see what you mean. You're saying that getCacheManager() in Cache returns a CacheContainer rather than an EmbeddedCacheManager. There's a reason for this:
>
> CacheContainer is the common demoninator for operations that are accessible by remote and local caches. EmbeddedCacheManager extends CacheContainer to add operations that only make sense on a local basis, whereas RemoteCacheManager does the same for remotely accessed Infinispan instances. For the moment, you cannot add listeners to remote cache/cachemanager instances, hence why those methods are only available in EmbeddedCacheManager.
>
> It might make sense to have an EmbeddedCache interface that ties up this up via covariant returns, i.e.:
>
> EmbeddedCacheManager getCacheManager();
>

Agree.

>
>>
>>> Wrt listener, what are you trying to say? That if a listener is registered to a Cache, then it should receive CacheManager notifications if it has methods annotated accordingly, i.e. @CacheStopped?
>>
>> Yes.
>
> I think this could be tricky cos if you promote something like this and you have @CacheStarted and @ViewChanged annotations, you're likely to miss them at least for the Cache in which you've added the cache manager listener, and you'd have maybe lost the first view change. This could be partially solved by adding the capability to get a cache without starting it, but you'd still have the issue of view change miss.
>
> To make sure you don't miss a notification, the recommended thing is add cache manager listeners after creating the cache manager and before starting it.
>

That can happen when adding a listener to CacheManager or Cache too,
there is no illegal state when adding listeners, right? :)

>>
>>>>
>>>> 3) Configuration stuff
>>>>
>>>> Generally I think the Configuration and GlobalConfiguration could be
>>>> simplified a bit, I found myself several times looking at the impl
>>>> code to understand how to achieve some configurations.
>>>
>>> Configurations such as?
>>
>> Example below, but this is a generic remark, more like a call to try
>> to simplify as much as possible, I can say that it took me much more
>> time to settle on configurations than to code.
>>
>>>
>>>> Better
>>>> documentation wouldn't hurt too, it's great to have a complete
>>>> reference, but the configuration samples are not ok, one is basically
>>>> empty, the other has all possible stuff, very unrealistic, would be
>>>> better to have reusable examples for each mode, with then
>>>> recommendations on how to improve these.
>>>
>>> Which configuration samples do you refer to? The ones in the testsuite?
>>
>> The ones bundled in etc/config-samples.
>
> Precisely for 5.0.0.ALPHA1 and 4.2.0.FINAL we removed all.xml and added sample.xml that has more meaningful configurations in there with several of them commented and ready for use.
>

I think it still needs more examples, and a docs section reviewing
those. It makes sense to have one example for each typical/mainstream
configuration used.

>>
>>>
>>> I suppose the reusable examples that you look for might be partially available via TestCacheManagerFactory except that these are oriented at testing (i.e. make sure each testsuite thread uses a different mcast address). Maybe we should extract these out and make more easily available.
>>>
>>>>
>>>> Infinispan 5 introduces a new global configuration setter to provide
>>>> an instance, with same method name as the one to provide the class
>>>> name. I believe one is enough, and to be more friendly with
>>>> Microcontainer and similar frameworks I would choose the one to set
>>>> the instance.
>>>
>>> Which configuration setter are you referring to?
>>
>> setMBeanServerLookup(...)
>
> Right. The one that takes a String is used by JAXB code whereas the one taking the instance is used by programmatic configuration.
>
> I think the latter should be renamed to setMBeanServerLookupInstance at least.
>

Agree.

> On a more global level, GlobalConfiguration might need separating into a superclass for programmatic configuration and a subclass for JAXB or something similar, in order to avoid confusion between the different methods. Vladimir WDYT?
>

That would even be better.

>>
>>> It's worth mentioning that GlobalConfiguration mixes both JAXB method requirements and programmatic configuration needs, so maybe that's what's confusing things.
>>
>> Perhaps.
>>
>>>>
>>>> 4) AtomicMap doubt
>>>>
>>>> I read in the Infinispan blog that AtomicMap provides colocation of
>>>> all entries, is that idea outdated? If not we may need a way to turn
>>>> that off :) For instance would not that mean the Tree API does not
>>>> works well with distribution mode? I apologize in advance if I'm
>>>> missing something, but if AtomicMap defines colocation, AtomicMap is
>>>> good for the node's data map, but not for the node's childs fqns.
>>>> Shouldn't each child fqn be freely distributed, being colocated
>>>> instead with the related node cache entry and data (atomic)map? Our
>>>> impl is kind of an "hybrid" of the Tree API, allows cache entries
>>>> references (similar to childs) but no data map, and the storage of
>>>> references through AtomicMap in same way as Tree API worries me.
>>>> Please clarify.
>>>
>>> Each FQN has underneath an AtomicMap, so each you won't find yourself finding k,v pairs belonging to a particular Fqn in different nodes.
>>>
>>> We make no guarantees wrt child fqn nodes. So, just cos FQN B is child of FQN A, it does not mean that the atomic map of B will be in same node as atomic map of B.
>>
>> Before I proceed with my reasoning, please clarify, the colocation
>> within AtomicMap is real? If I store data there, all data will be
>> colocated?
>
> Yup, all *data* stored in the AtomicMap will be located in the same node. It's treated as a single entity.
>

Ok, then lets think on the Tree API, typically/optimally you add, get
and remove a specific node in same cluster node/zone, iterating
through a node's childs is rare and usually without much perf
constraints. With current impl the node's child map entries are
colocated, since each node does that through an AtomicMap with child's
last FQN element, IMHO this is not great for performance:

1. Consider parent node P in node N1
2. In node N2, add a P child, this goes to N1. Adding P Child also
creates the child cache entries, all 3 seem to be colocated through
hashCode(), correct me if I'm wrong. Lets assume all hash ideally to
local node N2.
3. In node N2, get P child, this may go to N1 if we use P, skips it if
use Cache.get(...)
4. In node N2, remove P child, this needs to go to N1

Are you following the issue, if P is a popular parent node (which
happens a lot to root childs), N1 will be hammered by other nodes!

>>
>>>
>>>>
>>>> 5) Minor issues found
>>>>
>>>> See these a lot, forgotten info logging?
>>>>
>>>> 03:39:06,603 INFO  [TransactionLoggerImpl] Starting transaction logging
>>>> 03:39:06,623 INFO  [TransactionLoggerImpl] Stopping transaction logging
>>>
>>> Most likely: https://issues.jboss.org/browse/ISPN-852
>>>
>>>>
>>>> MBean registration tries twice the same MBean, the second time fails
>>>> and prints log (no harm besides that, the process continues without
>>>> failures):
>>>>
>>>> 03:39:06,395 INFO  [ComponentsJmxRegistration] Could not register
>>>> object with name:
>>>> org.infinispan:type=Cache,name="___defaultcache(dist_sync)",manager="DefaultCacheManager",component=Cache
>>>
>>> That should not happen: https://issues.jboss.org/browse/ISPN-853
>>>
>>>>
>>>> 6) Final thoughts
>>>>
>>>> Kind of feel bad to provide all these negative stuff in a single mail,
>>>> took me an hour to write it, but don't get me wrong, I really enjoy
>>>> Infinispan, it's a great improvement. I'm really excited to have it
>>>> plugged in AS7 (any plan on this?) and then migrate our platform to
>>>> this new cluster framework. I expect a big performance improvement, on
>>>> something already pretty fast, and much less memory usage, our
>>>> Infinispan impl "feels" very fine grained and optimized in all points
>>>> of view. Of course, the distribution mode is the cherry on top of the
>>>> cake, hello true scalability.
>>>
>>> I don't think it's negative stuff at all. On the contrary, I think it's really valuable feedback from another Infinispan user :)
>>
>> I don't think it's negative also, but some could interpret like that,
>> you know, too many complaints, whatever ;-)
>>
>>>
>>> AS7 will of course have Infinispan but in due course.
>>>
>>
>> So right now, there is no work being done to plug Infinispan in AS7?
>
> I can't say 100% for sure, Paul Ferraro or Scott Marlow will be able to clarify this further, but AFAIK, this is not being done yet.
>

Would be good to have a roadmap for this, to adjust our work too.

-- Eduardo
..............................................
http://emmartins.blogspot.com
http://redhat.com/solutions/telco

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Manik Surtani
Thanks for the feedback, Eduardo.  I'll address each one separately, although Galder's done an excellent job already.  :)

On 4 Jan 2011, at 16:12, Eduardo Martins wrote:

>>
>> Oh right, I see what you mean. You're saying that getCacheManager() in Cache returns a CacheContainer rather than an EmbeddedCacheManager. There's a reason for this:
>>
>> CacheContainer is the common demoninator for operations that are accessible by remote and local caches. EmbeddedCacheManager extends CacheContainer to add operations that only make sense on a local basis, whereas RemoteCacheManager does the same for remotely accessed Infinispan instances. For the moment, you cannot add listeners to remote cache/cachemanager instances, hence why those methods are only available in EmbeddedCacheManager.
>>
>> It might make sense to have an EmbeddedCache interface that ties up this up via covariant returns, i.e.:
>>
>> EmbeddedCacheManager getCacheManager();
>>
>
> Agree.

This was changed in 4.1.0 when we introduced the RemoteCacheManager (Hot Rod client).  But yeah I appreciate your concerns re: not being able to add a cache manager listener when all you have is a reference to a cache - however I would have expected that you'd also have a reference to an EmbeddedCacheManager (since you created the cache!) so you could add listeners there.

I don't think a covariant return type will work.  If Cache.getCacheManager() returns EmbeddedCacheManager, then RemoteCache.getCacheManager() cannot return RemoteCacheManager since it isn't a sub-interface of EmbeddedCacheManager.

TBH, RemoteCache.getCacheManager() is a no-op anyway - so I don't see why Cache.getCacheManager() doesn't just return EmbeddedCacheManager.

Galder, did you create a JIRA for this?

Cheers
Manik
--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Manik Surtani
In reply to this post by Eduardo Martins

On 4 Jan 2011, at 16:12, Eduardo Martins wrote:

>>>>>
>>>>> 3) Configuration stuff
>>>>>
>>>>> Generally I think the Configuration and GlobalConfiguration could be
>>>>> simplified a bit, I found myself several times looking at the impl
>>>>> code to understand how to achieve some configurations.
>>>>
>>>> Configurations such as?
>>>
>>> Example below, but this is a generic remark, more like a call to try
>>> to simplify as much as possible, I can say that it took me much more
>>> time to settle on configurations than to code.
>>>
>>>>
>>>>> Better
>>>>> documentation wouldn't hurt too, it's great to have a complete
>>>>> reference, but the configuration samples are not ok, one is basically
>>>>> empty, the other has all possible stuff, very unrealistic, would be
>>>>> better to have reusable examples for each mode, with then
>>>>> recommendations on how to improve these.
>>>>
>>>> Which configuration samples do you refer to? The ones in the testsuite?
>>>
>>> The ones bundled in etc/config-samples.
>>
>> Precisely for 5.0.0.ALPHA1 and 4.2.0.FINAL we removed all.xml and added sample.xml that has more meaningful configurations in there with several of them commented and ready for use.
>>
>
> I think it still needs more examples, and a docs section reviewing
> those. It makes sense to have one example for each typical/mainstream
> configuration used.

Again, these are in sample.xml but commented out.  Are you suggesting that each one of these becomes a separate config file?  E.g., sample-all.xml, sample-async-repl-queue.xml, sample-dist-l1.xml, etc?

>
>>>
>>>>
>>>> I suppose the reusable examples that you look for might be partially available via TestCacheManagerFactory except that these are oriented at testing (i.e. make sure each testsuite thread uses a different mcast address). Maybe we should extract these out and make more easily available.
>>>>
>>>>>
>>>>> Infinispan 5 introduces a new global configuration setter to provide
>>>>> an instance, with same method name as the one to provide the class
>>>>> name. I believe one is enough, and to be more friendly with
>>>>> Microcontainer and similar frameworks I would choose the one to set
>>>>> the instance.
>>>>
>>>> Which configuration setter are you referring to?
>>>
>>> setMBeanServerLookup(...)
>>
>> Right. The one that takes a String is used by JAXB code whereas the one taking the instance is used by programmatic configuration.
>>
>> I think the latter should be renamed to setMBeanServerLookupInstance at least.
>>
>
> Agree.

+1.  Again, do you have a JIRA for this?



--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Manik Surtani
In reply to this post by Eduardo Martins

On 4 Jan 2011, at 16:12, Eduardo Martins wrote:

4) AtomicMap doubt

I read in the Infinispan blog that AtomicMap provides colocation of
all entries, is that idea outdated? If not we may need a way to turn
that off :) For instance would not that mean the Tree API does not
works well with distribution mode? I apologize in advance if I'm
missing something, but if AtomicMap defines colocation, AtomicMap is
good for the node's data map, but not for the node's childs fqns.
Shouldn't each child fqn be freely distributed, being colocated
instead with the related node cache entry and data (atomic)map? Our
impl is kind of an "hybrid" of the Tree API, allows cache entries
references (similar to childs) but no data map, and the storage of
references through AtomicMap in same way as Tree API worries me.
Please clarify.

Each FQN has underneath an AtomicMap, so each you won't find yourself finding k,v pairs belonging to a particular Fqn in different nodes.

We make no guarantees wrt child fqn nodes. So, just cos FQN B is child of FQN A, it does not mean that the atomic map of B will be in same node as atomic map of B.

Before I proceed with my reasoning, please clarify, the colocation
within AtomicMap is real? If I store data there, all data will be
colocated?

Yup, all *data* stored in the AtomicMap will be located in the same node. It's treated as a single entity.


Ok, then lets think on the Tree API, typically/optimally you add, get
and remove a specific node in same cluster node/zone, iterating
through a node's childs is rare and usually without much perf
constraints. With current impl the node's child map entries are
colocated, since each node does that through an AtomicMap with child's
last FQN element, IMHO this is not great for performance:

1. Consider parent node P in node N1
2. In node N2, add a P child, this goes to N1. Adding P Child also
creates the child cache entries, all 3 seem to be colocated through
hashCode(), correct me if I'm wrong. Lets assume all hash ideally to
local node N2.
3. In node N2, get P child, this may go to N1 if we use P, skips it if
use Cache.get(...)
4. In node N2, remove P child, this needs to go to N1

Are you following the issue, if P is a popular parent node (which
happens a lot to root childs), N1 will be hammered by other nodes!

What you said above is not exactly true.  A TreeNode contains 2 AtomicMaps - one for data and one for structure.  The DataMap contains the K/V attribute pairs on the node.  The StructureMap contains information about children.  Firstly, these 2 AtomicMaps aren't necessarily colocated.  This is OK, since you rarely update structure (adding/removing children) and data (attributes on a node) in the same transaction.  Secondly, there is nothing that says parents and children are colocated since they use different keys.  So,

/a/b/c <-- could be on N1
/a/b/c/d <-- could be on N2

so transactions doing stuff on /a/b/c and /a/b/c/d won't be affecting the same node - unless it is structure that is changing.  So as per your example above, in step 2, all 3 don't necessarily go to the same node.  Also if you do something like TreeCache.getNode() with an FQN, you don't walk through parents.


HTH

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Eduardo Martins
Hi Manik, see inline:

On Wed, Jan 5, 2011 at 7:32 PM, Manik Surtani <[hidden email]> wrote:

>
> On 4 Jan 2011, at 16:12, Eduardo Martins wrote:
>
> 4) AtomicMap doubt
>
> I read in the Infinispan blog that AtomicMap provides colocation of
>
> all entries, is that idea outdated? If not we may need a way to turn
>
> that off :) For instance would not that mean the Tree API does not
>
> works well with distribution mode? I apologize in advance if I'm
>
> missing something, but if AtomicMap defines colocation, AtomicMap is
>
> good for the node's data map, but not for the node's childs fqns.
>
> Shouldn't each child fqn be freely distributed, being colocated
>
> instead with the related node cache entry and data (atomic)map? Our
>
> impl is kind of an "hybrid" of the Tree API, allows cache entries
>
> references (similar to childs) but no data map, and the storage of
>
> references through AtomicMap in same way as Tree API worries me.
>
> Please clarify.
>
> Each FQN has underneath an AtomicMap, so each you won't find yourself
> finding k,v pairs belonging to a particular Fqn in different nodes.
>
> We make no guarantees wrt child fqn nodes. So, just cos FQN B is child of
> FQN A, it does not mean that the atomic map of B will be in same node as
> atomic map of B.
>
> Before I proceed with my reasoning, please clarify, the colocation
>
> within AtomicMap is real? If I store data there, all data will be
>
> colocated?
>
> Yup, all *data* stored in the AtomicMap will be located in the same node.
> It's treated as a single entity.
>
>
> Ok, then lets think on the Tree API, typically/optimally you add, get
> and remove a specific node in same cluster node/zone, iterating
> through a node's childs is rare and usually without much perf
> constraints. With current impl the node's child map entries are
> colocated, since each node does that through an AtomicMap with child's
> last FQN element, IMHO this is not great for performance:
>
> 1. Consider parent node P in node N1
> 2. In node N2, add a P child, this goes to N1. Adding P Child also
> creates the child cache entries, all 3 seem to be colocated through
> hashCode(), correct me if I'm wrong. Lets assume all hash ideally to
> local node N2.
>
> 3. In node N2, get P child, this may go to N1 if we use P, skips it if
> use Cache.get(...)
> 4. In node N2, remove P child, this needs to go to N1
>
> Are you following the issue, if P is a popular parent node (which
> happens a lot to root childs), N1 will be hammered by other nodes!
>
> What you said above is not exactly true.  A TreeNode contains 2 AtomicMaps -
> one for data and one for structure.  The DataMap contains the K/V attribute
> pairs on the node.  The StructureMap contains information about children.
>  Firstly, these 2 AtomicMaps aren't necessarily colocated.  This is OK,

I just said both are colocated because both maps are stored in
Infinispan using an entry key object that defers hashCode() to fqn's
hashCode() only, it does not uses the NodeKey type also. I had the
idea that would result in colocation, is that incorrect?

> since you rarely update structure (adding/removing children) and data
> (attributes on a node) in the same transaction.  Secondly, there is nothing
> that says parents and children are colocated since they use different keys.
>  So,
> /a/b/c <-- could be on N1
> /a/b/c/d <-- could be on N2
> so transactions doing stuff on /a/b/c and /a/b/c/d won't be affecting the
> same node - unless it is structure that is changing.  So as per your example
> above, in step 2, all 3 don't necessarily go to the same node.  Also if you
> do something like TreeCache.getNode() with an FQN, you don't walk through
> parents.

That's what I mean in 3. using Cache.get(...) would not need parent.

I think you misunderstood the real issue, the fact that the node
structure is a map where all entries are colocated, as I said this
means that for populars parent nodes which have lots of childs, there
will be a lot of traffic to add/remove childs, this would not happen
if the parent's structure entry related with the child was colocated
with the child entries itself.

-- Eduardo
..............................................
http://emmartins.blogspot.com
http://redhat.com/solutions/telco

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Manik Surtani

On 5 Jan 2011, at 21:20, Eduardo Martins wrote:

>>
>> What you said above is not exactly true.  A TreeNode contains 2 AtomicMaps -
>> one for data and one for structure.  The DataMap contains the K/V attribute
>> pairs on the node.  The StructureMap contains information about children.
>>  Firstly, these 2 AtomicMaps aren't necessarily colocated.  This is OK,
>
> I just said both are colocated because both maps are stored in
> Infinispan using an entry key object that defers hashCode() to fqn's
> hashCode() only, it does not uses the NodeKey type also. I had the
> idea that would result in colocation, is that incorrect?

Ah ok, I didn't realise that.  We can experiment with using different hash codes so the 2 AtomicMaps are distributed independently, but then this still doesn't really solve your problem of all structural information being stored together.

>
>> since you rarely update structure (adding/removing children) and data
>> (attributes on a node) in the same transaction.  Secondly, there is nothing
>> that says parents and children are colocated since they use different keys.
>>  So,
>> /a/b/c <-- could be on N1
>> /a/b/c/d <-- could be on N2
>> so transactions doing stuff on /a/b/c and /a/b/c/d won't be affecting the
>> same node - unless it is structure that is changing.  So as per your example
>> above, in step 2, all 3 don't necessarily go to the same node.  Also if you
>> do something like TreeCache.getNode() with an FQN, you don't walk through
>> parents.
>
> That's what I mean in 3. using Cache.get(...) would not need parent.
>
> I think you misunderstood the real issue, the fact that the node
> structure is a map where all entries are colocated, as I said this
> means that for populars parent nodes which have lots of childs, there
> will be a lot of traffic to add/remove childs, this would not happen
> if the parent's structure entry related with the child was colocated
> with the child entries itself.

Right.  Well, if you have a better proposal to store such structural information, I'd be happy to hear it.  :-)  It would mean that you cannot use AtomicMaps for this purpose though, since anything stored in an AtomicMap will always be colocated (by design).

Cheers
Manik
--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Eduardo Martins
On Thu, Jan 6, 2011 at 12:04 PM, Manik Surtani <[hidden email]> wrote:

>
> On 5 Jan 2011, at 21:20, Eduardo Martins wrote:
>
>>>
>>> What you said above is not exactly true.  A TreeNode contains 2 AtomicMaps -
>>> one for data and one for structure.  The DataMap contains the K/V attribute
>>> pairs on the node.  The StructureMap contains information about children.
>>>  Firstly, these 2 AtomicMaps aren't necessarily colocated.  This is OK,
>>
>> I just said both are colocated because both maps are stored in
>> Infinispan using an entry key object that defers hashCode() to fqn's
>> hashCode() only, it does not uses the NodeKey type also. I had the
>> idea that would result in colocation, is that incorrect?
>
> Ah ok, I didn't realise that.  We can experiment with using different hash codes so the 2 AtomicMaps are distributed independently, but then this still doesn't really solve your problem of all structural information being stored together.
>

I believe all entries related to a specific node should be colocated,
so I guess it's fine as it is now.

>>
>>> since you rarely update structure (adding/removing children) and data
>>> (attributes on a node) in the same transaction.  Secondly, there is nothing
>>> that says parents and children are colocated since they use different keys.
>>>  So,
>>> /a/b/c <-- could be on N1
>>> /a/b/c/d <-- could be on N2
>>> so transactions doing stuff on /a/b/c and /a/b/c/d won't be affecting the
>>> same node - unless it is structure that is changing.  So as per your example
>>> above, in step 2, all 3 don't necessarily go to the same node.  Also if you
>>> do something like TreeCache.getNode() with an FQN, you don't walk through
>>> parents.
>>
>> That's what I mean in 3. using Cache.get(...) would not need parent.
>>
>> I think you misunderstood the real issue, the fact that the node
>> structure is a map where all entries are colocated, as I said this
>> means that for populars parent nodes which have lots of childs, there
>> will be a lot of traffic to add/remove childs, this would not happen
>> if the parent's structure entry related with the child was colocated
>> with the child entries itself.
>
> Right.  Well, if you have a better proposal to store such structural information, I'd be happy to hear it.  :-)  It would mean that you cannot use AtomicMaps for this purpose though, since anything stored in an AtomicMap will always be colocated (by design).
>

Right now, considering the types available, the AtomicMap is the best
option, yet ideally that structure containing the child fqns should be
a Map, without entry put/remove locks (using flags or not), with delta
replication, and without colocation :)

I really don't know much about Infinispan internals besides the Tree
API and its impl so let me ask you, is that possible to achieve?

-- Eduardo
..............................................
http://emmartins.blogspot.com
http://redhat.com/solutions/telco

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Manik Surtani

On 6 Jan 2011, at 12:28, Eduardo Martins wrote:

>
> Right now, considering the types available, the AtomicMap is the best
> option, yet ideally that structure containing the child fqns should be
> a Map, without entry put/remove locks (using flags or not), with delta
> replication, and without colocation :)

Well, any such map would colocate its elements by nature.  Consider:

        cache.put(key, map).

Anything in that map is stored under a single entry in the cache.  Hence colocation.

> I really don't know much about Infinispan internals besides the Tree
> API and its impl so let me ask you, is that possible to achieve?

If you break up that map and store its constituent entries as separate cache entries, then sure.  But then the trouble begins when you try and look up all of these entries again, to create a coherent and complete view.  :-)

--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Eduardo Martins
Just a wild guess, do we have cluster wide locks? Since the usage of
its collections (keyset(), etc) is rare we could opt to lock the entry
cluster wide and merge all entries from each different hashing cluster
part...

-- Eduardo
..............................................
http://emmartins.blogspot.com
http://redhat.com/solutions/telco

On Thu, Jan 6, 2011 at 12:33 PM, Manik Surtani <[hidden email]> wrote:

>
> On 6 Jan 2011, at 12:28, Eduardo Martins wrote:
>
>>
>> Right now, considering the types available, the AtomicMap is the best
>> option, yet ideally that structure containing the child fqns should be
>> a Map, without entry put/remove locks (using flags or not), with delta
>> replication, and without colocation :)
>
> Well, any such map would colocate its elements by nature.  Consider:
>
>        cache.put(key, map).
>
> Anything in that map is stored under a single entry in the cache.  Hence colocation.
>
>> I really don't know much about Infinispan internals besides the Tree
>> API and its impl so let me ask you, is that possible to achieve?
>
> If you break up that map and store its constituent entries as separate cache entries, then sure.  But then the trouble begins when you try and look up all of these entries again, to create a coherent and complete view.  :-)
>
> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Manik Surtani
Not sure I understand.  What would you lock?  :-)  And doesn't that still cause contention?

On 6 Jan 2011, at 13:25, Eduardo Martins wrote:

> Just a wild guess, do we have cluster wide locks? Since the usage of
> its collections (keyset(), etc) is rare we could opt to lock the entry
> cluster wide and merge all entries from each different hashing cluster
> part...
>
> -- Eduardo
> ..............................................
> http://emmartins.blogspot.com
> http://redhat.com/solutions/telco
>
> On Thu, Jan 6, 2011 at 12:33 PM, Manik Surtani <[hidden email]> wrote:
>>
>> On 6 Jan 2011, at 12:28, Eduardo Martins wrote:
>>
>>>
>>> Right now, considering the types available, the AtomicMap is the best
>>> option, yet ideally that structure containing the child fqns should be
>>> a Map, without entry put/remove locks (using flags or not), with delta
>>> replication, and without colocation :)
>>
>> Well, any such map would colocate its elements by nature.  Consider:
>>
>>        cache.put(key, map).
>>
>> Anything in that map is stored under a single entry in the cache.  Hence colocation.
>>
>>> I really don't know much about Infinispan internals besides the Tree
>>> API and its impl so let me ask you, is that possible to achieve?
>>
>> If you break up that map and store its constituent entries as separate cache entries, then sure.  But then the trouble begins when you try and look up all of these entries again, to create a coherent and complete view.  :-)
>>
>> --
>> Manik Surtani
>> [hidden email]
>> twitter.com/maniksurtani
>>
>> Lead, Infinispan
>> http://www.infinispan.org
>>
>>
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Eduardo Martins
Forget it, I was thinking in locking the map's entry in cache cluster
wide when accessing its collections, but that would also mean locks
for put/remove of data in map, so it would be even worse...

-- Eduardo
..............................................
http://emmartins.blogspot.com
http://redhat.com/solutions/telco



On Thu, Jan 6, 2011 at 1:29 PM, Manik Surtani <[hidden email]> wrote:

> Not sure I understand.  What would you lock?  :-)  And doesn't that still cause contention?
>
> On 6 Jan 2011, at 13:25, Eduardo Martins wrote:
>
>> Just a wild guess, do we have cluster wide locks? Since the usage of
>> its collections (keyset(), etc) is rare we could opt to lock the entry
>> cluster wide and merge all entries from each different hashing cluster
>> part...
>>
>> -- Eduardo
>> ..............................................
>> http://emmartins.blogspot.com
>> http://redhat.com/solutions/telco
>>
>> On Thu, Jan 6, 2011 at 12:33 PM, Manik Surtani <[hidden email]> wrote:
>>>
>>> On 6 Jan 2011, at 12:28, Eduardo Martins wrote:
>>>
>>>>
>>>> Right now, considering the types available, the AtomicMap is the best
>>>> option, yet ideally that structure containing the child fqns should be
>>>> a Map, without entry put/remove locks (using flags or not), with delta
>>>> replication, and without colocation :)
>>>
>>> Well, any such map would colocate its elements by nature.  Consider:
>>>
>>>        cache.put(key, map).
>>>
>>> Anything in that map is stored under a single entry in the cache.  Hence colocation.
>>>
>>>> I really don't know much about Infinispan internals besides the Tree
>>>> API and its impl so let me ask you, is that possible to achieve?
>>>
>>> If you break up that map and store its constituent entries as separate cache entries, then sure.  But then the trouble begins when you try and look up all of these entries again, to create a coherent and complete view.  :-)
>>>
>>> --
>>> Manik Surtani
>>> [hidden email]
>>> twitter.com/maniksurtani
>>>
>>> Lead, Infinispan
>>> http://www.infinispan.org
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Galder Zamarreño
In reply to this post by Eduardo Martins

On Jan 4, 2011, at 5:12 PM, Eduardo Martins wrote:

> Hi Galder, more comments inline:
>
> On Tue, Jan 4, 2011 at 10:30 AM, Galder Zamarreño <[hidden email]> wrote:
>> See comments inline:
>>
>> On Jan 3, 2011, at 3:44 PM, Eduardo Martins wrote:
>>
>>> See inline.
>>>
>>> On Mon, Jan 3, 2011 at 10:38 AM, Galder Zamarreño <[hidden email]> wrote:
>>>> See below my comments:
>>>>
>>>> On Dec 30, 2010, at 7:05 AM, Eduardo Martins wrote:
>>>>
>>> I actually don't know, it was not made due to an execution error. I
>>> saw there an Externalizer, I assume it's going to be "externalized",
>>> which means our DefaultConsistentHashExt that replaces it will also
>>> be.
>>>
>>> Out of curiosity, here is the code, it forces colocation of certain data:
>>>
>>> http://mobicents.googlecode.com/svn/branches/servers/cluster/2.x/infinispan/src/main/java/org/mobicents/cluster/infinispan/distribution/DefaultConsistentHashExt.java
>>
>> That code is a bit strange. If you need to make sure that the hash from InfinispanClusterDataKey is derived from its key's hash depending on whether the key is DependentClusterDataKey or something else, why not just adjust  InfinispanClusterDataKey.hashCode() accordingly? i.e.
>>
>> InfinispanClusterDataKey.hashCode() {
>>  if (getKey() instanceof DependentClusterDataKey) {
>>    return ((DependentClusterDataKey) clusterDataKey).dependsOn().hashCode();
>>  } else {
>>    return getKey().hashCode()
>>  }
>> }
>>
>> I don't think you need a different consistent hash implementation.
>>
>
> hashCode() is not used only by the hash impl, it should be as unique
> as possible elsewhere.

So? I still don't get why your hashCode() impl should not represent the hashCode() of what's really relevant for InfinispanClusterDataKey...

>>>
>>> So right now, there is no work being done to plug Infinispan in AS7?
>>
>> I can't say 100% for sure, Paul Ferraro or Scott Marlow will be able to clarify this further, but AFAIK, this is not being done yet.
>>
>
> Would be good to have a roadmap for this, to adjust our work too.

Again, this is something you need to ping the AS guys about: Paul F, Scott M and if needed Jason too.

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Galder Zamarreño
In reply to this post by Manik Surtani

On Jan 5, 2011, at 8:06 PM, Manik Surtani wrote:

> Thanks for the feedback, Eduardo.  I'll address each one separately, although Galder's done an excellent job already.  :)
>
> On 4 Jan 2011, at 16:12, Eduardo Martins wrote:
>
>>>
>>> Oh right, I see what you mean. You're saying that getCacheManager() in Cache returns a CacheContainer rather than an EmbeddedCacheManager. There's a reason for this:
>>>
>>> CacheContainer is the common demoninator for operations that are accessible by remote and local caches. EmbeddedCacheManager extends CacheContainer to add operations that only make sense on a local basis, whereas RemoteCacheManager does the same for remotely accessed Infinispan instances. For the moment, you cannot add listeners to remote cache/cachemanager instances, hence why those methods are only available in EmbeddedCacheManager.
>>>
>>> It might make sense to have an EmbeddedCache interface that ties up this up via covariant returns, i.e.:
>>>
>>> EmbeddedCacheManager getCacheManager();
>>>
>>
>> Agree.
>
> This was changed in 4.1.0 when we introduced the RemoteCacheManager (Hot Rod client).  But yeah I appreciate your concerns re: not being able to add a cache manager listener when all you have is a reference to a cache - however I would have expected that you'd also have a reference to an EmbeddedCacheManager (since you created the cache!) so you could add listeners there.
>
> I don't think a covariant return type will work.  If Cache.getCacheManager() returns EmbeddedCacheManager, then RemoteCache.getCacheManager() cannot return RemoteCacheManager since it isn't a sub-interface of EmbeddedCacheManager.

I'm not saying that Cache.getCacheManager should return EmbeddedCacheManager, see the example below.

>
> TBH, RemoteCache.getCacheManager() is a no-op anyway - so I don't see why Cache.getCacheManager() doesn't just return EmbeddedCacheManager.
>
> Galder, did you create a JIRA for this?

Hmmm, RemoteCache.getCacheManager() is a no-op cos RemoteCache defines:

   RemoteCacheManager getRemoteCacheManager();

AFAIK covariant returns means: "What this means is that a method in a subclass may return an object whose type is a subclass of the type returned by the method with the same signature in the superclass."

So, we could define have this:

Cache defines:
CacheContainer getCacheManager();

RemoteCache defines:
RemoteCacheManager getCacheManager();

If you had created a new subinterface called EmbeddedCache that extends Cache, you could do:

EmbeddedCache defines:
EmbeddedCacheManager getCacheManager();

This is possible cos both EmbeddedCacheManager and RemoteCacheManager are subclasses of CacheContainer.

>
> Cheers
> Manik
> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Feedback from Mobicents Cluster Framework on top of Infinispan 5.0 Alpha1

Galder Zamarreño
In reply to this post by Manik Surtani

On Jan 5, 2011, at 8:22 PM, Manik Surtani wrote:

>>
>>>>
>>>>>
>>>>> I suppose the reusable examples that you look for might be partially available via TestCacheManagerFactory except that these are oriented at testing (i.e. make sure each testsuite thread uses a different mcast address). Maybe we should extract these out and make more easily available.
>>>>>
>>>>>>
>>>>>> Infinispan 5 introduces a new global configuration setter to provide
>>>>>> an instance, with same method name as the one to provide the class
>>>>>> name. I believe one is enough, and to be more friendly with
>>>>>> Microcontainer and similar frameworks I would choose the one to set
>>>>>> the instance.
>>>>>
>>>>> Which configuration setter are you referring to?
>>>>
>>>> setMBeanServerLookup(...)
>>>
>>> Right. The one that takes a String is used by JAXB code whereas the one taking the instance is used by programmatic configuration.
>>>
>>> I think the latter should be renamed to setMBeanServerLookupInstance at least.
>>>
>>
>> Agree.
>
> +1.  Again, do you have a JIRA for this?

https://issues.jboss.org/browse/ISPN-866

>
>
>
> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Loading...