Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Jie Yu 

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

(Updated April 3, 2015, 9:29 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


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


Repository: mesos


Description
---

Terminated the perf subprocess once the parent exits.

The idea is to have a nanny process which installs a SIGTERM handler which will 
kill the process group (of course, we need to put the nanny and perf process to 
the same process group). We set the death signal of the nanny process to be 
SIGTERM. In that way, when slave exits, the nanny process will receive a 
SIGTERM, which will then kill all processes in the process group.


Diffs
-

  src/linux/perf.cpp cad6c80e2a9608ff02fc2b8976efba52713dd5a8 

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


Testing
---

sudo make check

I also manually verified it by terminating the slave while perf is in progress. 
The perf is killed immediately.


Thanks,

Jie Yu

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Jie Yu 

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

(Updated April 3, 2015, 9:29 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Changes
---

Review comments. Added more comments.


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


Repository: mesos


Description
---

Terminated the perf subprocess once the parent exits.

The idea is to have a nanny process which installs a SIGTERM handler which will 
kill the process group (of course, we need to put the nanny and perf process to 
the same process group). We set the death signal of the nanny process to be 
SIGTERM. In that way, when slave exits, the nanny process will receive a 
SIGTERM, which will then kill all processes in the process group.


Diffs (updated)
-

  src/linux/perf.cpp cad6c80e2a9608ff02fc2b8976efba52713dd5a8 

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


Testing
---

sudo make check

I also manually verified it by terminating the slave while perf is in progress. 
The perf is killed immediately.


Thanks,

Jie Yu

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [32820, 32805]

All tests passed.

- Mesos ReviewBot


On April 3, 2015, 5:11 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32805/
> ---
> 
> (Updated April 3, 2015, 5:11 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-2462
> https://issues.apache.org/jira/browse/MESOS-2462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Terminated the perf subprocess once the parent exits.
> 
> The idea is to have a nanny process which installs a SIGTERM handler which 
> will kill the process group (of course, we need to put the nanny and perf 
> process to the same process group). We set the death signal of the nanny 
> process to be SIGTERM. In that way, when slave exits, the nanny process will 
> receive a SIGTERM, which will then kill all processes in the process group.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cad6c80e2a9608ff02fc2b8976efba52713dd5a8 
> 
> Diff: https://reviews.apache.org/r/32805/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also manually verified it by terminating the slave while perf is in 
> progress. The perf is killed immediately.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Vinod Kone 

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

Ship it!



src/linux/perf.cpp


Add a comment here ?



src/linux/perf.cpp


Definitely needs a comment here on what you are trying to do.



src/linux/perf.cpp


Comment here that this child runs the perf command.



src/linux/perf.cpp


Comment that the parent blocks until the child (i.e., perf command) exits.



src/linux/perf.cpp


Comment here too.


- Vinod Kone


On April 3, 2015, 5:11 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32805/
> ---
> 
> (Updated April 3, 2015, 5:11 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-2462
> https://issues.apache.org/jira/browse/MESOS-2462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Terminated the perf subprocess once the parent exits.
> 
> The idea is to have a nanny process which installs a SIGTERM handler which 
> will kill the process group (of course, we need to put the nanny and perf 
> process to the same process group). We set the death signal of the nanny 
> process to be SIGTERM. In that way, when slave exits, the nanny process will 
> receive a SIGTERM, which will then kill all processes in the process group.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cad6c80e2a9608ff02fc2b8976efba52713dd5a8 
> 
> Diff: https://reviews.apache.org/r/32805/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also manually verified it by terminating the slave while perf is in 
> progress. The perf is killed immediately.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Jie Yu 

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

(Updated April 3, 2015, 5:11 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


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


Repository: mesos


Description
---

Terminated the perf subprocess once the parent exits.

The idea is to have a nanny process which installs a SIGTERM handler which will 
kill the process group (of course, we need to put the nanny and perf process to 
the same process group). We set the death signal of the nanny process to be 
SIGTERM. In that way, when slave exits, the nanny process will receive a 
SIGTERM, which will then kill all processes in the process group.


Diffs
-

  src/linux/perf.cpp cad6c80e2a9608ff02fc2b8976efba52713dd5a8 

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


Testing
---

sudo make check

I also manually verified it by terminating the slave while perf is in progress. 
The perf is killed immediately.


Thanks,

Jie Yu

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-02 Thread Mesos ReviewBot 

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


Bad patch!

Reviews applied: [32805]

Failed command: ./support/apply-review.sh -n -r 32805

Error:
 2015-04-03 02:04:58 URL:https://reviews.apache.org/r/32805/diff/raw/ 
[2268/2268] -> "32805.patch" [1]
error: patch failed: src/linux/perf.cpp:29
error: src/linux/perf.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On April 3, 2015, 1:42 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32805/
> ---
> 
> (Updated April 3, 2015, 1:42 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-2462
> https://issues.apache.org/jira/browse/MESOS-2462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Terminated the perf subprocess once the parent exits.
> 
> The idea is to have a nanny process which installs a SIGTERM handler which 
> will kill the process group (of course, we need to put the nanny and perf 
> process to the same process group). We set the death signal of the nanny 
> process to be SIGTERM. In that way, when slave exits, the nanny process will 
> receive a SIGTERM, which will then kill all processes in the process group.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cad6c80e2a9608ff02fc2b8976efba52713dd5a8 
> 
> Diff: https://reviews.apache.org/r/32805/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also manually verified it by terminating the slave while perf is in 
> progress. The perf is killed immediately.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>