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

Siddharth Ahuja commented on YARN-10075:
----------------------------------------

Just uploaded a patch that does the following:
# Removed "protected" attribute - _historyContext_ from JobHistoryServer. Only 
usage of historyContext in the class was to be passed in as an argument during 
the instantiation of the HistoryClientService and nothing else. Therefore, it 
is now cleaned up and the HistoryClientService is now instantiated by casting 
the jobHistoryService with HistoryContext.
# One test class - _TestJHSSecurity_ was found to be abusing this protected 
attribute during the creation of a jobHistoryServer inside this test class. The 
historyContext attribute was being referenced directly (bad) inside 
createHistoryClientService method during creation of the mock job history 
server. In fact, the only use of implementing this helper method seems to be 
passing in the "custom" jhsDTSecretManager (JHSDelegationTokenSecretManager) 
during the creation of the history client service. However, this is not 
required because jobHistoryServer.init(conf) will result in the same due to the 
serviceInit() call within JobHistoryServer that will call 
createHistoryClientService() which will end up using the custom 
jhsDTSecretManager created just earlier (createJHSSecretManager(...,...) 
happens before createHistoryClientService()).
# Cleaned up an unused commented line -  _final JobHistoryServer 
jobHistoryServer = jhServer;_ from the test class.

> historyContext doesn't need to be a class attribute inside JobHistoryServer
> ---------------------------------------------------------------------------
>
>                 Key: YARN-10075
>                 URL: https://issues.apache.org/jira/browse/YARN-10075
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: yarn
>            Reporter: Siddharth Ahuja
>            Assignee: Siddharth Ahuja
>            Priority: Minor
>         Attachments: YARN-10075.001.patch
>
>
> "historyContext" class attribute at 
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L67
>  is assigned a cast of another class attribute - "jobHistoryService" - 
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L131,
>  however it does not need to be stored separately because it is only ever 
> used once in the clas, and that too as an argument while instantiating the 
> HistoryClientService class at 
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L155.
> Therefore, we could just delete the lines at 
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L67
>  and 
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L131
>  completely and instantiate the HistoryClientService as follows:
> {code}
>   @VisibleForTesting
>   protected HistoryClientService createHistoryClientService() {
>     return new HistoryClientService((HistoryContext)jobHistoryService, 
>         this.jhsDTSecretManager);
>   }
> {code}



--
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