[ https://issues.apache.org/jira/browse/YARN-967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13828809#comment-13828809 ]
Mayank Bansal commented on YARN-967: ------------------------------------ Thanks [~zjshen] for review bq. 1. Change yarn.cmd accordingly. Done bq. 2. Not necessary, no log is written in AHSClientImpl. Done bq,3. Where're the following configurations? Defined in other patch? YARN-955 bq. 4. Should AHSClientImpl use YarnClient configurations? I think we should use same to maintian consistency betwwn ahsclient and yarn cli in terms of polling interval. I think keeping lots of confs doesn't make sense. bq. 5. Is the following condition correct? Done bq. 6. One important issue here is that the command change is incompatible. The users' old shell scripts will be break given the change here. It's good to make the command compatible. For example, by default, it's going to the info of the application(s). Or at least, we need to document the new behavior of the command. Vinod Kumar Vavilapalli, how do you say? As discussed its backward compatible. bq. 7. Rename it to appAttemptReportStr? Also the javadoc. Done bq. 8. Fix the above issue for printContainerReport as well. Done bq. 9. Does AHS RPC protocol throw not found exception as well? If not, I think it's good to do that to keep consistent. Maybe do the same for getApplicationAttemptReport and getContainerReport This is on purpose, as we first want to make call to RM and if app is not there then call AHS if not there then send exception to client. For attempt and contianer it only look into AHS and if not found send exception back to client. Thats the older behavior. bq. 10. Check getApplications as well. Make getApplicationAttempts and getContainers behave similarly. This and the one above are the server-side changes. Probably you'd like to coordinate your other patches. bq. 11. For listApplications, if the users want the applications in FINISHED/FAILED/KILLED states, why not going to historyClient as well? For listapplications we decide not to get info from AHS , we shall do it once we will have filters added. We are leaving it for now. bq. 12. AHSProxy is using a bunch of RM configurations instead of AHS ones. By the way, it seems AHSProxy is almost the same as RMProxy. Is it possible to reuse the code instead of duplicating it? Done bq. 13. In YarnCLI, should we make getter for historyClient as well, like client? Done bq. 14. The mock doesn't need to be defined in getXXXX and invoked every time getXXXX is called. Define it once, it will behave the same in the following. As discussed ignoring it bq. 15. It's better to mock multiple attempts/containers to test getXXXXs. Done bq. 16. The modified part of ApplicationCLI needs to be tested as well. Done > [YARN-321] Command Line Interface(CLI) for Reading Application History > Storage Data > ----------------------------------------------------------------------------------- > > Key: YARN-967 > URL: https://issues.apache.org/jira/browse/YARN-967 > Project: Hadoop YARN > Issue Type: Sub-task > Reporter: Devaraj K > Assignee: Mayank Bansal > Attachments: YARN-967-1.patch, YARN-967-2.patch, YARN-967-3.patch > > -- This message was sent by Atlassian JIRA (v6.1#6144)