JBCACHE-399
Most of the discussions for JBCACHE-399 have been private. Below is the text from a recent 1-1 conversation.
(07:47:49) smarlow_sssw: Hi Ben, is now a good time to talk about the issues with the original implementation approach suggested by JBCache-399 (support of Map Key as non-string)?
(07:49:41) ben_w_wang: sure.
(07:50:20) ben_w_wang: so you are saying the hashCode is not reliable across vm?
(07:52:57) smarlow_sssw: I understand what he was trying to suggest, however, to generate the hashcode style FQN, we would have to serialize the key object to a byte array and generate our own hashcode from that. That is the only way I think to have a hashcode style FQN that is the same across different VMs. VM implementations use the Java objects physical member address as the hashcode.
(07:53:52) ben_w_wang: Will that be too much if user provide his own?
(07:54:09) smarlow_sssw: sorry, I'm referring to the system.identityHashCode(Object x) method.
(07:54:54) smarlow_sssw: The default HashCode is also not dependable across different VMS as you don't know how it was implemented by the passed in object.
(07:55:41) smarlow_sssw: I like his idea but we would need a way to generate the hashcode ourselves I think.
(07:56:42) smarlow_sssw: sorry, I didn't read your response till now. If the user provides their own hashcode and we trust them, sure.
(07:57:18) smarlow_sssw: so we could document that users have to implement hashcode to be reliable across the cluster.
(07:57:45) ben_w_wang: yes, I think this is reasonable (at least to me).
(07:58:16) ben_w_wang: in generail, I don't expect the key object to be that complicated anyway.
(07:58:31) ben_w_wang: why won;t you throw it out to the forum to see what other people think?
(08:02:08) smarlow_sssw: The downside is that this approach isn't safe for objects that don't provide a reliable hashcode function and we cannot tell the difference between good and bad. We will get reports of failure to locate objects from the map (as we will look at the wrong location).
One other issue is dealing with collisions. We need to support multiple values being under the "value" location. I'm not sure how to do this yet.
(08:02:50) smarlow_sssw: the collision issue is because many different values can have the same hashcode.
(08:04:08) ben_w_wang: yeah, that's the problem, I think.
(08:04:57) ben_w_wang: what about the UID? It is unique.
(08:09:22) smarlow_sssw: We could map UID to the key and maintain the mapping across the cluster. The trick would just be to keep the key ==> UID map up to date. I think that the node that originates the Map update (at Map.Put time) will be responsible for generating the UID. Whichever node removes an entry from the map, will be responsible for removing its UID from the cluster.
(08:11:24) ben_w_wang: oh, so that doesn't quite work either. :-(
(08:12:27) smarlow_sssw: I think it could work but I think it would require support at the core Cache level (below PojoCache).
(08:12:41) smarlow_sssw: In JBC I mean.
(08:13:11) smarlow_sssw: On a different note, is my current implementation that bad?
(08:13:24) smarlow_sssw: other than jdbc cache loader, what else is wrong?
(08:13:48) smarlow_sssw: :)
(08:17:06) ben_w_wang: nothing except the cache loader and we have this problem of triggering a recursive infinite loop, remember? I am surprised that test case work for you. :-)
(08:17:41) smarlow_sssw: I thought the recursive loop only would occur if someone called Key.toString ?
(08:37:18) ben_w_wang: if user defines hashCode and equals in the Key object, I think you will trigger it.
(08:37:40) ben_w_wang: THis is one thing that I need Kabir to fix on the AOP side, if possible, actually.
(08:38:29) smarlow_sssw: that sounds like the test case I added, let me take a look.
(08:42:14) smarlow_sssw: I added "static class SerializableKeyValue implements Serializable" to the CachedMapAopTest.
There are two member variables:
Integer ivalue;
String svalue;
public int hashCode()
{
return ivalue.hashCode() + svalue.hashCode();
}
(08:42:27) smarlow_sssw: public boolean equals(Object o)
{
if(o == this)
{
return true;
}
if(! (o instanceof SerializableKeyValue)) {
System.out.println("equals: not instanceof SerializableKeyValue , it is a " + o.getClass().getName());
return false;
}
SerializableKeyValue val = (SerializableKeyValue)o;
return val.ivalue == this.ivalue && val.svalue == this.svalue;
}
(08:42:57) smarlow_sssw: perhaps I need to add a certain type of data to hit the recursive case?
(08:43:29) ben_w_wang: OK, Serializable won't have the problem. If it is PojoCacheable, then it is a problem.
(08:45:03) smarlow_sssw: If I remove the serializable tag, will be automatically try to treat it as PojoCacheable?
(08:45:29) ben_w_wang: No, it won't. You will need to declare it.
(08:46:06) ben_w_wang: But for now, we can say it has to be Serializable. Then only restriction is cache loader. I am fine with this solution untill we can solve it from AOP.
(08:49:00)
smarlow_sssw: before we leave the map problem though, should I add a test that the key is serializable? I did this at first but later removed it as I didn't know if the cache mode is local or replicated. Should I just always require the key to be serializable?
(08:49:38) smarlow_sssw: It might be helpful if I through an exception on non-serilizable key, so people know about the restriction (if they don't read the doc). :)
(08:49:57) ben_w_wang: yeah, can you create two separate test cases then?
(08:50:17) smarlow_sssw: yes, I'll create a negative test for this.
We can continue further discussions here, I thought the above conversation might be useful to others, which is why i'm including it here.
