[TEZ-4350] Remove synchronized from DAGAppMaster.serviceInit, serviceStart, serviceStop

• Type: Improvement
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 0.9.3, 0.10.2
• Assignee: László Bodor
• Reporter: László Bodor
• Created: 14/Nov/21 12:08
• Updated: 23/Jan/22 14:49
• Resolved: 23/Jan/22 14:49

Description

according to AbstractService.serviceInit javadoc

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

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

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]

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

Issue Links

• is related to: TEZ-537 Add support for session jars and pre-warming of containers to be re-used across a session
• is related to: YARN-530 Define Service model strictly, implement AbstractService for robust subclassing, migrate yarn-common services
• is related to: TEZ-2548 TezClient submitDAG can hang if the AM is in the process of shutting down
• is related to: MAPREDUCE-5298 Move MapReduce services to YARN-117 stricter lifecycle
• links to: GitHub Pull Request #162