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

Siddharth Seth commented on TEZ-1951:
-------------------------------------

{code}
<Class 
name="org.apache.tez.dag.app.rm.YarnTaskSchedulerService$DelayedContainerManager"/>
{code}
I think was meant to be inside the synchronized block in this case.

{code}+    this.localDirs = localDirs.clone();
+    this.logDirs = logDirs.clone();
{code}
localDirs and logDirs are only exposed to internal code. In runtime-internals 
I've ignored such warnings.

The other changes in DAGAppMaster - are they primarily formatting changes, with 
INTERNAL_ERROR added as a new case ?

{code}
-      this.pullAttempt = null;
       this.writeLock.unlock();
       // KKK Moves it out of the lock - so theoretically thread safe. Likely 
need a final inside
+      this.pullAttempt = null;
{code}
This moves resetting of pullAttempt outside of the lock, which can cause 
visibility issues. This may need to be a new finally block under the upper else 
condition.


{code}
+    return ( this.tsLogParams == null ? null : this.tsLogParams.clone() );
{code}
Never exposed to user code, safe to ignore ?

Rest looks good.


> Fix general findbugs warnings in tez-dag
> ----------------------------------------
>
>                 Key: TEZ-1951
>                 URL: https://issues.apache.org/jira/browse/TEZ-1951
>             Project: Apache Tez
>          Issue Type: Sub-task
>            Reporter: Hitesh Shah
>            Assignee: Hitesh Shah
>         Attachments: TEZ-1951.1.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to