> On Dec. 17, 2015, 8:36 a.m., Nate Cole wrote: > > I would say that if we're going to do this, then it should be to remove the > > member variable completely for getters and setters. Keeping them around in > > memory is what gets us into trouble. Any other classes where we follow > > this (anti)pattern? > > Jonathan Hurley wrote: > I agree - we need to remove the backing entity entirely and always get it > from the EM. We have to consider more than just "setters" here; anything that > touches things like OneToMany collections on the cluster entity also would > need to be considered a setter. We should just get rid of it entirely and > rely on the EM. > > Sid Wagle wrote: > @Nate: Yes we had this dettached entity reference caching problem in > ServiceImpl, ServiceComponentImpl and ServiceComponentHostImpl. > > @Jonathan: Agree with addressing the problem where related entites are > touched, in this patch I went after anything that does a *merge* on > clusterEntity, the other methods for example *applyLatestConfigurations* does > a merge on the related Collection entities and then a refresh() on the parent > clusterEntity which updates the reference, I don't think that would lead to a > problem because everything CASCADES down from cluster. Although, I will look > at this class again to make sure all such collection merges are covered. > > I still preserved reads from the dettached entites for things that rarely > or never change like *name* of Cluster / Service / Component. > Referencing changes done previously in this regard: > https://reviews.apache.org/r/39996/
If someone adds a new setMethod or a method that touches collections, they'll also have to remember to refresh the cluster entity. Instead, if we just retrieved it from the EM each time, we wouldn't need to worry about that invalidation, right? Is there a downside to always getting it from the EM? It should be cached in the EM's L1 cache, right? - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41477/#review110954 ----------------------------------------------------------- On Dec. 16, 2015, 7 p.m., Sid Wagle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41477/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2015, 7 p.m.) > > > Review request for Ambari, Jonathan Hurley, Myroslav Papirkovskyy, and Nate > Cole. > > > Bugs: AMBARI-14411 > https://issues.apache.org/jira/browse/AMBARI-14411 > > > Repository: ambari > > > Description > ------- > > *Preliminary patch* > > Symptom: > > {code} > Local Exception Stack: > Exception [EclipseLink-6004] (Eclipse Persistence Services - > 2.5.2.v20140319-9ad6abd): org.eclipse.persistence.exceptions.QueryException > Exception Description: The object > [org.apache.ambari.server.orm.entities.ClusterConfigEntity@3646b3a8], of > class [class org.apache.ambari.server.orm.entities.ClusterConfigEntity], with > identity hashcode (System.identityHashCode()) [364,075,546], > is not from this UnitOfWork object space, but the parent session's. The > object was never registered in this UnitOfWork, > but read from the parent session and related to an object registered in the > UnitOfWork. Ensure that you are correctly > registering your objects. If you are still having problems, you can use the > UnitOfWork.validateObjectSpace() method to > help debug where the error occurred. For more information, see the manual or > FAQ. > at > org.eclipse.persistence.exceptions.QueryException.backupCloneIsOriginalFromParent(QueryException.java:298) > at > org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.getBackupClone(UnitOfWorkImpl.java:1995) > at > org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerExistingObject(UnitOfWorkImpl.java:3976) > at > org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerExistingObject(UnitOfWorkImpl.java:3894) > at > org.eclipse.persistence.mappings.CollectionMapping.buildElementUnitOfWorkClone(CollectionMapping.java:308) > at > org.eclipse.persistence.mappings.CollectionMapping.buildElementClone(CollectionMapping.java:321) > at > org.eclipse.persistence.internal.queries.ContainerPolicy.addNextValueFromIteratorInto(ContainerPolicy.java:217) > {code} > > Likely Cause: > Stale clusterEntity reference points to a detached entity which gets tried to > be merged, the Cascaded relationship throws the error on persist. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java > 46dbe01 > > Diff: https://reviews.apache.org/r/41477/diff/ > > > Testing > ------- > > Unit testing in progress. > > > Thanks, > > Sid Wagle > >
