[ 
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


Reply via email to