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

Kuhu Shukla resolved TEZ-3725.
------------------------------
       Resolution: Fixed
    Fix Version/s: TEZ-3334

Committed to TEZ-3334 branch. Thanks [~jlowe] for the review comments.

> Cleanup http connections and other unnecessary fields in DAG Deletion tracker 
> classes.
> --------------------------------------------------------------------------------------
>
>                 Key: TEZ-3725
>                 URL: https://issues.apache.org/jira/browse/TEZ-3725
>             Project: Apache Tez
>          Issue Type: Sub-task
>            Reporter: Kuhu Shukla
>            Assignee: Kuhu Shukla
>             Fix For: TEZ-3334
>
>         Attachments: TEZ-3725.001.patch, TEZ-3725.002.patch
>
>
> JIRA to provides fixes for the following feedback comments from [~jlowe].
> {code}
> 1. LocalContainerLauncher
> I'm confused why we go through the trouble of serializing the hardcoded value 
> of zero into the aux service protocol buffer, stuff it into the env, then 
> immediately go fetch it back out and extract the integer from the byte 
> buffer. Isn't this a complicated way to say, shufflePort = 0?
> 2. tezDefaultComponentName only needs to be computed when 
> cleanupDagDataOnComplete is true. Actually may not be needed at all, see 
> related comment for DagDeleteRunnable below.
> 3. DagDeleteRunnable
> tezDefaultComponentName is unused? I think this transitively means pluginName 
> is unused in DeletionTracker which would simplify it's constructor signature.
> 4. DeletionTracker
> Nit: addNodeShufflePorts method name being plural implies more than one port 
> can be added but it's only for adding a single node, port pair.
> 5. AMContainerHelpers
> As Sidd mentioned before, we should avoid the redundant conf key lookup when 
> creating each container launch context.
> 6. DagDeleteRunnable
> Do we need to do any cleanup on the httpConnection?
> 7. DeletionTrackerImpl
> What if the submission to the executor throws RejectedExecutionException 
> because the executor was already shutdown and a late dagComplete was invoked?
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to