hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bikas Saha (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MAPREDUCE-4374) Fix child task environment variable config and add support for Windows
Date Fri, 29 Jun 2012 18:59:44 GMT

    [ https://issues.apache.org/jira/browse/MAPREDUCE-4374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13404122#comment-13404122
] 

Bikas Saha commented on MAPREDUCE-4374:
---------------------------------------

TaskRunner.java 
Should be easy to move to Shell.getUserHome()?
{code}
  static final String DEFAULT_HOME_DIR =
      System.getenv(Shell.WINDOWS ? "USERPROFILE" : "HOME");
{code}

Firstly, why do we need this when non-Windows gets away without it? It just seems to be pure
Java code. Secondly, if we know what abstract operation FOO we are doing here, then can we
push it into Shell.FOO()?
{code}
      // line 505
      if (Shell.WINDOWS) {
        // trim leading and trailing quotes on Windows
        String acmeDir = workDir.toString();
        acmeDir = acmeDir.replaceAll("^\"|\"$", "");
        tmpDir = new Path(acmeDir, tmp);
      } else
        tmpDir = new Path(workDir.toString(), tmp);

     ......
  // line 790
  private static void symlink(File workDir, String target, String link)
      throws IOException {
    if (link != null) {
      if (Shell.WINDOWS){
        String path = workDir.toString();
        // strip leading and trailing quotes
        path = path.replaceAll("^\"|\"$", "");
        link = path + "\\" + link;
      }
{code}

{code}
    } else if (createDir) {
      FileSystem localFs = FileSystem.getLocal(conf);
      if (!localFs.exists(tmpDir) && !localFs.mkdirs(tmpDir) && 
          !localFs.getFileStatus(tmpDir).isDir()) {
            throw new IOException("Mkdirs failed to create " +
                tmpDir.toString());
      }
    }
{code}
Why this code addition? Is this a general bug you have found?

Should be easy to get the regex pattern for env vars from Shell?
{code}
          if (Shell.WINDOWS)
            p = Pattern.compile("%([A-Za-z_][A-Za-z0-9_]*?)%");
          else
            p = Pattern.compile("\\$([A-Za-z_][A-Za-z0-9_]*)");
{code}

TestMiniMRChildTask.java
Should easily move inside Shell.getTempPath()?
{code}
  private final static String TMP_PATH = Shell.WINDOWS ? "C:\\tmp" : "/tmp";
{code}

Same question as above
{code}
  private static void checkEnv(String envName, String expValue, String mode) {
    String envValue = System.getenv(envName).trim();
    // trim leading and trailing quotes on Windows 
    if (Shell.WINDOWS)
      envValue = envValue.replaceAll("^\"|\"$", "");
{code}

I couldn't think of a simple way to move the Shell.Windows inside Shell. Mainly because of
the %being also used in String.Format.
{code}
    String envKey = String.format(Shell.WINDOWS ? "MY_PATH=%1$s,HOME=%1$s,"
        + "LD_LIBRARY_PATH=%%LD_LIBRARY_PATH%%;%1$s,"
        + "PATH=%%PATH%%;%1$s,NEW_PATH=%%NEW_PATH%%;%1$s"
        : "MY_PATH=%1$s,HOME=%1$s,LD_LIBRARY_PATH=$LD_LIBRARY_PATH:%1$s,"
            + "PATH=$PATH:%1$s,NEW_PATH=$NEW_PATH:%1$s", TMP_PATH);
{code}

TestTaskEnvironment.java
This could easily use Shell.getUserHome()?
{code}
    ttConf.set("mapred.child.env", (Shell.WINDOWS ? "ROOT=%USERPROFILE%"
        : "ROOT=$HOME"));
    ...
    assertTrue(root, root.contentEquals(System
        .getenv((Shell.WINDOWS ? "USERPROFILE" : "HOME"))));
{code}

Testing
Are there any tests that validate the changes or we should add new ones (Windows and Linux)?

                
> Fix child task environment variable config and add support for Windows
> ----------------------------------------------------------------------
>
>                 Key: MAPREDUCE-4374
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4374
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>    Affects Versions: 1-win
>            Reporter: Chuan Liu
>            Assignee: Chuan Liu
>            Priority: Minor
>         Attachments: MAPREDUCE-4374-branch-1-win.patch
>
>
> In HADOOP-2838, a new feature was introduced to set environment variables via the Hadoop
config 'mapred.child.env' for child tasks. There are some further fixes and improvements around
this feature, e.g. HADOOP-5981 were a bug fix; MAPREDUCE-478 broke the config into 'mapred.map.child.env'
and 'mapred.reduce.child.env'.  However the current implementation is still not complete.
It does not match its documentation or original intend as I believe. Also, by using ‘:’
(colon) and ‘;’ (semicolon) in the configuration syntax, we will have problems using them
on Windows because ‘:’ appears very often in Windows path as in “C:\”, and environment
variables are used very often to hold path names. The Jira is created to fix the problem and
provide support on Windows.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

Mime
View raw message