[
https://issues.apache.org/jira/browse/TEZ-1951?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14278215#comment-14278215
]
Siddharth Seth edited comment on TEZ-1951 at 1/15/15 3:59 AM:
--------------------------------------------------------------
{code}
<Class
name="org.apache.tez.dag.app.rm.YarnTaskSchedulerService$DelayedContainerManager"/>
{code}
I think tryAssigningAll 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.
was (Author: sseth):
{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)
