Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-4577

Enable aux services to have their own custom classpath/jar file

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.9.0, 3.0.0-alpha1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Right now, users have to add their jars to the NM classpath directly, thus put them on the system classloader. But if multiple versions of the plugin are present on the classpath, there is no control over which version actually gets loaded. Or if there are any conflicts between the dependencies introduced by the auxiliary service and the NM itself, they can break the NM, the auxiliary service, or both.

      The solution could be: to instantiate aux services using a classloader that is different from the system classloader.

      1. YARN-4577.1.patch
        7 kB
        Xuan Gong
      2. YARN-4577.2.patch
        9 kB
        Xuan Gong
      3. YARN-4577.20160119.1.patch
        10 kB
        Xuan Gong
      4. YARN-4577.20160204.patch
        13 kB
        Xuan Gong
      5. YARN-4577.20160428.patch
        13 kB
        Xuan Gong
      6. YARN-4577.20160509.patch
        20 kB
        Xuan Gong
      7. YARN-4577.20160510.patch
        15 kB
        Xuan Gong
      8. YARN-4577.20160511.1.patch
        23 kB
        Xuan Gong
      9. YARN-4577.20160511.patch
        23 kB
        Xuan Gong
      10. YARN-4577.3.patch
        10 kB
        Xuan Gong
      11. YARN-4577.3.rebase.patch
        11 kB
        Xuan Gong
      12. YARN-4577.4.patch
        12 kB
        Xuan Gong
      13. YARN-4577.5.patch
        11 kB
        Xuan Gong
      14. YARN-4577.poc.patch
        11 kB
        Xuan Gong

        Issue Links

          Activity

          Hide
          xgong Xuan Gong added a comment -

          Add a patch to have the ability to load the aux-service class from local path/hdfs.

          Show
          xgong Xuan Gong added a comment - Add a patch to have the ability to load the aux-service class from local path/hdfs.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 1s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 8m 0s trunk passed
          +1 compile 1m 51s trunk passed with JDK v1.8.0_66
          +1 compile 2m 10s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 0m 58s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 findbugs 2m 18s trunk passed
          +1 javadoc 0m 58s trunk passed with JDK v1.8.0_66
          +1 javadoc 3m 7s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 46s the patch passed
          +1 compile 2m 19s the patch passed with JDK v1.8.0_66
          +1 javac 2m 19s the patch passed
          +1 compile 2m 5s the patch passed with JDK v1.7.0_91
          +1 javac 2m 5s the patch passed
          -1 checkstyle 0m 31s Patch generated 4 new checkstyle issues in hadoop-yarn-project/hadoop-yarn (total was 261, now 264).
          +1 mvnsite 0m 55s the patch passed
          +1 mvneclipse 0m 23s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          -1 findbugs 1m 7s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager introduced 1 new FindBugs issues.
          +1 javadoc 0m 55s the patch passed with JDK v1.8.0_66
          +1 javadoc 3m 7s the patch passed with JDK v1.7.0_91
          +1 unit 0m 20s hadoop-yarn-api in the patch passed with JDK v1.8.0_66.
          +1 unit 8m 38s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.7.0_91.
          +1 unit 9m 10s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 18s Patch does not generate ASF License warnings.
          53m 54s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
            org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices.serviceInit(Configuration) creates a java.net.URLClassLoader classloader, which should be performed within a doPrivileged block At AuxServices.java:which should be performed within a doPrivileged block At AuxServices.java:[line 133]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781604/YARN-4577.1.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f64ed5ca774f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 95f3201
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10228/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/10228/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.html
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10228/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Max memory used 75MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/10228/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 1s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 0s trunk passed +1 compile 1m 51s trunk passed with JDK v1.8.0_66 +1 compile 2m 10s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 30s trunk passed +1 mvnsite 0m 58s trunk passed +1 mvneclipse 0m 22s trunk passed +1 findbugs 2m 18s trunk passed +1 javadoc 0m 58s trunk passed with JDK v1.8.0_66 +1 javadoc 3m 7s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 46s the patch passed +1 compile 2m 19s the patch passed with JDK v1.8.0_66 +1 javac 2m 19s the patch passed +1 compile 2m 5s the patch passed with JDK v1.7.0_91 +1 javac 2m 5s the patch passed -1 checkstyle 0m 31s Patch generated 4 new checkstyle issues in hadoop-yarn-project/hadoop-yarn (total was 261, now 264). +1 mvnsite 0m 55s the patch passed +1 mvneclipse 0m 23s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 1m 7s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager introduced 1 new FindBugs issues. +1 javadoc 0m 55s the patch passed with JDK v1.8.0_66 +1 javadoc 3m 7s the patch passed with JDK v1.7.0_91 +1 unit 0m 20s hadoop-yarn-api in the patch passed with JDK v1.8.0_66. +1 unit 8m 38s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_66. +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.7.0_91. +1 unit 9m 10s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 18s Patch does not generate ASF License warnings. 53m 54s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager   org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices.serviceInit(Configuration) creates a java.net.URLClassLoader classloader, which should be performed within a doPrivileged block At AuxServices.java:which should be performed within a doPrivileged block At AuxServices.java: [line 133] Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781604/YARN-4577.1.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f64ed5ca774f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 95f3201 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10228/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/10228/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.html JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10228/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Max memory used 75MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-YARN-Build/10228/console This message was automatically generated.
          Hide
          sjlee0 Sangjin Lee added a comment -

          We do have a fairly generic isolated classloader under hadoop-common (ApplicationClassLoader): https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ApplicationClassLoader.java

          It's currently used by mapreduce (job classloader and the client-side job launcher classloader).

          It seems that we should try to reuse the ApplicationClassLoader for this use case instead of creating another variant. Thoughts?

          Show
          sjlee0 Sangjin Lee added a comment - We do have a fairly generic isolated classloader under hadoop-common (ApplicationClassLoader): https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ApplicationClassLoader.java It's currently used by mapreduce (job classloader and the client-side job launcher classloader). It seems that we should try to reuse the ApplicationClassLoader for this use case instead of creating another variant. Thoughts?
          Hide
          xgong Xuan Gong added a comment -

          Sangjin Lee Thanks for the information. Will check if we could re-use it.

          Show
          xgong Xuan Gong added a comment - Sangjin Lee Thanks for the information. Will check if we could re-use it.
          Hide
          xgong Xuan Gong added a comment -

          Attached a new patch to fix -1 on findbug.

          It seems that we should try to reuse the ApplicationClassLoader for this use case instead of creating another variant. Thoughts?

          For my understanding, The applicationClassLoader is to append Classes from the application JARs in preference to the parent loader. It can not fix our problem completely.

          Show
          xgong Xuan Gong added a comment - Attached a new patch to fix -1 on findbug. It seems that we should try to reuse the ApplicationClassLoader for this use case instead of creating another variant. Thoughts? For my understanding, The applicationClassLoader is to append Classes from the application JARs in preference to the parent loader. It can not fix our problem completely.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 26s trunk passed
          +1 compile 1m 46s trunk passed with JDK v1.8.0_66
          +1 compile 2m 7s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 1m 28s trunk passed
          +1 mvneclipse 0m 38s trunk passed
          +1 findbugs 3m 30s trunk passed
          +1 javadoc 1m 21s trunk passed with JDK v1.8.0_66
          +1 javadoc 3m 36s trunk passed with JDK v1.7.0_91
          +1 mvninstall 1m 14s the patch passed
          +1 compile 1m 41s the patch passed with JDK v1.8.0_66
          +1 javac 1m 41s the patch passed
          +1 compile 2m 4s the patch passed with JDK v1.7.0_91
          +1 javac 2m 4s the patch passed
          -1 checkstyle 0m 30s Patch generated 2 new checkstyle issues in hadoop-yarn-project/hadoop-yarn (total was 261, now 262).
          +1 mvnsite 1m 23s the patch passed
          +1 mvneclipse 0m 31s the patch passed
          -1 whitespace 0m 0s The patch has 1 line(s) with tabs.
          +1 xml 0m 0s The patch has no ill-formed XML file.
          +1 findbugs 3m 49s the patch passed
          +1 javadoc 1m 17s the patch passed with JDK v1.8.0_66
          +1 javadoc 3m 33s the patch passed with JDK v1.7.0_91
          -1 unit 0m 20s hadoop-yarn-api in the patch failed with JDK v1.8.0_66.
          +1 unit 1m 53s hadoop-yarn-common in the patch passed with JDK v1.8.0_66.
          +1 unit 8m 30s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_66.
          -1 unit 0m 22s hadoop-yarn-api in the patch failed with JDK v1.7.0_91.
          +1 unit 2m 7s hadoop-yarn-common in the patch passed with JDK v1.7.0_91.
          +1 unit 9m 0s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 18s Patch does not generate ASF License warnings.
          62m 20s



          Reason Tests
          JDK v1.8.0_66 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields
          JDK v1.7.0_91 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781868/YARN-4577.2.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 7b4507c1515c 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 25051c3
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/whitespace-tabs.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10247/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Max memory used 75MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/10247/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 26s trunk passed +1 compile 1m 46s trunk passed with JDK v1.8.0_66 +1 compile 2m 7s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 28s trunk passed +1 mvneclipse 0m 38s trunk passed +1 findbugs 3m 30s trunk passed +1 javadoc 1m 21s trunk passed with JDK v1.8.0_66 +1 javadoc 3m 36s trunk passed with JDK v1.7.0_91 +1 mvninstall 1m 14s the patch passed +1 compile 1m 41s the patch passed with JDK v1.8.0_66 +1 javac 1m 41s the patch passed +1 compile 2m 4s the patch passed with JDK v1.7.0_91 +1 javac 2m 4s the patch passed -1 checkstyle 0m 30s Patch generated 2 new checkstyle issues in hadoop-yarn-project/hadoop-yarn (total was 261, now 262). +1 mvnsite 1m 23s the patch passed +1 mvneclipse 0m 31s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) with tabs. +1 xml 0m 0s The patch has no ill-formed XML file. +1 findbugs 3m 49s the patch passed +1 javadoc 1m 17s the patch passed with JDK v1.8.0_66 +1 javadoc 3m 33s the patch passed with JDK v1.7.0_91 -1 unit 0m 20s hadoop-yarn-api in the patch failed with JDK v1.8.0_66. +1 unit 1m 53s hadoop-yarn-common in the patch passed with JDK v1.8.0_66. +1 unit 8m 30s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_66. -1 unit 0m 22s hadoop-yarn-api in the patch failed with JDK v1.7.0_91. +1 unit 2m 7s hadoop-yarn-common in the patch passed with JDK v1.7.0_91. +1 unit 9m 0s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 18s Patch does not generate ASF License warnings. 62m 20s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields JDK v1.7.0_91 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781868/YARN-4577.2.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 7b4507c1515c 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 25051c3 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/whitespace-tabs.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/10247/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10247/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Max memory used 75MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-YARN-Build/10247/console This message was automatically generated.
          Hide
          kasha Karthik Kambatla added a comment -

          Would it make sense to run these auxiliary services as a separate process, may be even in a container in isolation? If one aux service is problematic, it wouldn't affect rest of the NM. And, we could monitor resources. I am fine with limiting this JIRA to the classpath, and filing another JIRA for this. Just wanted to bring this up and hear people's thoughts.

          Show
          kasha Karthik Kambatla added a comment - Would it make sense to run these auxiliary services as a separate process, may be even in a container in isolation? If one aux service is problematic, it wouldn't affect rest of the NM. And, we could monitor resources. I am fine with limiting this JIRA to the classpath, and filing another JIRA for this. Just wanted to bring this up and hear people's thoughts.
          Hide
          xgong Xuan Gong added a comment -

          Karthik Kambatla Yes, we will do that in future, and YARN-1593 has been created to track that.

          Show
          xgong Xuan Gong added a comment - Karthik Kambatla Yes, we will do that in future, and YARN-1593 has been created to track that.
          Hide
          sjlee0 Sangjin Lee added a comment -

          Hi Xuan Gong, it would be great if you could describe precisely the behavior of the classloading you desire in this solution. Then, we could discuss it a little better.

          For your reference, here is what the ApplicationClassLoader does.

          • isolates the classloading and the classpath of the "application" from the hadoop stack
          • while this classloader is in place, it tries to load classes first from the user classpath (as opposed to system classpath), and if not found tries to load it from the system classpath/classloader
          • there are "system classes" (mainly hadoop classes and JDK classes) that are always loaded only by the system classloader to ensure consistency
          • outside the context of this classloader, the hadoop code does not see the user classpath at all, and uses only the system classloader
          • each application classloader is isolated from one another (obviously)
          • when the application classloader is in scope, it gets set onto the Configuration as well as the current thread's context classloader to ensure consistency for reflection-based classloading

          This is essentially the same behavior as the webapp classloader of servlet engine implementations (Tomcat, Jetty, etc.).

          From the description and the inferred behavior from the provided patch, I didn't see much that the application classloader cannot work for this use case. The desire here is to see if we can have a single generic solution that can address all the needs for isolated classloading, rather than creating more solutions as new use cases arise. If there are some things that are not addressed by the current application classloader implementation, we could consider modifying it to make it wider in scope. There are some HADOOP JIRAs filed (see HADOOP-11656 for example) to adopt a single mechanism for classloading isolation and make it more formal and stricter .

          Thoughts?

          Show
          sjlee0 Sangjin Lee added a comment - Hi Xuan Gong , it would be great if you could describe precisely the behavior of the classloading you desire in this solution. Then, we could discuss it a little better. For your reference, here is what the ApplicationClassLoader does. isolates the classloading and the classpath of the "application" from the hadoop stack while this classloader is in place, it tries to load classes first from the user classpath (as opposed to system classpath), and if not found tries to load it from the system classpath/classloader there are "system classes" (mainly hadoop classes and JDK classes) that are always loaded only by the system classloader to ensure consistency outside the context of this classloader, the hadoop code does not see the user classpath at all, and uses only the system classloader each application classloader is isolated from one another (obviously) when the application classloader is in scope, it gets set onto the Configuration as well as the current thread's context classloader to ensure consistency for reflection-based classloading This is essentially the same behavior as the webapp classloader of servlet engine implementations (Tomcat, Jetty, etc.). From the description and the inferred behavior from the provided patch, I didn't see much that the application classloader cannot work for this use case. The desire here is to see if we can have a single generic solution that can address all the needs for isolated classloading, rather than creating more solutions as new use cases arise. If there are some things that are not addressed by the current application classloader implementation, we could consider modifying it to make it wider in scope. There are some HADOOP JIRAs filed (see HADOOP-11656 for example) to adopt a single mechanism for classloading isolation and make it more formal and stricter . Thoughts?
          Hide
          xgong Xuan Gong added a comment -

          Thanks, Sangjin Lee for the comments and suggestions.

          +1 for the suggestion to have a single generic solution that can address all the needs for isolated classloading. But i think that we still need some improvement on this.

          The use case here is simple: if we specify the aux-services classpath, either from local fs or from hdfs, we will load this service from the specified classpath (no matter we set the classpath in NM path or not). Otherwise, we load the service from the NM path.

          For ApplicationClassLoader,

          public ApplicationClassLoader(String classpath, ClassLoader parent,
                List<String> systemClasses)
          

          looks like we have to specify classpath (we can not set it null). Also, it needs me to specify systemClasses which is not required in this use-case. There are some un-necessary checks, such as isSystemClass() when we call loadClass. Overall, i think that the ApplicationClassLoader is too complicate for this use-case.

          Show
          xgong Xuan Gong added a comment - Thanks, Sangjin Lee for the comments and suggestions. +1 for the suggestion to have a single generic solution that can address all the needs for isolated classloading. But i think that we still need some improvement on this. The use case here is simple: if we specify the aux-services classpath, either from local fs or from hdfs, we will load this service from the specified classpath (no matter we set the classpath in NM path or not). Otherwise, we load the service from the NM path. For ApplicationClassLoader, public ApplicationClassLoader( String classpath, ClassLoader parent, List< String > systemClasses) looks like we have to specify classpath (we can not set it null). Also, it needs me to specify systemClasses which is not required in this use-case. There are some un-necessary checks, such as isSystemClass() when we call loadClass. Overall, i think that the ApplicationClassLoader is too complicate for this use-case.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Test wise

          • testLoadAuxServiceLocally should be calling aux.close() in finally{} clauses. It's idempotent so you could so a close() in the main path (and so test it), but still clean up after.
          • it'd be nice for the asserts to include some text about why the asserts are failing, especially simple assertTrue() calls. The goal is that enough information is printed to enable someone who sees the Jenkins log to be able to diagnose the problem. An "assert failed line 315" doesn't do that much, leads to the "add more test diagnostics" patches and more iterations.
          Show
          stevel@apache.org Steve Loughran added a comment - Test wise testLoadAuxServiceLocally should be calling aux.close() in finally{} clauses. It's idempotent so you could so a close() in the main path (and so test it), but still clean up after. it'd be nice for the asserts to include some text about why the asserts are failing, especially simple assertTrue() calls. The goal is that enough information is printed to enable someone who sees the Jenkins log to be able to diagnose the problem. An "assert failed line 315" doesn't do that much, leads to the "add more test diagnostics" patches and more iterations.
          Hide
          sjlee0 Sangjin Lee added a comment -

          The use case here is simple: if we specify the aux-services classpath, either from local fs or from hdfs, we will load this service from the specified classpath (no matter we set the classpath in NM path or not). Otherwise, we load the service from the NM path.

          Hmm, is one of the goals to preserve aux service's dependencies against hadoop's dependencies (as I see in the linked ticket SPARK-12807)? If so, I don't think the current approach in the patch does that. Note that URLClassLoader (or any simple extension of ClassLoader) always delegates classloading to the parent classloader first, and loads the class only if the parent classloader doesn't load/have it. In other words, any classpath the URLClassLoader owns is effectively appended, not prepended. That's precisely why ApplicationClassLoader inverts that order to create isolation.

          Could you write a simple test program to verify this behavior? I'm pretty sure you'll find that your classpath will still be shadowed by the system classpath.

          Also, as for using the ApplicationClassLoader, it shouldn't be too difficult. You pass in URL[] to the URLClassLoader too, so that's common. You can simply pass in the classloader of the calling class as the parent classloader. Also, you can simply pass null for the system classes, in which case the sensible default will be used.

          If it helps anyway, we could introduce a simpler constructor like the following:

          public ApplicationClassLoader(URL[] classpath) {
            this(classpath, getClass().getClassLoader(), null);
          }
          
          Show
          sjlee0 Sangjin Lee added a comment - The use case here is simple: if we specify the aux-services classpath, either from local fs or from hdfs, we will load this service from the specified classpath (no matter we set the classpath in NM path or not). Otherwise, we load the service from the NM path. Hmm, is one of the goals to preserve aux service's dependencies against hadoop's dependencies (as I see in the linked ticket SPARK-12807 )? If so, I don't think the current approach in the patch does that. Note that URLClassLoader (or any simple extension of ClassLoader) always delegates classloading to the parent classloader first , and loads the class only if the parent classloader doesn't load/have it. In other words, any classpath the URLClassLoader owns is effectively appended , not prepended. That's precisely why ApplicationClassLoader inverts that order to create isolation. Could you write a simple test program to verify this behavior? I'm pretty sure you'll find that your classpath will still be shadowed by the system classpath. Also, as for using the ApplicationClassLoader, it shouldn't be too difficult. You pass in URL[] to the URLClassLoader too, so that's common. You can simply pass in the classloader of the calling class as the parent classloader. Also, you can simply pass null for the system classes, in which case the sensible default will be used. If it helps anyway, we could introduce a simpler constructor like the following: public ApplicationClassLoader(URL[] classpath) { this (classpath, getClass().getClassLoader(), null ); }
          Hide
          xgong Xuan Gong added a comment -

          Thanks for the comments, Sangjin Lee

          Attached a new patch to use ApplicationClassLoader. One issue here is: ApplicationClassLoader does not work for HDFS, especially for loading jar if we only specify hdfs dir. Create a private function in AuxService.java to fix this. Maybe, this is the one that we can improve on ApplicationClassLoader.

          Show
          xgong Xuan Gong added a comment - Thanks for the comments, Sangjin Lee Attached a new patch to use ApplicationClassLoader. One issue here is: ApplicationClassLoader does not work for HDFS, especially for loading jar if we only specify hdfs dir. Create a private function in AuxService.java to fix this. Maybe, this is the one that we can improve on ApplicationClassLoader.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 4s YARN-4577 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12782606/YARN-4577.3.patch
          JIRA Issue YARN-4577
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/10307/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 4s YARN-4577 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12782606/YARN-4577.3.patch JIRA Issue YARN-4577 Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-YARN-Build/10307/console This message was automatically generated.
          Hide
          gtCarrera9 Li Lu added a comment -

          Thanks Xuan Gong for the work! The overall logic of reusing ApplicationClassLoader looks fine to me. While we are seeking for a solution to load jars from HDFS, maybe we can decide on the "interface" part of the design, such as the name of the config? I personally feel the "classloader.location" is a little bit confusing (which may mean the location of the classloader itself). Any special considerations here? Thanks!

          Show
          gtCarrera9 Li Lu added a comment - Thanks Xuan Gong for the work! The overall logic of reusing ApplicationClassLoader looks fine to me. While we are seeking for a solution to load jars from HDFS, maybe we can decide on the "interface" part of the design, such as the name of the config? I personally feel the "classloader.location" is a little bit confusing (which may mean the location of the classloader itself). Any special considerations here? Thanks!
          Hide
          gtCarrera9 Li Lu added a comment -

          BTW I tried this patch locally but it does not apply on the latest trunk. I hit some problems with yarn-default.xml. Not sure if this is a problem of the patch.

          Show
          gtCarrera9 Li Lu added a comment - BTW I tried this patch locally but it does not apply on the latest trunk. I hit some problems with yarn-default.xml. Not sure if this is a problem of the patch.
          Hide
          xgong Xuan Gong added a comment -

          rebase the patch

          Show
          xgong Xuan Gong added a comment - rebase the patch
          Hide
          sjlee0 Sangjin Lee added a comment -

          Thanks for the updated patch Xuan Gong!

          I had bit of time to sit down to go over the patch and think about related points.

          (1) URL.setURLStreamHandlerFactory() (and supporting non-local paths)
          Regarding setting the URLStreamHandlerFactory, you can call URL.setURLStreamHandlerFactory() at most once on a JVM, and any attempt to set it again within the same process will throw an error:

          Sets an application's URLStreamHandlerFactory. This method can be called at most once in a given Java Virtual Machine.

          In the patch, this is being called inside a for loop. This will throw an Error for any subsequent initialization of aux services. If this was needed to be able to handle non-local paths (like hdfs), then we would need to find a different way than this method to handle it.

          On a related note, how important is it to support non-local classpaths? If implementing it is not trivial, you might want to separate that work into a separate JIRA and address that. I'd be curious to hear how important that part of the feature is.

          (2) other types of classloading
          The patch will ensure that the aux service class itself will be loaded by the application classloader and any class that needs to be loaded in a normal manner as part of executing the aux service class. However, there are other types of classloading that can happen. Two major types you need to consider are classloading via Configuration.getClass() and reflection using thread context classloader (via Thread.currentThread().getContextClassLoader()).

          For example, if the aux service code depends on another class property (owned by the aux service) in the configuration, that will be invoked via Configuration.getClass(), and it will still use the system classloader to load that class. Then it's very likely that you'll get a ClassNotFoundException.

          The thread context classloader represents another similar problem. The moment the aux service code hits a code path that does Class.forName() that loads classes via the thread context classloader, and it needs to load an aux service-related class (that is not present in the main NM classpath), you will get a ClassNotFoundException.

          If you look at the existing uses of the ApplicationClassLoader, you'll see we usually try to demarcate the code regions that need to run under the ApplicationClassLoader, and set and unset both the Configuration classloader and the thread context classloader. You might need to do the same thing with executing the aux service code. Luckily, I believe there are well-defined entry points and exit points for the aux service code, so hopefully it is not too difficult to do it completely.

          (3) configuration property
          The configuration property "...classloader.location" doesn't seem quite natural. It is really about the classpath. How about simply "...classpath" instead?

          Also, it would be good to document that this is a comma-separated classpath somewhere.

          (4) unit test
          l.366: I'm quite confused by the comment and the code. If I'm reading this correctly, it is setting the classname configuration property with the TEST_DIR location, which is pretty much a bogus value. So it's understandable this won't work. But it's not really setting the classpath to a different location. Does it verify or confirm anything about this feature?

          l.383: should we delete TEST_DIR here? It seems like the directory is deleted in another unit test. I don't think it's created again for each of the unit tests. It doesn't seem like it's necessary to delete this directory at all here. Did I miss something?

          Overall, you might want to take a look at the TestApplicationClassLoader test to see how you can formulate the test. Note that all org.apache.hadoop.* classes are considered system classes and are exempt from the loading from the ApplicationClassLoader, so playing with the system classes is needed to test this (unless you can create test classes outside org.apache.hadoop).

          Show
          sjlee0 Sangjin Lee added a comment - Thanks for the updated patch Xuan Gong ! I had bit of time to sit down to go over the patch and think about related points. (1) URL.setURLStreamHandlerFactory() (and supporting non-local paths) Regarding setting the URLStreamHandlerFactory , you can call URL.setURLStreamHandlerFactory() at most once on a JVM, and any attempt to set it again within the same process will throw an error: Sets an application's URLStreamHandlerFactory . This method can be called at most once in a given Java Virtual Machine. In the patch, this is being called inside a for loop. This will throw an Error for any subsequent initialization of aux services. If this was needed to be able to handle non-local paths (like hdfs), then we would need to find a different way than this method to handle it. On a related note, how important is it to support non-local classpaths? If implementing it is not trivial, you might want to separate that work into a separate JIRA and address that. I'd be curious to hear how important that part of the feature is. (2) other types of classloading The patch will ensure that the aux service class itself will be loaded by the application classloader and any class that needs to be loaded in a normal manner as part of executing the aux service class. However, there are other types of classloading that can happen. Two major types you need to consider are classloading via Configuration.getClass() and reflection using thread context classloader (via Thread.currentThread().getContextClassLoader() ). For example, if the aux service code depends on another class property (owned by the aux service) in the configuration, that will be invoked via Configuration.getClass() , and it will still use the system classloader to load that class. Then it's very likely that you'll get a ClassNotFoundException . The thread context classloader represents another similar problem. The moment the aux service code hits a code path that does Class.forName() that loads classes via the thread context classloader, and it needs to load an aux service-related class (that is not present in the main NM classpath), you will get a ClassNotFoundException . If you look at the existing uses of the ApplicationClassLoader , you'll see we usually try to demarcate the code regions that need to run under the ApplicationClassLoader , and set and unset both the Configuration classloader and the thread context classloader. You might need to do the same thing with executing the aux service code. Luckily, I believe there are well-defined entry points and exit points for the aux service code, so hopefully it is not too difficult to do it completely. (3) configuration property The configuration property "...classloader.location" doesn't seem quite natural. It is really about the classpath. How about simply "...classpath" instead? Also, it would be good to document that this is a comma-separated classpath somewhere. (4) unit test l.366: I'm quite confused by the comment and the code. If I'm reading this correctly, it is setting the classname configuration property with the TEST_DIR location, which is pretty much a bogus value. So it's understandable this won't work. But it's not really setting the classpath to a different location. Does it verify or confirm anything about this feature? l.383: should we delete TEST_DIR here? It seems like the directory is deleted in another unit test. I don't think it's created again for each of the unit tests. It doesn't seem like it's necessary to delete this directory at all here. Did I miss something? Overall, you might want to take a look at the TestApplicationClassLoader test to see how you can formulate the test. Note that all org.apache.hadoop.* classes are considered system classes and are exempt from the loading from the ApplicationClassLoader , so playing with the system classes is needed to test this (unless you can create test classes outside org.apache.hadoop ).
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 31s Maven dependency ordering for branch
          +1 mvninstall 7m 56s trunk passed
          +1 compile 1m 56s trunk passed with JDK v1.8.0_66
          +1 compile 2m 12s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 1m 32s trunk passed
          +1 mvneclipse 0m 38s trunk passed
          +1 findbugs 3m 50s trunk passed
          +1 javadoc 1m 30s trunk passed with JDK v1.8.0_66
          +1 javadoc 3m 54s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 24s Maven dependency ordering for patch
          +1 mvninstall 1m 20s the patch passed
          +1 compile 2m 2s the patch passed with JDK v1.8.0_66
          +1 javac 2m 2s the patch passed
          +1 compile 2m 12s the patch passed with JDK v1.7.0_91
          +1 javac 2m 12s the patch passed
          -1 checkstyle 0m 32s Patch generated 5 new checkstyle issues in hadoop-yarn-project/hadoop-yarn (total was 258, now 263).
          +1 mvnsite 1m 28s the patch passed
          +1 mvneclipse 0m 33s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          -1 findbugs 1m 8s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager introduced 1 new FindBugs issues.
          +1 javadoc 1m 23s the patch passed with JDK v1.8.0_66
          +1 javadoc 3m 46s the patch passed with JDK v1.7.0_91
          -1 unit 0m 23s hadoop-yarn-api in the patch failed with JDK v1.8.0_66.
          +1 unit 2m 0s hadoop-yarn-common in the patch passed with JDK v1.8.0_66.
          -1 unit 8m 46s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.8.0_66.
          -1 unit 0m 25s hadoop-yarn-api in the patch failed with JDK v1.7.0_91.
          +1 unit 2m 14s hadoop-yarn-common in the patch passed with JDK v1.7.0_91.
          -1 unit 9m 12s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.7.0_91.
          +1 asflicense 0m 19s Patch does not generate ASF License warnings.
          67m 11s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
            org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices.serviceInit(Configuration) creates a org.apache.hadoop.util.ApplicationClassLoader classloader, which should be performed within a doPrivileged block At AuxServices.java:which should be performed within a doPrivileged block At AuxServices.java:[line 147]
          JDK v1.8.0_66 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields
          JDK v1.7.0_91 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12782643/YARN-4577.3.rebase.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 997b7054b090 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 2a30386
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.html
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.7.0_91.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10310/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Max memory used 76MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/10310/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 31s Maven dependency ordering for branch +1 mvninstall 7m 56s trunk passed +1 compile 1m 56s trunk passed with JDK v1.8.0_66 +1 compile 2m 12s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 32s trunk passed +1 mvnsite 1m 32s trunk passed +1 mvneclipse 0m 38s trunk passed +1 findbugs 3m 50s trunk passed +1 javadoc 1m 30s trunk passed with JDK v1.8.0_66 +1 javadoc 3m 54s trunk passed with JDK v1.7.0_91 0 mvndep 0m 24s Maven dependency ordering for patch +1 mvninstall 1m 20s the patch passed +1 compile 2m 2s the patch passed with JDK v1.8.0_66 +1 javac 2m 2s the patch passed +1 compile 2m 12s the patch passed with JDK v1.7.0_91 +1 javac 2m 12s the patch passed -1 checkstyle 0m 32s Patch generated 5 new checkstyle issues in hadoop-yarn-project/hadoop-yarn (total was 258, now 263). +1 mvnsite 1m 28s the patch passed +1 mvneclipse 0m 33s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. -1 findbugs 1m 8s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager introduced 1 new FindBugs issues. +1 javadoc 1m 23s the patch passed with JDK v1.8.0_66 +1 javadoc 3m 46s the patch passed with JDK v1.7.0_91 -1 unit 0m 23s hadoop-yarn-api in the patch failed with JDK v1.8.0_66. +1 unit 2m 0s hadoop-yarn-common in the patch passed with JDK v1.8.0_66. -1 unit 8m 46s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.8.0_66. -1 unit 0m 25s hadoop-yarn-api in the patch failed with JDK v1.7.0_91. +1 unit 2m 14s hadoop-yarn-common in the patch passed with JDK v1.7.0_91. -1 unit 9m 12s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.7.0_91. +1 asflicense 0m 19s Patch does not generate ASF License warnings. 67m 11s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager   org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices.serviceInit(Configuration) creates a org.apache.hadoop.util.ApplicationClassLoader classloader, which should be performed within a doPrivileged block At AuxServices.java:which should be performed within a doPrivileged block At AuxServices.java: [line 147] JDK v1.8.0_66 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields JDK v1.7.0_91 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12782643/YARN-4577.3.rebase.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 997b7054b090 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2a30386 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.html unit https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/10310/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10310/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Max memory used 76MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-YARN-Build/10310/console This message was automatically generated.
          Hide
          xgong Xuan Gong added a comment -

          Thanks for the comments, Sangjin Lee

          "how important is it to support non-local classpaths"

          It is important to support non-local classpath, especially HDFS classpath. It is one of the requirement for this feature. Of course, the changes are not trivial. I think that supporting HDFS could be one of the improvement for the ApplicationClassLoader if we are planning to do it. If ApplicationClassLoader supports it in future, we could replace it. But i still prefer to do it here since it is part of the requirement.

          and "Regarding setting the URLStreamHandlerFactory, you can call URL.setURLStreamHandlerFactory() at most once on a JVM, and any attempt to set it again within the same process will throw an error:"

          Good point. Added a static method to call it in AuxService.java. But I can not find a better way to solve it. Any better suggestions ?

          other types of classloading

          Actually, I do not even need a parent classloader here. For me, if the user provided a specific classpath for the aux-service, it is user's responsibility to make sure the provided jar file includes everything, includes the dependency. And when we initiate the related aux-service, we only look for the specific classpath. If the aux-service can not be initiated successfully with the specific classpath, it should throw an exception instead of trying to load it from parent classloader.

          unit test: l.366: I'm quite confused by the comment and the code

          This unit test is used to test my previous comment.

          Show
          xgong Xuan Gong added a comment - Thanks for the comments, Sangjin Lee "how important is it to support non-local classpaths" It is important to support non-local classpath, especially HDFS classpath. It is one of the requirement for this feature. Of course, the changes are not trivial. I think that supporting HDFS could be one of the improvement for the ApplicationClassLoader if we are planning to do it. If ApplicationClassLoader supports it in future, we could replace it. But i still prefer to do it here since it is part of the requirement. and "Regarding setting the URLStreamHandlerFactory, you can call URL.setURLStreamHandlerFactory() at most once on a JVM, and any attempt to set it again within the same process will throw an error:" Good point. Added a static method to call it in AuxService.java. But I can not find a better way to solve it. Any better suggestions ? other types of classloading Actually, I do not even need a parent classloader here. For me, if the user provided a specific classpath for the aux-service, it is user's responsibility to make sure the provided jar file includes everything, includes the dependency. And when we initiate the related aux-service, we only look for the specific classpath. If the aux-service can not be initiated successfully with the specific classpath, it should throw an exception instead of trying to load it from parent classloader. unit test: l.366: I'm quite confused by the comment and the code This unit test is used to test my previous comment.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 32s Maven dependency ordering for branch
          +1 mvninstall 7m 54s trunk passed
          +1 compile 1m 48s trunk passed with JDK v1.8.0_66
          +1 compile 2m 10s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 1m 32s trunk passed
          +1 mvneclipse 0m 41s trunk passed
          +1 findbugs 3m 38s trunk passed
          +1 javadoc 1m 27s trunk passed with JDK v1.8.0_66
          +1 javadoc 3m 55s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 24s Maven dependency ordering for patch
          +1 mvninstall 1m 13s the patch passed
          +1 compile 1m 52s the patch passed with JDK v1.8.0_66
          +1 javac 1m 52s the patch passed
          +1 compile 2m 10s the patch passed with JDK v1.7.0_91
          +1 javac 2m 10s the patch passed
          -1 checkstyle 0m 31s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 258 unchanged - 0 fixed = 259 total (was 258)
          +1 mvnsite 1m 23s the patch passed
          +1 mvneclipse 0m 32s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 xml 0m 0s The patch has no ill-formed XML file.
          +1 findbugs 4m 12s the patch passed
          +1 javadoc 1m 21s the patch passed with JDK v1.8.0_66
          +1 javadoc 3m 38s the patch passed with JDK v1.7.0_91
          -1 unit 0m 21s hadoop-yarn-api in the patch failed with JDK v1.8.0_66.
          +1 unit 1m 58s hadoop-yarn-common in the patch passed with JDK v1.8.0_66.
          -1 unit 8m 45s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.8.0_66.
          -1 unit 0m 23s hadoop-yarn-api in the patch failed with JDK v1.7.0_91.
          +1 unit 2m 10s hadoop-yarn-common in the patch passed with JDK v1.7.0_91.
          -1 unit 9m 21s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.7.0_91.
          +1 asflicense 0m 20s Patch does not generate ASF License warnings.
          66m 32s



          Reason Tests
          JDK v1.8.0_66 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields
          JDK v1.7.0_91 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12782816/YARN-4577.4.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 3b687b178b1f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b08ecf5
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.7.0_91.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10316/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Max memory used 77MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/10316/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 32s Maven dependency ordering for branch +1 mvninstall 7m 54s trunk passed +1 compile 1m 48s trunk passed with JDK v1.8.0_66 +1 compile 2m 10s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 32s trunk passed +1 mvnsite 1m 32s trunk passed +1 mvneclipse 0m 41s trunk passed +1 findbugs 3m 38s trunk passed +1 javadoc 1m 27s trunk passed with JDK v1.8.0_66 +1 javadoc 3m 55s trunk passed with JDK v1.7.0_91 0 mvndep 0m 24s Maven dependency ordering for patch +1 mvninstall 1m 13s the patch passed +1 compile 1m 52s the patch passed with JDK v1.8.0_66 +1 javac 1m 52s the patch passed +1 compile 2m 10s the patch passed with JDK v1.7.0_91 +1 javac 2m 10s the patch passed -1 checkstyle 0m 31s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 258 unchanged - 0 fixed = 259 total (was 258) +1 mvnsite 1m 23s the patch passed +1 mvneclipse 0m 32s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 xml 0m 0s The patch has no ill-formed XML file. +1 findbugs 4m 12s the patch passed +1 javadoc 1m 21s the patch passed with JDK v1.8.0_66 +1 javadoc 3m 38s the patch passed with JDK v1.7.0_91 -1 unit 0m 21s hadoop-yarn-api in the patch failed with JDK v1.8.0_66. +1 unit 1m 58s hadoop-yarn-common in the patch passed with JDK v1.8.0_66. -1 unit 8m 45s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.8.0_66. -1 unit 0m 23s hadoop-yarn-api in the patch failed with JDK v1.7.0_91. +1 unit 2m 10s hadoop-yarn-common in the patch passed with JDK v1.7.0_91. -1 unit 9m 21s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.7.0_91. +1 asflicense 0m 20s Patch does not generate ASF License warnings. 66m 32s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields JDK v1.7.0_91 Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12782816/YARN-4577.4.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 3b687b178b1f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b08ecf5 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/10316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10316/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Max memory used 77MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-YARN-Build/10316/console This message was automatically generated.
          Hide
          xgong Xuan Gong added a comment -

          Sangjin Lee

          how important is it to support non-local classpaths

          We can do it separately. Right now, for this ticket, we are focusing on supporting the local classpath.

          other types of classloading

          Do we have any examples for this ? I can only find the MRApp and RunJar are using ApplicationClassLoader.

          Show
          xgong Xuan Gong added a comment - Sangjin Lee how important is it to support non-local classpaths We can do it separately. Right now, for this ticket, we are focusing on supporting the local classpath. other types of classloading Do we have any examples for this ? I can only find the MRApp and RunJar are using ApplicationClassLoader.
          Hide
          sjlee0 Sangjin Lee added a comment -

          Do we have any examples for this ? I can only find the MRApp and RunJar are using ApplicationClassLoader.

          Currently there are 3 places where the application classloader is used: RunJar (for running the hadoop jar command), YarnChild and MRAppMaster for MR.

          A standard pattern is to surround the code that needs to execute user code (and needs the user classloader) with setting the config classloader as well as the thread context classloader. You can look at things like MRApps.setJobClassLoader() and MRAppMaster.callWithJobClassLoader().

          Show
          sjlee0 Sangjin Lee added a comment - Do we have any examples for this ? I can only find the MRApp and RunJar are using ApplicationClassLoader. Currently there are 3 places where the application classloader is used: RunJar (for running the hadoop jar command), YarnChild and MRAppMaster for MR. A standard pattern is to surround the code that needs to execute user code (and needs the user classloader) with setting the config classloader as well as the thread context classloader. You can look at things like MRApps.setJobClassLoader() and MRAppMaster.callWithJobClassLoader().
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 34s Maven dependency ordering for branch
          +1 mvninstall 8m 49s trunk passed
          +1 compile 2m 48s trunk passed with JDK v1.8.0_66
          +1 compile 2m 20s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 39s trunk passed
          +1 mvnsite 1m 38s trunk passed
          +1 mvneclipse 0m 42s trunk passed
          +1 findbugs 4m 8s trunk passed
          +1 javadoc 1m 32s trunk passed with JDK v1.8.0_66
          +1 javadoc 3m 58s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 26s Maven dependency ordering for patch
          +1 mvninstall 1m 23s the patch passed
          +1 compile 2m 20s the patch passed with JDK v1.8.0_66
          +1 javac 2m 20s the patch passed
          +1 compile 2m 20s the patch passed with JDK v1.7.0_91
          +1 javac 2m 20s the patch passed
          -1 checkstyle 0m 38s hadoop-yarn-project/hadoop-yarn: patch generated 13 new + 258 unchanged - 0 fixed = 271 total (was 258)
          +1 mvnsite 1m 31s the patch passed
          +1 mvneclipse 0m 34s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 4m 32s the patch passed
          +1 javadoc 1m 26s the patch passed with JDK v1.8.0_66
          +1 javadoc 3m 43s the patch passed with JDK v1.7.0_91
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_66.
          +1 unit 2m 5s hadoop-yarn-common in the patch passed with JDK v1.8.0_66.
          -1 unit 8m 53s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.8.0_66.
          +1 unit 0m 25s hadoop-yarn-api in the patch passed with JDK v1.7.0_91.
          +1 unit 2m 16s hadoop-yarn-common in the patch passed with JDK v1.7.0_91.
          -1 unit 9m 15s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.7.0_91.
          +1 asflicense 0m 20s Patch does not generate ASF License warnings.
          71m 13s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12783119/YARN-4577.20160119.1.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 9446af938ee9 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 2085e60
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10385/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10385/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.8.0_66.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/10385/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10385/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Max memory used 77MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/10385/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 34s Maven dependency ordering for branch +1 mvninstall 8m 49s trunk passed +1 compile 2m 48s trunk passed with JDK v1.8.0_66 +1 compile 2m 20s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 39s trunk passed +1 mvnsite 1m 38s trunk passed +1 mvneclipse 0m 42s trunk passed +1 findbugs 4m 8s trunk passed +1 javadoc 1m 32s trunk passed with JDK v1.8.0_66 +1 javadoc 3m 58s trunk passed with JDK v1.7.0_91 0 mvndep 0m 26s Maven dependency ordering for patch +1 mvninstall 1m 23s the patch passed +1 compile 2m 20s the patch passed with JDK v1.8.0_66 +1 javac 2m 20s the patch passed +1 compile 2m 20s the patch passed with JDK v1.7.0_91 +1 javac 2m 20s the patch passed -1 checkstyle 0m 38s hadoop-yarn-project/hadoop-yarn: patch generated 13 new + 258 unchanged - 0 fixed = 271 total (was 258) +1 mvnsite 1m 31s the patch passed +1 mvneclipse 0m 34s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 4m 32s the patch passed +1 javadoc 1m 26s the patch passed with JDK v1.8.0_66 +1 javadoc 3m 43s the patch passed with JDK v1.7.0_91 +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_66. +1 unit 2m 5s hadoop-yarn-common in the patch passed with JDK v1.8.0_66. -1 unit 8m 53s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.8.0_66. +1 unit 0m 25s hadoop-yarn-api in the patch passed with JDK v1.7.0_91. +1 unit 2m 16s hadoop-yarn-common in the patch passed with JDK v1.7.0_91. -1 unit 9m 15s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.7.0_91. +1 asflicense 0m 20s Patch does not generate ASF License warnings. 71m 13s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12783119/YARN-4577.20160119.1.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 9446af938ee9 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2085e60 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10385/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10385/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10385/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10385/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Max memory used 77MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-YARN-Build/10385/console This message was automatically generated.
          Hide
          xgong Xuan Gong added a comment -

          Sangjin Lee
          Thanks for the information.

          Attached a new patch. Could you take a look, please ?

          Show
          xgong Xuan Gong added a comment - Sangjin Lee Thanks for the information. Attached a new patch. Could you take a look, please ?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 26s Maven dependency ordering for branch
          +1 mvninstall 7m 13s trunk passed
          +1 compile 2m 4s trunk passed with JDK v1.8.0_66
          +1 compile 2m 18s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 38s trunk passed
          +1 mvnsite 1m 32s trunk passed
          +1 mvneclipse 0m 37s trunk passed
          +1 findbugs 3m 22s trunk passed
          +1 javadoc 1m 31s trunk passed with JDK v1.8.0_66
          +1 javadoc 4m 5s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 25s Maven dependency ordering for patch
          +1 mvninstall 1m 18s the patch passed
          +1 compile 2m 3s the patch passed with JDK v1.8.0_66
          +1 javac 2m 3s the patch passed
          +1 compile 2m 14s the patch passed with JDK v1.7.0_91
          +1 javac 2m 14s the patch passed
          -1 checkstyle 0m 35s hadoop-yarn-project/hadoop-yarn: patch generated 2 new + 258 unchanged - 0 fixed = 260 total (was 258)
          +1 mvnsite 1m 28s the patch passed
          +1 mvneclipse 0m 34s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 4m 17s the patch passed
          +1 javadoc 1m 26s the patch passed with JDK v1.8.0_66
          +1 javadoc 3m 56s the patch passed with JDK v1.7.0_91
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_66.
          +1 unit 2m 0s hadoop-yarn-common in the patch passed with JDK v1.8.0_66.
          +1 unit 8m 57s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 25s hadoop-yarn-api in the patch passed with JDK v1.7.0_91.
          +1 unit 2m 22s hadoop-yarn-common in the patch passed with JDK v1.7.0_91.
          +1 unit 9m 48s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 23s Patch does not generate ASF License warnings.
          68m 0s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786365/YARN-4577.20160204.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux ab1657c5306b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 1bcfab8
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10496/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10496/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Max memory used 77MB
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/10496/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 26s Maven dependency ordering for branch +1 mvninstall 7m 13s trunk passed +1 compile 2m 4s trunk passed with JDK v1.8.0_66 +1 compile 2m 18s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 38s trunk passed +1 mvnsite 1m 32s trunk passed +1 mvneclipse 0m 37s trunk passed +1 findbugs 3m 22s trunk passed +1 javadoc 1m 31s trunk passed with JDK v1.8.0_66 +1 javadoc 4m 5s trunk passed with JDK v1.7.0_91 0 mvndep 0m 25s Maven dependency ordering for patch +1 mvninstall 1m 18s the patch passed +1 compile 2m 3s the patch passed with JDK v1.8.0_66 +1 javac 2m 3s the patch passed +1 compile 2m 14s the patch passed with JDK v1.7.0_91 +1 javac 2m 14s the patch passed -1 checkstyle 0m 35s hadoop-yarn-project/hadoop-yarn: patch generated 2 new + 258 unchanged - 0 fixed = 260 total (was 258) +1 mvnsite 1m 28s the patch passed +1 mvneclipse 0m 34s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 4m 17s the patch passed +1 javadoc 1m 26s the patch passed with JDK v1.8.0_66 +1 javadoc 3m 56s the patch passed with JDK v1.7.0_91 +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_66. +1 unit 2m 0s hadoop-yarn-common in the patch passed with JDK v1.8.0_66. +1 unit 8m 57s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_66. +1 unit 0m 25s hadoop-yarn-api in the patch passed with JDK v1.7.0_91. +1 unit 2m 22s hadoop-yarn-common in the patch passed with JDK v1.7.0_91. +1 unit 9m 48s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 23s Patch does not generate ASF License warnings. 68m 0s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786365/YARN-4577.20160204.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux ab1657c5306b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1bcfab8 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/10496/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10496/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Max memory used 77MB Console output https://builds.apache.org/job/PreCommit-YARN-Build/10496/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          sjlee0 Sangjin Lee added a comment -

          Thanks for updating the patch Xuan Gong.

          (1)
          It looks like the secondary classloading pattern has still not been addressed. I'm referring to setting the created classloader onto the configuration as well as thread context classloader. It is not sufficient to simply load the main aux service class using that classloader. That works for the cases where other dependent classes are loaded via normal class references from it, but does nothing to handle classloading via Configuration.getClass() or reflection using the thread context classloader. If we do not address them, we will get fatal errors the moment an aux service does those types of classloading, each of which will become a bug. This is definitely a requirement IMO.

          Fortunately we have a fairly well-defined set of call points that call into the aux services. We can surround them with setting and unsetting the configuration classloader as well as thread context classloader (see the comments above for how it is done). It is not exact, but it is certainly necessary. Let me know if you have any questions.

          (2)
          Are we supporting hdfs paths as part of the aux classpaths? I thought that you mentioned that it does not have to be done as part of this JIRA. If that is the case, why do we still need to set the URL stream handler factory? The JVM's URL stream handler factory is capable of handling all local paths.

          (3)
          Assuming we don't need to support hdfs paths, can't we simply rely on ApplicationClassLoader to construct the URLs from the classpath? Is there a reason we need to replicate the constructUrlsFromClasspath()? It would be good if we can rely on the common implementation, and improve it if there is a missing piece.

          Show
          sjlee0 Sangjin Lee added a comment - Thanks for updating the patch Xuan Gong . (1) It looks like the secondary classloading pattern has still not been addressed. I'm referring to setting the created classloader onto the configuration as well as thread context classloader. It is not sufficient to simply load the main aux service class using that classloader. That works for the cases where other dependent classes are loaded via normal class references from it, but does nothing to handle classloading via Configuration.getClass() or reflection using the thread context classloader. If we do not address them, we will get fatal errors the moment an aux service does those types of classloading, each of which will become a bug. This is definitely a requirement IMO. Fortunately we have a fairly well-defined set of call points that call into the aux services. We can surround them with setting and unsetting the configuration classloader as well as thread context classloader (see the comments above for how it is done). It is not exact, but it is certainly necessary. Let me know if you have any questions. (2) Are we supporting hdfs paths as part of the aux classpaths? I thought that you mentioned that it does not have to be done as part of this JIRA. If that is the case, why do we still need to set the URL stream handler factory? The JVM's URL stream handler factory is capable of handling all local paths. (3) Assuming we don't need to support hdfs paths, can't we simply rely on ApplicationClassLoader to construct the URLs from the classpath? Is there a reason we need to replicate the constructUrlsFromClasspath() ? It would be good if we can rely on the common implementation, and improve it if there is a missing piece.
          Hide
          xgong Xuan Gong added a comment -

          Sangjin Lee Thanks for the review. Looks like we reviewed a incorrect patch.

          https://issues.apache.org/jira/secure/attachment/12786365/YARN-4577.20160204.patch is correct one..

          Sorry for the inconsistent naming of the patch. Could you review the patch ?

          Show
          xgong Xuan Gong added a comment - Sangjin Lee Thanks for the review. Looks like we reviewed a incorrect patch. https://issues.apache.org/jira/secure/attachment/12786365/YARN-4577.20160204.patch is correct one.. Sorry for the inconsistent naming of the patch. Could you review the patch ?
          Hide
          sjlee0 Sangjin Lee added a comment -

          My apologies Xuan Gong! I must have looked at the last attachments. I'll take a look at it, and get back to you soon.

          Show
          sjlee0 Sangjin Lee added a comment - My apologies Xuan Gong ! I must have looked at the last attachments. I'll take a look at it, and get back to you soon.
          Hide
          sjlee0 Sangjin Lee added a comment -

          I think I need to clarify a little more what needs to be done with the custom classloader. The custom classloader for a given aux service must be created only once and used throughout for any code that exercises that aux service's code.

          I don't think you need to wrap creating the aux service with a callWithClassLoader() call. That's really not necessary. What you do need is, you need to intercept subsequent calls on the aux service and wrap them with callWithClassLoader(). And when you do, you'd need to provide that exact custom classloader instance you created when you created the aux service. Otherwise, things like ClassCastException can ensue.

          So all overridable calls to the public methods like initializeApplication(), stopApplication(), getMetaData(), initializeContainer(), stopContainer(), as well as service lifecycle methods such as serviceStart(), serviceInit(), and serviceStop() should be intercepted.

          Now, intercepting the aux-service-specific methods is easier as there are explicit places where they are invoked. What's more difficult is the service lifecycle methods as they are invoked from AuxServices only indirectly.

          I can see 2 approaches to this (there may be more). First, we could consider adding a wrapper class that can do this within the wrapper code. It might be something like

          class AuxiliaryServiceWithCustomClassLoader extends AuxiliaryService {
            private final AuxiliaryService wrapped;
            private final ClassLoader customClassLoader;
            
            public AuxiliaryServiceWithCustomClassLoader(AuxiliaryService s, ClassLoader cl) {
              this.wrapped = s;
              this.customClassLoader = cl;
            }
            ...
            @Override
            protected void serviceStart() throws Exception {
              callWithCustomClassLoader(wrapped.serviceStart());
            }
            ...
            @Override
            public void initializeApplication(ApplicationInitializationContext context) {
              callWithCustomClassLoader(wrapper.initializeApplication(context));
            }
            ...
            private callWithCustomClassLoader(...) {
            }
          }
          

          The other approach may be to abandon dealing with the Configuration.getClass() and the thread context classloader scenario, and requiring any aux service that wants to use a custom classpath NOT to do anything that will trigger Configuration.getClass() or the use of the TCCL, either directly or indirectly. Then we don't need to do all this wrapping business.

          What I'm not sure of is how practical this requirement would be for those aux services. Note that this has to be true even directly.

          I also need to point out one potential risk with this wrapping. One potential risk is that the configuration is a shared object. If one aux service resets the configuration classloader to invoke its code, any other NM code that's running concurrently could be impacted potentially. For example, if two aux services have a race (on separate threads) setting the configuration class, then we could have a nasty problem.

          Could you consider the implications of these options? Perhaps requiring these aux services not to do any Configuration.getClass() might not be that unreasonable...

          Show
          sjlee0 Sangjin Lee added a comment - I think I need to clarify a little more what needs to be done with the custom classloader. The custom classloader for a given aux service must be created only once and used throughout for any code that exercises that aux service's code. I don't think you need to wrap creating the aux service with a callWithClassLoader() call. That's really not necessary. What you do need is, you need to intercept subsequent calls on the aux service and wrap them with callWithClassLoader() . And when you do, you'd need to provide that exact custom classloader instance you created when you created the aux service. Otherwise, things like ClassCastException can ensue. So all overridable calls to the public methods like initializeApplication() , stopApplication() , getMetaData() , initializeContainer() , stopContainer() , as well as service lifecycle methods such as serviceStart() , serviceInit() , and serviceStop() should be intercepted. Now, intercepting the aux-service-specific methods is easier as there are explicit places where they are invoked. What's more difficult is the service lifecycle methods as they are invoked from AuxServices only indirectly. I can see 2 approaches to this (there may be more). First, we could consider adding a wrapper class that can do this within the wrapper code. It might be something like class AuxiliaryServiceWithCustomClassLoader extends AuxiliaryService { private final AuxiliaryService wrapped; private final ClassLoader customClassLoader; public AuxiliaryServiceWithCustomClassLoader(AuxiliaryService s, ClassLoader cl) { this .wrapped = s; this .customClassLoader = cl; } ... @Override protected void serviceStart() throws Exception { callWithCustomClassLoader(wrapped.serviceStart()); } ... @Override public void initializeApplication(ApplicationInitializationContext context) { callWithCustomClassLoader(wrapper.initializeApplication(context)); } ... private callWithCustomClassLoader(...) { } } The other approach may be to abandon dealing with the Configuration.getClass() and the thread context classloader scenario, and requiring any aux service that wants to use a custom classpath NOT to do anything that will trigger Configuration.getClass() or the use of the TCCL, either directly or indirectly. Then we don't need to do all this wrapping business. What I'm not sure of is how practical this requirement would be for those aux services. Note that this has to be true even directly. I also need to point out one potential risk with this wrapping. One potential risk is that the configuration is a shared object. If one aux service resets the configuration classloader to invoke its code, any other NM code that's running concurrently could be impacted potentially. For example, if two aux services have a race (on separate threads) setting the configuration class, then we could have a nasty problem. Could you consider the implications of these options? Perhaps requiring these aux services not to do any Configuration.getClass() might not be that unreasonable...
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Just caught up with the wall of comments.

          +1 in general for the ApplicationClassloader based solution. Aux-services was always a hack since Chris Douglas and I originally introduced it, the better solution is to move these services as first-class apps on top of YARN, but we are where we are.

          Sangjin Lee,

          For example, if the aux service code depends on another class property (owned by the aux service) in the configuration, that will be invoked via Configuration.getClass(), and it will still use the system classloader to load that class. Then it's very likely that you'll get a ClassNotFoundException.

          Sangjin, you may be missing one important thing here - unlike in the MapReduce case, there is no shared Configuration object between NodeManager and the specific aux-service implementation here. We simply do not pass in any configuration anywhere as part of the AuxService APIs - so this entire thread of reasoning about getClass() is no long a problem? If needed, we can document advising against adding Conf as part of future API changes.

          The thread context classloader represents another similar problem. The moment the aux service code hits a code path that does Class.forName() that loads classes via the thread context classloader, and it needs to load an aux service-related class (that is not present in the main NM classpath), you will get a ClassNotFoundException.

          In addition to wrapping aux-service API calls under a class-loader, wouldn't it suffice to simply have NM make all aux-services API calls in a separate thread whose ContextClassLoader is changed to be another custom one that resolves both System classes as well as AuxServices classes?

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Just caught up with the wall of comments. +1 in general for the ApplicationClassloader based solution. Aux-services was always a hack since Chris Douglas and I originally introduced it, the better solution is to move these services as first-class apps on top of YARN, but we are where we are. Sangjin Lee , For example, if the aux service code depends on another class property (owned by the aux service) in the configuration, that will be invoked via Configuration.getClass(), and it will still use the system classloader to load that class. Then it's very likely that you'll get a ClassNotFoundException. Sangjin, you may be missing one important thing here - unlike in the MapReduce case, there is no shared Configuration object between NodeManager and the specific aux-service implementation here. We simply do not pass in any configuration anywhere as part of the AuxService APIs - so this entire thread of reasoning about getClass() is no long a problem? If needed, we can document advising against adding Conf as part of future API changes. The thread context classloader represents another similar problem. The moment the aux service code hits a code path that does Class.forName() that loads classes via the thread context classloader, and it needs to load an aux service-related class (that is not present in the main NM classpath), you will get a ClassNotFoundException. In addition to wrapping aux-service API calls under a class-loader, wouldn't it suffice to simply have NM make all aux-services API calls in a separate thread whose ContextClassLoader is changed to be another custom one that resolves both System classes as well as AuxServices classes?
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          We simply do not pass in any configuration anywhere as part of the AuxService APIs - so this entire thread of reasoning about getClass() is no long a problem?

          Xuan Gong reminded me offline that we do pass a shared configuration as part of serviceInit(). In that case, the solution is simply to pass a private cloned Configuration for each of the aux-services?

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - We simply do not pass in any configuration anywhere as part of the AuxService APIs - so this entire thread of reasoning about getClass() is no long a problem? Xuan Gong reminded me offline that we do pass a shared configuration as part of serviceInit(). In that case, the solution is simply to pass a private cloned Configuration for each of the aux-services?
          Hide
          sjlee0 Sangjin Lee added a comment -

          Thanks for your comments Vinod Kumar Vavilapalli.

          Xuan Gong reminded me offline that we do pass a shared configuration as part of serviceInit(). In that case, the solution is simply to pass a private cloned Configuration for each of the aux-services?

          Yes, that's exactly the scenario under which an aux service gets a reference to the shared configuration. Resetting the configuration classloader on a shared configuration is a big risk.

          I like your idea about creating a copy of the configuration. The only implication is that the aux services will then never see any changes made to the configuration past serviceInit(). I don't think that's a deal breaker. We could tweak the above code pattern I suggested to achieve this:

          class AuxiliaryServiceWithCustomClassLoader extends AuxiliaryService {
            private final AuxiliaryService wrapped;
            private final ClassLoader customClassLoader;
            private Configuration conf;
            
            public AuxiliaryServiceWithCustomClassLoader(AuxiliaryService s, ClassLoader cl) {
              this.wrapped = s;
              this.customClassLoader = cl;
            }
          
            @Override
            protected void serviceInit(Configuration conf) throws Exception {
              this.conf = new Configuration(conf);
              this.conf.setClassLoader(customClassLoader);
              // call the wrapped service
              callWithCustomClassLoader(wrapped.serviceInit(this.conf));
            }
            ...
            @Override
            protected void serviceStart() throws Exception {
              callWithCustomClassLoader(wrapped.serviceStart());
            }
            ...
            @Override
            public void initializeApplication(ApplicationInitializationContext context) {
              callWithCustomClassLoader(wrapper.initializeApplication(context));
            }
            ...
            private callWithCustomClassLoader(...) {
            }
          }
          

          Then the only thing callWithCustomClassLoader() needs to deal with is setting and unsetting the thread context classloader.

          In addition to wrapping aux-service API calls under a class-loader, wouldn't it suffice to simply have NM make all aux-services API calls in a separate thread whose ContextClassLoader is changed to be another custom one that resolves both System classes as well as AuxServices classes?

          If you can dedicate a thread for each aux service, that can work too. However, we need to ensure all calls to the aux services (including the service*() methods) are made on those threads and not on any other thread. I think that might make things more complicated than the other way around. The callWithCustomClassLoader() method can be very simple. All it needs to do is

          ClassLoader original = Thread.currentThread().getContextClassLoader();
          Thread.currentThread().setContextClassLoader(customClassLoader);
          try {
            // the code that is being wrapped
          } finally {
            Thread.currentThread().setContextClassLoader(original);
          }
          
          Show
          sjlee0 Sangjin Lee added a comment - Thanks for your comments Vinod Kumar Vavilapalli . Xuan Gong reminded me offline that we do pass a shared configuration as part of serviceInit(). In that case, the solution is simply to pass a private cloned Configuration for each of the aux-services? Yes, that's exactly the scenario under which an aux service gets a reference to the shared configuration. Resetting the configuration classloader on a shared configuration is a big risk. I like your idea about creating a copy of the configuration. The only implication is that the aux services will then never see any changes made to the configuration past serviceInit() . I don't think that's a deal breaker. We could tweak the above code pattern I suggested to achieve this: class AuxiliaryServiceWithCustomClassLoader extends AuxiliaryService { private final AuxiliaryService wrapped; private final ClassLoader customClassLoader; private Configuration conf; public AuxiliaryServiceWithCustomClassLoader(AuxiliaryService s, ClassLoader cl) { this .wrapped = s; this .customClassLoader = cl; } @Override protected void serviceInit(Configuration conf) throws Exception { this .conf = new Configuration(conf); this .conf.setClassLoader(customClassLoader); // call the wrapped service callWithCustomClassLoader(wrapped.serviceInit( this .conf)); } ... @Override protected void serviceStart() throws Exception { callWithCustomClassLoader(wrapped.serviceStart()); } ... @Override public void initializeApplication(ApplicationInitializationContext context) { callWithCustomClassLoader(wrapper.initializeApplication(context)); } ... private callWithCustomClassLoader(...) { } } Then the only thing callWithCustomClassLoader() needs to deal with is setting and unsetting the thread context classloader. In addition to wrapping aux-service API calls under a class-loader, wouldn't it suffice to simply have NM make all aux-services API calls in a separate thread whose ContextClassLoader is changed to be another custom one that resolves both System classes as well as AuxServices classes? If you can dedicate a thread for each aux service, that can work too. However, we need to ensure all calls to the aux services ( including the service*() methods) are made on those threads and not on any other thread. I think that might make things more complicated than the other way around. The callWithCustomClassLoader() method can be very simple. All it needs to do is ClassLoader original = Thread .currentThread().getContextClassLoader(); Thread .currentThread().setContextClassLoader(customClassLoader); try { // the code that is being wrapped } finally { Thread .currentThread().setContextClassLoader(original); }
          Hide
          sjlee0 Sangjin Lee added a comment -

          I think the above should be a viable solution that addresses all types of classloading.

          Show
          sjlee0 Sangjin Lee added a comment - I think the above should be a viable solution that addresses all types of classloading.
          Hide
          xgong Xuan Gong added a comment -

          Sangjin Lee Thanks for the suggestion. I have uploaded a poc patch for this. Could you take a look, please ?

          Show
          xgong Xuan Gong added a comment - Sangjin Lee Thanks for the suggestion. I have uploaded a poc patch for this. Could you take a look, please ?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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.
          0 mvndep 0m 20s Maven dependency ordering for branch
          +1 mvninstall 6m 57s trunk passed
          +1 compile 1m 50s trunk passed with JDK v1.8.0_77
          +1 compile 2m 6s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 37s trunk passed
          +1 mvnsite 0m 55s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 2m 3s trunk passed
          +1 javadoc 0m 57s trunk passed with JDK v1.8.0_77
          +1 javadoc 3m 16s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 0m 46s the patch passed
          +1 compile 1m 50s the patch passed with JDK v1.8.0_77
          -1 javac 2m 46s hadoop-yarn-project_hadoop-yarn-jdk1.8.0_77 with JDK v1.8.0_77 generated 1 new + 22 unchanged - 0 fixed = 23 total (was 22)
          +1 javac 1m 50s the patch passed
          +1 compile 2m 7s the patch passed with JDK v1.7.0_95
          -1 javac 4m 53s hadoop-yarn-project_hadoop-yarn-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 25 unchanged - 0 fixed = 26 total (was 25)
          +1 javac 2m 7s the patch passed
          -1 checkstyle 0m 35s hadoop-yarn-project/hadoop-yarn: patch generated 2 new + 261 unchanged - 0 fixed = 263 total (was 261)
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 20s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          -1 findbugs 1m 4s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 0m 53s the patch passed with JDK v1.8.0_77
          +1 javadoc 3m 21s the patch passed with JDK v1.7.0_95
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_77.
          +1 unit 11m 2s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_77.
          +1 unit 0m 23s hadoop-yarn-api in the patch passed with JDK v1.7.0_95.
          +1 unit 11m 33s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 19s Patch does not generate ASF License warnings.
          57m 49s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
            org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices.serviceInit(Configuration) creates a org.apache.hadoop.util.ApplicationClassLoader classloader, which should be performed within a doPrivileged block At AuxServices.java:which should be performed within a doPrivileged block At AuxServices.java:[line 133]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799895/YARN-4577.poc.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 5189f947e974 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 1e48eef
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac hadoop-yarn-project_hadoop-yarn-jdk1.8.0_77: https://builds.apache.org/job/PreCommit-YARN-Build/11153/artifact/patchprocess/diff-compile-javac-hadoop-yarn-project_hadoop-yarn-jdk1.8.0_77.txt
          javac hadoop-yarn-project_hadoop-yarn-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-YARN-Build/11153/artifact/patchprocess/diff-compile-javac-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_95.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11153/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11153/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.html
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11153/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11153/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 0m 20s Maven dependency ordering for branch +1 mvninstall 6m 57s trunk passed +1 compile 1m 50s trunk passed with JDK v1.8.0_77 +1 compile 2m 6s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 37s trunk passed +1 mvnsite 0m 55s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 2m 3s trunk passed +1 javadoc 0m 57s trunk passed with JDK v1.8.0_77 +1 javadoc 3m 16s trunk passed with JDK v1.7.0_95 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 0m 46s the patch passed +1 compile 1m 50s the patch passed with JDK v1.8.0_77 -1 javac 2m 46s hadoop-yarn-project_hadoop-yarn-jdk1.8.0_77 with JDK v1.8.0_77 generated 1 new + 22 unchanged - 0 fixed = 23 total (was 22) +1 javac 1m 50s the patch passed +1 compile 2m 7s the patch passed with JDK v1.7.0_95 -1 javac 4m 53s hadoop-yarn-project_hadoop-yarn-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 25 unchanged - 0 fixed = 26 total (was 25) +1 javac 2m 7s the patch passed -1 checkstyle 0m 35s hadoop-yarn-project/hadoop-yarn: patch generated 2 new + 261 unchanged - 0 fixed = 263 total (was 261) +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 20s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 1m 4s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 53s the patch passed with JDK v1.8.0_77 +1 javadoc 3m 21s the patch passed with JDK v1.7.0_95 +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_77. +1 unit 11m 2s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_77. +1 unit 0m 23s hadoop-yarn-api in the patch passed with JDK v1.7.0_95. +1 unit 11m 33s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 19s Patch does not generate ASF License warnings. 57m 49s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager   org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices.serviceInit(Configuration) creates a org.apache.hadoop.util.ApplicationClassLoader classloader, which should be performed within a doPrivileged block At AuxServices.java:which should be performed within a doPrivileged block At AuxServices.java: [line 133] Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799895/YARN-4577.poc.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 5189f947e974 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1e48eef Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac hadoop-yarn-project_hadoop-yarn-jdk1.8.0_77: https://builds.apache.org/job/PreCommit-YARN-Build/11153/artifact/patchprocess/diff-compile-javac-hadoop-yarn-project_hadoop-yarn-jdk1.8.0_77.txt javac hadoop-yarn-project_hadoop-yarn-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-YARN-Build/11153/artifact/patchprocess/diff-compile-javac-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_95.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11153/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11153/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.html JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11153/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11153/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sjlee0 Sangjin Lee added a comment -

          Yes I think the POC patch is pretty close to what I had in mind too.

          A couple of more minor suggestions:

          • I probably wouldn't make AuxServiceWithCustomClassLoader public. It should be really visible only to AuxServices. Package scope should be fine.
          • I understand callWithCustomClassLoader() is bit complicated because it has to support methods with different signatures. I would simply inline the code (as you are doing with service*() methods). Then you don't have to do any reflection business to do this.

          Don't forget to test it with a real-life use case! Thanks.

          Show
          sjlee0 Sangjin Lee added a comment - Yes I think the POC patch is pretty close to what I had in mind too. A couple of more minor suggestions: I probably wouldn't make AuxServiceWithCustomClassLoader public. It should be really visible only to AuxServices . Package scope should be fine. I understand callWithCustomClassLoader() is bit complicated because it has to support methods with different signatures. I would simply inline the code (as you are doing with service*() methods). Then you don't have to do any reflection business to do this. Don't forget to test it with a real-life use case! Thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 docker 0m 2s Docker failed to build yetus/hadoop:7b1c37a.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801074/YARN-4577.5.patch
          JIRA Issue YARN-4577
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11242/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 docker 0m 2s Docker failed to build yetus/hadoop:7b1c37a. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801074/YARN-4577.5.patch JIRA Issue YARN-4577 Console output https://builds.apache.org/job/PreCommit-YARN-Build/11242/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          xgong Xuan Gong added a comment -

          Sangjin Lee Thanks for the review.

          Attached a new patch to address the comments.

          Unfortunately, I am not able to create a unit test for this. But I did test it manually.

          Here is how I test it:
          1. Create a customized TestAuxService which extends AuxiliaryService.
          2. Create two jar file which have the same jar file name: TestAuxSerivce.jar and have the same class name: TestAuxService.java
          3. Each TestAuxService.java has different log message. something like "TestAuxService in NM ClassPath" and "TestAuxService in Customer ClassPath"
          4. Put one TestAuxService.jar into NM ClassPath, and put another TestAuxService.jar into customer class path, such as "/Users/xuan/dep/TestAuxService.jar"
          5. modify several configuration in YARN-SITE.XML

              <property>
                  <name>yarn.nodemanager.aux-services</name>
                  <value>mapreduce_shuffle,TestAuxService</value>
                  <description>shuffle service that needs to be set for Map Reduce to run </description>
              </property>
          
            <property>
                <name>yarn.nodemanager.aux-services.TestAuxService.class</name>
                <value>org.aux.TestAuxService</value>
            </property>
          

          6. start NM, and verified the log message in NM logs, we can see

          Test My AuxService in NM ClassPath in Service Init stage
          Test My AuxService in NM ClassPath in Service Start stage
          

          And we can verify that we load the TestAuxService class from NM Class Path
          7. add one more configuration into yarn-site.xml

              <property>
                  <name>yarn.nodemanager.aux-services.TestAuxService.class.classpath</name>
                  <value>/Users/xuan/dep/TestAuxService.jar</value>
              </property>
          

          8. Start NM, and check log message in NM log, we can find

          Test My AuxService in Customer ClassPath in Service Init stage
          Test My AuxService in Customer ClassPath in Service Start stage
          

          we can verify that if we set the customer class path, we would load TestAuxService from customer class path instead of NM classpath.

          Show
          xgong Xuan Gong added a comment - Sangjin Lee Thanks for the review. Attached a new patch to address the comments. Unfortunately, I am not able to create a unit test for this. But I did test it manually. Here is how I test it: 1. Create a customized TestAuxService which extends AuxiliaryService. 2. Create two jar file which have the same jar file name: TestAuxSerivce.jar and have the same class name: TestAuxService.java 3. Each TestAuxService.java has different log message. something like "TestAuxService in NM ClassPath" and "TestAuxService in Customer ClassPath" 4. Put one TestAuxService.jar into NM ClassPath, and put another TestAuxService.jar into customer class path, such as "/Users/xuan/dep/TestAuxService.jar" 5. modify several configuration in YARN-SITE.XML <property> <name>yarn.nodemanager.aux-services</name> <value>mapreduce_shuffle,TestAuxService</value> <description>shuffle service that needs to be set for Map Reduce to run </description> </property> <property> <name>yarn.nodemanager.aux-services.TestAuxService.class</name> <value>org.aux.TestAuxService</value> </property> 6. start NM, and verified the log message in NM logs, we can see Test My AuxService in NM ClassPath in Service Init stage Test My AuxService in NM ClassPath in Service Start stage And we can verify that we load the TestAuxService class from NM Class Path 7. add one more configuration into yarn-site.xml <property> <name>yarn.nodemanager.aux-services.TestAuxService.class.classpath</name> <value>/Users/xuan/dep/TestAuxService.jar</value> </property> 8. Start NM, and check log message in NM log, we can find Test My AuxService in Customer ClassPath in Service Init stage Test My AuxService in Customer ClassPath in Service Start stage we can verify that if we set the customer class path, we would load TestAuxService from customer class path instead of NM classpath.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 docker 0m 4s Docker failed to build yetus/hadoop:7b1c37a.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801074/YARN-4577.5.patch
          JIRA Issue YARN-4577
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11244/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 docker 0m 4s Docker failed to build yetus/hadoop:7b1c37a. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801074/YARN-4577.5.patch JIRA Issue YARN-4577 Console output https://builds.apache.org/job/PreCommit-YARN-Build/11244/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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.
          0 mvndep 0m 11s Maven dependency ordering for branch
          +1 mvninstall 8m 20s trunk passed
          +1 compile 2m 52s trunk passed with JDK v1.8.0_92
          +1 compile 2m 46s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 40s trunk passed
          +1 mvnsite 1m 6s trunk passed
          +1 mvneclipse 0m 27s trunk passed
          +1 findbugs 2m 26s trunk passed
          +1 javadoc 1m 20s trunk passed with JDK v1.8.0_92
          +1 javadoc 3m 26s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 1m 0s the patch passed
          +1 compile 3m 35s the patch passed with JDK v1.8.0_92
          +1 javac 3m 35s the patch passed
          +1 compile 2m 47s the patch passed with JDK v1.7.0_95
          +1 javac 2m 47s the patch passed
          -1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 263 unchanged - 0 fixed = 264 total (was 263)
          +1 mvnsite 1m 5s the patch passed
          +1 mvneclipse 0m 26s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          -1 findbugs 1m 23s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 1m 26s the patch passed with JDK v1.8.0_92
          +1 javadoc 3m 23s the patch passed with JDK v1.7.0_95
          +1 unit 0m 37s hadoop-yarn-api in the patch passed with JDK v1.8.0_92.
          +1 unit 12m 43s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_92.
          +1 unit 0m 30s hadoop-yarn-api in the patch passed with JDK v1.7.0_95.
          +1 unit 12m 28s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 25s Patch does not generate ASF License warnings.
          69m 51s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
            org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices.serviceInit(Configuration) creates a org.apache.hadoop.util.ApplicationClassLoader classloader, which should be performed within a doPrivileged block At AuxServices.java:which should be performed within a doPrivileged block At AuxServices.java:[line 132]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801074/YARN-4577.5.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a23f8d9763a7 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6243eab
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_92 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11276/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11276/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.html
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11276/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11276/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 0m 11s Maven dependency ordering for branch +1 mvninstall 8m 20s trunk passed +1 compile 2m 52s trunk passed with JDK v1.8.0_92 +1 compile 2m 46s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 40s trunk passed +1 mvnsite 1m 6s trunk passed +1 mvneclipse 0m 27s trunk passed +1 findbugs 2m 26s trunk passed +1 javadoc 1m 20s trunk passed with JDK v1.8.0_92 +1 javadoc 3m 26s trunk passed with JDK v1.7.0_95 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 0s the patch passed +1 compile 3m 35s the patch passed with JDK v1.8.0_92 +1 javac 3m 35s the patch passed +1 compile 2m 47s the patch passed with JDK v1.7.0_95 +1 javac 2m 47s the patch passed -1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 263 unchanged - 0 fixed = 264 total (was 263) +1 mvnsite 1m 5s the patch passed +1 mvneclipse 0m 26s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 1m 23s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 1m 26s the patch passed with JDK v1.8.0_92 +1 javadoc 3m 23s the patch passed with JDK v1.7.0_95 +1 unit 0m 37s hadoop-yarn-api in the patch passed with JDK v1.8.0_92. +1 unit 12m 43s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_92. +1 unit 0m 30s hadoop-yarn-api in the patch passed with JDK v1.7.0_95. +1 unit 12m 28s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 69m 51s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager   org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServices.serviceInit(Configuration) creates a org.apache.hadoop.util.ApplicationClassLoader classloader, which should be performed within a doPrivileged block At AuxServices.java:which should be performed within a doPrivileged block At AuxServices.java: [line 132] Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801074/YARN-4577.5.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a23f8d9763a7 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6243eab Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_92 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11276/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11276/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.html JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11276/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11276/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sjlee0 Sangjin Lee added a comment -

          Would you be able to come up with a unit test still? I know it's somewhat tricky, but you might be able to find a way to test a few things. You might want to take a look at TestRunJar.testClientClassLoader() to see if you can reuse some of the ideas there.

          Coupled with the unit test strategy, you might want to consider supporting some type of an override mechanism for system classes. Other use cases of ApplicationClassLoader provide this (although it's case by case). If users ever run into a classloading issue which can be fixed by a small modification to the system classes, providing the override mechanism would be a big win. And it might come in handy in writing unit tests too.

          Also, please take a look at the checkstyle and findbug issues.

          (AuxServices.java)

          • l.132: some INFO level logging here would be helpful (like the name of the aux service that's using the custom classloader, etc.)
          • l.146: I know it's existing code, but the C-style equal check is not necessary with java. I would simply go with sClass == null.
          • l.160: same as above.

          (AuxiliaryServiceWithCustomClassLoader.java)

          • in serviceInit(), serviceStart(), serviceStop(), shouldn't we call wrapped.serviceInit() instead of wrapper.init(), and so on?
          • l.46: We might want to change this a little. It appears AbstractService.init() already sets the configuration (with the original configuration) before AuxiliaryServiceWithCustomClassLoader.serviceInit() is invoked. This may lead to confusion down the road. Let's reset the configuration here via setConfig(). In other words,
            @Override
            protected void serviceInit(Configuration conf) throws Exception {
              Configuration config = new Configuration(conf);
              // reset the service configuration
              setConfig(config);
              config.setClassLoader(customClassLoader);
              ClassLoader original = Thread.currentThread().getContextClassLoader();
              Thread.currentThread().setContextClassLoader(customClassLoader);
              try {
                wrapped.serviceInit(config);
              } finally {
                Thread.currentThread().setContextClassLoader(original);
              }
            }
            

            Also, I don't think we need Configuration as a member variable.

          Show
          sjlee0 Sangjin Lee added a comment - Would you be able to come up with a unit test still? I know it's somewhat tricky, but you might be able to find a way to test a few things. You might want to take a look at TestRunJar.testClientClassLoader() to see if you can reuse some of the ideas there. Coupled with the unit test strategy, you might want to consider supporting some type of an override mechanism for system classes. Other use cases of ApplicationClassLoader provide this (although it's case by case). If users ever run into a classloading issue which can be fixed by a small modification to the system classes, providing the override mechanism would be a big win. And it might come in handy in writing unit tests too. Also, please take a look at the checkstyle and findbug issues. (AuxServices.java) l.132: some INFO level logging here would be helpful (like the name of the aux service that's using the custom classloader, etc.) l.146: I know it's existing code, but the C-style equal check is not necessary with java. I would simply go with sClass == null . l.160: same as above. (AuxiliaryServiceWithCustomClassLoader.java) in serviceInit() , serviceStart() , serviceStop() , shouldn't we call wrapped.serviceInit() instead of wrapper.init() , and so on? l.46: We might want to change this a little. It appears AbstractService.init() already sets the configuration (with the original configuration) before AuxiliaryServiceWithCustomClassLoader.serviceInit() is invoked. This may lead to confusion down the road. Let's reset the configuration here via setConfig() . In other words, @Override protected void serviceInit(Configuration conf) throws Exception { Configuration config = new Configuration(conf); // reset the service configuration setConfig(config); config.setClassLoader(customClassLoader); ClassLoader original = Thread .currentThread().getContextClassLoader(); Thread .currentThread().setContextClassLoader(customClassLoader); try { wrapped.serviceInit(config); } finally { Thread .currentThread().setContextClassLoader(original); } } Also, I don't think we need Configuration as a member variable.
          Hide
          xgong Xuan Gong added a comment -

          Thanks for the view, Sangjin Lee

          in serviceInit(), serviceStart(), serviceStop(), shouldn't we call wrapped.serviceInit() instead of wrapper.init(), and so on?

          Looks like the serviceInit/serviceStart/serviceStop are protected functions, so we can not do that. When we call init(), it would automatically call serviceInit.

          Coupled with the unit test strategy, you might want to consider supporting some type of an override mechanism for system classes. Other use cases of ApplicationClassLoader provide this (although it's case by case).

          Thought about it. Add the improvement for it.

          Would you be able to come up with a unit test still? I know it's somewhat tricky, but you might be able to find a way to test a few things. You might want to take a look at TestRunJar.testClientClassLoader() to see if you can reuse some of the ideas there.

          Tried several different ways to test it. But it is hard. A good unit test for this could be similar as I did for the manual testing. But unfortunately, I do not know how to do it.

          Show
          xgong Xuan Gong added a comment - Thanks for the view, Sangjin Lee in serviceInit(), serviceStart(), serviceStop(), shouldn't we call wrapped.serviceInit() instead of wrapper.init(), and so on? Looks like the serviceInit/serviceStart/serviceStop are protected functions, so we can not do that. When we call init(), it would automatically call serviceInit. Coupled with the unit test strategy, you might want to consider supporting some type of an override mechanism for system classes. Other use cases of ApplicationClassLoader provide this (although it's case by case). Thought about it. Add the improvement for it. Would you be able to come up with a unit test still? I know it's somewhat tricky, but you might be able to find a way to test a few things. You might want to take a look at TestRunJar.testClientClassLoader() to see if you can reuse some of the ideas there. Tried several different ways to test it. But it is hard. A good unit test for this could be similar as I did for the manual testing. But unfortunately, I do not know how to do it.
          Show
          xgong Xuan Gong added a comment - Uploaded a new patch: https://issues.apache.org/jira/secure/attachment/12801375/YARN-4577.20160428.patch
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 27s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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.
          0 mvndep 0m 15s Maven dependency ordering for branch
          +1 mvninstall 6m 31s trunk passed
          +1 compile 1m 45s trunk passed with JDK v1.8.0_91
          +1 compile 2m 4s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 35s trunk passed
          +1 mvnsite 0m 54s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 2m 0s trunk passed
          +1 javadoc 0m 54s trunk passed with JDK v1.8.0_91
          +1 javadoc 3m 13s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 0m 47s the patch passed
          +1 compile 1m 43s the patch passed with JDK v1.8.0_91
          +1 javac 1m 43s the patch passed
          +1 compile 2m 2s the patch passed with JDK v1.7.0_95
          +1 javac 2m 2s the patch passed
          -1 checkstyle 0m 34s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 263 unchanged - 0 fixed = 264 total (was 263)
          +1 mvnsite 0m 51s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 21s the patch passed
          +1 javadoc 0m 50s the patch passed with JDK v1.8.0_91
          +1 javadoc 3m 13s the patch passed with JDK v1.7.0_95
          +1 unit 0m 20s hadoop-yarn-api in the patch passed with JDK v1.8.0_91.
          +1 unit 11m 6s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_91.
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.7.0_95.
          +1 unit 11m 44s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 20s Patch does not generate ASF License warnings.
          57m 2s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801375/YARN-4577.20160428.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f6d325834bde 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6243eab
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11277/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11277/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11277/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 27s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 0m 15s Maven dependency ordering for branch +1 mvninstall 6m 31s trunk passed +1 compile 1m 45s trunk passed with JDK v1.8.0_91 +1 compile 2m 4s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 35s trunk passed +1 mvnsite 0m 54s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 2m 0s trunk passed +1 javadoc 0m 54s trunk passed with JDK v1.8.0_91 +1 javadoc 3m 13s trunk passed with JDK v1.7.0_95 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 0m 47s the patch passed +1 compile 1m 43s the patch passed with JDK v1.8.0_91 +1 javac 1m 43s the patch passed +1 compile 2m 2s the patch passed with JDK v1.7.0_95 +1 javac 2m 2s the patch passed -1 checkstyle 0m 34s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 263 unchanged - 0 fixed = 264 total (was 263) +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 21s the patch passed +1 javadoc 0m 50s the patch passed with JDK v1.8.0_91 +1 javadoc 3m 13s the patch passed with JDK v1.7.0_95 +1 unit 0m 20s hadoop-yarn-api in the patch passed with JDK v1.8.0_91. +1 unit 11m 6s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_91. +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.7.0_95. +1 unit 11m 44s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 20s Patch does not generate ASF License warnings. 57m 2s Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801375/YARN-4577.20160428.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f6d325834bde 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6243eab Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11277/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11277/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11277/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Tx for taking care of this, Xuan Gong and Sangjin Lee!

          Few comments on the patch

          • YarnConfiguration:
            • class.classpath -> classpath
            • classloader.system.classes -> system-classes
            • Similarly rename the corresponding constants
            • Add the new configs to yarn-default.xml, if only commented out
          • Change definitions of NM_AUX_SERVICE_CLASS_CLASSPATH, NM_AUX_SERVICE_CLASSLOADER_SYSTEM_CLASSES to use the other constant NM_AUX_SERVICES.
          • AuxServices
            • createJobClassLoader -> createAuxServiceClassLoader()
            • systemClass -> systemClasses
          • The validation code to check the aux-service names so that ServiceData can be read and written properly should be done for aux-services loaded from a different classpath too.
          • AuxiliaryServiceWithCustomClassLoader
            • Add a code comment as to why we are create new Configuration objects (a summary of what we discussed above).
            • Can we change the code such that AuxiliaryServiceWithCustomClassLoader simply takes in the class-name, classpath and system-classpath and does everything internally? This will keep the AuxServices class code cleaner.
          • We should definitely add a test-case. I think the simpler way to do this is to dynamically generate classes. Let me think about a bit while you address the remaining comments.
          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Tx for taking care of this, Xuan Gong and Sangjin Lee ! Few comments on the patch YarnConfiguration: class.classpath -> classpath classloader.system.classes -> system-classes Similarly rename the corresponding constants Add the new configs to yarn-default.xml, if only commented out Change definitions of NM_AUX_SERVICE_CLASS_CLASSPATH, NM_AUX_SERVICE_CLASSLOADER_SYSTEM_CLASSES to use the other constant NM_AUX_SERVICES. AuxServices createJobClassLoader -> createAuxServiceClassLoader() systemClass -> systemClasses The validation code to check the aux-service names so that ServiceData can be read and written properly should be done for aux-services loaded from a different classpath too. AuxiliaryServiceWithCustomClassLoader Add a code comment as to why we are create new Configuration objects (a summary of what we discussed above). Can we change the code such that AuxiliaryServiceWithCustomClassLoader simply takes in the class-name, classpath and system-classpath and does everything internally? This will keep the AuxServices class code cleaner. We should definitely add a test-case. I think the simpler way to do this is to dynamically generate classes. Let me think about a bit while you address the remaining comments.
          Hide
          sjlee0 Sangjin Lee added a comment -

          If you push more into AuxiliaryServiceWithCustomClassLoader from AuxServices, it may give you more opportunities to do targeted unit tests.

          Just FYI, in case of TestRunJar, I created a few test classes (ClassLoaderCheckMain, etc.), created a jar on the fly (makeClassLoaderTestJar()), changed the system classes to include/exclude some of these test classes using the override, and test scenarios. Your mileage may vary.

          Show
          sjlee0 Sangjin Lee added a comment - If you push more into AuxiliaryServiceWithCustomClassLoader from AuxServices , it may give you more opportunities to do targeted unit tests. Just FYI, in case of TestRunJar , I created a few test classes ( ClassLoaderCheckMain , etc.), created a jar on the fly ( makeClassLoaderTestJar() ), changed the system classes to include/exclude some of these test classes using the override, and test scenarios. Your mileage may vary.
          Hide
          xgong Xuan Gong added a comment -

          Attached a new patch with a test case.

          Show
          xgong Xuan Gong added a comment - Attached a new patch with a test case.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 10s Maven dependency ordering for branch
          +1 mvninstall 6m 30s trunk passed
          +1 compile 1m 45s trunk passed with JDK v1.8.0_91
          +1 compile 2m 3s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 41s trunk passed
          +1 mvnsite 0m 54s trunk passed
          +1 mvneclipse 0m 24s trunk passed
          +1 findbugs 2m 1s trunk passed
          +1 javadoc 0m 56s trunk passed with JDK v1.8.0_91
          +1 javadoc 3m 20s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 0m 47s the patch passed
          +1 compile 1m 53s the patch passed with JDK v1.8.0_91
          +1 javac 1m 53s the patch passed
          +1 compile 2m 8s the patch passed with JDK v1.7.0_95
          +1 javac 2m 8s the patch passed
          +1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: patch generated 0 new + 304 unchanged - 2 fixed = 304 total (was 306)
          +1 mvnsite 0m 51s the patch passed
          +1 mvneclipse 0m 22s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 23s the patch passed
          +1 javadoc 0m 52s the patch passed with JDK v1.8.0_91
          +1 javadoc 3m 22s the patch passed with JDK v1.7.0_95
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_91.
          +1 unit 11m 7s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_91.
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.7.0_95.
          +1 unit 11m 33s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 20s Patch does not generate ASF License warnings.
          57m 21s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803148/YARN-4577.20160509.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 85cb2a13c3c3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 87f5e35
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11396/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11396/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 6m 30s trunk passed +1 compile 1m 45s trunk passed with JDK v1.8.0_91 +1 compile 2m 3s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 41s trunk passed +1 mvnsite 0m 54s trunk passed +1 mvneclipse 0m 24s trunk passed +1 findbugs 2m 1s trunk passed +1 javadoc 0m 56s trunk passed with JDK v1.8.0_91 +1 javadoc 3m 20s trunk passed with JDK v1.7.0_95 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 0m 47s the patch passed +1 compile 1m 53s the patch passed with JDK v1.8.0_91 +1 javac 1m 53s the patch passed +1 compile 2m 8s the patch passed with JDK v1.7.0_95 +1 javac 2m 8s the patch passed +1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: patch generated 0 new + 304 unchanged - 2 fixed = 304 total (was 306) +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 22s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 23s the patch passed +1 javadoc 0m 52s the patch passed with JDK v1.8.0_91 +1 javadoc 3m 22s the patch passed with JDK v1.7.0_95 +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_91. +1 unit 11m 7s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_91. +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.7.0_95. +1 unit 11m 33s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 20s Patch does not generate ASF License warnings. 57m 21s Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803148/YARN-4577.20160509.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 85cb2a13c3c3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 87f5e35 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11396/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11396/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          vvasudev Varun Vasudev added a comment - - edited

          Xuan Gong - TestAuxServices#makeClassLoaderTestJar and TestRunJar#makeClassLoaderTestJar seem to be the same with minor differences - can you refactor the code to use the same underlying logic? And can you also add a comment explaining the test logic? Thanks!

          Show
          vvasudev Varun Vasudev added a comment - - edited Xuan Gong - TestAuxServices#makeClassLoaderTestJar and TestRunJar#makeClassLoaderTestJar seem to be the same with minor differences - can you refactor the code to use the same underlying logic? And can you also add a comment explaining the test logic? Thanks!
          Hide
          xgong Xuan Gong added a comment -

          Thanks for the review. Varun Vasudev
          I have uploaded a new patch to address all your comments.

          Show
          xgong Xuan Gong added a comment - Thanks for the review. Varun Vasudev I have uploaded a new patch to address all your comments.
          Hide
          sjlee0 Sangjin Lee added a comment -

          It appears the latest patch is missing AuxiliaryServiceWithCustomClassLoader.java?

          Show
          sjlee0 Sangjin Lee added a comment - It appears the latest patch is missing AuxiliaryServiceWithCustomClassLoader.java ?
          Hide
          sjlee0 Sangjin Lee added a comment -

          One minor nit on AuxiliaryServiceWithCustomClassLoader.java:

          • l.45: now that we have a static factory method, it would be better to make the constructor private
          Show
          sjlee0 Sangjin Lee added a comment - One minor nit on AuxiliaryServiceWithCustomClassLoader.java : l.45: now that we have a static factory method, it would be better to make the constructor private
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 36s Maven dependency ordering for branch
          +1 mvninstall 9m 27s trunk passed
          +1 compile 11m 47s trunk passed with JDK v1.8.0_91
          +1 compile 8m 50s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 45s trunk passed
          +1 mvnsite 2m 20s trunk passed
          +1 mvneclipse 0m 46s trunk passed
          +1 findbugs 4m 17s trunk passed
          +1 javadoc 2m 24s trunk passed with JDK v1.8.0_91
          +1 javadoc 5m 4s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 18s Maven dependency ordering for patch
          -1 mvninstall 0m 19s hadoop-yarn-server-nodemanager in the patch failed.
          -1 compile 4m 7s root in the patch failed with JDK v1.8.0_91.
          -1 javac 4m 7s root in the patch failed with JDK v1.8.0_91.
          -1 compile 3m 49s root in the patch failed with JDK v1.7.0_95.
          -1 javac 3m 49s root in the patch failed with JDK v1.7.0_95.
          +1 checkstyle 1m 43s root: patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318)
          -1 mvnsite 0m 21s hadoop-yarn-server-nodemanager in the patch failed.
          +1 mvneclipse 0m 43s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          -1 findbugs 0m 16s hadoop-yarn-server-nodemanager in the patch failed.
          +1 javadoc 2m 23s the patch passed with JDK v1.8.0_91
          +1 javadoc 5m 4s the patch passed with JDK v1.7.0_95
          +1 unit 10m 24s hadoop-common in the patch passed with JDK v1.8.0_91.
          +1 unit 0m 31s hadoop-yarn-api in the patch passed with JDK v1.8.0_91.
          -1 unit 0m 20s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.8.0_91.
          +1 unit 10m 0s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 unit 0m 30s hadoop-yarn-api in the patch passed with JDK v1.7.0_95.
          -1 unit 0m 21s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 25s Patch does not generate ASF License warnings.
          97m 37s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803326/YARN-4577.20160510.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e92da668fdd3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6e56578
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          compile https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-compile-root-jdk1.8.0_91.txt
          javac https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-compile-root-jdk1.8.0_91.txt
          compile https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-compile-root-jdk1.7.0_95.txt
          javac https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-compile-root-jdk1.7.0_95.txt
          mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11404/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: .
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11404/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 36s Maven dependency ordering for branch +1 mvninstall 9m 27s trunk passed +1 compile 11m 47s trunk passed with JDK v1.8.0_91 +1 compile 8m 50s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 45s trunk passed +1 mvnsite 2m 20s trunk passed +1 mvneclipse 0m 46s trunk passed +1 findbugs 4m 17s trunk passed +1 javadoc 2m 24s trunk passed with JDK v1.8.0_91 +1 javadoc 5m 4s trunk passed with JDK v1.7.0_95 0 mvndep 0m 18s Maven dependency ordering for patch -1 mvninstall 0m 19s hadoop-yarn-server-nodemanager in the patch failed. -1 compile 4m 7s root in the patch failed with JDK v1.8.0_91. -1 javac 4m 7s root in the patch failed with JDK v1.8.0_91. -1 compile 3m 49s root in the patch failed with JDK v1.7.0_95. -1 javac 3m 49s root in the patch failed with JDK v1.7.0_95. +1 checkstyle 1m 43s root: patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318) -1 mvnsite 0m 21s hadoop-yarn-server-nodemanager in the patch failed. +1 mvneclipse 0m 43s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 0m 16s hadoop-yarn-server-nodemanager in the patch failed. +1 javadoc 2m 23s the patch passed with JDK v1.8.0_91 +1 javadoc 5m 4s the patch passed with JDK v1.7.0_95 +1 unit 10m 24s hadoop-common in the patch passed with JDK v1.8.0_91. +1 unit 0m 31s hadoop-yarn-api in the patch passed with JDK v1.8.0_91. -1 unit 0m 20s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.8.0_91. +1 unit 10m 0s hadoop-common in the patch passed with JDK v1.7.0_95. +1 unit 0m 30s hadoop-yarn-api in the patch passed with JDK v1.7.0_95. -1 unit 0m 21s hadoop-yarn-server-nodemanager in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 97m 37s Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803326/YARN-4577.20160510.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e92da668fdd3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6e56578 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt compile https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-compile-root-jdk1.8.0_91.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-compile-root-jdk1.8.0_91.txt compile https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-compile-root-jdk1.7.0_95.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-compile-root-jdk1.7.0_95.txt mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11404/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11404/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11404/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          vvasudev Varun Vasudev added a comment -

          Xuan Gong - as Sangjin Lee mentioned the latest patch is missing AuxiliaryServiceWithCustomClassLoader.java.

          Show
          vvasudev Varun Vasudev added a comment - Xuan Gong - as Sangjin Lee mentioned the latest patch is missing AuxiliaryServiceWithCustomClassLoader.java.
          Hide
          xgong Xuan Gong added a comment -

          Varun Vasudev, Sangjin Lee
          Sorry about that. I have uploaded a new patch for this

          Show
          xgong Xuan Gong added a comment - Varun Vasudev , Sangjin Lee Sorry about that. I have uploaded a new patch for this
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 16s Maven dependency ordering for branch
          +1 mvninstall 7m 14s trunk passed
          +1 compile 6m 52s trunk passed with JDK v1.8.0_91
          +1 compile 7m 2s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 31s trunk passed
          +1 mvnsite 1m 52s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 3m 37s trunk passed
          +1 javadoc 1m 54s trunk passed with JDK v1.8.0_91
          +1 javadoc 4m 24s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 31s the patch passed
          +1 compile 5m 50s the patch passed with JDK v1.8.0_91
          +1 javac 5m 50s the patch passed
          +1 compile 6m 47s the patch passed with JDK v1.7.0_95
          +1 javac 6m 47s the patch passed
          -1 checkstyle 1m 29s root: patch generated 1 new + 316 unchanged - 2 fixed = 317 total (was 318)
          +1 mvnsite 1m 54s the patch passed
          +1 mvneclipse 0m 39s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 4m 19s the patch passed
          +1 javadoc 1m 52s the patch passed with JDK v1.8.0_91
          +1 javadoc 4m 21s the patch passed with JDK v1.7.0_95
          -1 unit 7m 21s hadoop-common in the patch failed with JDK v1.8.0_91.
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_91.
          +1 unit 11m 4s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_91.
          +1 unit 7m 56s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 unit 0m 26s hadoop-yarn-api in the patch passed with JDK v1.7.0_95.
          +1 unit 11m 45s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 23s Patch does not generate ASF License warnings.
          105m 21s



          Reason Tests
          JDK v1.8.0_91 Failed junit tests hadoop.net.TestDNS



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803459/YARN-4577.20160511.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c9bed34a7662 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 687233f
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11420/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11420/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11420/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11420/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: .
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11420/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 16s Maven dependency ordering for branch +1 mvninstall 7m 14s trunk passed +1 compile 6m 52s trunk passed with JDK v1.8.0_91 +1 compile 7m 2s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 31s trunk passed +1 mvnsite 1m 52s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 3m 37s trunk passed +1 javadoc 1m 54s trunk passed with JDK v1.8.0_91 +1 javadoc 4m 24s trunk passed with JDK v1.7.0_95 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 31s the patch passed +1 compile 5m 50s the patch passed with JDK v1.8.0_91 +1 javac 5m 50s the patch passed +1 compile 6m 47s the patch passed with JDK v1.7.0_95 +1 javac 6m 47s the patch passed -1 checkstyle 1m 29s root: patch generated 1 new + 316 unchanged - 2 fixed = 317 total (was 318) +1 mvnsite 1m 54s the patch passed +1 mvneclipse 0m 39s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 4m 19s the patch passed +1 javadoc 1m 52s the patch passed with JDK v1.8.0_91 +1 javadoc 4m 21s the patch passed with JDK v1.7.0_95 -1 unit 7m 21s hadoop-common in the patch failed with JDK v1.8.0_91. +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_91. +1 unit 11m 4s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_91. +1 unit 7m 56s hadoop-common in the patch passed with JDK v1.7.0_95. +1 unit 0m 26s hadoop-yarn-api in the patch passed with JDK v1.7.0_95. +1 unit 11m 45s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 23s Patch does not generate ASF License warnings. 105m 21s Reason Tests JDK v1.8.0_91 Failed junit tests hadoop.net.TestDNS Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803459/YARN-4577.20160511.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c9bed34a7662 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 687233f Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11420/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11420/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11420/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11420/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11420/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sjlee0 Sangjin Lee added a comment -

          I think it's quite close. The hadoop-common test failure is unrelated. The checkstyle issue should be trivial to fix. Hi Xuan Gong, could you please fix the checkstyle issue? Then I think it's good to go.

          Varun Vasudev, let me know if you're OK with that.

          Show
          sjlee0 Sangjin Lee added a comment - I think it's quite close. The hadoop-common test failure is unrelated. The checkstyle issue should be trivial to fix. Hi Xuan Gong , could you please fix the checkstyle issue? Then I think it's good to go. Varun Vasudev , let me know if you're OK with that.
          Hide
          vvasudev Varun Vasudev added a comment -

          Agree with Sangjin Lee; Xuan Gong - can you please fix the checkstyle issue and we can commit it.

          Show
          vvasudev Varun Vasudev added a comment - Agree with Sangjin Lee ; Xuan Gong - can you please fix the checkstyle issue and we can commit it.
          Hide
          xgong Xuan Gong added a comment -

          Thanks for the review. Uploaded a new patch which made the AuxiliaryServiceWithCustomClassLoader as final

          Show
          xgong Xuan Gong added a comment - Thanks for the review. Uploaded a new patch which made the AuxiliaryServiceWithCustomClassLoader as final
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 15s Maven dependency ordering for branch
          +1 mvninstall 6m 41s trunk passed
          +1 compile 6m 3s trunk passed with JDK v1.8.0_91
          +1 compile 6m 53s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 30s trunk passed
          +1 mvnsite 1m 52s trunk passed
          +1 mvneclipse 0m 40s trunk passed
          +1 findbugs 3m 43s trunk passed
          +1 javadoc 2m 31s trunk passed with JDK v1.8.0_91
          +1 javadoc 5m 25s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 1m 31s the patch passed
          +1 compile 6m 4s the patch passed with JDK v1.8.0_91
          +1 javac 6m 4s the patch passed
          +1 compile 6m 50s the patch passed with JDK v1.7.0_95
          +1 javac 6m 50s the patch passed
          +1 checkstyle 1m 30s root: patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318)
          +1 mvnsite 1m 53s the patch passed
          +1 mvneclipse 0m 38s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 4m 18s the patch passed
          +1 javadoc 1m 52s the patch passed with JDK v1.8.0_91
          +1 javadoc 4m 24s the patch passed with JDK v1.7.0_95
          +1 unit 7m 50s hadoop-common in the patch passed with JDK v1.8.0_91.
          +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_91.
          +1 unit 11m 6s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_91.
          +1 unit 7m 48s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 unit 0m 24s hadoop-yarn-api in the patch passed with JDK v1.7.0_95.
          +1 unit 11m 36s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 23s Patch does not generate ASF License warnings.
          106m 1s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803562/YARN-4577.20160511.1.patch
          JIRA Issue YARN-4577
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e71cd162ac8f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / d464f4d
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11426/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: .
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11426/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 15s Maven dependency ordering for branch +1 mvninstall 6m 41s trunk passed +1 compile 6m 3s trunk passed with JDK v1.8.0_91 +1 compile 6m 53s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 30s trunk passed +1 mvnsite 1m 52s trunk passed +1 mvneclipse 0m 40s trunk passed +1 findbugs 3m 43s trunk passed +1 javadoc 2m 31s trunk passed with JDK v1.8.0_91 +1 javadoc 5m 25s trunk passed with JDK v1.7.0_95 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 1m 31s the patch passed +1 compile 6m 4s the patch passed with JDK v1.8.0_91 +1 javac 6m 4s the patch passed +1 compile 6m 50s the patch passed with JDK v1.7.0_95 +1 javac 6m 50s the patch passed +1 checkstyle 1m 30s root: patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318) +1 mvnsite 1m 53s the patch passed +1 mvneclipse 0m 38s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 4m 18s the patch passed +1 javadoc 1m 52s the patch passed with JDK v1.8.0_91 +1 javadoc 4m 24s the patch passed with JDK v1.7.0_95 +1 unit 7m 50s hadoop-common in the patch passed with JDK v1.8.0_91. +1 unit 0m 22s hadoop-yarn-api in the patch passed with JDK v1.8.0_91. +1 unit 11m 6s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_91. +1 unit 7m 48s hadoop-common in the patch passed with JDK v1.7.0_95. +1 unit 0m 24s hadoop-yarn-api in the patch passed with JDK v1.7.0_95. +1 unit 11m 36s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 23s Patch does not generate ASF License warnings. 106m 1s Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803562/YARN-4577.20160511.1.patch JIRA Issue YARN-4577 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e71cd162ac8f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d464f4d Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11426/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11426/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          vvasudev Varun Vasudev added a comment -

          Latest patch looks good to me. Sangjin Lee what do you think?

          Show
          vvasudev Varun Vasudev added a comment - Latest patch looks good to me. Sangjin Lee what do you think?
          Hide
          sjlee0 Sangjin Lee added a comment -

          +1. Committing shortly. I'm going to commit to trunk and branch-2 (2.9). Do we need/want this for 2.8.0 as well?

          Show
          sjlee0 Sangjin Lee added a comment - +1. Committing shortly. I'm going to commit to trunk and branch-2 (2.9). Do we need/want this for 2.8.0 as well?
          Hide
          xgong Xuan Gong added a comment -

          Sangjin Lee
          Please commit into trunk and branch-2. Thanks

          Show
          xgong Xuan Gong added a comment - Sangjin Lee Please commit into trunk and branch-2. Thanks
          Hide
          sjlee0 Sangjin Lee added a comment -

          Committed the patch to trunk and branch-2. Thanks Xuan Gong for your contribution! Thanks Steve Loughran, Li Lu, Vinod Kumar Vavilapalli, and Varun Vasudev for your reviews.

          Xuan Gong, do you think this warrants some release notes? If so, do you mind adding a little?

          Show
          sjlee0 Sangjin Lee added a comment - Committed the patch to trunk and branch-2. Thanks Xuan Gong for your contribution! Thanks Steve Loughran , Li Lu , Vinod Kumar Vavilapalli , and Varun Vasudev for your reviews. Xuan Gong , do you think this warrants some release notes? If so, do you mind adding a little?
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #9753 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9753/)
          YARN-4577. Enable aux services to have their own custom classpath/jar (sjlee: rev 0bbe01f8d56191edfba3b50fb9f8859a0b3f826f)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestAuxServices.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/JarFinder.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxiliaryServiceWithCustomClassLoader.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #9753 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9753/ ) YARN-4577 . Enable aux services to have their own custom classpath/jar (sjlee: rev 0bbe01f8d56191edfba3b50fb9f8859a0b3f826f) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxServices.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestAuxServices.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/JarFinder.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/AuxiliaryServiceWithCustomClassLoader.java

            People

            • Assignee:
              xgong Xuan Gong
              Reporter:
              xgong Xuan Gong
            • Votes:
              0 Vote for this issue
              Watchers:
              17 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development