Hadoop Common
  1. Hadoop Common
  2. HADOOP-4975

CompositeRecordReader: ClassLoader set in JobConf is not passed onto WrappedRecordReaders

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.19.0
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      I am using a custom ClassLoader which I set in my JobConf via setClassLoader(). The ClassLoader is loaded key and value classes which are required to read records from SequenceFiles that were written out in a previous MapReduce job.

      However, I am getting a ClassNotFoundException when using the CompositeInputFormat to create a RecordReader to read these SequenceFiles from HDFS. It occurs when the SequenceFile.Reader tries to create an instance of the Key/Value classes, presumably because the class loader SequenceFile.Reader is using is not the one I set with JobConf.setClassLoader. Below is an example of the stack trace I get:

      Caused by: java.io.IOException: WritableName can't load class
      	at org.apache.hadoop.io.WritableName.getClass(WritableName.java:73)
      	at org.apache.hadoop.io.SequenceFile$Reader.getKeyClass(SequenceFile.java:1596)
      	... 33 more
      Caused by: java.lang.ClassNotFoundException: org.apache.hadoop.MyWritableClass
      	at java.net.URLClassLoader$1.run(URLClassLoader.java:200)
      	at java.security.AccessController.doPrivileged(Native Method)
      	at java.net.URLClassLoader.findClass(URLClassLoader.java:188)
      	at java.lang.ClassLoader.loadClass(ClassLoader.java:307)
      	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:301)
      	at java.lang.ClassLoader.loadClass(ClassLoader.java:252)
      	at java.lang.ClassLoader.loadClassInternal(ClassLoader.java:320)
      	at java.lang.Class.forName0(Native Method)
      	at java.lang.Class.forName(Class.java:247)
      	at org.apache.hadoop.conf.Configuration.getClassByName(Configuration.java:673)
      	at org.apache.hadoop.io.WritableName.getClass(WritableName.java:71)
      	... 34 more
      

      I'll attach a unit test that can demonstrate this more clearly....

      1. HADOOP-4975-2.patch
        7 kB
        Jingkei Ly
      2. HADOOP-4975-1.patch
        0.8 kB
        Jingkei Ly
      3. break-wrapped-rr-test.patch
        6 kB
        Jingkei Ly

        Issue Links

          Activity

          Hide
          Jingkei Ly added a comment -

          Unit test to demonstrate HADOOP-4975.

          Show
          Jingkei Ly added a comment - Unit test to demonstrate HADOOP-4975 .
          Hide
          Jingkei Ly added a comment -

          I believe the test above fails because in Parser.java [line: 313] calls Parser.getConf() to create a copy of the JobConf. Parser.getConf() which in turn uses the JobConf(JobConf) [JobConf.java line:138] constructor to create a copied instance. I think problem is that that particular JobConf constructor doesn't copy the reference to the classLoader to the new JobConf instance.

          Should the JobConf/Configuration constructor copy the classLoader reference, or should it be set in Parser.getConf()?

          Show
          Jingkei Ly added a comment - I believe the test above fails because in Parser.java [line: 313] calls Parser.getConf() to create a copy of the JobConf. Parser.getConf() which in turn uses the JobConf(JobConf) [JobConf.java line:138] constructor to create a copied instance. I think problem is that that particular JobConf constructor doesn't copy the reference to the classLoader to the new JobConf instance. Should the JobConf/Configuration constructor copy the classLoader reference, or should it be set in Parser.getConf()?
          Hide
          Jingkei Ly added a comment -

          I've attached the one line change that fixes this problem for me and allows the test case previously submitted pass - I made the change to the Parser class, but I think the question is still open about whether the change should go into the Configuration(Configuration) constructor.

          Show
          Jingkei Ly added a comment - I've attached the one line change that fixes this problem for me and allows the test case previously submitted pass - I made the change to the Parser class, but I think the question is still open about whether the change should go into the Configuration(Configuration) constructor.
          Hide
          Chris Douglas added a comment -

          The fix looks good.

          Would you mind combining your proposed fix and the unit test into a single patch, per the contribution guidelines (per said guidelines, please replace tabs with two spaces)?

          Show
          Chris Douglas added a comment - The fix looks good. Would you mind combining your proposed fix and the unit test into a single patch, per the contribution guidelines (per said guidelines, please replace tabs with two spaces)?
          Hide
          Jingkei Ly added a comment -

          Combined unit test and fix in a single patch and fixed formatting as requested.

          Show
          Jingkei Ly added a comment - Combined unit test and fix in a single patch and fixed formatting as requested.
          Hide
          Chris Douglas added a comment -
               [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 3 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.
          
          Show
          Chris Douglas added a comment - [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 3 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.
          Hide
          Chris Douglas added a comment -

          I committed this. Thanks, Jingkei

          Show
          Chris Douglas added a comment - I committed this. Thanks, Jingkei
          Hide
          Jingkei Ly added a comment -

          Is it possible to get this patch into the 20.0 release?

          Show
          Jingkei Ly added a comment - Is it possible to get this patch into the 20.0 release?
          Hide
          Amareshwari Sriramadasu added a comment -

          I made the change to the Parser class, but I think the question is still open about whether the change should go into the Configuration(Configuration) constructor.

          Yes. I figured out that the solution is to move the changes to Configuration(Configuration), when I'm trying to change join package to use new api. Raised HADOOP-6103 for the Configuration changes.

          Show
          Amareshwari Sriramadasu added a comment - I made the change to the Parser class, but I think the question is still open about whether the change should go into the Configuration(Configuration) constructor. Yes. I figured out that the solution is to move the changes to Configuration(Configuration), when I'm trying to change join package to use new api. Raised HADOOP-6103 for the Configuration changes.

            People

            • Assignee:
              Jingkei Ly
              Reporter:
              Jingkei Ly
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development