[
https://issues.apache.org/jira/browse/YARN-233?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Chris Nauroth updated YARN-233:
-------------------------------
Attachment: YARN-233-branch-trunk-win.patch
Thanks for the review, Bikas. I have attached an updated patch to address some
of your feedback.
{quote}
Didnt quite get this removal?
{quote}
The "exec" to launch a command varies per platform. It needs to be "@call" on
Windows. I chose to move this logic into
ContainerLaunch.ShellScriptBuilder#command and its subclasses, so that
MapReduce and any other YARN applications don't need to repeat if (WINDOWS)
checks. However, it occurs to me that this could be considered a
backwards-incompatible change if existing contract is that the application must
submit the "exec" in its command. I don't know how significant the impact is,
so let me know what you think.
Yes, I agree with your assessment that "setsid" in the code is really checking
for "process groups supported". I've moved MAPREDUCE-4325 to YARN-240 for the
rename work. Meanwhile, I have updated this patch to return true for
isSetsidSupported when running on Windows. (We are saying that WinUtils must
be present for Hadoop to work on Windows, so it must be true that process
groups are supported.)
I have been trying to minimize the if (WINDOWS) checks. For the two examples
you gave, I didn't see any other logical place to move the checks. It's so
specific to DefaultContainerExecutor that it didn't seem to fit inside Shell.
Let me know if you have any suggestions on this.
I've updated the patch to add a kill command helper method in Shell, but it's
slightly different from the branch-1-win version. YARN uses "kill -0" for
checking validity of a pid, so I needed to add a special case for signal 0 to
use "winutils task isAlive" instead of "winutils task kill". In order for
error handling to work correctly, I needed to change winutils to return a
non-zero exit code from "winutils task isAlive". I refactored
isSetsidAvailable into Shell too.
I'm curious if we actually need this validation logic in ProcessIdFileReader or
if we can simplify by discarding it. From what I can tell, the pids are always
generated by YARN code, so they are always going to be the expected numeric pid
on Unix or the expected container ID on Windows. It would seem that we can
just trust them. What is the goal of this validation logic? Is it security
protection against an external process feeding in bogus pid values? I'd expect
that to be protected by proper permissions and ownership settings on the node
manager's working directories, so the extra logic wouldn't be necessary. Some
other requirement?
It looks like the closest equivalent of TestKillSubProcesses is some
combination of the tests in TestContainerLaunch, TestContainerManager, and
TestContainersMonitor. Right now, nearly all YARN tests fail early on Windows
due to validation failures on the test data path containing a drive
specification. I'd prefer to address those test infrastructure failures in a
separate jira.
> YARN support for execution of container on Windows
> --------------------------------------------------
>
> Key: YARN-233
> URL: https://issues.apache.org/jira/browse/YARN-233
> Project: Hadoop YARN
> Issue Type: Bug
> Components: nodemanager
> Affects Versions: trunk-win
> Reporter: Chris Nauroth
> Assignee: Chris Nauroth
> Attachments: YARN-233-branch-trunk-win.patch,
> YARN-233-branch-trunk-win.patch
>
>
> YARN container launch code requires updates for compatibility with Windows.
> Instead of launching and killing tasks via generated bash scripts and kill,
> use generated cmd scripts and winutils for task management.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
