[jira] [Updated] (TEZ-3725) Cleanup http connections and other unnecessary fields in DAG Deletion tracker classes.

["Kuhu Shukla (JIRA)" <ji...@apache.org>]

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

Kuhu Shukla updated TEZ-3725:
-----------------------------
    Attachment: TEZ-3725.001.patch

Thank you [~jlowe] for the review comments. This patch attempts to address a major subset of them. Appreciate your feedback on the attached patch.

I added Log at info level when the submission for DagDeleteRunnable fails with rejected exception and when http connection or the input stream close throws an exception. It could be debug level as well but since the .post log file for AM logs is generally pretty small, I have added them at info level.

Looking at java documentation, closing the input stream without reading it does not seem to be an issue. Let me know if this makes sense.

> 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
>         Attachments: TEZ-3725.001.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)