You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2017/02/07 02:31:27 UTC

Review Request 56367: Stout: Windows: patch `os::killtree` to terminate job objects.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56367/
-----------------------------------------------------------

Review request for mesos, Alex Clemmer and Joseph Wu.


Bugs: MESOS-6892
    https://issues.apache.org/jira/browse/MESOS-6892


Repository: mesos


Description
-------

Before we deprecate `os::killtree`, we need to it at least work
with the new job object semantics of the `WindowsLauncher`.
Given the PID of the "process tree" to kill, this maps it to the
name used for the job object, opens a new handle to it, and calls
`os::kill_job` to terminate it.


Diffs
-----

  3rdparty/stout/include/stout/os/windows/killtree.hpp 2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 

Diff: https://reviews.apache.org/r/56367/diff/


Testing
-------


Thanks,

Andrew Schwartzmeyer


Re: Review Request 56367: Windows: Stout: Shim `os::killtree` to terminate job objects.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56367/
-----------------------------------------------------------

(Updated March 27, 2017, 10:12 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Changes
-------

Updated for `JobObjectManager` changes.


Summary (updated)
-----------------

Windows: Stout: Shim `os::killtree` to terminate job objects.


Bugs: MESOS-6892
    https://issues.apache.org/jira/browse/MESOS-6892


Repository: mesos


Description (updated)
-------

On Windows, a "process tree" is in fact a job object. Thus, the
implementation of `os::killtree` on Windows is a shim which terminates
the job object corresponding to the process group represented by the
given PID.


Diffs (updated)
-----

  3rdparty/stout/include/stout/os/windows/killtree.hpp 2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 


Diff: https://reviews.apache.org/r/56367/diff/2/

Changes: https://reviews.apache.org/r/56367/diff/1-2/


Testing
-------


Thanks,

Andrew Schwartzmeyer


Re: Review Request 56367: Stout: Windows: patch `os::killtree` to terminate job objects.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56367/#review165590
-----------------------------------------------------------



Test again review bot!

- Andrew Schwartzmeyer


On Feb. 8, 2017, 9:14 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56367/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2017, 9:14 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-6892
>     https://issues.apache.org/jira/browse/MESOS-6892
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before we deprecate `os::killtree`, we need to it at least work
> with the new job object semantics of the `WindowsLauncher`.
> Given the PID of the "process tree" to kill, this maps it to the
> name used for the job object, opens a new handle to it, and calls
> `os::kill_job` to terminate it.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 
> 
> Diff: https://reviews.apache.org/r/56367/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 56367: Stout: Windows: patch `os::killtree` to terminate job objects.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.

> On Feb. 8, 2017, 10:53 p.m., Mesos Reviewbot wrote:
> > Bad review!
> > 
> > Reviews applied: [56367, 56362, 56364, 56363]
> > 
> > Error:
> > Circular dependency detected for review 56363.Please fix the 'depends_on' field.

Reviewbot, that's not enough information. I see no circular dependency in these reviews.

67 -> [62]
62 -> [64, 63]
64 -> [63]
63 -> []

63 did depend on 62, but that was corrected over two hours ago. It's the only thing I could that would create a circular dependency.
There's a join dependeny where [62, 64] -> [63], but that's not circular and shouldn't be a problem.


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56367/#review164803
-----------------------------------------------------------


On Feb. 8, 2017, 9:14 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56367/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2017, 9:14 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-6892
>     https://issues.apache.org/jira/browse/MESOS-6892
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before we deprecate `os::killtree`, we need to it at least work
> with the new job object semantics of the `WindowsLauncher`.
> Given the PID of the "process tree" to kill, this maps it to the
> name used for the job object, opens a new handle to it, and calls
> `os::kill_job` to terminate it.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 
> 
> Diff: https://reviews.apache.org/r/56367/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 56367: Stout: Windows: patch `os::killtree` to terminate job objects.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56367/#review164803
-----------------------------------------------------------



Bad review!

Reviews applied: [56367, 56362, 56364, 56363]

Error:
Circular dependency detected for review 56363.Please fix the 'depends_on' field.

- Mesos Reviewbot


On Feb. 8, 2017, 9:14 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56367/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2017, 9:14 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-6892
>     https://issues.apache.org/jira/browse/MESOS-6892
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before we deprecate `os::killtree`, we need to it at least work
> with the new job object semantics of the `WindowsLauncher`.
> Given the PID of the "process tree" to kill, this maps it to the
> name used for the job object, opens a new handle to it, and calls
> `os::kill_job` to terminate it.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 
> 
> Diff: https://reviews.apache.org/r/56367/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 56367: Stout: Windows: patch `os::killtree` to terminate job objects.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56367/
-----------------------------------------------------------

(Updated Feb. 8, 2017, 9:14 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Changes
-------

Fix dependency.


Bugs: MESOS-6892
    https://issues.apache.org/jira/browse/MESOS-6892


Repository: mesos


Description
-------

Before we deprecate `os::killtree`, we need to it at least work
with the new job object semantics of the `WindowsLauncher`.
Given the PID of the "process tree" to kill, this maps it to the
name used for the job object, opens a new handle to it, and calls
`os::kill_job` to terminate it.


Diffs
-----

  3rdparty/stout/include/stout/os/windows/killtree.hpp 2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 

Diff: https://reviews.apache.org/r/56367/diff/


Testing
-------


Thanks,

Andrew Schwartzmeyer