Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 0.23.0
    • Component/s: mrv2
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      After MAPREDUCE-2880, my classpath was missing key jar files.

      1. notworking.classpath.uniq
        7 kB
        Ravi Prakash
      2. working.classpath.uniq
        11 kB
        Ravi Prakash
      3. MAPREDUCE-3087.patch
        0.9 kB
        Ravi Prakash
      4. MAPREDUCE-3087.patch
        3 kB
        Ravi Prakash
      5. MAPREDUCE-3087.patch
        0.9 kB
        Ravi Prakash
      6. MAPREDUCE-3087.patch
        4 kB
        Ravi Prakash
      7. test-mr-app.jar
        0.5 kB
        Ravi Prakash
      8. MAPREDUCE-3087-3.patch
        0.9 kB
        Vinod Kumar Vavilapalli

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #838 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/838/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #838 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/838/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Build #59 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/59/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.
          svn merge -c r1187658 --ignore-ancestry ../../trunk/

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Build #59 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/59/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. svn merge -c r1187658 --ignore-ancestry ../../trunk/ vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #868 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/868/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #868 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/868/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #47 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/47/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.
          svn merge -c r1187658 --ignore-ancestry ../../trunk/

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #47 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/47/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. svn merge -c r1187658 --ignore-ancestry ../../trunk/ vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Commit #40 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/40/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.
          svn merge -c r1187658 --ignore-ancestry ../../trunk/

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Commit #40 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/40/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. svn merge -c r1187658 --ignore-ancestry ../../trunk/ vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #1151 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1151/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1151 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1151/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #1136 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1136/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1136 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1136/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Commit #41 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/41/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.
          svn merge -c r1187658 --ignore-ancestry ../../trunk/

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Commit #41 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/41/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. svn merge -c r1187658 --ignore-ancestry ../../trunk/ vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-0.23-Commit #40 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/40/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.
          svn merge -c r1187658 --ignore-ancestry ../../trunk/

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-0.23-Commit #40 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/40/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. svn merge -c r1187658 --ignore-ancestry ../../trunk/ vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187659 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #1214 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1214/)
          MAPREDUCE-3087. Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash.

          vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1214 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1214/ ) MAPREDUCE-3087 . Fixed the mapreduce classpath to correctly include the generated-classpath file needed for tests. Contributed by Ravi Prakash. vinodkv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1187658 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java
          Hide
          Vinod Kumar Vavilapalli added a comment -

          I just committed this to trunk and branch-0.23. Thanks Ravi Prakash!

          Show
          Vinod Kumar Vavilapalli added a comment - I just committed this to trunk and branch-0.23. Thanks Ravi Prakash!
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12500303/MAPREDUCE-3087-3.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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 appears to introduce 160 new Findbugs (version 1.3.9) warnings.

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

          +1 core tests. The patch passed unit tests in .

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

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//artifact/trunk/hadoop-mapreduce-project/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-app.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//artifact/trunk/hadoop-mapreduce-project/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-core.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//artifact/trunk/hadoop-mapreduce-project/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-common.html
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//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/12500303/MAPREDUCE-3087-3.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 appears to introduce 160 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//artifact/trunk/hadoop-mapreduce-project/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-app.html Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//artifact/trunk/hadoop-mapreduce-project/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-core.html Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//artifact/trunk/hadoop-mapreduce-project/patchprocess/newPatchFindbugsWarningshadoop-mapreduce-client-common.html Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1112//console This message is automatically generated.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Attaching Ravi's first patch again to run through Jenkins.

          Show
          Vinod Kumar Vavilapalli added a comment - Attaching Ravi's first patch again to run through Jenkins.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Okay, I now get the picture. I am fine with the original patch with no tests. Existing TestMRJobs should already validate this fix.

          Show
          Vinod Kumar Vavilapalli added a comment - Okay, I now get the picture. I am fine with the original patch with no tests. Existing TestMRJobs should already validate this fix.
          Hide
          Ravi Prakash added a comment -

          Oh, and my earlier patch which had changes in the pom would not work, because while testing, the line I mentioned (MRApps.java:181)

          classpathFileStream = thisClassLoader.getResourceAsStream(mrAppGeneratedClasspathFile);

          Would again not find the mrapp-generated-classpath file. That is the reason I had to change the pom.xml in the first place.

          Show
          Ravi Prakash added a comment - Oh, and my earlier patch which had changes in the pom would not work, because while testing, the line I mentioned (MRApps.java:181) classpathFileStream = thisClassLoader.getResourceAsStream(mrAppGeneratedClasspathFile); Would again not find the mrapp-generated-classpath file. That is the reason I had to change the pom.xml in the first place.
          Hide
          Ravi Prakash added a comment -

          Vinod! Thanks for your comment. The contents of the mrapp-generated-classpath in the test-mr-app.jar file I uploaded are :

          $ cat mrapp-generated-classpath
          /somePath1/someJar1.jar:/somePath2/someJar2.jar:/somePath3/someJar3.jar
          

          It does not need to be automatically generated. It is present ONLY because setMRFrameworkClasspath looks for mrapp-generated-classpath on MRApps.java:181

          classpathFileStream = thisClassLoader.getResourceAsStream(mrAppGeneratedClasspathFile);

          Without it, my test would throw an exception because the call above would return null. The specific line I want to exercise (MRApps.java:192) in the unit test comes after this line.

          If you have another way to structure the test, please suggest it.

          OR we can check in the single line change (MRApps.java:192) which we all agree is right without writing this test.

          Show
          Ravi Prakash added a comment - Vinod! Thanks for your comment. The contents of the mrapp-generated-classpath in the test-mr-app.jar file I uploaded are : $ cat mrapp-generated-classpath /somePath1/someJar1.jar:/somePath2/someJar2.jar:/somePath3/someJar3.jar It does not need to be automatically generated. It is present ONLY because setMRFrameworkClasspath looks for mrapp-generated-classpath on MRApps.java:181 classpathFileStream = thisClassLoader.getResourceAsStream(mrAppGeneratedClasspathFile); Without it, my test would throw an exception because the call above would return null. The specific line I want to exercise (MRApps.java:192) in the unit test comes after this line. If you have another way to structure the test, please suggest it. OR we can check in the single line change (MRApps.java:192) which we all agree is right without writing this test.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          I've been conveniently ignoring this all this while.

          mr-generated-classpath needs to be automatically generated. And it(its content) is tied to the app module (hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/pom.xml). We cannot check-in some version of it; if something changes in app module, this file(and/or the test-jar) needs to change and thus is unmaintainable.

          So we should only add the real-app jar as the dependency. We only need it to be present for jobclient module which uses the setFrameWorkClasspath() util, which already is there.

          So in sum, your patch without any pom changes should work for you, I think. Shouldn't it?

          Show
          Vinod Kumar Vavilapalli added a comment - I've been conveniently ignoring this all this while. mr-generated-classpath needs to be automatically generated. And it(its content) is tied to the app module (hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/pom.xml). We cannot check-in some version of it; if something changes in app module, this file(and/or the test-jar) needs to change and thus is unmaintainable. So we should only add the real-app jar as the dependency. We only need it to be present for jobclient module which uses the setFrameWorkClasspath() util, which already is there. So in sum, your patch without any pom changes should work for you, I think. Shouldn't it?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12498954/test-mr-app.jar
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1013//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/12498954/test-mr-app.jar against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1013//console This message is automatically generated.
          Hide
          Ravi Prakash added a comment -

          I do not know how to include the jar file in a patch. After applying the MAPREDUCE-3087.patch, the test-mr-app.jar file needs to go in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/resources/

          Can someone please tell me how else to include a test jar file? Or if there is another way write this unit test?

          Show
          Ravi Prakash added a comment - I do not know how to include the jar file in a patch. After applying the MAPREDUCE-3087 .patch, the test-mr-app.jar file needs to go in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/resources/ Can someone please tell me how else to include a test jar file? Or if there is another way write this unit test?
          Hide
          Ravi Prakash added a comment -

          Arun. It was a blocker for me to do any dev work because all my jobs were failing

          I don't know if we ever want to have something in the CLASSPATH which doesn't make sense to the JVM, and /somepath/mr-apps.jar!/mrapp-generated-classpath doesn't make sense to the JVM (so it will be ignored).

          Please ignore the last patch. It was an attempt to include tests, but it seems pretty hard because:
          1. If I include the real mr-app.jar (using pom.xml), then a circular dependency is created.
          2. If I don't include the mr-app.jar, then I must checkin and include in the CLASSPATH some jar containing a file mrapp-generated-classpath. This seemed too much effort for a simple one line change.

          I'm re-uploading the 1st patch with is a single line change.

          Show
          Ravi Prakash added a comment - Arun. It was a blocker for me to do any dev work because all my jobs were failing I don't know if we ever want to have something in the CLASSPATH which doesn't make sense to the JVM, and /somepath/mr-apps.jar!/mrapp-generated-classpath doesn't make sense to the JVM (so it will be ignored). Please ignore the last patch. It was an attempt to include tests, but it seems pretty hard because: 1. If I include the real mr-app.jar (using pom.xml), then a circular dependency is created. 2. If I don't include the mr-app.jar, then I must checkin and include in the CLASSPATH some jar containing a file mrapp-generated-classpath. This seemed too much effort for a simple one line change. I'm re-uploading the 1st patch with is a single line change.
          Hide
          Arun C Murthy added a comment -

          Ravi, thanks for digging through this.

          Patch looks ok, not sure if there is a better way.

          OTOH, I don't get the changes to pom deps?

          Also, pls fix the javac warning. Thanks.

          Show
          Arun C Murthy added a comment - Ravi, thanks for digging through this. Patch looks ok, not sure if there is a better way. OTOH, I don't get the changes to pom deps? Also, pls fix the javac warning. Thanks.
          Hide
          Ravi Prakash added a comment -

          No No Hadoop QA. Please don't be cross with me. I like you. I really do.

          The trouble you see is, that MRApps:setMRFrameworkClasspath() queries the classloader for mrapp-generated-classpath. If the CLASSPATH doesn't have the mr-app.jar, reader (MRApps.java:182) throws an NPE (because classpathFileStream is null). So the classpath MUST have a valid jar containing mrapp-generated-classpath. When I modified pom.xml to include hadoop-mapreduce-client-app, a circular dependency was introduced.

          So should I really go through the trouble of having a test-jar containing mrapp-generated-classpath or will a nice brave committer approve my one line change? That would be the first patch.

          Show
          Ravi Prakash added a comment - No No Hadoop QA. Please don't be cross with me. I like you. I really do. The trouble you see is, that MRApps:setMRFrameworkClasspath() queries the classloader for mrapp-generated-classpath. If the CLASSPATH doesn't have the mr-app.jar, reader (MRApps.java:182) throws an NPE (because classpathFileStream is null). So the classpath MUST have a valid jar containing mrapp-generated-classpath. When I modified pom.xml to include hadoop-mapreduce-client-app, a circular dependency was introduced. So should I really go through the trouble of having a test-jar containing mrapp-generated-classpath or will a nice brave committer approve my one line change? That would be the first patch.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12497818/MAPREDUCE-3087.patch
          against trunk revision .

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

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

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

          -1 javac. The patch appears to cause tar ant target to fail.

          -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.

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

          +1 core tests. The patch passed unit tests in .

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

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/953//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/953//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/12497818/MAPREDUCE-3087.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The patch appears to cause tar ant target to fail. -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/953//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/953//console This message is automatically generated.
          Hide
          Ravi Prakash added a comment -

          Geez Hadoop QA. Why do you have be to such a stickler for tests?
          All right, all right. Here you go. Happy now?

          Show
          Ravi Prakash added a comment - Geez Hadoop QA. Why do you have be to such a stickler for tests? All right, all right. Here you go. Happy now?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12497799/MAPREDUCE-3087.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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 (version 1.3.9) warnings.

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

          +1 core tests. The patch passed unit tests in .

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

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/949//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/949//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/12497799/MAPREDUCE-3087.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/949//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/949//console This message is automatically generated.
          Hide
          Ravi Prakash added a comment -

          Seems like I finally figured this one out.

          The line at issue is MRApps.java:191

          thisClassLoader.getResource(mrAppGeneratedClasspathFile).getFile() );

          The output of the getFile() method includes the "!mrapp-generated-classpath" . e.g. in a test program

          URL url = new URL("file:/somepath/somewhere/mr-apps.jar!mrapp-generated-classpath");
          System.out.println("Added " + url.getFile());
          System.out.println("Should add " + url.getFile().split("!")[0] );
          

          outputs

          Added /somepath/somewhere/mr-apps.jar!mrapp-generated-classpath
          Should add /somepath/somewhere/mr-apps.jar
          

          So the line 191 was simply adding /somepath/somewhere/mr-app.jar!mrapp-generated-classpath to the classpath. This prevents my JVM to read mr-app.jar as included in the classpath.

          Attaching a small patch to fix this issue. Can someone please review and commit this?

          Show
          Ravi Prakash added a comment - Seems like I finally figured this one out. The line at issue is MRApps.java:191 thisClassLoader.getResource(mrAppGeneratedClasspathFile).getFile() ); The output of the getFile() method includes the "!mrapp-generated-classpath" . e.g. in a test program URL url = new URL("file:/somepath/somewhere/mr-apps.jar!mrapp-generated-classpath"); System.out.println("Added " + url.getFile()); System.out.println("Should add " + url.getFile().split("!")[0] ); outputs Added /somepath/somewhere/mr-apps.jar!mrapp-generated-classpath Should add /somepath/somewhere/mr-apps.jar So the line 191 was simply adding /somepath/somewhere/mr-app.jar!mrapp-generated-classpath to the classpath. This prevents my JVM to read mr-app.jar as included in the classpath. Attaching a small patch to fix this issue. Can someone please review and commit this?
          Hide
          Ravi Prakash added a comment -

          Ok! So here's what I did. I breakpointed at ContainerLaunch.java:197 (which basically launches the containers). While execution was suspended, I manually edited the task.sh to change mr-app.jar!/mrapp-generated-classpath to simply mr-app.jar.

          I had to do this thrice for a simple word-count (once for the AM, once for the map and once for the reduce I guess). This let me job succeed.

          Did MR-2880 simplify the expansion of !/mrapp-generated-classpath, or is this some cool JAVA classpath trick that I don't know about?

          Show
          Ravi Prakash added a comment - Ok! So here's what I did. I breakpointed at ContainerLaunch.java:197 (which basically launches the containers). While execution was suspended, I manually edited the task.sh to change mr-app.jar!/mrapp-generated-classpath to simply mr-app.jar. I had to do this thrice for a simple word-count (once for the AM, once for the map and once for the reduce I guess). This let me job succeed. Did MR-2880 simplify the expansion of !/mrapp-generated-classpath, or is this some cool JAVA classpath trick that I don't know about?
          Hide
          Arun C Murthy added a comment -

          If the assumption going forward is that we don't want to run the yarn servers from source, then we can probably mark this ticket as invalid.

          We want to run this from source, but the fix is to get YARNRunner/MRAppMaster to add the .class files to the CLASSPATH, not via the NodeManager. Not critical for now, but are you interested in taking this up?

          Show
          Arun C Murthy added a comment - If the assumption going forward is that we don't want to run the yarn servers from source, then we can probably mark this ticket as invalid. We want to run this from source, but the fix is to get YARNRunner/MRAppMaster to add the .class files to the CLASSPATH, not via the NodeManager. Not critical for now, but are you interested in taking this up?
          Hide
          Ravi Prakash added a comment -

          The way I start my YARN servers may have something to do with it. In my yarn file (the script in hadoop-yarn/bin) I modify

          CLASSPATH="${HADOOP_CONF_DIR}:${YARN_CONF_DIR}"
          to
          bq. CLASSPATH="${HADOOP_CONF_DIR}:${YARN_CONF_DIR}:${CLASSPATH}"

          so that I can put all the jars on the classpath on the command line and run from source.

          If the assumption going forward is that we don't want to run the yarn servers from source, then we can probably mark this ticket as invalid.

          Show
          Ravi Prakash added a comment - The way I start my YARN servers may have something to do with it. In my yarn file (the script in hadoop-yarn/bin) I modify CLASSPATH="${HADOOP_CONF_DIR}:${YARN_CONF_DIR}" to bq. CLASSPATH="${HADOOP_CONF_DIR}:${YARN_CONF_DIR}:${CLASSPATH}" so that I can put all the jars on the classpath on the command line and run from source. If the assumption going forward is that we don't want to run the yarn servers from source, then we can probably mark this ticket as invalid.
          Hide
          Ravi Prakash added a comment -

          The classpath I grepped | sort | uniq from the two task.sh files I got (one from the working version and the other from the notworking version). Looks like the classpath which worked had some other jars not present in the new classpath.

          I'm guessing as part of the simplification for CLASSPATH maybe we missed something that was being included earlier?

          Show
          Ravi Prakash added a comment - The classpath I grepped | sort | uniq from the two task.sh files I got (one from the working version and the other from the notworking version). Looks like the classpath which worked had some other jars not present in the new classpath. I'm guessing as part of the simplification for CLASSPATH maybe we missed something that was being included earlier?

            People

            • Assignee:
              Ravi Prakash
              Reporter:
              Ravi Prakash
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development