[ 
https://issues.apache.org/jira/browse/AMBARI-4357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13877660#comment-13877660
 ] 

John Speidel commented on AMBARI-4357:
--------------------------------------

After looking further at other JPA related code, it is clear that the issue of 
incorrect/inconsistent transactional semantics is a bigger issue than this 
particular class.  I see many similar cases where incorrect or suboptimal 
transactional semantics are specified.  In many classes all methods, including 
getters have the '@transactional' annotation.  The annotation for the getters 
doesn't specify 'readOnly' or other non default attributes such as an isolation 
level.  In other cases, only a single getter uses the '@transactional' 
annotation while all other methods have no annotation. 

Incorrect transactional usage can cause many issues ranging from potentially 
serious performance degradation to inconsistent or invalid data. 

Instead of focusing specifically on the initial issues with 
ServiceResourceProvider, an overview of all JPA related code needs to occur to 
ensure correct and consistent usage of transactions.  Ideally, a transaction 
best practices document will be created for new classes to follow.



> ServiceResourceProvider appears to use transactions incorrectly
> ---------------------------------------------------------------
>
>                 Key: AMBARI-4357
>                 URL: https://issues.apache.org/jira/browse/AMBARI-4357
>             Project: Ambari
>          Issue Type: Bug
>          Components: controller
>            Reporter: John Speidel
>
> I am looking at the ServiceResourceProvider class which is responsible for 
> setting/getting information related to the service resource from the Ambari 
> DB and see that JPA is being used.  
> Looking at how transactions are demarcated in this class, I have serious 
> concerns.
> - The getter method 'getResources' has a '@Transactional' annotation even 
> though it doesn't perform any database writes.
> - The create and update methods 'createResources' and 'updateResources', 
> which do update the database, have no transactional annotations.
> I can think of no explanation where this transactional behavior would be 
> correct.  This behavior needs to be understood, reviewed and corrected.  If 
> this is the desired behavior, it needs to be clarified.
> The required concurrency guarantees for the writes and reads need to be 
> explained.  For example, what read consistency is required.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to