[ https://issues.apache.org/jira/browse/MAPREDUCE-5648?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13856673#comment-13856673 ]
Karthik Kambatla commented on MAPREDUCE-5648: --------------------------------------------- Patch looks good overall. Minor comments/ nits: # Instead of calling the underlying method directly, should probably call the new version of killTask {code} public boolean killTask(TaskAttemptID arg0, boolean arg1) throws IOException, InterruptedException { - return clientCache.getClient(arg0.getJobID()).killTask(arg0, arg1); + return clientCache.getClient(arg0.getJobID()).killTask(arg0, arg1, null); + } + + @Override + public boolean killTask(TaskAttemptID arg0, boolean arg1, String arg2) throws IOException, + InterruptedException { {code} # Can move throws to the line above {code} + public boolean killTask(final TaskAttemptID taskId, final boolean shouldFail, + final String reason) + throws IOException, InterruptedException { {code} # The following method signature can be fit in two lines {code} /** Throws {@link UnsupportedOperationException} */ public boolean killTask(org.apache.hadoop.mapreduce.TaskAttemptID taskId, boolean shouldFail, String reason) throws IOException { throw new UnsupportedOperationException("Killing tasks in " + "LocalJobRunner is not supported"); } {code} # Unit tests to verify the new code? > Allow user-specified diagnostics for killed tasks > ------------------------------------------------- > > Key: MAPREDUCE-5648 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-5648 > Project: Hadoop Map/Reduce > Issue Type: Improvement > Components: client, mr-am, mrv2 > Affects Versions: 2.2.0 > Reporter: Gera Shegalov > Assignee: Gera Shegalov > Attachments: MAPREDUCE-5648.v01.patch, MAPREDUCE-5648.v02.patch, > Screen Shot 2013-11-23 at 11.12.15 AM.png > > > Our users and tools want to be able to supply additional custom diagnostic > messages to mapreduce ClientProtocol killTask. -- This message was sent by Atlassian JIRA (v6.1.5#6160)