[jira] [Comment Edited] (YARN-9534) TimelineV2ClientImpl.pollTimelineServiceAddress() throws a YarnException when it is interrupted

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

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

eBugs edited comment on YARN-9534 at 5/7/19 1:22 PM:
-----------------------------------------------------

Thanks for looking into this.

I agree that switching to a completely new exception is a bad idea. *Is it better to wrap the InterruptedException?* In this way, the inner stack trace is included, and users can know which method is interrupted by looking at the stack trace. Another benefit of wrapping the cause exception is that if callers want to perform accurate exception handling, they can extract the cause exception instead of parsing the error message to understand want's really going on.

 

I also found a similar code in one of pollTimelineServiceAddress()'s caller. The InterruptedException is wrapped here.:

 
{code:java}
public void dispatchEntities(boolean sync,
    TimelineEntity[] entitiesTobePublished, boolean subappwrite)
    throws YarnException {
  ...
  // created a holder and place it in queue
  EntitiesHolder entitiesHolder =
      new EntitiesHolder(entities, sync, subappwrite); // <-- This eventually calls pollTimelineServiceAddress()
  try {
    timelineEntityQueue.put(entitiesHolder);
  } catch (InterruptedException e) {
    Thread.currentThread().interrupt();
    throw new YarnException(
        "Failed while adding entity to the queue for publishing", e); // <-- Here, YarnException wraps InterruptedException
  }
}
{code}
 


was (Author: ebugs-in-cloud-systems):
Thanks for looking into this.

I agree that switching to a completely new exception is a bad idea. *Is it better to wrap the InterruptedException?* In this way, the inner stack trace is included, and users can know which method is interrupted by looking at the stack trace. Another benefit of wrapping the cause exception is that if callers want to perform accurate exception handling, they can extract the cause exception instead of parsing the error message to understand want's really going on.

 

I also found a similar code in one of pollTimelineServiceAddress()'s caller. The InterruptedException is wrapped here.:

 
{code:java}
public void dispatchEntities(boolean sync,
    TimelineEntity[] entitiesTobePublished, boolean subappwrite)
    throws YarnException {
  ...
  // created a holder and place it in queue
  EntitiesHolder entitiesHolder =
      new EntitiesHolder(entities, sync, subappwrite); // <-- This will eventually call pollTimelineServiceAddress()
  try {
    timelineEntityQueue.put(entitiesHolder);
  } catch (InterruptedException e) {
    Thread.currentThread().interrupt();
    throw new YarnException(
        "Failed while adding entity to the queue for publishing", e); // <-- Here, YarnException wraps InterruptedException
  }
}
{code}
 

> TimelineV2ClientImpl.pollTimelineServiceAddress() throws a YarnException when it is interrupted
> -----------------------------------------------------------------------------------------------
>
>                 Key: YARN-9534
>                 URL: https://issues.apache.org/jira/browse/YARN-9534
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: eBugs
>            Priority: Minor
>
> Dear YARN developers, we are developing a tool to detect exception-related bugs in Java. Our prototype has spotted the following throw statement whose exception class and error message indicate different error conditions. 
>   
> Version: Hadoop-3.1.2 
> File: HADOOP-ROOT/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/client/api/impl/TimelineV2ClientImpl.java
> Line: 354
> {code:java}
> try {
>   Thread.sleep(this.serviceRetryInterval);
> } catch (InterruptedException e) {
>   Thread.currentThread().interrupt();
>   throw new YarnException("Interrupted while trying to connect ATS");
> }{code}
>  
> The exception is triggered when {{pollTimelineServiceAddress()}} is interrupted. However, throwing a {{YarnException}} is too general and makes accurate exception handling more difficult. If throwing the {{InterruptedException}} is not preferred, throwing an {{InterruptedIOException}} or wrapping the {{InterruptedException}} could be more accurate here.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org