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

Zhijie Shen commented on YARN-3477:
-----------------------------------

+1 for unwrap the failing exception. Some thoughts and comments about the patch:

1. Shall we avoid throwing the runtime exception? Nowadays token operations can 
throw RTE to the caller, which makes a bit difficult for users to handle.

2. Do you think if it is better to change the log message level to DEBUG during 
the retry?

3. It seems we don't need to remember last exp, but throw it immediately if 
{{if (leftRetries == 0)}}.
{code}
189             } catch (IOException | RuntimeException e) {
190               lastException = e;
191               // break if there's no retries left
192               if (leftRetries == 0) {
193                 break;
{code}

4. You may want to update this test code like one of your other change.
{code}
108         } catch (RuntimeException re) {
109           Assert.assertTrue(re instanceof ClientHandlerException);
110         }
{code}

> TimelineClientImpl swallows exceptions
> --------------------------------------
>
>                 Key: YARN-3477
>                 URL: https://issues.apache.org/jira/browse/YARN-3477
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: timelineserver
>    Affects Versions: 2.6.0, 2.7.0
>            Reporter: Steve Loughran
>            Assignee: Steve Loughran
>         Attachments: YARN-3477-001.patch, YARN-3477-002.patch
>
>
> If timeline client fails more than the retry count, the original exception is 
> not thrown. Instead some runtime exception is raised saying "retries run out"
> # the failing exception should be rethrown, ideally via 
> NetUtils.wrapException to include URL of the failing endpoing
> # Otherwise, the raised RTE should (a) state that URL and (b) set the 
> original fault as the inner cause



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to