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

Iñigo Goiri commented on YARN-9768:
-----------------------------------

Thanks [~maniraj...@gmail.com] for [^YARN-9768.002.patch].
As we are using getTimeDuration(), the variables should be also time durations, 
I usually do:
{code}
public static final long 
DEFAULT_RM_DELEGATION_TOKEN_RENEWER_THREAD_RETRY_INTERVAL = 
TimeUni.SECONDS.toMillis(60);
{code}

Regarding reading these variables, I prefer using the following indentation:
{code}
tokenRenewerThreadRetryInterval = conf.getTimeDuration(
    YarnConfiguration.RM_DELEGATION_TOKEN_RENEWER_THREAD_RETRY_INTERVAL,
    YarnConfiguration.DEFAULT_RM_DELEGATION_TOKEN_RENEWER_THREAD_RETRY_INTERVAL,
    TimeUnit.MILLISECONDS);
{code}

DelegationTokenRenewer#215 should be a single line.
In DelegationTokenRenewer#227, you should do {{catch(TimeOutException toe)}} 
then add an extra {{catch(Exception e)}}.
I also think DelegationTokenRenewer#234 can be a lambda.

Avoid DelegationTokenRenewer#442, it just adds churn in an unrelated patch. 
Same for #691 and #508.

Why are we making DelegationTokenRenewer#551 a debug message? If we change 
that, let's also use logger style with {}.

DelegationTokenRenewer#1107 should be a single line. Same as 1129 and 1047 with 
the end of file.


For TestDelegationTokenRenewer, let's also avoid the changes like #169.
#1567 should be a single line. Same for 1571 and 1573.

I'm not a big fan of this long sleeps (#1591).

You have a print in 1593, which could be done properly adding a message to the 
assertTrue (which could use a extracted version of the conf).

> RM Renew Delegation token thread should timeout and retry
> ---------------------------------------------------------
>
>                 Key: YARN-9768
>                 URL: https://issues.apache.org/jira/browse/YARN-9768
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: CR Hota
>            Priority: Major
>         Attachments: YARN-9768.001.patch, YARN-9768.002.patch
>
>
> Delegation token renewer thread in RM (DelegationTokenRenewer.java) renews 
> HDFS tokens received to check for validity and expiration time.
> This call is made to an underlying HDFS NN or Router Node (which has exact 
> APIs as HDFS NN). If one of the nodes is bad and the renew call is stuck the 
> thread remains stuck indefinitely. The thread should ideally timeout the 
> renewToken and retry from the client's perspective.
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to