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

Varun Vasudev commented on YARN-5621:
-------------------------------------

1)
{code}
+      File srcFile =
+          writeScriptToNMPrivateDir(nmPrivateDir, scriptBuilder.toString());
+      String dstFile =
+          container.getWorkDir() + Path.SEPARATOR + srcFile.getName();
{code}

Rename srcFile to privateScriptFile and dstFile to userScriptFile

2)
{code}
+    } catch (IOException e) {
+      LOG.error("Error when creating symlink for  " + 
container.getContainerId()
+          + ": " + src + " -> " + symlink, e);
+    }
{code}
We should continue the exception or at least surface that the resource 
localization has failed?

3)
{code}
+      LOG.info("Copy " + srcFile + " to " + dstFile);
{code}
Debug instead of info? We already log successful symlink creation in 
ContainerImpl.java

4)
{code}
+    } catch (PrivilegedOperationException e) {
+      int exitCode = e.getExitCode();
+      LOG.error("Error when running script [" + dstFile + "], exitcode = "
+          + exitCode + ", output: " + e.getOutput(), e);
+    }
{code}
Like (2) above, we should surface the error to the AM somehow?

5)
{code}
-      if (dst.isAbsolute()) {
-        throw new IOException("Destination must be relative");
-      }
{code}
Can you explain why we need to remove this check? Can’t we just pass the 
absolute path of linkFile?

6)
{code}
+    // give up root privs
+    if (change_user(user_detail->pw_uid, user_detail->pw_gid) != 0) {
+        unlink(src_script_file);
+        return -1;
+    }
+
{code}
Instead of -1 we should return an error code saying the change user failed.

7)
{code}
+    unlink(src_script_file);
+    unlink(dst_script_file);
+    return 0;
{code}
This code will never be executed - excel doesn’t return unless there’s an error 
- you’ll need to use fork/exec or clean up in the NodeManager itself

> Support LinuxContainerExecutor to create symlinks
> -------------------------------------------------
>
>                 Key: YARN-5621
>                 URL: https://issues.apache.org/jira/browse/YARN-5621
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Jian He
>            Assignee: Jian He
>         Attachments: YARN-5621.1.patch, YARN-5621.2.patch
>
>
> When new resources are localized, new symlink needs to be created for the 
> localized resource. This is the change for the LinuxContainerExecutor to 
> create the symlinks.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to