Mailing-List: contact mapreduce-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: mapreduce-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r833010 - in /hadoop/mapreduce/branches/branch-0.21:
 CHANGES.txt
 src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java
 src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java
Date: Thu, 05 Nov 2009 06:13:50 -0000
To: mapreduce-commits@hadoop.apache.org
From: yhemanth@apache.org
Message-Id: <20091105061350.674BA2388901@eris.apache.org>

Author: yhemanth
Date: Thu Nov  5 06:13:49 2009
New Revision: 833010

URL: http://svn.apache.org/viewvc?rev=833010&view=rev
Log:
Merge -r 833001:833002 from trunk to branch-0.21 to fix MAPREDUCE-962.

Modified:
    hadoop/mapreduce/branches/branch-0.21/CHANGES.txt
    hadoop/mapreduce/branches/branch-0.21/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java
    hadoop/mapreduce/branches/branch-0.21/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java

Modified: hadoop/mapreduce/branches/branch-0.21/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/branches/branch-0.21/CHANGES.txt?rev=833010&r1=833009&r2=833010&view=diff
==============================================================================
--- hadoop/mapreduce/branches/branch-0.21/CHANGES.txt (original)
+++ hadoop/mapreduce/branches/branch-0.21/CHANGES.txt Thu Nov  5 06:13:49 2009
@@ -797,3 +797,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/branches/branch-0.21/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/branches/branch-0.21/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java?rev=833010&r1=833009&r2=833010&view=diff
==============================================================================
--- hadoop/mapreduce/branches/branch-0.21/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java (original)
+++ hadoop/mapreduce/branches/branch-0.21/src/java/org/apache/hadoop/mapreduce/util/ProcfsBasedProcessTree.java Thu Nov  5 06:13:49 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/branches/branch-0.21/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/branches/branch-0.21/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java?rev=833010&r1=833009&r2=833010&view=diff
==============================================================================
--- hadoop/mapreduce/branches/branch-0.21/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java (original)
+++ hadoop/mapreduce/branches/branch-0.21/src/test/mapred/org/apache/hadoop/mapreduce/util/TestProcfsBasedProcessTree.java Thu Nov  5 06:13:49 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