[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- will probably try to fix for 4.0.1 -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow updated HHH-6897: ------------------------------ Summary: serialization of the EntityManager should be possible (was: for serialization of the EntityManager, org.hibernate.service.internal.StandardServiceRegistryImpl and its contents should be serializable) -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow reassigned HHH-6897: --------------------------------- Assignee: Scott Marlow -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- I like #1. I added some notes previously On HHH-6727. I think that a unique property should be passed in, otherwise an UUID can be used. I agree that the persistence unit name will serve as the unique property but to really make it more unique, I will pass in the application scoped pu name (will look like applicationEarName_JarName#PuName). So, we will need a new property for passing that in. I'll start working on this. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- The container will always know the fully scoped pu name, when it calls PersistenceProvider.createContainerEntityManagerFactory, it can pass the unique name in as a property. If its some other container (different EE server) calling in, some other well known name could be passed in (I think). -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- Definitely, the other containers can also pass in the unique property (if they want to cluster also) or they can simply let an internal uuid be generated (which I think should be the default). -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Emmanuel Bernard edited comment on HHH-6897 at 12/15/11 7:48 AM: ----------------------------------------------------------------- I've been thinking about it some more. The problem with UUID is that it won't be shared between different nodes in a cluster. I think we should - support an explicit property as EMF name - use the non qualified persistence unit name if none is provided This scheme has the benefit of working both in cluster and non cluster modes and JBoss AS can make it a bit more safe by refixing the PU name with the ear name. No need for a UUID scheme that only covers the case were no name can be inferred and serializability ought to happen in the same VM. was (Author: emmanuel): I've been thinking about it some more. The problem with UUID is that it won't be shared between different nodes in a cluster. I'd we should - support an explicit property as EMF name - use the non qualified persistence unit name if none is provided This scheme has the benefit of working both in cluster and non cluster modes and JBoss AS can make it a bit more safe by refixing the PU name with the ear name -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Steve Ebersole commented on HHH-6897: ------------------------------------- The reason for the UUID is for zero-conf, within-the-same-VM ser/deser. Its a tad overkill to make users specify a config option in the single-VM case when we dont really need it. Yes for clusters, etc we will need a unique identifier for identifying which EMF. But isnt that there already a notion of a "PU name"? Does that not suffice? -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Steve Ebersole commented on HHH-6897: ------------------------------------- That is exactly what core does for SF. If there is a name for the SF, that is used. If not its UUID is used. The problem in core is that the name is the {{hibernate.session_factory_name}} setting, which is explicitly a JNDI name. Ideally I'd like those notions split so that you could name a SF without having to push it into JNDI. So maybe {{hibernate.session_factory_name}} and a new {{hibernate.session_factory_name.bindable}} which is true/false. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Emmanuel Bernard commented on HHH-6897: --------------------------------------- I'm -1 on UUID as by definition it does *not* solve the cluster problem you are trying to fix. I'm attaching the logs of the discussion we had on UUID vs pu name {quote} emmanuel: Hi Scott I don't follow you reasoning emmanuel: https://hibernate.onjira.com/browse/HHH-6897?focusedCommentId=44600 emmanuel: "Even with containers that know to pass in the unique EMF name, application managed entity managers will not likely specify the unique EMF name. So, I agree that defaulting to UUID covers more cases." smarlow: the part about using uuid? emmanuel: So all app managed PU will *not* work in a cluster *for sure* emmanuel: as a UUID will be generated emmanuel: Actually application managed entity managers work just like container managed entity managers. It's more app managed EMF vs container managed EMF emmanuel: but even that, all EMF defined in a persistence.xml should be deployed by the container emmanuel: so all are container based emmanuel: theiur lifecycle I mena emmanuel: What you are proposing is that in a cluster, if the explicit name is not provided, then we use UUID => clustering won't work in any container besides Jboss AS, let's be realistic about that emmanuel: I agree the PU name is not "unique enough" pontentially but we could definitely raise a warning in this case (ie recording two different instances of EMF under the same name smarlow: is it a warning or a deployment error ? smarlow: as long as closing the emf, doesn't close the wrong em, its just a warning emmanuel: I feel that a deployment error could be too constraining smarlow: a warning is fine, if closing the emf, doesn't close the wrong em smarlow: I think we talked recently about what happens when the emf gets closed. didn't you say that the em isn't referenced from the emf? emmanuel: Well what we can do is the following, if two emf are found with the same name, we register them and when unserializing an EM fron that name, we can raise a deserialization error explaining two EMF are registered with the same name emmanuel: that way we can do the warning ahead of time emmanuel: and the be safe for their data emmanuel: makes ense? emmanuel: right the EMF has not reference to an EM emmanuel: So the EM won't find it's EMF (as it's closed) and wont be able to be deserialized smarlow: that makes sense also (they can change their pu name then) emmanuel: yes emmanuel: with a good log / exception text we ahve a win win situation smarlow: yup, simpler is better and reusing what is already available emmanuel: ie easy 95% of the time on any container and safe 100% of the time on JBoss AS smarlow: steve said that we should be able to do a 4.0.1 release next week, after this is fixed emmanuel: k emmanuel: cool smarlow: emmanuel: when we create two EMFs with the same pu name (different application deployments), would we not be putting the EMF pu name into a global map, with the pu name as the key? smarlow: which would be a problem, as the map would only accept one EMF emmanuel: no if your map accepts a Set<EntityManagerFactory> emmanuel: s/no/not/ smarlow: what would be the map key be? the Set<EMF> is the value? emmanuel: I was thinking Map<String,Set<EMF>> smarlow: String is the pu name, right? emmanuel: Where the key is either the specific property passed in when creating the EMF or if not the PU name smarlow: okay, so we still support a key passed in also emmanuel: Right so that JBoss AS passes a truly unique PU name on steroid and is always 100% safe and working emmanuel: But still works 99% of the time on other containers that don't handle Hibernae specifically smarlow: that makes sense, for app managed entity managers, are used in the cluster, they can either use a unique pu name or specify the property smarlow: s/for/when emmanuel: What do you mean by app managed EM? emmanuel: @Inject EntityManagerFactory factory; public doStuff() { EntityManager em = emf.createEntityManager(); … em.close(); } ? emmanuel: if that's the case then, these are handled the same as container maanged EM emmanuel: the property must be passed at the EMF creation emmanuel: not at the EM creation emmanuel: ie during PersistenceProvider.createContainerEntityManagerFactory(persistenceUnitInfo, propertiesMap); smarlow: xchat on Fedora 16 seg faulted (first time since installing F16) emmanuel: Let me replay emmanuel: What do you mean by app managed EM? emmanuel: @Inject EntityManagerFactory factory; smarlow: I got up to "<emmanuel> ie during PersistenceProvider.createContainerEntityManagerFactory(persistenceUnitInfo, propertiesMap);" emmanuel: public doStuff() {   EntityManager em = emf.createEntityManager();   …   em.close(); } emmanuel: if that's the case then, these are handled the same as container maanged EM emmanuel: the property must be passed at the EMF creation emmanuel: not at the EM creation emmanuel: ie during  PersistenceProvider.createContainerEntityManagerFactory(persistenceUnitInfo, propertiesMap); emmanuel: and that's it smarlow: that makes sense, since even the application managed EMF comes from the  PersistenceProvider.createContainerEntityManagerFactory(persistenceUnitInfo, propertiesMap); emmanuel: yep, exactly {quote} -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- I thought more about the idea of mutating the PU name to always be fully qualified. That would be an unexpected change for everyone that has already written applications for Hibernate. Not all Hibernate JPA applications that deploy on AS7, are container managed either (they can use the PersistenceProviderResolverHolder as some users/projects do). Also, mutating the pu name to be fully qualified, would mean that references to the pu name would have to also be fully qualified or be able to ignore the qualifying element. As far as the error check, it would need to fail the deployment, as that is when we would see that a EMF with the specified name already exists. That reminds me, we need to add that error check not matter what, in case the application/container specifies a duplicate EMF name. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Emmanuel Bernard commented on HHH-6897: --------------------------------------- I'm trying to think it through but it seems to me that a PU is unique within a VM wrt ser/deser. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- As far as the PU name being unique within a VM, its not. The PU is only unique within the application scope. Applications cannot not see the PUs of other applications (AS5/6 did but only as an extension). -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow edited comment on HHH-6897 at 12/19/11 10:31 AM: -------------------------------------------------------------- At application deployment time, a warning will be generated if the EMF registry Map<String,Set<EMF>> already contains an entry for the EMF. The warning can be ignored for applications that don't use passivation or cluster replication of the EM. When the EM is deserialized, if more than one entry is in the set (Map<String,Set<EMF>>), an error is thrown. was (Author: smarlow(a)redhat.com): At application deployment time, a DEBUG will be generated if the EMF registry Map<String,Set<EMF>> already contains an entry for the EMF. The warning can be ignored for applications that don't use passivation or cluster replication of the EM. When the EM is deserialized, if more than one entry is in the set (Map<String,Set<EMF>>), an error is thrown. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- I pushed the changes to https://github.com/scottmarlow/hibernate-core/tree/HHH-6897. I'll push them to Hibernate master in the morning after my test finishes running (not yet running clustering tests but checking other test results). -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Emmanuel Bernard commented on HHH-6897: --------------------------------------- Looks good Scott. I have attached some minor comments to the commit https://github.com/scottmarlow/hibernate-core/commit/305a8ab21f829cb7d21f... -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- change merged to master (https://github.com/hibernate/hibernate-core/commit/e18d0875927ba4520461ab...) -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-6897?page=c... ] Scott Marlow commented on HHH-6897: ----------------------------------- Added nonserializable object to EMF for test in EntityManagerSerializationTest, to ensure that this jira doesn't regress. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira