Hi, I'm wondering why org.hibernate.ogm.grid.AssociationKey implements Serializable. When is it actually serialized?

I'm suspecting the answer is "never", as the type of the "metadata" member is not serializable, so I'd expect a NotSerializableException to be raised.

If so, does anything speak against making AssociationKey not implement Serializable?

Thanks, --Gunnar _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

2014/2/3 Sanne Grinovero <sanne(a)hibernate.org&gt; > > If you store anything in Infinispan, it should at the very least be > Serializable. > > A better solution would be to provide custom Externalizers: > https://hibernate.atlassian.net/browse/OGM-37 Ah, I see. I like the Externalizer approach as I found it surprising that AK implements Serializable as there apparently is no need for it from the perspective of the core engine itself. > > > > On 3 February 2014 10:56, Gunnar Morling <gunnar(a)hibernate.org&gt; wrote: > > Hi, > > > > I'm wondering why org.hibernate.ogm.grid.AssociationKey implements > > Serializable. When is it actually serialized? > > When you use Infinispan in clustered mode. > > > > > I'm suspecting the answer is "never", as the type of the "metadata" > > member > > is not serializable, so I'd expect a NotSerializableException to be > > raised. > > Our problem is probably that we don't have integration tests using > Infinispan in modes other than "Local". > I suspect I avoided making them as they make the testsuite slower, but > it seems you just highlighted an important reason to add them :-/ Ok, I've created https://hibernate.atlassian.net/browse/OGM-434 for adding a test for this scenario. I'm still wondering how it supposed to work given that AssociationKey contains a non-serializable non-transient member.

> > > > > If so, does anything speak against making AssociationKey not implement > > Serializable? > > It's needed. Also I think it's an important "warning flag" in terms of > design: never consider hooking references to services in such a key, > and strive to keep it as a very simple value holder. Hum, not sure whether making a type Serializable is the right way to indicate such a matter. But of course the requirement stands as is for Infinispan as you describe, at least unless that Externalizer-based approach gets implemented.

Thanks for the clarification, --Gunnar > > > -- Sanne > > > > > Thanks, > > > > --Gunnar > > _______________________________________________ > > hibernate-dev mailing list > > hibernate-dev(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev

Hi, I'm wondering why org.hibernate.ogm.grid.AssociationKey implements Serializable. When is it actually serialized? I'm suspecting the answer is "never", as the type of the "metadata" member is not serializable, so I'd expect a NotSerializableException to be raised. If so, does anything speak against making AssociationKey not implement Serializable? Thanks, --Gunnar _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

On 3 February 2014 13:33, Emmanuel Bernard <emmanuel(a)hibernate.org&gt; wrote: > Hum, with the fact that this object now points to metadata, I wonder if > we should force EhCache, Map and Infinispan to have their own version of > EntityKey / RowKey and AssociationKey (or an appropriate externalizer). Ok to consider it, but let's keep an eye on performance requirements: allocating twice the objects which are strictly necessary is probably not going to help.

Maybe if these where interfaces, each GridDialect could make his own type ?

Sanne > > Emmanuel > > On Mon 2014-02-03 11:56, Gunnar Morling wrote: >> Hi, >> >> I'm wondering why org.hibernate.ogm.grid.AssociationKey implements >> Serializable. When is it actually serialized? >> >> I'm suspecting the answer is "never", as the type of the "metadata" member >> is not serializable, so I'd expect a NotSerializableException to be raised. >> >> If so, does anything speak against making AssociationKey not implement >> Serializable? >> >> Thanks, >> >> --Gunnar >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev

> On 3 févr. 2014, at 15:21, Sanne Grinovero <sanne(a)hibernate.org&gt; wrote: > >> On 3 February 2014 13:33, Emmanuel Bernard <emmanuel(a)hibernate.org&gt; wrote: >> Hum, with the fact that this object now points to metadata, I wonder if >> we should force EhCache, Map and Infinispan to have their own version of >> EntityKey / RowKey and AssociationKey (or an appropriate externalizer). > > Ok to consider it, but let's keep an eye on performance requirements: > allocating twice the objects which are strictly necessary is probably > not going to help. > Maybe if these where interfaces, each GridDialect could make his own type ? > The thing is only the key/value dialects do make use of the keys objects. And even in that case, it might not make sense to actually reuse these objects. For example if the cache name is the table and only the id columns are part of the key. Is that edge case worth this complexity (interfaces and splitting the metadata)? Maybe with optional methods in the grid dialect? That would still make me sad I guess :)

_______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

>> The thing is only the key/value dialects do make use of the keys objects. >> And even in that case, it might not make sense to actually reuse these >> objects. For example if the cache name is the table and only the id columns >> are part of the key. >> Is that edge case worth this complexity (interfaces and splitting the >> metadata)? Maybe with optional methods in the grid dialect? That would still >> make me sad I guess :) Actually these keys where originally designed to be the keys in key/value stores and the cost of not reusing them is high, so I'm not following why you would like to make them not Serializable; indeed some have some fields which need to be moved out, but there are JIRA's open for that already as it's an important consideration for storage size -> memory consumption: OGM-151 I'm not sure I understood what you are suggesting we could do when the "cache name is the table and only the id columns are part of the key": to use just the array for the key? Sadly we need to override equals of the array, so at least one wrapper is needed (unless we find a way to override comparisons in Infinispan and all others). But a second wrapper is simply "one too much" because conceptually we don't need it.

2014/2/13 Emmanuel Bernard <emmanuel(a)hibernate.org&gt; > > >> The thing is only the key/value dialects do make use of the keys > objects. > > >> And even in that case, it might not make sense to actually reuse these > > >> objects. For example if the cache name is the table and only the id > columns > > >> are part of the key. > > >> Is that edge case worth this complexity (interfaces and splitting the > > >> metadata)? Maybe with optional methods in the grid dialect? That > would still > > >> make me sad I guess :) > > > > Actually these keys where originally designed to be the keys in > > key/value stores and the cost of not reusing them is high, so I'm not > > following why you would like to make them not Serializable; indeed > > some have some fields which need to be moved out, but there are JIRA's > > open for that already as it's an important consideration for storage > > size -> memory consumption: OGM-151 > > > > I'm not sure I understood what you are suggesting we could do when the > > "cache name is the table and only the id columns are part of the key": > > to use just the array for the key? Sadly we need to override equals of > > the array, so at least one wrapper is needed (unless we find a way to > > override comparisons in Infinispan and all others). But a second > > wrapper is simply "one too much" because conceptually we don't need > > it. > > Sanne and I took the conversation online and we could only agree that > the other one was crazy. > But we also agree that making AssociationMetadata Serializable was > cheap. So to be sure, you mean these keys *should* be serializable and stores should be allowed to use them as is as the persistent representation? > Hopefully we won't regret that contract enhancement. > To me it seems a bit like premature optimization. There is one known case which actually benefits from this and we don't really know the penalty for creating the wrapper in this case. In most stores we need to create another representation anyways (e.g. DBObject for MongoDB). But once used as Serializable in the wild, we can't get back out of this and must be very careful or may be limited when evolving these types.