[ 
https://issues.apache.org/jira/browse/TEZ-4350?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

László Bodor updated TEZ-4350:
------------------------------
    Description: 
according to AbstractService.serviceInit javadoc
{code}
   * This method will only ever be called once during the lifecycle of
   * a specific service instance.
   *
   * Implementations do not need to be synchronized as the logic
   * in {@link #init(Configuration)} prevents re-entrancy.
{code}

moreover, it generates findbugs alerts for every field that is accessed 
somewhere else in DAGAppMaster in an unsynchronized fashion:

{code}
Code    Warning
IS      Inconsistent synchronization of 
org.apache.tez.dag.app.DAGAppMaster.webUIService; locked 57% of time
Bug type IS2_INCONSISTENT_SYNC (click for details)
In class org.apache.tez.dag.app.DAGAppMaster
Field org.apache.tez.dag.app.DAGAppMaster.webUIService
Synchronized 57% of the time
Unsynchronized access at DAGAppMaster.java:[line 2623]
Unsynchronized access at DAGAppMaster.java:[line 2623]
Synchronized access at DAGAppMaster.java:[line 568]
Synchronized access at DAGAppMaster.java:[line 569]
Synchronized access at DAGAppMaster.java:[line 578]
Synchronized access at DAGAppMaster.java:[line 656]
{code}

I cannot see any value now in having it synchronized (most probably this wasn't 
the case 8 years ago at the time of TEZ-537)

UPDATE: double-checked, AbstractService lifecycle methods are protected with a 
lock since YARN-530

  was:
according to AbstractService.serviceInit javadoc
{code}
   * This method will only ever be called once during the lifecycle of
   * a specific service instance.
   *
   * Implementations do not need to be synchronized as the logic
   * in {@link #init(Configuration)} prevents re-entrancy.
{code}

moreover, it generates findbugs alerts for every field that is accessed 
somewhere else in DAGAppMaster in an unsynchronized fashion:

{code}
Code    Warning
IS      Inconsistent synchronization of 
org.apache.tez.dag.app.DAGAppMaster.webUIService; locked 57% of time
Bug type IS2_INCONSISTENT_SYNC (click for details)
In class org.apache.tez.dag.app.DAGAppMaster
Field org.apache.tez.dag.app.DAGAppMaster.webUIService
Synchronized 57% of the time
Unsynchronized access at DAGAppMaster.java:[line 2623]
Unsynchronized access at DAGAppMaster.java:[line 2623]
Synchronized access at DAGAppMaster.java:[line 568]
Synchronized access at DAGAppMaster.java:[line 569]
Synchronized access at DAGAppMaster.java:[line 578]
Synchronized access at DAGAppMaster.java:[line 656]
{code}

I cannot see any value now in having it synchronized (most probably this wasn't 
the case 8 years ago at the time of TEZ-537)


> Remove synchronized  from DAGAppMaster.serviceInit, serviceStart, serviceStop
> -----------------------------------------------------------------------------
>
>                 Key: TEZ-4350
>                 URL: https://issues.apache.org/jira/browse/TEZ-4350
>             Project: Apache Tez
>          Issue Type: Improvement
>            Reporter: László Bodor
>            Assignee: László Bodor
>            Priority: Major
>          Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> according to AbstractService.serviceInit javadoc
> {code}
>    * This method will only ever be called once during the lifecycle of
>    * a specific service instance.
>    *
>    * Implementations do not need to be synchronized as the logic
>    * in {@link #init(Configuration)} prevents re-entrancy.
> {code}
> moreover, it generates findbugs alerts for every field that is accessed 
> somewhere else in DAGAppMaster in an unsynchronized fashion:
> {code}
> Code  Warning
> IS    Inconsistent synchronization of 
> org.apache.tez.dag.app.DAGAppMaster.webUIService; locked 57% of time
> Bug type IS2_INCONSISTENT_SYNC (click for details)
> In class org.apache.tez.dag.app.DAGAppMaster
> Field org.apache.tez.dag.app.DAGAppMaster.webUIService
> Synchronized 57% of the time
> Unsynchronized access at DAGAppMaster.java:[line 2623]
> Unsynchronized access at DAGAppMaster.java:[line 2623]
> Synchronized access at DAGAppMaster.java:[line 568]
> Synchronized access at DAGAppMaster.java:[line 569]
> Synchronized access at DAGAppMaster.java:[line 578]
> Synchronized access at DAGAppMaster.java:[line 656]
> {code}
> I cannot see any value now in having it synchronized (most probably this 
> wasn't the case 8 years ago at the time of TEZ-537)
> UPDATE: double-checked, AbstractService lifecycle methods are protected with 
> a lock since YARN-530



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to