[ https://issues.apache.org/jira/browse/YARN-9699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16946979#comment-16946979 ]
Szilard Nemeth edited comment on YARN-9699 at 10/8/19 3:26 PM: --------------------------------------------------------------- Hi [~pbacsko], [~sunilg] ,[~shuzirra]! *Changes I've made between patch 004 and 005 (my patch):* - Fixed print usage of converter parameters in: ResourceManager#printUsage - FSConfigToCSConfigConverter#capacitySchedulerOutputStream: Renamed field - TestFSQueueConverter: Removed 'throws Exception' from many method signatures where this was possible. *Fixed comments from Gergo:* - {quote}line 127-128: Even in console mode the directory parameter is mandatory, I think we should only check for directory argument if we are not in console output mode. {quote} Actually, when we set up the commons CLI Parser (FSConfigToCSConfigArgumentHandler#parseAndConvert), we need to provide if a parameter is mandatory or not so we can't define it like "be mandatory, but not in case of any other parameter is provided." So the best I could do: In FSConfigToCSConfigConverter#validateParams: If output directory is not defined and console mode is not set (with -p option), we throw an exception. So if the output directory is not defined and console mode is set with -p, we can safely continue the execution. - {quote}Also what happens if both console and directory provided? {quote} Fixed this, added the printing of the output files to an else-clause like: {code:java} if (consoleMode) { System.out.println("======= capacity-scheduler.xml ======="); } else { capacitySchedulerConfig.writeXml(capacitySchedulerOutputStream); } {code} *Comments from Sunil:* {quote}yarn resourcemanager CLI is used here. hence ruling out the factor that RM need nt have to run. I am thinking abt a case on a live system where we need to upgrade. more thoughts are there on this which CLI is better, however we can address this later considering the completeness in this. I ll open a Jira to discuss the CLI in details later. {quote} Let's discuss that in Phase 2. Have you opened the jira for this discussion yet? Anyway, I will file a jira soon for Phase 2 FS -> CS converter. {quote}I would like to see more test results. kindly attach the same with patch. {quote} Can we have this on Phase 2 as well (as we are a bit behind the schedule with Phase 1)? In a separate patch (006), in order to help reviewers: Moved all converter-related classes to separate package. With patch 008, I executed intellij's code inspections and did minor code cleanup. was (Author: snemeth): Hi [~pbacsko], [~sunilg] ,[~shuzirra]! *Changes I've made between patch 004 and 005 (my patch):* - Fixed print usage of converter parameters in: ResourceManager#printUsage - FSConfigToCSConfigConverter#capacitySchedulerOutputStream: Renamed field - TestFSQueueConverter: Removed 'throws Exception' from many method signatures where this was possible. *Fixed comments from Gergo:* - {quote}line 127-128: Even in console mode the directory parameter is mandatory, I think we should only check for directory argument if we are not in console output mode. {quote} Actually, when we set up the ommons CLI Parser (FSConfigToCSConfigArgumentHandler#parseAndConvert), we need to provide if a parameter is mandatory or not so we can't define it like "be mandatory, but not in case of any other parameter is provided." So the best I could do: In FSConfigToCSConfigConverter#validateParams: If output directory is not defined and console mode is not set (with -p option), we throw an exception. So if the output directory is not defined and console mode is set with -p, we can safely continue the execution. - {quote}Also what happens if both console and directory provided? {quote} Fixed this, added the printing of the output files to an else-clause like: {code:java} if (consoleMode) { System.out.println("======= capacity-scheduler.xml ======="); } else { capacitySchedulerConfig.writeXml(capacitySchedulerOutputStream); } {code} *Comments from Sunil:* {quote}yarn resourcemanager CLI is used here. hence ruling out the factor that RM need nt have to run. I am thinking abt a case on a live system where we need to upgrade. more thoughts are there on this which CLI is better, however we can address this later considering the completeness in this. I ll open a Jira to discuss the CLI in details later. {quote} Let's discuss that in Phase 2. Have you opened the jira for this discussion yet? Anyway, I will file a jira soon for Phase 2 FS -> CS converter. {quote}I would like to see more test results. kindly attach the same with patch. {quote} Can we have this on Phase 2 as well (as we are a bit behind the schedule with Phase 1)? In a separate patch (006), in order to help reviewers: Moved all converter-related classes to separate package. With patch 008, I executed intellij's code inspections and did minor code cleanup. > Migration tool that help to generate CS config based on FS config (Phase 1) > --------------------------------------------------------------------------- > > Key: YARN-9699 > URL: https://issues.apache.org/jira/browse/YARN-9699 > Project: Hadoop YARN > Issue Type: Sub-task > Reporter: Wanqiang Ji > Assignee: Peter Bacsko > Priority: Major > Attachments: FS_to_CS_migration_POC.patch, YARN-9699-003.patch, > YARN-9699-004.patch, YARN-9699-005.patch, YARN-9699-006.patch, > YARN-9699-007.patch, YARN-9699-008.patch, YARN-9699.001.patch, > YARN-9699.002.patch > > -- 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