Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-08-17 Thread Timothy Chen 

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

Ship it!


Ship It!


src/linux/cgroups.cpp (line 1686)


More like a code suggestion, looks like reap is always called after kill, 
and just waiting on all the statuses you gathered.

I wonder why not just move collect inside of  kill(processes) in the end 
and return that instead of another callback?


- Timothy Chen


On Aug. 13, 2015, 1:52 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated Aug. 13, 2015, 1:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> + manual tests
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-08-13 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36612, 36620]

All tests passed.

- Mesos ReviewBot


On Aug. 13, 2015, 1:52 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated Aug. 13, 2015, 1:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> + manual tests
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-08-13 Thread Joerg Schad 

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

(Updated Aug. 13, 2015, 1:52 p.m.)


Review request for mesos, Benjamin Hindman and Timothy Chen.


Changes
---

Addressed comment and rebased.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 

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


Testing (updated)
---

sudo make check
+ manual tests


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-08-13 Thread Joerg Schad 


> On July 28, 2015, 7:20 p.m., Timothy Chen wrote:
> > I notice there are no new tests added for this, can you add a test to 
> > verify the new change works?
> 
> Timothy Chen wrote:
> Are you able to add this? Otherwise let's add a TODO and get this in.

Created MESOS-3255 to follow up with tests.


- Joerg


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


On July 24, 2015, 1:07 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 24, 2015, 1:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-08-12 Thread Timothy Chen 


> On July 28, 2015, 7:20 p.m., Timothy Chen wrote:
> > I notice there are no new tests added for this, can you add a test to 
> > verify the new change works?

Are you able to add this? Otherwise let's add a TODO and get this in.


- Timothy


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


On July 24, 2015, 1:07 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 24, 2015, 1:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-28 Thread Timothy Chen 

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


I notice there are no new tests added for this, can you add a test to verify 
the new change works?


src/linux/cgroups.cpp (line 1689)


I'll move the onFailed to next line so we line up the indentation.


- Timothy Chen


On July 24, 2015, 1:07 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 24, 2015, 1:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-27 Thread Joerg Schad 


> On July 23, 2015, 5:42 p.m., Timothy Chen wrote:
> > src/linux/cgroups.cpp, line 1696
> > 
> >
> > Seems like we can just a lambda instead of a new fail method, then we 
> > don't even need to store chain variable right?

We need to store the chain variable for the finalize call, e.g. when the 
timeout kills the process.


- Joerg


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


On July 24, 2015, 1:07 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 24, 2015, 1:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-24 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36612, 36620]

All tests passed.

- Mesos ReviewBot


On July 24, 2015, 1:07 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 24, 2015, 1:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-24 Thread Joerg Schad 

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

(Updated July 24, 2015, 1:07 p.m.)


Review request for mesos, Benjamin Hindman and Timothy Chen.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-24 Thread Mesos ReviewBot 

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


Bad patch!

Reviews applied: [36612, 36620]

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

Error:
 2015-07-24 12:57:59 URL:https://reviews.apache.org/r/36620/diff/raw/ 
[234469/234469] -> "36620.patch" [1]
Successfully applied: WIP Added Non-Freezeer Task Killer.

WIP Added Non-Freezeer Task Killer.


Review: https://reviews.apache.org/r/36620
Checking 54 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
mesos:
  cmake/MesosConfigure.cmake
  docs/app-framework-development-guide.md
  docs/fetcher-cache-internals.md
  docs/release-guide.md
  include/mesos/mesos.proto
  include/mesos/scheduler/scheduler.proto
  include/mesos/slave/isolator.hpp
  include/mesos/slave/isolator.proto
  src/Makefile.am
  src/common/protobuf_utils.cpp
  src/common/protobuf_utils.hpp
  src/exec/exec.cpp
  src/linux/cgroups.cpp
  src/master/master.cpp
  src/master/master.hpp
  src/sched/sched.cpp
  src/scheduler/scheduler.cpp
  src/slave/containerizer/isolator.cpp
  src/slave/containerizer/isolators/cgroups/cpushare.cpp
  src/slave/containerizer/isolators/cgroups/cpushare.hpp
  src/slave/containerizer/isolators/cgroups/mem.cpp
  src/slave/containerizer/isolators/cgroups/mem.hpp
  src/slave/containerizer/isolators/cgroups/perf_event.cpp
  src/slave/containerizer/isolators/cgroups/perf_event.hpp
  src/slave/containerizer/isolators/filesystem/posix.cpp
  src/slave/containerizer/isolators/filesystem/posix.hpp
  src/slave/containerizer/isolators/filesystem/shared.cpp
  src/slave/containerizer/isolators/filesystem/shared.hpp
  src/slave/containerizer/isolators/namespaces/pid.cpp
  src/slave/containerizer/isolators/namespaces/pid.hpp
  src/slave/containerizer/isolators/network/port_mapping.cpp
  src/slave/containerizer/isolators/network/port_mapping.hpp
  src/slave/containerizer/isolators/posix.hpp
  src/slave/containerizer/isolators/posix/disk.cpp
  src/slave/containerizer/isolators/posix/disk.hpp
  src/slave/containerizer/launcher.cpp
  src/slave/containerizer/linux_launcher.cpp
  src/slave/containerizer/mesos/containerizer.cpp
  src/slave/containerizer/mesos/containerizer.hpp
  src/tests/containerizer_tests.cpp
  src/tests/disk_quota_tests.cpp
  src/tests/docker_containerizer_tests.cpp
  src/tests/external_containerizer_test.cpp
  src/tests/fault_tolerance_tests.cpp
  src/tests/hook_tests.cpp
  src/tests/isolator.hpp
  src/tests/master_authorization_tests.cpp
  src/tests/master_slave_reconciliation_tests.cpp
  src/tests/master_tests.cpp
  src/tests/master_validation_tests.cpp
  src/tests/memory_pressure_tests.cpp
  src/tests/partition_tests.cpp
  src/tests/persistent_volume_tests.cpp
  src/tests/port_mapping_tests.cpp
  src/tests/rate_limiting_tests.cpp
  src/tests/reconciliation_tests.cpp
  src/tests/registrar_zookeeper_tests.cpp
  src/tests/scheduler_tests.cpp
  src/tests/slave_recovery_tests.cpp
  src/tests/slave_tests.cpp
  support/tag.sh
stout:
  3rdparty/libprocess/3rdparty/stout/cmake/FindApr.cmake
  3rdparty/libprocess/3rdparty/stout/cmake/FindSvn.cmake
  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake
  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt
libprocess:
  3rdparty/libprocess/3rdparty/CMakeLists.txt
  3rdparty/libprocess/cmake/ProcessConfigure.cmake
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake
  3rdparty/libprocess/include/process/http.hpp
  3rdparty/libprocess/src/tests/CMakeLists.txt
Failed to commit patch

- Mesos ReviewBot


On July 24, 2015, 12:15 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 24, 2015, 12:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> d7c6dfb04120a7fd28e1a4682f39015bd149eb40 
>   3rdparty/libprocess/3rdparty/stout/cmake/FindApr.cmake 
> 4b28aa170f48d37ae9096bc28a64d8a32e8d35dd 
>   3rdparty/libprocess/3r

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-24 Thread Joerg Schad 

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

(Updated July 24, 2015, 12:15 p.m.)


Review request for mesos, Benjamin Hindman and Timothy Chen.


Changes
---

Addressed comments


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
d7c6dfb04120a7fd28e1a4682f39015bd149eb40 
  3rdparty/libprocess/3rdparty/stout/cmake/FindApr.cmake 
4b28aa170f48d37ae9096bc28a64d8a32e8d35dd 
  3rdparty/libprocess/3rdparty/stout/cmake/FindSvn.cmake 
87737395d011393e7ef0c86fa18a3d31f3045fb0 
  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
6e7eb00b79e81cb6426cf08d50e22075898f2616 
  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
d58f8a4ab8f6f761adb6a9db2dac438ffe0e211b 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
b1a519007e5bed196541f294c4676277f9708714 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
4681eef1347fd46e85e1768795cc2f34aaba02b2 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 
  3rdparty/libprocess/include/process/http.hpp 
9faed55247a3ccd629db7b85dbf31d3117e120e9 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
b61ca1e5be2054c1156205c62a1e8311a575bfa5 
  cmake/MesosConfigure.cmake ee47842a5a2681221838e736738943572e96e702 
  docs/app-framework-development-guide.md 
db0181c0b82fded1860ef636747e70d80e3884f2 
  docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b 
  docs/release-guide.md a946c7e2c57632d3ff1b4cd6ac6c47df69dcf80c 
  include/mesos/mesos.proto e015c81d5052214ef8207642e23b3892a6123c9a 
  include/mesos/scheduler/scheduler.proto 
89daf8a6b74057ee156b3ad691397e76fcb835b8 
  include/mesos/slave/isolator.hpp 8387efd39a0e868038350ab620b38e4ab89327db 
  include/mesos/slave/isolator.proto 07c1c1a09d1578dd4c3abd8bb1773782b4aa9549 
  src/Makefile.am 9f2d7e38b28c1208cd819d2470c7f6ec6d5a71f9 
  src/common/protobuf_utils.hpp 22046bad118818dc815f1752fa805adaba136bab 
  src/common/protobuf_utils.cpp 4cfbda4696d4abc169b3d1a05dc522889232ebcd 
  src/exec/exec.cpp 54ef622ca3e9a004b065e2f3a496a1b87902f722 
  src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
  src/master/master.hpp bf61bb2deb61a73303f4122383dcb7e8f5d726de 
  src/master/master.cpp bab04feaf238e51c4d50feb1d3a9c0735ec68fec 
  src/sched/sched.cpp e411f376938a2c6793621de42e60425e5faed453 
  src/scheduler/scheduler.cpp badc107dbf4e2bd9146f9244724e0db5c2ae05d3 
  src/slave/containerizer/isolator.cpp ec14b207be36af7ca601e0f327e7025a155f41e4 
  src/slave/containerizer/isolators/cgroups/cpushare.hpp 
4fa9015e445872ec4a060e62dc2b5f2b30fc0f47 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp 
b1ebdaddc211bab023be85084bc82b6b82093f0b 
  src/slave/containerizer/isolators/cgroups/mem.hpp 
c198c832f8fb3765e932f1f20d3e16524340a7f2 
  src/slave/containerizer/isolators/cgroups/mem.cpp 
919e0f7eccff1d4c10852260965bda2bdb1c4267 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
243cf5a6ff699aa37f885cc164977126413d155b 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
367cb4364212ce057a474b574b8dbf8ccc1ae235 
  src/slave/containerizer/isolators/filesystem/posix.hpp 
d44023eb95c48d6ffbd69334f91a2b4c4b760db0 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
2aa8406b8386c7e407c9330400629a651f346ddf 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
4a5dcc35b6aea665893a1406fe4e2a97cf4e5d1a 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
f90045e7696b54b7b5082b9aa16c78d0347afde6 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
702f331d39acc75e9c98183c30227ec47f10e904 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
4241fa7a917ab31f52ece4caf16e94b3ef9ac353 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
6ffd7290eef2f0c3d3dc1c6236248e498c5be6ca 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
39d1813ee534df184cd5cf2b11af22e0195b0802 
  src/slave/containerizer/isolators/posix.hpp 
6ddab7df489feb2a7762178183191d5d1dc99e15 
  src/slave/containerizer/isolators/posix/disk.hpp 
fdf24a18bfca97863b7c15c39378b879d7b43517 
  src/slave/containerizer/isolators/posix/disk.cpp 
238f17941712e0709eba1cf78c6bc825b3b4459e 
  src/slave/containerizer/launcher.cpp ecb33309ff0941985eebf7a94d831330076978de 
  src/slave/containerizer/linux_launcher.cpp 
ed2e881f555d651ad1e5f1b62f32848ade09b424 
  src/slave/containerizer/mesos/containerizer.hpp 
5155362bef9163af3fa5334be3a39b91549e1ec1 
  src/slave/containerizer/mesos/containerizer.cpp 
c21e925e9584bc914f492d7f459498f732779fc8 
  src/tests/containerizer_tests.cpp 0b1338107ebe7b78821fddbefec90524f35e3858 
  src/tests/disk_quota_tests.cpp 1ed1342a10a4d59b84ef13a3d66eef95150c4e53 
  src/tests

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-23 Thread Timothy Chen 

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



src/linux/cgroups.cpp (line 1672)


Why we need a new promise here? Why can't we use promise variable you 
created in the class?



src/linux/cgroups.cpp (line 1678)


"Failed to get cgroup processes: " + processes.error()



src/linux/cgroups.cpp (line 1696)


Seems like we can just a lambda instead of a new fail method, then we don't 
even need to store chain variable right?



src/linux/cgroups.cpp (line 1713)


I don't think the error message includes the original intention right?

"Failed to cgroups kill: " + kill.error()


- Timothy Chen


On July 23, 2015, 12:21 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 23, 2015, 12:21 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-23 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36612, 36620]

All tests passed.

- Mesos ReviewBot


On July 23, 2015, 12:21 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 23, 2015, 12:21 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-23 Thread Joerg Schad 

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

(Updated July 23, 2015, 12:21 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Improved comments.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-23 Thread Joerg Schad 

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

(Updated July 23, 2015, 12:19 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Improved comments.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-22 Thread Timothy Chen 

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



src/linux/cgroups.cpp (line 1627)


I think it's good to also describe what the killer does and why it needs to 
recursively kill.



src/linux/cgroups.cpp (line 1859)


I don't think this log is necessary, at very least VLOG(1)


- Timothy Chen


On July 22, 2015, 10:04 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 22, 2015, 10:04 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-22 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36612, 36620]

All tests passed.

- Mesos ReviewBot


On July 22, 2015, 10:04 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 22, 2015, 10:04 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-22 Thread Joerg Schad 

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

(Updated July 22, 2015, 10:04 a.m.)


Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Mesos ReviewBot 

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


Bad patch!

Reviews applied: [36612]

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

Error:
 2015-07-22 05:28:09 URL:https://reviews.apache.org/r/36612/diff/raw/ 
[12620/12620] -> "36612.patch" [1]
error: patch failed: src/linux/cgroups.hpp:92
error: src/linux/cgroups.hpp: patch does not apply
error: patch failed: src/linux/cgroups.cpp:110
error: src/linux/cgroups.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On July 22, 2015, 4:37 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 22, 2015, 4:37 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Joerg Schad 

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

(Updated July 22, 2015, 4:37 a.m.)


Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Timothy Chen 

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



src/linux/cgroups.cpp (line 1671)


It's very odd to see the failure message set here but then a lot more 
attempts to try to set it in the finished callback.

How about having a failed method that sets the failure and another finished 
callback that is what you have?



src/linux/cgroups.cpp (line 1683)


Very wierd identation?



src/linux/cgroups.cpp (line 1686)


//check -> // Check



src/linux/cgroups.cpp (line 1688)


one more space to the right



src/linux/cgroups.cpp (line 1708)


extra space between "the" and "pids"


- Timothy Chen


On July 21, 2015, 5:18 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 21, 2015, 5:18 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Timothy Chen 

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



src/linux/cgroups.cpp (line 1504)


This is actually the Freezer right? 
And we should make this VLOG(1)



src/linux/cgroups.cpp (line 1627)


To me this doesn't killer doesn't "atomically" kill all of them, as that 
implies either all or none.

I think the TaskKiller actually best effort try to kill by signal and 
reintrospect all tasks in a cgroup within the timeout



src/linux/cgroups.cpp (line 1650)


100 ms seems way too fast?
And why not use the default timeout that's already there?



src/linux/cgroups.cpp (line 1668)


This should go to line 1674?



src/linux/cgroups.cpp (line 1676)


Move to previous line.



src/linux/cgroups.cpp (line 1684)


This fits on to previous line?


- Timothy Chen


On July 21, 2015, 5:18 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 21, 2015, 5:18 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Joerg Schad 

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

(Updated July 21, 2015, 5:18 p.m.)


Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs
-

  src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 

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


Testing (updated)
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Joerg Schad 

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

(Updated July 21, 2015, 5:17 p.m.)


Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs
-

  src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Jie Yu 

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


Could you please attach a ticket or explain in the description about the 
motivation?

- Jie Yu


On July 21, 2015, 4:42 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 21, 2015, 4:42 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36612, 36620]

All tests passed.

- Mesos ReviewBot


On July 21, 2015, 4:42 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated July 21, 2015, 4:42 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Joerg Schad 

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

(Updated July 21, 2015, 4:42 p.m.)


Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-07-21 Thread Joerg Schad 

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

(Updated July 21, 2015, 3:59 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Style fixes


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs (updated)
-

  src/linux/cgroups.cpp b7d11ac4eb8b78664c8c7bd7a5b0d566b5bf2f0e 

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


Testing
---


Thanks,

Joerg Schad