[
https://issues.apache.org/jira/browse/HADOOP-5488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12700127#action_12700127
]
Vinod K V commented on HADOOP-5488:
-----------------------------------
Gone through the patch. Looks good overall. Some code comments:
- JvmContext : Constructor should just take a pid and child jvm should pass it
after reading it from the environment.
- The 'isProcessGroup' check is not needed in
ProcessTree.sigKillInCurrentThread() as we may want to send SIGKILL to
processes not started as sessions also.
- Can you please remove the unused import in DefaultTaskController that was
perhaps mistakenly left in HADOOP-4490:
import org.apache.hadoop.fs.Path;
- No need for changing the controller.killTaskJVM() to return a boolean. If
jvmmanager.kill() has to be reentrant it can just maintain the variables.
-- Is it needed to make kill to be reentrant now?
- I think DefaultTaskController.killTaskJvm() method should be better
commented, especially the various conditionals.
- Fix the LOG var of the abstract class Task (HADOOP-5351)
- TaskLog: The place where the comment says pid files are not used anymore, we
shoujld also suggest the method that should instead be used directly.
- There should be a comment about the bump up of the version number of
TaskUmbilicalProtocol saying there is a change in signature of getTask()
- TestKillSubProcesses:
-- Most of the methods in this class can be private instead of
package-private unless explicitly needed.
-- I see some hardcoded values 5 in the code like in
"scriptDir.toString().substring(5)". This is perhaps related to parsing out the
filesystem scheme info. We should instead use URI methods to get the actual
path name.
-- KillingMapperWithChildren, FailingMapperWithChildren and
SucceedMapperWithChildren can be futher refactored.
-- Can we use some job in UtilsForTest instead of our own runJob() ?
- Is this possible to test if this test-case fails without the changes in
the patch and succeeds with the changes?
> HADOOP-2721 doesn't clean up descendant processes of a jvm that exits cleanly
> after running a task successfully
> ---------------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-5488
> URL: https://issues.apache.org/jira/browse/HADOOP-5488
> Project: Hadoop Core
> Issue Type: Bug
> Components: mapred
> Reporter: Vinod K V
> Assignee: Ravi Gummadi
> Attachments: HADOOP-5488.patch, HADOOP_5488_5614.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
