[
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)
