Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-1686

ClassNotFoundException for custom format classes provided in libjars

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.20.1
    • Fix Version/s: 0.22.0
    • Component/s: contrib/streaming, test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The StreamUtil::goodClassOrNull method assumes user-provided classes have package names and if not, they are part of the Hadoop Streaming package. For example, using custom InputFormat or OutputFormat classes without package names will fail with a ClassNotFound exception which is not indicative given the classes are provided in the libjars option. Admittedly, most Java packages should have a package name so this should rarely come up.

      Possible resolution options:

      1) modify the error message to include the actual classname that was attempted in the goodClassOrNull method
      2) call the Configuration::getClassByName method first and if class not found check for default package name and try the call again

          public static Class goodClassOrNull(Configuration conf, String className, String defaultPackage) {
              Class clazz = null;
              try {
                  clazz = conf.getClassByName(className);
              } catch (ClassNotFoundException cnf) {
              }
              if (clazz == null) {
                  if (className.indexOf('.') == -1 && defaultPackage != null) {
                      className = defaultPackage + "." + className;
                      try {
                          clazz = conf.getClassByName(className);
                      } catch (ClassNotFoundException cnf) {
                      }
                  }
              }
              return clazz;
          }
      
      1. HADOOP-1686.patch
        5 kB
        Paul Burkhardt
      2. HADOOP-1686-1.patch
        5 kB
        Paul Burkhardt
      3. HADOOP-1686-2.patch
        5 kB
        Paul Burkhardt

        Activity

        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #523 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/523/)

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #523 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/523/ )
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Setting the assignee to Paul.

        Show
        Tsz Wo Nicholas Sze added a comment - Setting the assignee to Paul.
        Hide
        Amareshwari Sriramadasu added a comment -

        I just committed this. Thanks Paul !

        Show
        Amareshwari Sriramadasu added a comment - I just committed this. Thanks Paul !
        Hide
        Amareshwari Sriramadasu added a comment -

        +1
        Patch looks good.

        Test failure is because of MAPREDUCE-1834. Will check this in.

        Show
        Amareshwari Sriramadasu added a comment - +1 Patch looks good. Test failure is because of MAPREDUCE-1834 . Will check this in.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12450844/HADOOP-1686-2.patch
        against trunk revision 980316.

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

        +1 tests included. The patch appears to include 7 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 release audit. The applied patch does not increase the total number of release audit warnings.

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/600/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/600/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/600/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/600/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/12450844/HADOOP-1686-2.patch against trunk revision 980316. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 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 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/600/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/600/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/600/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/600/console This message is automatically generated.
        Hide
        Paul Burkhardt added a comment -

        New patch against the trunk. The previous patches were against the tag release-0.20.1 since it was our intent to get it in the 0.20.x baseline. Also removed the extra assert as suggested.

        Show
        Paul Burkhardt added a comment - New patch against the trunk. The previous patches were against the tag release-0.20.1 since it was our intent to get it in the 0.20.x baseline. Also removed the extra assert as suggested.
        Hide
        Paul Burkhardt added a comment -

        New patch against the trunk.

        Show
        Paul Burkhardt added a comment - New patch against the trunk.
        Hide
        Amareshwari Sriramadasu added a comment -

        I ran both the ant test and ant test-patch targets with success because the testjar is created in the compile-core-test target. I need more information to understand your request.

        I still could not succeed running the newly added test. Are you testing your patch against branch 0.20 instead of trunk? because there is no such target as "compile-core-test" in trunk. It is compile-mapred-test and path for testjar is src/test/mapred/testjar in trunk. Please provide a patch for trunk.

        The loadLibJar() and assert mentioned in your third comment is there to abort the unit test if the classpath is not properly loaded.

        Yes. I think the method and the assert associated with it, is unnecessary because StreamUtil.goodClassOrNull is doing the same thing :

              clazz = conf.getClassByName(className);
        
        Show
        Amareshwari Sriramadasu added a comment - I ran both the ant test and ant test-patch targets with success because the testjar is created in the compile-core-test target. I need more information to understand your request. I still could not succeed running the newly added test. Are you testing your patch against branch 0.20 instead of trunk? because there is no such target as "compile-core-test" in trunk. It is compile-mapred-test and path for testjar is src/test/mapred/testjar in trunk. Please provide a patch for trunk. The loadLibJar() and assert mentioned in your third comment is there to abort the unit test if the classpath is not properly loaded. Yes. I think the method and the assert associated with it, is unnecessary because StreamUtil.goodClassOrNull is doing the same thing : clazz = conf.getClassByName(className);
        Hide
        Paul Burkhardt added a comment -

        I attached a new patch to remove the printStackTrace() call in accordance to MAPREDUCE-571. Note that the pre-patched version will fail with a Java stack trace for ClassNotFoundException because it is thrown outside the method.

        I ran both the ant test and ant test-patch targets with success because the testjar is created in the compile-core-test target. I need more information to understand your request.

        The loadLibJar() and assert mentioned in your third comment is there to abort the unit test if the classpath is not properly loaded. Otherwise it may not be clear that the unit test failed because there was an issue in the configuration of the unit test. Let me know if you still want me to remove the assertion.

        Thank you for the comments.

        Show
        Paul Burkhardt added a comment - I attached a new patch to remove the printStackTrace() call in accordance to MAPREDUCE-571 . Note that the pre-patched version will fail with a Java stack trace for ClassNotFoundException because it is thrown outside the method. I ran both the ant test and ant test-patch targets with success because the testjar is created in the compile-core-test target. I need more information to understand your request. The loadLibJar() and assert mentioned in your third comment is there to abort the unit test if the classpath is not properly loaded. Otherwise it may not be clear that the unit test failed because there was an issue in the configuration of the unit test. Let me know if you still want me to remove the assertion. Thank you for the comments.
        Hide
        Amareshwari Sriramadasu added a comment -

        Thanks Paul for the patch. Some comments on the patch:

        • Please remove printStackTrace() calls in catch blocks in StreamUtil. Since StreamUtil.goodClassOrNull is used to find whether the passed mapper/reducer value is class or command, we don't want to print the stacktrace. Also, see MAPREDUCE-571.
        • The testcase does not pass even after the fix because the path given for the jar is never built. For example, see the testjar directory in src/test/mapred/testjar and how it is built.
        • In the testcase, loadLibJar() and assert associated with it, seems unnecessary.
        Show
        Amareshwari Sriramadasu added a comment - Thanks Paul for the patch. Some comments on the patch: Please remove printStackTrace() calls in catch blocks in StreamUtil. Since StreamUtil.goodClassOrNull is used to find whether the passed mapper/reducer value is class or command, we don't want to print the stacktrace. Also, see MAPREDUCE-571 . The testcase does not pass even after the fix because the path given for the jar is never built. For example, see the testjar directory in src/test/mapred/testjar and how it is built. In the testcase, loadLibJar() and assert associated with it, seems unnecessary.
        Hide
        Paul Burkhardt added a comment -

        Patch has been created and submitted.

        Show
        Paul Burkhardt added a comment - Patch has been created and submitted.
        Hide
        Paul Burkhardt added a comment -

        Okay, I'll try and do that.

        Paul

        Show
        Paul Burkhardt added a comment - Okay, I'll try and do that. Paul
        Hide
        Amareshwari Sriramadasu added a comment -

        Paul, Can you create the patch with suggested change and a unit test, and upload here?

        Show
        Amareshwari Sriramadasu added a comment - Paul, Can you create the patch with suggested change and a unit test, and upload here?

          People

          • Assignee:
            Paul Burkhardt
            Reporter:
            Paul Burkhardt
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development