> On Dec. 17, 2015, 1:36 p.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.
@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/ - Sid ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41477/#review110954 ----------------------------------------------------------- On Dec. 17, 2015, midnight, Sid Wagle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41477/ > ----------------------------------------------------------- > > (Updated Dec. 17, 2015, midnight) > > > 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 > >
