Zhijie Shen commented on YARN-2709:

[~gtCarrera], thanks for the patch. Here're some comments.

1. Any reason why TimelineClientRetryOp, TimelineClientConnectionRetry and 
TimelineJerseyRetryFilter are not private?

2. Redundant reference.

3. Not sure why can't this code be put into run() directly? At least it 
shouldn't be public.
  public Token<TimelineDelegationTokenIdentifier>
    getDelegationTokenInternal(final String renewer) throws IOException {

4. It's safer to create connectionRetry before retryFilter, because retryFilter 
may invoke retryFilter, though it won't actually in practice.
      TimelineJerseyRetryFilter retryFilter = new TimelineJerseyRetryFilter();
      client = new Client(new URLConnectionClientHandler(
          new TimelineURLConnectionFactory()), cc);
      token = new DelegationTokenAuthenticatedURL.Token();
      connectionRetry = new TimelineClientConnectionRetry(conf);

5. Unnecessary import in TimelineClientImpl

6. I believe the following mock is not necessary. The reason why you want to 
tack this code is because of HADOOP-11215. Due to this issue, it will throw 
cast exception here. Please leave a comment about the mock code bellow.
    doThrow(new ConnectException("Connection refused")).when(client)

7. It's not meaningful renewer. You can say 
UserGroupInformation.getCurrentUser().getShortUserName() here.
      Token<TimelineDelegationTokenIdentifier> token = client

> Add retry for timeline client getDelegationToken method
> -------------------------------------------------------
>                 Key: YARN-2709
>                 URL: https://issues.apache.org/jira/browse/YARN-2709
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Li Lu
>            Assignee: Li Lu
>         Attachments: YARN-2709-102014-1.patch, YARN-2709-102014.patch
> As mentioned in YARN-2673, we need to add retry mechanism to timeline client 
> for secured clusters. This means if the timeline server is not available, a 
> timeline client needs to retry to get a delegation token. 

This message was sent by Atlassian JIRA

Reply via email to