Re: Review Request 61408: Added test cases for V1 teardown Call.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61222, 61408]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 15, 2017, 12:07 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61408/
> ---
> 
> (Updated Aug. 15, 2017, 12:07 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-6846
> https://issues.apache.org/jira/browse/MESOS-6846
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for teardown operation in V1 operator API.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
> 
> 
> Diff: https://reviews.apache.org/r/61408/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61408: Added test cases for V1 teardown Call.

[Quinn Leng]

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

(Updated Aug. 15, 2017, 12:07 a.m.)


Review request for mesos, Anand Mazumdar and Greg Mann.


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


Repository: mesos


Description
---

Added test cases for teardown operation in V1 operator API.


Diffs (updated)
-

  src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 


Diff: https://reviews.apache.org/r/61408/diff/3/

Changes: https://reviews.apache.org/r/61408/diff/2-3/


Testing
---

make check


Thanks,

Quinn Leng

Re: Review Request 60495: Added network ports isolator listen socket utilities.

[James Peach]

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

(Updated Aug. 14, 2017, 11:40 p.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


Changes
---

Rebased and addressed review feedback.


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


Repository: mesos


Description
---

Implemented network ports isolator socket utilities to find the
inode numbers of all the listening sockets, and the inodes of the
open sockets for a process. Together, these utilities can tell us
which sockets a process is listening on.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/60495/diff/9/

Changes: https://reviews.apache.org/r/60495/diff/8-9/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60494: Exposed LinuxLauncher cgroups helper.

[James Peach]

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

(Updated Aug. 14, 2017, 11:30 p.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


Changes
---

Rebased and addressed review feedback.


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


Repository: mesos


Description
---

Expose the LinuxLauncher cgroups helper to generate the cgroups
path from a container ID. This is needed by the `network/ports`
isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/linux_launcher.hpp 
e1525231e0a374bc044e929e82b8d051732e97cb 
  src/slave/containerizer/mesos/linux_launcher.cpp 
1cea04edac8e0c4aea8c1c7d946b5065f3eac931 


Diff: https://reviews.apache.org/r/60494/diff/9/

Changes: https://reviews.apache.org/r/60494/diff/8-9/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 61574: Added `kill()` support to the test containerizer interface.

[Anand Mazumdar]

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

(Updated Aug. 14, 2017, 11:11 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments.


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


Repository: mesos


Description
---

This is needed to make some tests pass that rely on the test
containerizer.


Diffs (updated)
-

  src/tests/containerizer.hpp 4bd40c32625bc1f7998d523c6ee81bf78ac74538 
  src/tests/containerizer.cpp 1d2b6391cf7a7545fa44206c59d05764f3e8cdfb 


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

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 61570: Added the `kill()` function to the containerizer interface.

[Anand Mazumdar]

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

(Updated Aug. 14, 2017, 11:09 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This would be used by now on for killing a container by sending
a signal to it similar to the linux equivalent `kill()` system call.


Diffs (updated)
-

  src/slave/containerizer/containerizer.hpp 
0954ed6175e4c8f963bf371e54e0f9ffe7bc9c1c 
  src/slave/containerizer/mesos/containerizer.hpp 
fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
  src/slave/containerizer/mesos/containerizer.cpp 
ff192bb085f3554ad1b1f20fb73bf827bf04ef13 


Diff: https://reviews.apache.org/r/61570/diff/3/

Changes: https://reviews.apache.org/r/61570/diff/2-3/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 61493: Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.

[Gilbert Song]

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


Fix it, then Ship it!





src/tests/default_executor_tests.cpp
Line 1375 (original), 1375 (patched)


this test is linux specific, need `#ifdef __linux__`.



src/tests/default_executor_tests.cpp
Lines 1378 (patched)


should be a `ROOT` test.



src/tests/default_executor_tests.cpp
Lines 1383-1384 (patched)


`namespces/pid` isolator is not specified! all containers are sharing the 
agent's pid ns by default.



src/tests/default_executor_tests.cpp
Lines 1414-1419 (patched)


not yours. this is a tech debt in this file. let's use 
`v1::scheduler::SendSubscribe()`.



src/tests/default_executor_tests.cpp
Lines 1427-1431 (patched)


use `v1::createExecutorInfo()`



src/tests/default_executor_tests.cpp
Lines 1463-1466 (patched)


oneline instead.



src/tests/default_executor_tests.cpp
Lines 1498-1501 (patched)


ditto.



src/tests/default_executor_tests.cpp
Lines 1510-1514 (patched)


save as `executorSandbox`.


- Gilbert Song


On Aug. 13, 2017, 7:06 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61493/
> ---
> 
> (Updated Aug. 13, 2017, 7:06 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 3ac9f292f928aa67ee380a63233832365daa2cc2 
> 
> 
> Diff: https://reviews.apache.org/r/61493/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61262: Added 'heartbeat' event for the operator API.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [61262]

Failed command: python support/apply-reviews.py -n -r 61262

Error:
error: patch failed: src/tests/api_tests.cpp:1590
error: src/tests/api_tests.cpp: patch does not apply

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/223/console

- Mesos Reviewbot Windows


On Aug. 14, 2017, 9:51 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61262/
> ---
> 
> (Updated Aug. 14, 2017, 9:51 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-7695
> https://issues.apache.org/jira/browse/MESOS-7695
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the 'HEARTBEAT' event for the operator API, modified other
> related test cases to accept heartbeats.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b 
>   include/mesos/v1/master/master.proto 
> c3fb31de2509adcdec8204f8bbe46b46a31540e8 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
> 
> 
> Diff: https://reviews.apache.org/r/61262/diff/4/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61262: Added 'heartbeat' event for the operator API.

[Quinn Leng]

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

(Updated Aug. 14, 2017, 9:51 p.m.)


Review request for mesos, Anand Mazumdar and Greg Mann.


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


Repository: mesos


Description
---

Added the 'HEARTBEAT' event for the operator API, modified other
related test cases to accept heartbeats.


Diffs (updated)
-

  include/mesos/master/master.proto fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b 
  include/mesos/v1/master/master.proto c3fb31de2509adcdec8204f8bbe46b46a31540e8 
  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 


Diff: https://reviews.apache.org/r/61262/diff/4/

Changes: https://reviews.apache.org/r/61262/diff/3-4/


Testing
---

make check -j48


Thanks,

Quinn Leng

Re: Review Request 61621: Added MESOS-7660 to the changelog.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Aug. 14, 2017, 5:44 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61621/
> ---
> 
> (Updated Aug. 14, 2017, 5:44 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7660
> https://issues.apache.org/jira/browse/MESOS-7660
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-7660 to the changelog.
> 
> 
> Diffs
> -
> 
>   CHANGELOG abb0e3bea95648b91605cec37b21f079d2b15b41 
> 
> 
> Diff: https://reviews.apache.org/r/61621/diff/1/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61572: Made `killNestedContainer()` use `kill()` on the containerizer.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Aug. 14, 2017, 5:20 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61572/
> ---
> 
> (Updated Aug. 14, 2017, 5:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of invoking `destroy()` directly, `killNestedContainer()`
> would invoke `kill()` to terminate the nested container.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 2d33f0b498c8c819d1aaa6b39ae38b1009fda3e4 
> 
> 
> Diff: https://reviews.apache.org/r/61572/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61574: Added `kill()` support to the test containerizer interface.

[Vinod Kone]

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




src/tests/containerizer.hpp
Lines 168 (patched)


s/status/signal/



src/tests/containerizer.cpp
Lines 534-541 (patched)


Can you do this inside `TestContainerizerProcess::kill` instead of here? 
Seems more consistent with how we handled `attach` for example.


- Vinod Kone


On Aug. 10, 2017, 7:32 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61574/
> ---
> 
> (Updated Aug. 10, 2017, 7:32 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is needed to make some tests pass that rely on the test
> containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer.hpp 4bd40c32625bc1f7998d523c6ee81bf78ac74538 
>   src/tests/containerizer.cpp 1d2b6391cf7a7545fa44206c59d05764f3e8cdfb 
> 
> 
> Diff: https://reviews.apache.org/r/61574/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61573: Made the default executor support signal escalation.

[Vinod Kone]

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




src/launcher/default_executor.cpp
Lines 1044 (patched)


s/retry attempt/signal escalation timeout/


- Vinod Kone


On Aug. 14, 2017, 5:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61573/
> ---
> 
> (Updated Aug. 14, 2017, 5:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This modifies the default executor to perform signal escalation
> via the 'KILL_NESTED_CONTAINER' call i.e., send a SIGTERM followed
> by a SIGKILL after some grace period. Note that support for kill
> policies still needs to be done instead of using a constant grace
> period.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 
> 
> 
> Diff: https://reviews.apache.org/r/61573/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61573: Made the default executor support signal escalation.

[Vinod Kone]

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




src/launcher/default_executor.cpp
Lines 1000 (patched)


Can you add a TODO here to use kill policy set on TaskInfo (if present) and 
override with kill policy set on kill event (if present)?


- Vinod Kone


On Aug. 14, 2017, 5:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61573/
> ---
> 
> (Updated Aug. 14, 2017, 5:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This modifies the default executor to perform signal escalation
> via the 'KILL_NESTED_CONTAINER' call i.e., send a SIGTERM followed
> by a SIGKILL after some grace period. Note that support for kill
> policies still needs to be done instead of using a constant grace
> period.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 
> 
> 
> Diff: https://reviews.apache.org/r/61573/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61572: Made `killNestedContainer()` use `kill()` on the containerizer.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 14, 2017, 5:20 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61572/
> ---
> 
> (Updated Aug. 14, 2017, 5:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of invoking `destroy()` directly, `killNestedContainer()`
> would invoke `kill()` to terminate the nested container.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 2d33f0b498c8c819d1aaa6b39ae38b1009fda3e4 
> 
> 
> Diff: https://reviews.apache.org/r/61572/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61575: Added a test for verifying signal escalation on the default executor.

[Gastón Kleiman]

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




src/tests/default_executor_tests.cpp
Lines 537 (patched)


s/terimate/terminate/



src/tests/default_executor_tests.cpp
Lines 593-597 (patched)


In some tests we do this instead:

```
  v1::ExecutorInfo executorInfo = v1::createExecutorInfo(
  v1::DEFAULT_EXECUTOR_ID.value(),
  None(),
  None(),
  v1::ExecutorInfo::DEFAULT);
```

At some point we might want to choose one approach and do a sweeping change 
in this file.



src/tests/default_executor_tests.cpp
Lines 600 (patched)


s/EXPECT_NE(0, 
offers->offers().size());/EXPECT_FALSE(offers->offers().empty)());



src/tests/default_executor_tests.cpp
Lines 613-661 (patched)


What do you think about the following shorter (and IMHO more readable) 
approach?

```
  v1::Offer::Operation launchGroup = v1::LAUNCH_GROUP(
  executorInfo,
  v1::createTaskGroupInfo({taskInfo}));

  Future update1;
  EXPECT_CALL(*scheduler, update(_, _))
.WillOnce(DoAll(
FutureArg<1>(),
v1::scheduler::SendAcknowledge(
frameworkId,
offer.agent_id(;

  mesos.send(v1::createCallAccept(
  frameworkId,
  offer,
  {reserve, create, launchGroup}));
```


- Gastón Kleiman


On Aug. 14, 2017, 5:59 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61575/
> ---
> 
> (Updated Aug. 14, 2017, 5:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test uses the kill policy helper and blocks the SIGTERM signal.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> b9776314a8781963b92ba9ac297654f61a443bc8 
> 
> 
> Diff: https://reviews.apache.org/r/61575/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 60524: Stout: Made the `Duration` operators handle int overflows explicitly.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60524, 61621, 60525, 60815, 61601, 60721]

Failed command: python support/apply-reviews.py -n -r 60721

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File "support/apply-reviews.py", line 268, in commit_patch
shell(cmd, options['dry_run'])
  File "support/apply-reviews.py", line 144, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
  File "C:\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
  File "C:\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
  File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 
25: ordinal not in range(128)

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/221/console

- Mesos Reviewbot Windows


On July 26, 2017, 11:31 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60524/
> ---
> 
> (Updated July 26, 2017, 11:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7661
> https://issues.apache.org/jira/browse/MESOS-7661
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the `Duration` arithmetic operators return `Duration::max()` if the
> operation would result in an integer overflow, and `Duration::min()` if
> it would result in an underflow.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/duration.hpp 
> b0cd77b833f6fbf752b4db820fd43b87e1d1e476 
>   3rdparty/stout/tests/duration_tests.cpp 
> 59b08f14849a8db31f11fbd0b2e1248c99afd9dd 
> 
> 
> Diff: https://reviews.apache.org/r/60524/diff/5/
> 
> 
> Testing
> ---
> 
> Added new Stout tests and confirmed that the Mesos test suite still passes.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61570: Added the `kill()` function to the containerizer interface.

[Vinod Kone]

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




src/slave/containerizer/containerizer.hpp
Lines 149 (patched)


I see you followed the `attach` pattern here, but another option is to make 
this pure virtual and explicitly implement kill (returns unsupported failure) 
on docker containerizer. An advantate I see that you can look at docker 
containerizer and quickly see what are the things you need to implement in the 
future.


- Vinod Kone


On Aug. 14, 2017, 5:20 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61570/
> ---
> 
> (Updated Aug. 14, 2017, 5:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would be used by now on for killing a container by sending
> a signal to it similar to the linux equivalent `kill()` system call.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> 0954ed6175e4c8f963bf371e54e0f9ffe7bc9c1c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ff192bb085f3554ad1b1f20fb73bf827bf04ef13 
> 
> 
> Diff: https://reviews.apache.org/r/61570/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61620: Tracked successful task fetches rather than total.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61620]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 14, 2017, 5:01 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61620/
> ---
> 
> (Updated Aug. 14, 2017, 5:01 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7888
> https://issues.apache.org/jira/browse/MESOS-7888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the fetcher task metrics, track the number of times we
> successfully fetch all the task URIs rather than the total
> number of times we try to fetch task URIs. The success count
> is easier to observe and explain, and is consistent with the
> metrics nomenclature for other pending metrics.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
>   src/slave/containerizer/fetcher.cpp 
> fdeb9de346534fa7e223f90db9ba17f77a81611f 
>   src/slave/containerizer/fetcher_process.hpp 
> 36010e3b3f2bd5b67272baecb490dd2a39a4974b 
>   src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 
> 
> 
> Diff: https://reviews.apache.org/r/61620/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61573: Made the default executor support signal escalation.

[Gastón Kleiman]

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




src/launcher/default_executor.cpp
Line 996 (original), 996-997 (patched)


I think that it'd be useful to include the task ID in this message.



src/launcher/default_executor.cpp
Lines 1002-1003 (patched)


Ditto (and also the container ID).



src/launcher/default_executor.cpp
Lines 1053-1054 (patched)


Ditto.



src/launcher/default_executor.cpp
Lines 1058-1059 (patched)


Ditto.


- Gastón Kleiman


On Aug. 14, 2017, 5:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61573/
> ---
> 
> (Updated Aug. 14, 2017, 5:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This modifies the default executor to perform signal escalation
> via the 'KILL_NESTED_CONTAINER' call i.e., send a SIGTERM followed
> by a SIGKILL after some grace period. Note that support for kill
> policies still needs to be done instead of using a constant grace
> period.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 
> 
> 
> Diff: https://reviews.apache.org/r/61573/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61571: Added `kill()` call to the composing containerizer.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 10, 2017, 7:33 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61571/
> ---
> 
> (Updated Aug. 10, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `kill()` call to the composing containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> bef6d880ca1d815fc21d9d017f6de2d26ad5218f 
>   src/slave/containerizer/composing.cpp 
> a003e1b80dc9b4dec5b3fbbadb2daecf855c90c7 
> 
> 
> Diff: https://reviews.apache.org/r/61571/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61570: Added the `kill()` function to the containerizer interface.

[Vinod Kone]

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




src/slave/containerizer/mesos/containerizer.cpp
Lines 582 (patched)


s/status/signal/



src/slave/containerizer/mesos/containerizer.cpp
Lines 587 (patched)


s/status/signal/



src/slave/containerizer/mesos/containerizer.cpp
Lines 2455 (patched)


Can you add a TODO to chain this on the launch completion future? Seems a 
bit unfortunate to destroy the container irrespective of the signal.



src/slave/containerizer/mesos/containerizer.cpp
Lines 2466 (patched)


s/running//



src/slave/containerizer/mesos/containerizer.cpp
Lines 2467 (patched)


no need to include the container id because the caller knows it already.


- Vinod Kone


On Aug. 14, 2017, 5:20 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61570/
> ---
> 
> (Updated Aug. 14, 2017, 5:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would be used by now on for killing a container by sending
> a signal to it similar to the linux equivalent `kill()` system call.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> 0954ed6175e4c8f963bf371e54e0f9ffe7bc9c1c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ff192bb085f3554ad1b1f20fb73bf827bf04ef13 
> 
> 
> Diff: https://reviews.apache.org/r/61570/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61573: Made the default executor support signal escalation.

[Anand Mazumdar]

> On Aug. 14, 2017, 6:45 p.m., Gastón Kleiman wrote:
> > src/launcher/default_executor.cpp
> > Line 233 (original), 233 (patched)
> > 
> >
> > shouldn't we also pass the `event.kill().kill_policy()`? It allows 
> > frameworks/operators to override the kill policy, see: 
> > https://github.com/apache/mesos/blob/628d6609b6eeb90767e5799d0177bfe4828d71aa/include/mesos/executor/executor.proto#L96-L106
> 
> Gastón Kleiman wrote:
> Droppping the issue, I just noticed that the commit message says that 
> this doesn't include support for kill policies.

Yeah, we still need to add support for `KillPolicy` later. Hopefully, it should 
be trivial after this change (still would need an additional test)

I had added this information to the review description already:
```Note that support for kill
policies still needs to be done instead of using a constant grace
period.```


- Anand


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


On Aug. 14, 2017, 5:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61573/
> ---
> 
> (Updated Aug. 14, 2017, 5:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This modifies the default executor to perform signal escalation
> via the 'KILL_NESTED_CONTAINER' call i.e., send a SIGTERM followed
> by a SIGKILL after some grace period. Note that support for kill
> policies still needs to be done instead of using a constant grace
> period.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 
> 
> 
> Diff: https://reviews.apache.org/r/61573/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61573: Made the default executor support signal escalation.

[Gastón Kleiman]

> On Aug. 14, 2017, 6:45 p.m., Gastón Kleiman wrote:
> > src/launcher/default_executor.cpp
> > Line 233 (original), 233 (patched)
> > 
> >
> > shouldn't we also pass the `event.kill().kill_policy()`? It allows 
> > frameworks/operators to override the kill policy, see: 
> > https://github.com/apache/mesos/blob/628d6609b6eeb90767e5799d0177bfe4828d71aa/include/mesos/executor/executor.proto#L96-L106

Droppping the issue, I just noticed that the commit message says that this 
doesn't include support for kill policies.


- Gastón


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


On Aug. 14, 2017, 5:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61573/
> ---
> 
> (Updated Aug. 14, 2017, 5:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This modifies the default executor to perform signal escalation
> via the 'KILL_NESTED_CONTAINER' call i.e., send a SIGTERM followed
> by a SIGKILL after some grace period. Note that support for kill
> policies still needs to be done instead of using a constant grace
> period.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 
> 
> 
> Diff: https://reviews.apache.org/r/61573/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61573: Made the default executor support signal escalation.

[Gastón Kleiman]

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




src/launcher/default_executor.cpp
Line 233 (original), 233 (patched)


shouldn't we also pass the `event.kill().kill_policy()`? It allows 
frameworks/operators to override the kill policy, see: 
https://github.com/apache/mesos/blob/628d6609b6eeb90767e5799d0177bfe4828d71aa/include/mesos/executor/executor.proto#L96-L106


- Gastón Kleiman


On Aug. 14, 2017, 5:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61573/
> ---
> 
> (Updated Aug. 14, 2017, 5:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This modifies the default executor to perform signal escalation
> via the 'KILL_NESTED_CONTAINER' call i.e., send a SIGTERM followed
> by a SIGKILL after some grace period. Note that support for kill
> policies still needs to be done instead of using a constant grace
> period.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 
> 
> 
> Diff: https://reviews.apache.org/r/61573/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61571: Added `kill()` call to the composing containerizer.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Aug. 10, 2017, 7:33 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61571/
> ---
> 
> (Updated Aug. 10, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `kill()` call to the composing containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> bef6d880ca1d815fc21d9d017f6de2d26ad5218f 
>   src/slave/containerizer/composing.cpp 
> a003e1b80dc9b4dec5b3fbbadb2daecf855c90c7 
> 
> 
> Diff: https://reviews.apache.org/r/61571/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61569: Added the field `signal` to the `KillNestedContainer` call.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 10, 2017, 7:32 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61569/
> ---
> 
> (Updated Aug. 10, 2017, 7:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would be used later for sending signals (SIGTERM, SIGKILL etc.)
> to the running container. Previously, SIGKILL was used by default.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 9bac9541acd24e1123ca5dd5925e2a1381d13b4a 
>   include/mesos/v1/agent/agent.proto ea9282cf12fbe1c2ddeaa37223e4811685263734 
> 
> 
> Diff: https://reviews.apache.org/r/61569/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61493: Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Aug. 14, 2017, 2:06 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61493/
> ---
> 
> (Updated Aug. 14, 2017, 2:06 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 3ac9f292f928aa67ee380a63233832365daa2cc2 
> 
> 
> Diff: https://reviews.apache.org/r/61493/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61570: Added the `kill()` function to the containerizer interface.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Aug. 14, 2017, 5:20 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61570/
> ---
> 
> (Updated Aug. 14, 2017, 5:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would be used by now on for killing a container by sending
> a signal to it similar to the linux equivalent `kill()` system call.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> 0954ed6175e4c8f963bf371e54e0f9ffe7bc9c1c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ff192bb085f3554ad1b1f20fb73bf827bf04ef13 
> 
> 
> Diff: https://reviews.apache.org/r/61570/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61569: Added the field `signal` to the `KillNestedContainer` call.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Aug. 10, 2017, 7:32 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61569/
> ---
> 
> (Updated Aug. 10, 2017, 7:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-7879
> https://issues.apache.org/jira/browse/MESOS-7879
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would be used later for sending signals (SIGTERM, SIGKILL etc.)
> to the running container. Previously, SIGKILL was used by default.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 9bac9541acd24e1123ca5dd5925e2a1381d13b4a 
>   include/mesos/v1/agent/agent.proto ea9282cf12fbe1c2ddeaa37223e4811685263734 
> 
> 
> Diff: https://reviews.apache.org/r/61569/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 61220: Added MESOS-5116 to the CHANGELOG.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61220]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 14, 2017, 3:45 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61220/
> ---
> 
> (Updated Aug. 14, 2017, 3:45 p.m.)
> 
> 
> Review request for mesos, Kapil Arya and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-5116 (XFS disk isolator support for no-enforce mode) to
> the CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG abb0e3bea95648b91605cec37b21f079d2b15b41 
>   docs/configuration.md 6ab2d1a06de9e4fff9e6769d2b2cbd05e8f3d17a 
>   docs/upgrades.md 349d8a144097d9be354a95c904ab7209ae3cc442 
> 
> 
> Diff: https://reviews.apache.org/r/61220/diff/3/
> 
> 
> Testing
> ---
> 
> Generated website and verified correct links and content.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61575: Added a test for verifying signal escalation on the default executor.

[Anand Mazumdar]

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

(Updated Aug. 14, 2017, 5:59 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Minor change to a comment.


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


Repository: mesos


Description
---

This test uses the kill policy helper and blocks the SIGTERM signal.


Diffs (updated)
-

  src/tests/default_executor_tests.cpp b9776314a8781963b92ba9ac297654f61a443bc8 


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

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 61621: Added MESOS-7660 to the changelog.

[Gastón Kleiman]

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


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


Repository: mesos


Description
---

Added MESOS-7660 to the changelog.


Diffs
-

  CHANGELOG abb0e3bea95648b91605cec37b21f079d2b15b41 


Diff: https://reviews.apache.org/r/61621/diff/1/


Testing
---

Not a functional change.


Thanks,

Gastón Kleiman

Re: Review Request 60525: Fixed the default filter used by the allocator.

[Gastón Kleiman]

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

(Updated Aug. 14, 2017, 5:44 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


Changes
---

Moved the changelog changes to a different patch, improved the documentation.


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


Repository: mesos


Description
---

If a framework accepts/refuses an offer using a very long filter, the
`HierarchicalAllocator` will use the default filter instead. Meaning
that it will filter the resources for only 5 seconds. This can happen
when a framework sets `Filter::refuse_seconds` to a number of seconds
larger than what fits in `Duration`.

This patch makes the hierarchical allocator cap the filter duration to
at most 365 days.


Diffs (updated)
-

  docs/scheduler-http-api.md 4a5d77b88ae7cf0a0d8d39fe2579eb68bf33059a 
  include/mesos/mesos.proto 50a5caf2623fff7d371753cbc6e842779fbf9769 
  include/mesos/v1/mesos.proto 9d47a8a3a2f10fa493b4db35a73f33e6ee1ac727 
  src/master/allocator/mesos/hierarchical.cpp 
f021c34ef11aac42026ba39c5a1b775794982035 


Diff: https://reviews.apache.org/r/60525/diff/9/

Changes: https://reviews.apache.org/r/60525/diff/8-9/


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 61570: Added the `kill()` function to the containerizer interface.

[Anand Mazumdar]

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

(Updated Aug. 14, 2017, 5:20 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


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


Repository: mesos


Description
---

This would be used by now on for killing a container by sending
a signal to it similar to the linux equivalent `kill()` system call.


Diffs (updated)
-

  src/slave/containerizer/containerizer.hpp 
0954ed6175e4c8f963bf371e54e0f9ffe7bc9c1c 
  src/slave/containerizer/mesos/containerizer.hpp 
fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
  src/slave/containerizer/mesos/containerizer.cpp 
ff192bb085f3554ad1b1f20fb73bf827bf04ef13 


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

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 61572: Made `killNestedContainer()` use `kill()` on the containerizer.

[Anand Mazumdar]

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

(Updated Aug. 14, 2017, 5:20 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Changed `Optional` to `int` for `signal` variable.


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


Repository: mesos


Description
---

Instead of invoking `destroy()` directly, `killNestedContainer()`
would invoke `kill()` to terminate the nested container.


Diffs (updated)
-

  src/slave/http.cpp 2d33f0b498c8c819d1aaa6b39ae38b1009fda3e4 


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

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 61573: Made the default executor support signal escalation.

[Anand Mazumdar]

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

(Updated Aug. 14, 2017, 5:19 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Minor changes in code for readability.


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


Repository: mesos


Description
---

This modifies the default executor to perform signal escalation
via the 'KILL_NESTED_CONTAINER' call i.e., send a SIGTERM followed
by a SIGKILL after some grace period. Note that support for kill
policies still needs to be done instead of using a constant grace
period.


Diffs (updated)
-

  src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 


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

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 60721: Stout: Made `Duration::parse()` handle durations out of range.

[Gastón Kleiman]

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

(Updated Aug. 14, 2017, 5:12 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


Changes
---

Magic numbers are bad (even though I like the band).


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


Repository: mesos


Description
---

Made `Duration:parse()` return an error if the argument is out of the
range that a `Duration` can represent.


Diffs (updated)
-

  3rdparty/stout/include/stout/duration.hpp 
b0cd77b833f6fbf752b4db820fd43b87e1d1e476 
  3rdparty/stout/tests/duration_tests.cpp 
59b08f14849a8db31f11fbd0b2e1248c99afd9dd 


Diff: https://reviews.apache.org/r/60721/diff/6/

Changes: https://reviews.apache.org/r/60721/diff/5-6/


Testing
---

Added a new expectation to an existing test and confirmed that tests still pass.


Thanks,

Gastón Kleiman

Review Request 61620: Tracked successful task fetches rather than total.

[James Peach]

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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

In the fetcher task metrics, track the number of times we
successfully fetch all the task URIs rather than the total
number of times we try to fetch task URIs. The success count
is easier to observe and explain, and is consistent with the
metrics nomenclature for other pending metrics.


Diffs
-

  docs/monitoring.md d1f64d46b22e79c16a680a2e7fff9a01202c5457 
  src/slave/containerizer/fetcher.cpp fdeb9de346534fa7e223f90db9ba17f77a81611f 
  src/slave/containerizer/fetcher_process.hpp 
36010e3b3f2bd5b67272baecb490dd2a39a4974b 
  src/tests/fetcher_tests.cpp 01da9fef8e6766cc550d6dc5fe595de86906fb54 


Diff: https://reviews.apache.org/r/61620/diff/1/


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60525: Fixed the default filter used by the allocator.

[Greg Mann]

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


Fix it, then Ship it!





CHANGELOG
Line 35 (original), 35 (patched)


Nit: this change doesn't belong in this patch :)



docs/scheduler-http-api.md
Line 168 (original), 168 (patched)


Let's tweak this line a bit to accommodate your new additions.

How about moving the following portion:
"Also, any of the offer's resources not used in the `ACCEPT` call (e.g., to 
launch a task or task group) are considered declined and might be reoffered to 
other frameworks."

and merging it with your next sentence, "The unused resources ..."


- Greg Mann


On Aug. 12, 2017, 1:07 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60525/
> ---
> 
> (Updated Aug. 12, 2017, 1:07 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7660
> https://issues.apache.org/jira/browse/MESOS-7660
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework accepts/refuses an offer using a very long filter, the
> `HierarchicalAllocator` will use the default filter instead. Meaning
> that it will filter the resources for only 5 seconds. This can happen
> when a framework sets `Filter::refuse_seconds` to a number of seconds
> larger than what fits in `Duration`.
> 
> This patch makes the hierarchical allocator cap the filter duration to
> at most 365 days.
> 
> 
> Diffs
> -
> 
>   CHANGELOG abb0e3bea95648b91605cec37b21f079d2b15b41 
>   docs/scheduler-http-api.md 4a5d77b88ae7cf0a0d8d39fe2579eb68bf33059a 
>   include/mesos/mesos.proto 50a5caf2623fff7d371753cbc6e842779fbf9769 
>   include/mesos/v1/mesos.proto 9d47a8a3a2f10fa493b4db35a73f33e6ee1ac727 
>   src/master/allocator/mesos/hierarchical.cpp 
> f021c34ef11aac42026ba39c5a1b775794982035 
> 
> 
> Diff: https://reviews.apache.org/r/60525/diff/8/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 60815: Prefer leading instead of trailing spaces for allocator log messages.

[Greg Mann]

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


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.cpp
Lines 1173-1174 (original), 1173-1174 (patched)


Need to remove the trailing space.


- Greg Mann


On July 12, 2017, 9:49 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60815/
> ---
> 
> (Updated July 12, 2017, 9:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer leading instead of trailing spaces for allocator log messages.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f021c34ef11aac42026ba39c5a1b775794982035 
> 
> 
> Diff: https://reviews.apache.org/r/60815/diff/3/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61601: Stout: make boundary checking in Duration consistent.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Aug. 12, 2017, 1:06 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61601/
> ---
> 
> (Updated Aug. 12, 2017, 1:06 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7661
> https://issues.apache.org/jira/browse/MESOS-7661
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: make boundary checking in Duration consistent.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/duration.hpp 
> b0cd77b833f6fbf752b4db820fd43b87e1d1e476 
> 
> 
> Diff: https://reviews.apache.org/r/61601/diff/1/
> 
> 
> Testing
> ---
> 
> Stout tests still pass.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61220: Added MESOS-5116 to the CHANGELOG.

[James Peach]

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

(Updated Aug. 14, 2017, 3:45 p.m.)


Review request for mesos, Kapil Arya and Jiang Yan Xu.


Repository: mesos


Description
---

Added MESOS-5116 (XFS disk isolator support for no-enforce mode) to
the CHANGELOG.


Diffs (updated)
-

  CHANGELOG abb0e3bea95648b91605cec37b21f079d2b15b41 
  docs/configuration.md 6ab2d1a06de9e4fff9e6769d2b2cbd05e8f3d17a 
  docs/upgrades.md 349d8a144097d9be354a95c904ab7209ae3cc442 


Diff: https://reviews.apache.org/r/61220/diff/3/

Changes: https://reviews.apache.org/r/61220/diff/2-3/


Testing (updated)
---

Generated website and verified correct links and content.


Thanks,

James Peach

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61597: Fixed linking to `IPHlpAPI` library.

[Mesos Reviewbot Windows]

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



Bad review!

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot Windows


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>