Jason Lowe updated YARN-1342:

    Attachment: YARN-1342v5.patch

Thanks for the review, Devaraj!

bq. I think we can get the state from nmStore inside recover() instead of 
getting as an argument.

I fixed this for NMContainerTokenSecretManager and NMTokenSecretManagerInNM.

bq. Here e.getMessage() may not be required to pass as message since we are 
wrapping the same exception.

I originally used the (message, throwable) form because the resulting exception 
message is subtly different than just passing the throwable. 
org.fusesource.leveldbjni.internal.JniDB converts exceptions into DBException 
using the (message, throwable) form, and I was trying to be consistent.  
However I don't think it really matters that much what the message is, so I 
went ahead and changed all the conversions from DBException to IOException to 
just use the throwable form.

bq. Can we move the CONTAINER_TOKENS_KEY_PREFIX.length() to outside of the 
while loop? 

I'm skeptical of this change assuming any decent JVM environment.  The 
String.length() method is just returning a member, and the JIT eats this kind 
of stuff up all the time.  I went ahead and made the change anyway, but let me 
know if I'm missing the motivations for it.

bq. Can we make the string container_ as a constant? 

Replaced it with ConverterUtils.CONTAINER_PREFIX as it's close enough in this 

bq. What do you think of having the names like RecoveredContainerTokensState, 

Sounds good.  For consistency I also changed the corresponding class and 
methods for NM tokens.

> Recover container tokens upon nodemanager restart
> -------------------------------------------------
>                 Key: YARN-1342
>                 URL: https://issues.apache.org/jira/browse/YARN-1342
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager
>    Affects Versions: 2.3.0
>            Reporter: Jason Lowe
>            Assignee: Jason Lowe
>         Attachments: YARN-1342.patch, YARN-1342v2.patch, 
> YARN-1342v3-and-YARN-1987.patch, YARN-1342v4.patch, YARN-1342v5.patch

This message was sent by Atlassian JIRA

Reply via email to