[ https://issues.apache.org/jira/browse/MAPREDUCE-1338?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12801075#action_12801075 ]
Devaraj Das commented on MAPREDUCE-1338: ---------------------------------------- Looks good overall. Some comments: 1) Rename the shuffletoken to jobtoken (TokenStorage.setShuffle, etc.) 2) Rename loadJobToken to loadTokens 3) The javadoc at the beginning of the TokenCache file is misleading 3) Ideally the alias for the secrets in TokenStorage should be a Text object instead of String 4) Is there any reason why TestTokenCache is in mapred package whereas TestTokenStorage is in the security package? 5) I think there should be an API to get all the tokens (TokenCache.getAllTokens()). That will be helpful for HADOOP-6299. 6) Also, i think there should be an API to add tokens (TokenCache.addToken(Token)). That will be helpful for MAPREDUCE-1383. One other thing that I realized while reviewing this patch was that the persistence of the token cache should happen in the submitJob method in the JobTracker, rather than in the Job's initialization, in order to better support job recovery. For example, we could have a case where the user submits a job, and then before the job initialization, the JobTracker gets restarted for some reason. If job recovery is enabled, the job submitted earlier will be considered for initialization at some point of time after the restart. But the job's tokencache wouldn't be there then.. But I am okay to handle this issue in a follow up jira. > need security keys storage solution > ----------------------------------- > > Key: MAPREDUCE-1338 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-1338 > Project: Hadoop Map/Reduce > Issue Type: New Feature > Reporter: Boris Shkolnik > Assignee: Boris Shkolnik > Attachments: HADOOP-6325.patch, MAPREDUCE-1338-2.patch, > MAPREDUCE-1338-4.patch, MAPREDUCE-1338-6.patch, MAPREDUCE-1338-7.patch, > MAPREDUCE-1338.patch > > > set, get, store, load security keys > key alias - byte[] > key value - byte[] > store/load from DataInput/Output stream -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.