Jason Lowe updated YARN-1342:
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
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