[jira] [Commented] (MAPREDUCE-6449) MR Code should not throw and catch YarnRuntimeException to communicate internal exceptions
[ https://issues.apache.org/jira/browse/MAPREDUCE-6449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14971033#comment-14971033 ] Junping Du commented on MAPREDUCE-6449: --- Is this patch breaking our MR rolling upgrade story that old MR job and new MR job can coexist in a single cluster (during upgrade)? At least, changes on hs part sounds like this. If so, I would be very concern on this. > MR Code should not throw and catch YarnRuntimeException to communicate > internal exceptions > -- > > Key: MAPREDUCE-6449 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-6449 > Project: Hadoop Map/Reduce > Issue Type: Bug >Reporter: Anubhav Dhoot >Assignee: Neelesh Srinivas Salian > Labels: mapreduce > Attachments: MAPREDUCE-6449.001.patch, MAPREDUCE-6449.002.patch, > MAPREDUCE-6499-prelim.patch > > > In discussion of MAPREDUCE-6439 we discussed how throwing and catching > YarnRuntimeException in MR code is incorrect and we should instead use some > MR specific exception. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MAPREDUCE-6449) MR Code should not throw and catch YarnRuntimeException to communicate internal exceptions
[ https://issues.apache.org/jira/browse/MAPREDUCE-6449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14963606#comment-14963606 ] Neelesh Srinivas Salian commented on MAPREDUCE-6449: These passed locally for me except Profiler. Will upload a patch with changes and testing locally. > MR Code should not throw and catch YarnRuntimeException to communicate > internal exceptions > -- > > Key: MAPREDUCE-6449 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-6449 > Project: Hadoop Map/Reduce > Issue Type: Bug >Reporter: Anubhav Dhoot >Assignee: Neelesh Srinivas Salian > Labels: mapreduce > Attachments: MAPREDUCE-6449.001.patch, MAPREDUCE-6449.002.patch, > MAPREDUCE-6499-prelim.patch > > > In discussion of MAPREDUCE-6439 we discussed how throwing and catching > YarnRuntimeException in MR code is incorrect and we should instead use some > MR specific exception. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MAPREDUCE-6449) MR Code should not throw and catch YarnRuntimeException to communicate internal exceptions
[ https://issues.apache.org/jira/browse/MAPREDUCE-6449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14953851#comment-14953851 ] Anubhav Dhoot commented on MAPREDUCE-6449: -- Changes look good overall AMWebServices and MRApps pair seems safe to replace. This is of the variety where you are replacing an existing try catch pair of YarnRuntimeException with MRRuntimeException. TypeConverter#toYarn also looks like a candidate. CachedHistoryStorage, ClientCache, DefaultSpeculator, HistoryFileManager, LocalContainerAllocator seems of the type where this is no existing catch block in the callers and is basically replacing one uncaught RuntimeException with another. So seems fine. YarnRunner change seems unnecessary. Minor comment, you can remove unused imports of YarnRuntimeException as you go along for eg in MRApps. > MR Code should not throw and catch YarnRuntimeException to communicate > internal exceptions > -- > > Key: MAPREDUCE-6449 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-6449 > Project: Hadoop Map/Reduce > Issue Type: Bug >Reporter: Anubhav Dhoot >Assignee: Neelesh Srinivas Salian > Labels: mapreduce > Attachments: MAPREDUCE-6449.001.patch, MAPREDUCE-6499-prelim.patch > > > In discussion of MAPREDUCE-6439 we discussed how throwing and catching > YarnRuntimeException in MR code is incorrect and we should instead use some > MR specific exception. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MAPREDUCE-6449) MR Code should not throw and catch YarnRuntimeException to communicate internal exceptions
[ https://issues.apache.org/jira/browse/MAPREDUCE-6449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14953944#comment-14953944 ] Neelesh Srinivas Salian commented on MAPREDUCE-6449: [~adhoot] thanks for the review. I'll address the comments in the next patch. Not sure if there can be a good test for this. Will test if it keeps the project stable. Thank you. > MR Code should not throw and catch YarnRuntimeException to communicate > internal exceptions > -- > > Key: MAPREDUCE-6449 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-6449 > Project: Hadoop Map/Reduce > Issue Type: Bug >Reporter: Anubhav Dhoot >Assignee: Neelesh Srinivas Salian > Labels: mapreduce > Attachments: MAPREDUCE-6449.001.patch, MAPREDUCE-6499-prelim.patch > > > In discussion of MAPREDUCE-6439 we discussed how throwing and catching > YarnRuntimeException in MR code is incorrect and we should instead use some > MR specific exception. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MAPREDUCE-6449) MR Code should not throw and catch YarnRuntimeException to communicate internal exceptions
[ https://issues.apache.org/jira/browse/MAPREDUCE-6449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14935362#comment-14935362 ] Neelesh Srinivas Salian commented on MAPREDUCE-6449: Fixing the patch on my end. Will add one soon with test(s). > MR Code should not throw and catch YarnRuntimeException to communicate > internal exceptions > -- > > Key: MAPREDUCE-6449 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-6449 > Project: Hadoop Map/Reduce > Issue Type: Bug >Reporter: Anubhav Dhoot >Assignee: Neelesh Srinivas Salian > Labels: mapreduce > Attachments: MAPREDUCE-6449.001.patch > > > In discussion of MAPREDUCE-6439 we discussed how throwing and catching > YarnRuntimeException in MR code is incorrect and we should instead use some > MR specific exception. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MAPREDUCE-6449) MR Code should not throw and catch YarnRuntimeException to communicate internal exceptions
[ https://issues.apache.org/jira/browse/MAPREDUCE-6449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14907284#comment-14907284 ] Hadoop QA commented on MAPREDUCE-6449: -- \\ \\ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | pre-patch | 19m 12s | Pre-patch trunk compilation is healthy. | | {color:green}+1{color} | @author | 0m 0s | The patch does not contain any @author tags. | | {color:green}+1{color} | tests included | 0m 0s | The patch appears to include 14 new or modified test files. | | {color:red}-1{color} | javac | 4m 26s | The patch appears to cause the build to fail. | \\ \\ || Subsystem || Report/Notes || | Patch URL | http://issues.apache.org/jira/secure/attachment/12762165/MAPREDUCE-6449.001.patch | | Optional Tests | javadoc javac unit findbugs checkstyle | | git revision | trunk / d1b9b85 | | Console output | https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6016/console | This message was automatically generated. > MR Code should not throw and catch YarnRuntimeException to communicate > internal exceptions > -- > > Key: MAPREDUCE-6449 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-6449 > Project: Hadoop Map/Reduce > Issue Type: Bug >Reporter: Anubhav Dhoot >Assignee: Neelesh Srinivas Salian > Labels: mapreduce > Attachments: MAPREDUCE-6449.001.patch > > > In discussion of MAPREDUCE-6439 we discussed how throwing and catching > YarnRuntimeException in MR code is incorrect and we should instead use some > MR specific exception. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MAPREDUCE-6449) MR Code should not throw and catch YarnRuntimeException to communicate internal exceptions
[ https://issues.apache.org/jira/browse/MAPREDUCE-6449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14904725#comment-14904725 ] Neelesh Srinivas Salian commented on MAPREDUCE-6449: Started on the version 1 of this patch. Questions to confirm my understanding: 1) As mentioned in MAPREDUCE-6439, I agree that there are 3 files in MR that have the catch (YarnRuntimeException e) {} block that needs to be addressed in this JIRA. These files include ~/mapreduce/v2/app/MRAppMaster.java ~/mapreduce/v2/hs/webapp/HsWebServices.java ~/mapreduce/v2/app/webapp/AMWebServices.java The other occurrences are under YARN which I believe is taken in YARN-4021. 2) The objective in this JIRA is to distinguish the calls for the Exception from remote versus local and wrap these under a unified Exception in MR that also helps in backwards compatiibility. 3) I observed that each of the other files in the mapred modules have specific actions in the catch block. Like in TestRecordFactory: catch (YarnRuntimeException e) { e.printStackTrace(); Assert.fail("Failed to crete record"); } So, the idea in this JIRA is to simply map the name of YarnRuntimeException to a single Wrapper for MR Exception? There are instances where the YarnException is expected to be caught: As in TestLocalContainerAllocator which is the local exception catch block. catch (YarnException e) { // YarnException is expected } Please correct/augment this comment to help confirm my understanding. Thank you. > MR Code should not throw and catch YarnRuntimeException to communicate > internal exceptions > -- > > Key: MAPREDUCE-6449 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-6449 > Project: Hadoop Map/Reduce > Issue Type: Bug >Reporter: Anubhav Dhoot >Assignee: Neelesh Srinivas Salian > > In discussion of MAPREDUCE-6439 we discussed how throwing and catching > YarnRuntimeException in MR code is incorrect and we should instead use some > MR specific exception. -- This message was sent by Atlassian JIRA (v6.3.4#6332)