Details

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

      Description

      I have noticed that org.apache.hadoop.mapred.MapReduceChildJVM doesn't forward the value of -Djava.library.path= from the parent JVM to the child JVM. Thus if one wants to use native libraries for compression the only option seems to be to manually include relevant java.library.path settings into the mapred-site.xml (as mapred.[map|reduce].child.java.opts).

      This seems to be a change in behavior compared to MR1 where TaskRunner.java used to do that:

      String libraryPath = System.getProperty("java.library.path");
          if (libraryPath == null) {
            libraryPath = workDir.getAbsolutePath();
          } else {
            libraryPath += SYSTEM_PATH_SEPARATOR + workDir;
          }
          boolean hasUserLDPath = false;
          for(int i=0; i<javaOptsSplit.length ;i++) {
            if(javaOptsSplit[i].startsWith("-Djava.library.path=")) {
              javaOptsSplit[i] += SYSTEM_PATH_SEPARATOR + libraryPath;
              hasUserLDPath = true;
              break;
            }
          }
          if(!hasUserLDPath) {
            vargs.add("-Djava.library.path=" + libraryPath);
          }
          for (int i = 0; i < javaOptsSplit.length; i++) {
            vargs.add(javaOptsSplit[i]);
          }
      

      Is this a regression or a deliberate choice?

      1. MAPREDUCE-3693.patch.txt
        2 kB
        Roman Shaposhnik
      2. MAPREDUCE-3693-2.patch.txt
        1 kB
        Roman Shaposhnik

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Build #173 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/173/)
          Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml.

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

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Build #173 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/173/ ) Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236596 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #971 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/971/)
          MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik.

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #971 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/971/ ) MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236594 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #151 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/151/)
          Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml.

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

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #151 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/151/ ) Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236596 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #938 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/938/)
          MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik.

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #938 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/938/ ) MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236594 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Commit #449 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/449/)
          Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml.

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

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Commit #449 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/449/ ) Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236596 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #1618 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1618/)
          MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik.

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1618 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1618/ ) MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236594 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #1674 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1674/)
          MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik.

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1674 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1674/ ) MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236594 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Commit #424 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/424/)
          Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml.

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

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Commit #424 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/424/ ) Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236596 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-0.23-Commit #433 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/433/)
          Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml.

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

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Common-0.23-Commit #433 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/433/ ) Merge -c 1236594 from trunk to branch-0.23 to fix MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236596 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #1602 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1602/)
          MAPREDUCE-3693. Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik.

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

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1602 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1602/ ) MAPREDUCE-3693 . Added mapreduce.admin.user.env to mapred-default.xml. Contributed by Roman Shapshonik. acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236594 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml
          Hide
          Arun C Murthy added a comment -

          I just committed this. Thanks Roman!

          Show
          Arun C Murthy added a comment - I just committed this. Thanks Roman!
          Hide
          Eli Collins added a comment -

          +1 Thanks Roman!

          Show
          Eli Collins added a comment - +1 Thanks Roman!
          Hide
          Arun C Murthy added a comment -

          I mean changing the value of default to blank string is unnecessary.

          Show
          Arun C Murthy added a comment - I mean changing the value of default to blank string is unnecessary.
          Hide
          Arun C Murthy added a comment -

          Roman, the patch is fine. However, there is no need to remove the DEFAULT_* variable, it should be passed in as the 'default' value.

          Show
          Arun C Murthy added a comment - Roman, the patch is fine. However, there is no need to remove the DEFAULT_* variable, it should be passed in as the 'default' value.
          Hide
          Eli Collins added a comment -

          Oops, I forgot that mapred-default.xml isn't just docs, is actually used. In any case, we typically make the default value in the code match the default.xml value (see HADOOP-7956 for removing this duplication) so good to be consistent.

          (btw I assume you mean mapred-default.xml instead of yarn-default.xml)

          Show
          Eli Collins added a comment - Oops, I forgot that mapred-default.xml isn't just docs, is actually used. In any case, we typically make the default value in the code match the default.xml value (see HADOOP-7956 for removing this duplication) so good to be consistent. (btw I assume you mean mapred-default.xml instead of yarn-default.xml)
          Hide
          Roman Shaposhnik added a comment -

          @Eli,

          what's the value to have the same string in both places (yarn-default.xml and as a string constant)?

          Also, I don't understand your comment "This will break tasks that use native libs". Isn't yarn-default.xml guaranteed to be on a classpath and thus the behavior remains exactly what it used to be (IOW, the default value is no longer needed since the property always gets loaded from yarn-default.xml)?

          Show
          Roman Shaposhnik added a comment - @Eli, what's the value to have the same string in both places (yarn-default.xml and as a string constant)? Also, I don't understand your comment "This will break tasks that use native libs". Isn't yarn-default.xml guaranteed to be on a classpath and thus the behavior remains exactly what it used to be (IOW, the default value is no longer needed since the property always gets loaded from yarn-default.xml)?
          Hide
          Eli Collins added a comment -

          Forgot to mention, the idea isn't to change the default value, but to make it clear (1)what the default value is and (2) that they need to explicitly include the default value in their override, if they choose to override it.

          Show
          Eli Collins added a comment - Forgot to mention, the idea isn't to change the default value, but to make it clear (1)what the default value is and (2) that they need to explicitly include the default value in their override, if they choose to override it.
          Hide
          Eli Collins added a comment -

          Why make DEFAULT_MAPRED_ADMIN_USER_ENV the empty string? This will break tasks that use native libs, and per your addition to mapred-default.xml the default value should be LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native (what it is set to currentlY). Otherwise looks good.

          Show
          Eli Collins added a comment - Why make DEFAULT_MAPRED_ADMIN_USER_ENV the empty string? This will break tasks that use native libs, and per your addition to mapred-default.xml the default value should be LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native (what it is set to currentlY). Otherwise looks good.
          Hide
          Roman Shaposhnik added a comment -

          Given that this patch only touches configuration/docs, I don't think I can adequately supply one. As for the 3 failing tests looks like they've been failing for at least 6 builds: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/lastCompletedBuild/testReport/

          Show
          Roman Shaposhnik added a comment - Given that this patch only touches configuration/docs, I don't think I can adequately supply one. As for the 3 failing tests looks like they've been failing for at least 6 builds: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/lastCompletedBuild/testReport/
          Hide
          Hadoop QA added a comment -

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

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

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

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

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

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.mapred.TestJobClientGetJob
          org.apache.hadoop.mapred.TestMRWithDistributedCache
          org.apache.hadoop.mapred.TestLocalModeWithNewApis

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

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1674//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1674//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12511784/MAPREDUCE-3693.patch.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.mapred.TestJobClientGetJob org.apache.hadoop.mapred.TestMRWithDistributedCache org.apache.hadoop.mapred.TestLocalModeWithNewApis +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1674//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/1674//console This message is automatically generated.
          Hide
          Eli Collins added a comment -

          If you want to support multiple versions of the native libraries, or the job depends on native libraries that conflict with those in hadoop's lib/native.

          Show
          Eli Collins added a comment - If you want to support multiple versions of the native libraries, or the job depends on native libraries that conflict with those in hadoop's lib/native.
          Hide
          Roman Shaposhnik added a comment -

          @Eli,

          I'd be curious to know what are the usecases where excluding HADOOP_COMMON_HOME/lib/native is a requirement.

          Show
          Roman Shaposhnik added a comment - @Eli, I'd be curious to know what are the usecases where excluding HADOOP_COMMON_HOME/lib/native is a requirement.
          Hide
          Eli Collins added a comment -

          Sure. I still think that the right thing to do is to add LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native to thmakee env unconditionally

          But then an admin would have no way (using mapreduce.admin.user.env) of excluding HADOOP_COMMON_HOME/lib/native right? There are valid use cases where you don't want to pollute LI_LIBRARY_PATH with the native bits in hadoop.

          Show
          Eli Collins added a comment - Sure. I still think that the right thing to do is to add LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native to thmakee env unconditionally But then an admin would have no way (using mapreduce.admin.user.env) of excluding HADOOP_COMMON_HOME/lib/native right? There are valid use cases where you don't want to pollute LI_LIBRARY_PATH with the native bits in hadoop.
          Hide
          Roman Shaposhnik added a comment -

          @Eli, Arun

          Sure. I still think that the right thing to do is to add LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native to thmakee env unconditionally AND add to that whatever the property specifying. That said, if the attached patch can be committed I think it'll be a nice enough compromise.

          Show
          Roman Shaposhnik added a comment - @Eli, Arun Sure. I still think that the right thing to do is to add LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native to thmakee env unconditionally AND add to that whatever the property specifying. That said, if the attached patch can be committed I think it'll be a nice enough compromise.
          Hide
          Eli Collins added a comment -

          If mapreduce.admin.user.env were documented in mapred-default.xml with its default value (which sets LD_LIBRARY_PATH), then it would be clear to admins/users that LD_LIBRARY_PATH needs to be included if they override it.

          Roman, how about just submitting a patch to update mapred-default.xml?

          Show
          Eli Collins added a comment - If mapreduce.admin.user.env were documented in mapred-default.xml with its default value (which sets LD_LIBRARY_PATH), then it would be clear to admins/users that LD_LIBRARY_PATH needs to be included if they override it. Roman, how about just submitting a patch to update mapred-default.xml?
          Hide
          Roman Shaposhnik added a comment -

          Now that I've looked at the actual implementation (Arun, thanks a lot for pointing out where it is!) the only concern that I have left is that it is extremely difficult to realize that mapreduce.admin.user.env is what actually adds
          LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native to the YarnChild's environment.

          In other words, if the end user wants to tweak mapreduce.admin.user.env it is a hard requirement to NOT forget adding LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native back to whatever user wants to pass via that property. This strikes me as a pretty brittle implementation, unless of course mapreduce.admin.user.env is meant to be one of those hidden options that
          you're better off not tweaking.

          In summary, I really do think that one of the following needs to happen:

          • $HADOOP_COMMON_HOME/lib/native needs to be appended to whatever value of LD_LIBRARY_PATH there is in all cases
          • mapreduce.admin.user.env needs to be documents as a do not touch option
          • mapreduce.admin.user.env needs to be documents with a NOTE saying that setting it will invalidate the default setting of LD_LIBRARY_PATH (which will break loading of your native libraries)
          Show
          Roman Shaposhnik added a comment - Now that I've looked at the actual implementation (Arun, thanks a lot for pointing out where it is!) the only concern that I have left is that it is extremely difficult to realize that mapreduce.admin.user.env is what actually adds LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native to the YarnChild's environment. In other words, if the end user wants to tweak mapreduce.admin.user.env it is a hard requirement to NOT forget adding LD_LIBRARY_PATH=$HADOOP_COMMON_HOME/lib/native back to whatever user wants to pass via that property. This strikes me as a pretty brittle implementation, unless of course mapreduce.admin.user.env is meant to be one of those hidden options that you're better off not tweaking. In summary, I really do think that one of the following needs to happen: $HADOOP_COMMON_HOME/lib/native needs to be appended to whatever value of LD_LIBRARY_PATH there is in all cases mapreduce.admin.user.env needs to be documents as a do not touch option mapreduce.admin.user.env needs to be documents with a NOTE saying that setting it will invalidate the default setting of LD_LIBRARY_PATH (which will break loading of your native libraries)
          Hide
          Roman Shaposhnik added a comment -

          Ok, so the implementation now relies on setting LD_LIBRARY_PATH instead of java.library.path. Didn't know that. Let me see whether it actually gets passed into the env of the child.

          Show
          Roman Shaposhnik added a comment - Ok, so the implementation now relies on setting LD_LIBRARY_PATH instead of java.library.path. Didn't know that. Let me see whether it actually gets passed into the env of the child.
          Hide
          Arun C Murthy added a comment -

          Ok, now I'm completely confused.

          The original bug asked for a piece of code which isn't needed anymore and now the description changed on me...

          We should have opened a separate bug, IAC, LD_LIBRARY_PATH is already setup correctly via mapreduce.admin.user.env which defaults to right path for native libs.

          Show
          Arun C Murthy added a comment - Ok, now I'm completely confused. The original bug asked for a piece of code which isn't needed anymore and now the description changed on me... We should have opened a separate bug, IAC, LD_LIBRARY_PATH is already setup correctly via mapreduce.admin.user.env which defaults to right path for native libs.
          Hide
          Todd Lipcon added a comment -

          Hey Arun. I agree with you in principle here... but the use case is still valid and needs to be addressed – users shouldn't have to set up their individual configurations to point to wherever the compression libraries are on the server. We need some way of setting up the environment here in a sensible way - same as how users "automatically" get HDFS on their classpath via the NM, for example.

          I haven't looked into the confs, etc, yet, so apologies if I'm missing the obvious right answer.

          Show
          Todd Lipcon added a comment - Hey Arun. I agree with you in principle here... but the use case is still valid and needs to be addressed – users shouldn't have to set up their individual configurations to point to wherever the compression libraries are on the server. We need some way of setting up the environment here in a sensible way - same as how users "automatically" get HDFS on their classpath via the NM, for example. I haven't looked into the confs, etc, yet, so apologies if I'm missing the obvious right answer.
          Hide
          Arun C Murthy added a comment -

          In MR2 the parent JVM i.e. MR AM is unrelated to child JVM (map/reduce) and hence it's java.library.path isn't passed along.

          Very different from MR1 which, arguably, was a bug to pass TT's java.library.path to child (map/reduce).

          This is a good time to not do this anymore.

          Similarly, we shouldn't pass NM's library path either. It's the AM's responsibility to set it up.

          Show
          Arun C Murthy added a comment - In MR2 the parent JVM i.e. MR AM is unrelated to child JVM (map/reduce) and hence it's java.library.path isn't passed along. Very different from MR1 which, arguably, was a bug to pass TT's java.library.path to child (map/reduce). This is a good time to not do this anymore. Similarly, we shouldn't pass NM's library path either. It's the AM's responsibility to set it up.

            People

            • Assignee:
              Roman Shaposhnik
              Reporter:
              Roman Shaposhnik
            • Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development