ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sid Wagle" <swa...@hortonworks.com>
Subject Re: Review Request 41477: Fix stale cluster entity reference which results in merge issues
Date Thu, 17 Dec 2015 19:55:11 GMT


> 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.
> 
> 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/
> 
> Jonathan Hurley wrote:
>     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: Agree and Agree. I will change patch accordingly. Should we try to address Host_Role_command
and Exectuion_Command - disable caching as well? The reason is these would goto L1 cache despite
being cached explicity. Technially all of the out getter would be served from cache unless
the commands result in contention. I believe @Noncacheable might work here, let me know your
thoughts on this. I think we should to make sure these gets are not actual DB reads.


- 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message