[jira] [Commented] (TEZ-919) Fix shutdown handling for Shuffle (regular and broadcast)

["Hitesh Shah (JIRA)" <ji...@apache.org>]

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

Hitesh Shah commented on TEZ-919:
---------------------------------

typo - // Synchronizing on isAtive

bq.  LOG.info("Closing input");
  - is the thread that calls close appropriately named? Or should the log contain an identifier? Likewise for connection close log message. same for impl/Fetcher and Fetcher. 

bq. cleanupCurrentConnection
   - looks like this function may end up getting called multiple times in a race - should the input and connection objects be set to null after calling close? Or set an atomic flag to denote already cleaned up ? 

{code}
+        LOG.info("Exception while shutting down fetcher : " + e.getMessage());
+        if (LOG.isDebugEnabled()) {
+          LOG.debug(e);
+        }
{code}
   - log messages could be interleaved due to multiple threads. Maybe just use if/else based on log level and print message in non-debug and full stack in debug? i.e Stack trace without "Exception while shutting down fetcher" may not be useful.

bq. // TODO NEWTEZ Limit # fetchers to number of inputs 
   - jira for this? 

bq. cleanupFetchers
  - when flag is set to false, some fetchers will be closed but others will not. Is this fine? Will cleanupIgnoreErrors always be called? If not, maybe close all and throw an exception from any one of the ones that failed? 

- Fix TODOs in InputAlreadyClosedException

- s/pendingHosts.size() == 0/isEmpty()/

More comments on the way. 









> Fix shutdown handling for Shuffle (regular and broadcast)
> ---------------------------------------------------------
>
>                 Key: TEZ-919
>                 URL: https://issues.apache.org/jira/browse/TEZ-919
>             Project: Apache Tez
>          Issue Type: Bug
>            Reporter: Siddharth Seth
>            Assignee: Siddharth Seth
>         Attachments: TEZ-919.1.txt
>
>
> Split from TEZ-711. Shutting down ShuffledInput and ShuffledUnsortedInput needs some work.



--
This message was sent by Atlassian JIRA
(v6.2#6252)