Hadoop Common
  1. Hadoop Common
  2. HADOOP-5623

Streaming: process provided status messages are overwritten every 10 seoncds

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.19.0, 0.19.1, 0.20.0
    • Fix Version/s: 0.20.2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Every 10 seconds (if the streaming process is producing output key/values on stdout), PipeMapRed sets the task's status string to "Records R/W=N/N". This replaces any custom task status that the streaming process may have specified using the "reporter:status:" stderr lines.

      1. HADOOP-5623-streaming-status.patch
        7 kB
        Rick Cox
      2. HADOOP-5623-streaming-status.patch
        7 kB
        Rick Cox
      3. hadoop-5623-v1.patch
        7 kB
        Jothi Padmanabhan
      4. 5623.20S.patch
        7 kB
        Ravi Gummadi

        Issue Links

          Activity

          Hide
          Rick Cox added a comment -

          This patch calls reporter.progress() instead of reporter.setStatus("Records R/W...") in the case where the streaming process has provided a status message.

          It also adds a test case for the "reporter:status:" syntax, which covers this problem since the test task writes the status before writing the first line of output, so without this patch, the status is immediately overwritten:

          [junit] Running org.apache.hadoop.streaming.TestStreamingStatus
          [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 19.121 sec

          ant test-patch indicates an eclipse classpath change, which I don't understand:

          [exec] -1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 6 new or modified tests.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] -1 Eclipse classpath. The patch causes the Eclipse classpath to differ from the contents of the lib directories.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          [exec]
          [exec]

          Show
          Rick Cox added a comment - This patch calls reporter.progress() instead of reporter.setStatus("Records R/W...") in the case where the streaming process has provided a status message. It also adds a test case for the "reporter:status:" syntax, which covers this problem since the test task writes the status before writing the first line of output, so without this patch, the status is immediately overwritten: [junit] Running org.apache.hadoop.streaming.TestStreamingStatus [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 19.121 sec ant test-patch indicates an eclipse classpath change, which I don't understand: [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 6 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] -1 Eclipse classpath. The patch causes the Eclipse classpath to differ from the contents of the lib directories. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12404573/HADOOP-5623-streaming-status.patch
          against trunk revision 761632.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 Eclipse classpath. The patch retains Eclipse classpath integrity.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/111/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/111/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/111/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/111/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12404573/HADOOP-5623-streaming-status.patch against trunk revision 761632. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/111/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/111/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/111/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/111/console This message is automatically generated.
          Hide
          Rick Cox added a comment -

          It appears core test failure is due to compilation failing in the C++ StringUtils.cc, and is unrelated to this patch, which only changes files in src/contrib/streaming.

          Show
          Rick Cox added a comment - It appears core test failure is due to compilation failing in the C++ StringUtils.cc, and is unrelated to this patch, which only changes files in src/contrib/streaming.
          Hide
          Jothi Padmanabhan added a comment -

          Changes look fine.
          However, the test case fails. That is because HADOOP-5572 introduced progress phases for the map task too, so the status messages get appended by " > whichphase". In TestStreamingStatus, reports[0].getState is returning "starting echo > sort" which is different from the expected string.

          Show
          Jothi Padmanabhan added a comment - Changes look fine. However, the test case fails. That is because HADOOP-5572 introduced progress phases for the map task too, so the status messages get appended by " > whichphase". In TestStreamingStatus, reports [0] .getState is returning "starting echo > sort" which is different from the expected string.
          Hide
          Rick Cox added a comment -

          Thanks Jothi, I'm attaching a new version of the patch to fix the test case.

          Show
          Rick Cox added a comment - Thanks Jothi, I'm attaching a new version of the patch to fix the test case.
          Hide
          Jothi Padmanabhan added a comment -

          The changes look fine. However, one minor nit. Since StderrApp is public and StderrApp.go is public static, we probably could preserve the existing API and make it call the new one with status = false.
          In the interest of time, I will just upload a patch with those changes on your behalf.

          Show
          Jothi Padmanabhan added a comment - The changes look fine. However, one minor nit. Since StderrApp is public and StderrApp.go is public static, we probably could preserve the existing API and make it call the new one with status = false. In the interest of time, I will just upload a patch with those changes on your behalf.
          Hide
          Jothi Padmanabhan added a comment -

          Rick's patch with a very minor modification to preserve the go method with the existing signature

          Show
          Jothi Padmanabhan added a comment - Rick's patch with a very minor modification to preserve the go method with the existing signature
          Hide
          Jothi Padmanabhan added a comment -

          I ran all the streaming tests and they passed.
          Ant test results:
          [exec] +1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 6 new or modified tests.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

          Show
          Jothi Padmanabhan added a comment - I ran all the streaming tests and they passed. Ant test results: [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 6 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          Hide
          Rick Cox added a comment -

          Thanks, the new patch looks good to me.

          (So that I can do the right thing on future changes, are test case public methods considered part of the public API, or is it just that someone might be using that method and it's easy to provide compatibility here?)

          Show
          Rick Cox added a comment - Thanks, the new patch looks good to me. (So that I can do the right thing on future changes, are test case public methods considered part of the public API, or is it just that someone might be using that method and it's easy to provide compatibility here?)
          Hide
          Devaraj Das added a comment -

          I just committed this. Thanks, Rick and Jothi!

          Show
          Devaraj Das added a comment - I just committed this. Thanks, Rick and Jothi!
          Hide
          Tom White added a comment -

          (So that I can do the right thing on future changes, are test case public methods considered part of the public API, or is it just that someone might be using that method and it's easy to provide compatibility here?)

          The latter: test cases are not a part of the public API. The public API is defined by the published javadoc.

          Show
          Tom White added a comment - (So that I can do the right thing on future changes, are test case public methods considered part of the public API, or is it just that someone might be using that method and it's easy to provide compatibility here?) The latter: test cases are not a part of the public API. The public API is defined by the published javadoc.
          Hide
          Todd Lipcon added a comment -

          Reopening - this patch needs to be committed to branch-20 as well for 0.20.2. See MAPREDUCE-576.

          I applied this patch to branch 20, and then ran the test case provided in that other JIRA like this:

          ./bin/hadoop jar build/contrib/streaming/hadoop-0.20.2-dev-streaming.jar -jt local -Dfs.default.name=file:/// -mapper cat -reducer /tmp/test.py -input file:///etc/motd -output file:///tmp/foo
          

          Before the patch, it failed as noted in that JIRA. After the patch, passed.

          Show
          Todd Lipcon added a comment - Reopening - this patch needs to be committed to branch-20 as well for 0.20.2. See MAPREDUCE-576 . I applied this patch to branch 20, and then ran the test case provided in that other JIRA like this: ./bin/hadoop jar build/contrib/streaming/hadoop-0.20.2-dev-streaming.jar -jt local -Dfs.default.name=file:/// -mapper cat -reducer /tmp/test.py -input file:///etc/motd -output file:///tmp/foo Before the patch, it failed as noted in that JIRA. After the patch, passed.
          Hide
          Tom White added a comment -

          I've just committed this to the 0.20 branch.

          Show
          Tom White added a comment - I've just committed this to the 0.20 branch.
          Hide
          Ravi Gummadi added a comment -

          Patch for Y20S branch. Not for commit here.

          Show
          Ravi Gummadi added a comment - Patch for Y20S branch. Not for commit here.

            People

            • Assignee:
              Rick Cox
              Reporter:
              Rick Cox
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development