You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mapreduce-commits@hadoop.apache.org by yh...@apache.org on 2009/11/05 06:53:31 UTC
svn commit: r833002 - in /hadoop/mapreduce/trunk: CHANGES.txt
src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java
src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java
Author: yhemanth
Date: Thu Nov 5 05:53:31 2009
New Revision: 833002
URL: http://svn.apache.org/viewvc?rev=833002&view=rev
Log:
MAPREDUCE-962. Fix a NullPointerException while killing task process trees. Contributed by Ravi Gummadi.
Modified:
hadoop/mapreduce/trunk/CHANGES.txt
hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java
hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java
Modified: hadoop/mapreduce/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/CHANGES.txt?rev=833002&r1=833001&r2=833002&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/CHANGES.txt (original)
+++ hadoop/mapreduce/trunk/CHANGES.txt Thu Nov 5 05:53:31 2009
@@ -854,3 +854,7 @@
MAPREDUCE-1163. Remove unused, hard-coded paths from libhdfs. (Allen
Wittenauer via cdouglas)
+
+ MAPREDUCE-962. Fix a NullPointerException while killing task process
+ trees. (Ravi Gummadi via yhemanth)
+
Modified: hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java?rev=833002&r1=833001&r2=833002&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java (original)
+++ hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java Thu Nov 5 05:53:31 2009
@@ -228,12 +228,19 @@
/** Verify that the given process id is same as its process group id.
* @param pidStr Process id of the to-be-verified-process
+ * @param procfsDir Procfs root dir
*/
- private static boolean assertPidPgrpidForMatch(String pidStr) {
+ static boolean checkPidPgrpidForMatch(String pidStr, String procfsDir) {
Integer pId = Integer.parseInt(pidStr);
// Get information for this process
ProcessInfo pInfo = new ProcessInfo(pId);
- pInfo = constructProcessInfo(pInfo);
+ pInfo = constructProcessInfo(pInfo, procfsDir);
+ if (pInfo == null) {
+ // process group leader may have finished execution, but we still need to
+ // kill the subProcesses in the process group.
+ return true;
+ }
+
//make sure that pId and its pgrpId match
if (!pInfo.getPgrpId().equals(pId)) {
LOG.warn("Unexpected: Process with PID " + pId +
@@ -258,7 +265,7 @@
boolean inBackground)
throws IOException {
// Make sure that the pid given is a process group leader
- if (!assertPidPgrpidForMatch(pgrpId)) {
+ if (!checkPidPgrpidForMatch(pgrpId, PROCFS)) {
throw new IOException("Process with PID " + pgrpId +
" is not a process group leader.");
}
@@ -391,15 +398,6 @@
}
/**
- *
- * Construct the ProcessInfo using the process' PID and procfs and return the
- * same. Returns null on failing to read from procfs,
- */
- private static ProcessInfo constructProcessInfo(ProcessInfo pinfo) {
- return constructProcessInfo(pinfo, PROCFS);
- }
-
- /**
* Construct the ProcessInfo using the process' PID and procfs rooted at the
* specified directory and return the same. It is provided mainly to assist
* testing purposes.
@@ -422,6 +420,8 @@
in = new BufferedReader(fReader);
} catch (FileNotFoundException f) {
// The process vanished in the interim!
+ LOG.warn("The process " + pinfo.getPid()
+ + " may have finished in the interim.");
return ret;
}
@@ -436,6 +436,11 @@
.parseInt(m.group(4)), Integer.parseInt(m.group(5)), Long
.parseLong(m.group(7)));
}
+ else {
+ LOG.warn("Unexpected: procfs stat file is not in the expected format"
+ + " for process with pid " + pinfo.getPid());
+ ret = null;
+ }
} catch (IOException io) {
LOG.warn("Error reading the stream " + io);
ret = null;
Modified: hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java?rev=833002&r1=833001&r2=833002&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java (original)
+++ hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java Thu Nov 5 05:53:31 2009
@@ -423,6 +423,34 @@
}
/**
+ * Verifies ProcfsBasedProcessTree.checkPidPgrpidForMatch() in case of
+ * 'constructProcessInfo() returning null' by not writing stat file for the
+ * mock process
+ * @throws IOException if there was a problem setting up the
+ * fake procfs directories or files.
+ */
+ public void testDestroyProcessTree() throws IOException {
+ // test process
+ String pid = "100";
+ // create the fake procfs root directory.
+ File procfsRootDir = new File(TEST_ROOT_DIR, "proc");
+
+ try {
+ setupProcfsRootDir(procfsRootDir);
+
+ // crank up the process tree class.
+ ProcfsBasedProcessTree processTree = new ProcfsBasedProcessTree(
+ pid, true, 100L, procfsRootDir.getAbsolutePath());
+
+ // Let us not create stat file for pid 100.
+ assertTrue(ProcfsBasedProcessTree.checkPidPgrpidForMatch(
+ pid, procfsRootDir.getAbsolutePath()));
+ } finally {
+ FileUtil.fullyDelete(procfsRootDir);
+ }
+ }
+
+ /**
* Test the correctness of process-tree dump.
*
* @throws IOException