Re: Review Request 62230: Avoid GC pruning events from blocking other processes.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62252', '62230']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62230

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62230/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (253 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (241 ms)
[--] 30 tests from ContentType/SchedulerTest (24924 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (961 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 (995 
ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2054 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (131 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (160 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (335 ms 
total)

[--] Global test environment tear-down
[==] 627 tests from 66 test cases ran. (333988 ms total)
[  PASSED  ] 625 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] ContentType/MasterAPITest.EventAuthorizationFiltering/1, where 
GetParam() = application/json
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.SigkillExecutor/0, where 
GetParam() = "mesos"

 2 FAILED TESTS
  YOU HAVE 174 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62230/logs/mesos-tests-stderr.log):

```
I0913 05:23:46.984045 14064 master.cpp:8418] Removing framework 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a- (default)
I0913 05:23:46.985044 14064 master.cpp:3267] Deactivating framework 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a- (default)
I0913 05:23:46.999840 13252 hierarchical.cpp:412] Deactivated framework 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a-
I0913 05:23:46.999840 14108 slave.cpp:3235] Shutting down framework 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a-
I0913 05:23:47.000609 14064 master.cpp:8993] Updating the state of task 
74bc6c1f-13f2-482c-b54f-f99aa688d800 of framework 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0913 05:23:47.000609 14108 slave.cpp:5731] Shutting down executor 'default' of 
framework 9333fbac-335a-4c17-8fa6-0d297bcc8a1a- (via HTTP)
I0913 05:23:47.004621 14064 master.cpp:9087] Removing task 
74bc6c1f-13f2-482c-b54f-f99aa688d800 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework 9333fbac-335a-4c17-8fa6-0d297bcc8a1a- on agent 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a-S0 at slave(254)@10.3.1.5:49240 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0913 05:23:47.024621 14064 master.cpp:9116] Removing executor 'default' with 
resources [] of framework 9333fbac-335a-4c17-8fa6-0d297bcc8a1a- on agent 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a-S0 at slave(254)@10.3.1.5:49240 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0913 05:23:47.027621  9232 hierarchical.cpp:355] Removed framework 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a-
E0913 05:23:47.028898 14108 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0913 05:23:47.029620  9232 scheduler.cpp:444] Re-detecting master
I0913 05:23:47.032620 11676 scheduler.cpp:470] New master detected at 
master@10.3.1.5:49240
I0913 05:23:47.047621 11676 slave.cpp:5407] Executor 'default' of framework 
9333fbac-335a-4c17-8fa6-0d297bcc8a1a- exited with status 0
I0913 05:23:47.057621 11676 slave.cpp:5511] Cleaning up executor 'default' of 
framework 9333fbac-335a-4c17-8fa6-0d297bcc8a1a- (via HTTP)
W0913 05:23:47.057621 11832 master.cpp:7021] Ignoring unknown exited executor 
'default' of framework 

Re: Review Request 62263: Add test to browse and read in sandbox virtual path.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62040', '62174', '62047', '62263']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62263

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62263/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (247 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (257 ms)
[--] 30 tests from ContentType/SchedulerTest (24722 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (900 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 
(1023 ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (1995 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (141 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (153 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (344 ms 
total)

[--] Global test environment tear-down
[==] 628 tests from 66 test cases ran. (340272 ms total)
[  PASSED  ] 626 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] ContentType/MasterAPITest.EventAuthorizationFiltering/1, where 
GetParam() = application/json
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.SigkillExecutor/0, where 
GetParam() = "mesos"

 2 FAILED TESTS
  YOU HAVE 174 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62263/logs/mesos-tests-stderr.log):

```
I0913 05:23:24.409837 10332 master.cpp:8418] Removing framework 
a1994e31-61ae-4465-9f33-a8df4e1213df- (default)
I0913 05:23:24.409837 10332 master.cpp:3267] Deactivating framework 
a1994e31-61ae-4465-9f33-a8df4e1213df- (default)
I0913 05:23:24.409837 14024 hierarchical.cpp:412] Deactivated framework 
a1994e31-61ae-4465-9f33-a8df4e1213df-
I0913 05:23:24.410820 13384 slave.cpp:3245] Shutting down framework 
a1994e31-61ae-4465-9f33-a8df4e1213df-
I0913 05:23:24.410820 10332 master.cpp:8993] Updating the state of task 
ac5711a8-999c-41be-9d60-7a4fb942fe07 of framework 
a1994e31-61ae-4465-9f33-a8df4e1213df- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0913 05:23:24.410820 13384 slave.cpp:5752] Shutting down executor 'default' of 
framework a1994e31-61ae-4465-9f33-a8df4e1213df- (via HTTP)
I0913 05:23:24.418820 10332 master.cpp:9087] Removing task 
ac5711a8-999c-41be-9d60-7a4fb942fe07 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework a1994e31-61ae-4465-9f33-a8df4e1213df- on agent 
a1994e31-61ae-4465-9f33-a8df4e1213df-S0 at slave(255)@10.3.1.7:58816 
(mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0913 05:23:24.438820 10332 master.cpp:9116] Removing executor 'default' with 
resources [] of framework a1994e31-61ae-4465-9f33-a8df4e1213df- on agent 
a1994e31-61ae-4465-9f33-a8df4e1213df-S0 at slave(255)@10.3.1.7:58816 
(mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0913 05:23:24.442886 13772 hierarchical.cpp:355] Removed framework 
a1994e31-61ae-4465-9f33-a8df4e1213df-
E0913 05:23:24.443821 10920 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0913 05:23:24.453820 10920 scheduler.cpp:444] Re-detecting master
I0913 05:23:24.456822 10920 scheduler.cpp:470] New master detected at 
master@10.3.1.7:58816
I0913 05:23:24.463822 11232 slave.cpp:5417] Executor 'default' of framework 
a1994e31-61ae-4465-9f33-a8df4e1213df- exited with status 0
I0913 05:23:24.463822 11232 slave.cpp:5521] Cleaning up executor 'default' of 
framework a1994e31-61ae-4465-9f33-a8df4e1213df- (via HTTP)
W0913 05:23:24.463822 13772 master.cpp:7021] Ignoring unknown exited executor 
'default' 

Re: Review Request 62252: Added `process::Executor::execute()`.

2017-09-12 Thread Benjamin Hindman

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



Thanks for taking this on Chun! A few high level comments to start.

(1) I don't think we need to implement a version of `execute()` that takes 
arguments that we'll apply to the function. With lambda captures we can easily 
and cleanly accomplish that and any of the corner cases will be cleanly solved 
for with C++14 which we should move too sooner rather than later. If folks 
really want that functionality I'd rather just see them use `std::bind()` and 
do `executor.execute(std::bind(f, arg1, arg2))`. It's not that many more 
characters and it greately simplifies your implementation!

(2) Given the simplification of (1) I feel like you probably don't need a 
separate `Executor::Process` class, and instead can just use `dispatch()` on 
the existing `process`. You should be able to simplify your SFINAE by 
leveraging the return type `dispatch()` since it already takes care of the 
`void` issue for you.

- Benjamin Hindman


On Sept. 12, 2017, 8:12 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62252/
> ---
> 
> (Updated Sept. 12, 2017, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7970
> https://issues.apache.org/jira/browse/MESOS-7970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a convenient interface to `process::Executor` to
> asynchronously execute arbitrary functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/executor.hpp 
> cd2f2f03cba8a435f142b31def9f89a6cd61af73 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 82efb2f8449e4cb13620cae1a49321fc42e2db60 
> 
> 
> Diff: https://reviews.apache.org/r/62252/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 62237: Fixed a memory leak in composing containerizer.

2017-09-12 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On Sept. 12, 2017, 7:27 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62237/
> ---
> 
> (Updated Sept. 12, 2017, 7:27 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Bugs: MESOS-7927
> https://issues.apache.org/jira/browse/MESOS-7927
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a memory leak in composing containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> c076c74bb09e5d528420cece38e583a1e1ae07c9 
> 
> 
> Diff: https://reviews.apache.org/r/62237/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 62246: Added workaround for a docker bug in docker build helper.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62246']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62246

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62246/logs/mesos-tests-stdout.log):

```

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (890 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 
(1017 ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2025 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (145 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (161 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (347 ms 
total)

[--] Global test environment tear-down
C:\mesos\mesos\src\tests\environment.cpp(756): error: Failed
Tests completed with child processes remaining:
-+- 10360 00DEB5EFE47C
 --- 12976 00DEB5EFE47C
[==] 627 tests from 66 test cases ran. (334671 ms total)
[  PASSED  ] 625 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] ContentType/MasterAPITest.EventAuthorizationFiltering/1, where 
GetParam() = application/json
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.SigkillExecutor/0, where 
GetParam() = "mesos"

 2 FAILED TESTS
  YOU HAVE 174 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62246/logs/mesos-tests-stderr.log):

```
I0913 03:40:09.330443 10836 master.cpp:8418] Removing framework 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f- (default)
I0913 03:40:09.330443 10836 master.cpp:3267] Deactivating framework 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f- (default)
I0913 03:40:09.330443  3004 hierarchical.cpp:412] Deactivated framework 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f-
I0913 03:40:09.331439 10512 slave.cpp:3235] Shutting down framework 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f-
I0913 03:40:09.331439 10836 master.cpp:8993] Updating the state of task 
1276933e-df6b-4126-b9d2-ab93a59f58f1 of framework 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0913 03:40:09.331439 10512 slave.cpp:5731] Shutting down executor 'default' of 
framework 3541ce17-6339-4cd6-b93d-afd3e7da6b7f- (via HTTP)
I0913 03:40:09.344758 10836 master.cpp:9087] Removing task 
1276933e-df6b-4126-b9d2-ab93a59f58f1 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework 3541ce17-6339-4cd6-b93d-afd3e7da6b7f- on agent 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f-S0 at slave(254)@10.3.1.5:64581 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0913 03:40:09.355754 10836 master.cpp:9116] Removing executor 'default' with 
resources [] of framework 3541ce17-6339-4cd6-b93d-afd3e7da6b7f- on agent 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f-S0 at slave(254)@10.3.1.5:64581 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0913 03:40:09.368953  2780 hierarchical.cpp:355] Removed framework 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f-
E0913 03:40:09.368953  3004 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0913 03:40:09.369956 12192 scheduler.cpp:444] Re-detecting master
I0913 03:40:09.373952 12192 scheduler.cpp:470] New master detected at 
master@10.3.1.5:64581
I0913 03:40:09.386953 12192 slave.cpp:5407] Executor 'default' of framework 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f- exited with status 0
I0913 03:40:09.386953 12192 slave.cpp:5511] Cleaning up executor 'default' of 
framework 3541ce17-6339-4cd6-b93d-afd3e7da6b7f- (via HTTP)
W0913 03:40:09.388953 12284 master.cpp:7021] Ignoring unknown exited executor 
'default' of framework 3541ce17-6339-4cd6-b93d-afd3e7da6b7f- on agent 
3541ce17-6339-4cd6-b93d-afd3e7da6b7f-S0 at 

Review Request 62263: Add test to browse and read in sandbox virtual path.

2017-09-12 Thread Zhitao Li

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

Review request for mesos, Benjamin Mahler and Jason Lai.


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


Repository: mesos


Description
---

Add test to browse and read in sandbox virtual path.


Diffs
-

  src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 


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


Testing
---


Thanks,

Zhitao Li



Re: Review Request 62047: Allowed look up latest executor directory by virtual path.

2017-09-12 Thread Zhitao Li

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

(Updated Sept. 13, 2017, 1:25 a.m.)


Review request for mesos, Benjamin Mahler and Jason Lai.


Changes
---

Separate out test to a differnet patch.


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


Repository: mesos


Description
---

This patch added support to attach a virtual path
`/frameworks/{framework-id}/executors/{executor-id}/latest` to `Files` class
in agent, which can be used to navigate an executor's latest sandbox without
needing to know `work_dir` and `slave_id`.


Diffs (updated)
-

  src/slave/paths.hpp 51b481fc0870f1e95448f85ee2fd485fceea1919 
  src/slave/paths.cpp 08177bcef63b998f691be6893ab35891098a2886 
  src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 


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

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


Testing (updated)
---

Created new unit test in https://reviews.apache.org/r/62263/


Thanks,

Zhitao Li



Re: Review Request 62254: Ignored /proc/self/ns/pid_for_children when listing namespaces.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62253', '62254']`

Failed command: `C:\mesos\src\mesos-tests.exe`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62254

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62254/logs/mesos-tests-stdout.log):

```
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (199 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (200 ms)
[--] 30 tests from ContentType/SchedulerTest (21219 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (702 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 (855 
ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (1649 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (134 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (154 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (329 ms 
total)

[--] Global test environment tear-down
[==] 627 tests from 66 test cases ran. (284404 ms total)
[  PASSED  ] 626 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.SigkillExecutor/0, where 
GetParam() = "mesos"

 1 FAILED TEST
  YOU HAVE 174 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62254/logs/mesos-tests-stderr.log):

```
I0912 23:17:48.760558 12128 default_executor.cpp:185] Received ACKNOWLEDGED 
event
I0912 23:17:49.272580 11968 default_executor.cpp:842] Child container 
724375d3-5c44-48de-ab1a-e621a5ead895.51c7115b-4f38-401d-934d-23c4cced1f66 of 
task 'eee2f001-d3f6-4842-a5af-f500d949ef0d' in state TASK_FAILED exited with 
status 1
I0912 23:17:49.272580 11968 default_executor.cpp:964] Terminating after 1secs
I0912 23:17:50.275440 10392 process.cpp:1068] Failed to accept socket: future 
discarded
'\?\C:\Users\mesos\AppData\Local\Temp\2\v5qPD5\slaves\32e4c40a-33ec-4144-84db-c1519b62de43-S0\frameworks\32e4c40a-33ec-4144-84db-c1519b62de43-\executors\default\runs\bf8acb4d-41db-49d5-9548-ba37e82b1d1c\containers\10fc31f1-e35a-4236-9a7b-39f62570289e'
CMD.EXE was started with the above path as the current directory.
UNC paths are not supported.  Defaulting to Windows directory.
'\?\C:\Users\mesos\AppData\Local\Temp\2\v5qPD5\slaves\32e4c40a-33ec-4144-84db-c1519b62de43-S0\frameworks\32e4c40a-33ec-4144-84db-c1519b62de43-\executors\default\runs\bf8acb4d-41db-49d5-9548-ba37e82b1d1c\containers\79bf6b5e-3c03-4452-b327-339cb0058608'
CMD.EXE was started with the above path as the current directory.
UNC paths are not supported.  Defaulting to Windows directory.
I0912 23:17:51.654733 12976 executor.cpp:192] Version: 1.5.0
I0912 23:17:51.713738 12920 default_executor.cpp:185] Received SUBSCRIBED event
I0912 23:17:51.728744 12920 default_executor.cpp:189] Subscribed executor on 
mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0912 23:17:51.733744 12920 default_executor.cpp:185] Received LAUNCH_GROUP 
event
I0912 23:17:51.736744 12920 default_executor.cpp:394] Setting 
'MESOS_CONTAINER_IP' to: 10.3.1.5
I0912 23:17:52.016747 12920 default_executor.cpp:624] Successfully launched 
tasks [ ba34a8c9-cc79-49fe-8472-bf3b271d9ad9, 
4ca81226-60ab-4cea-a946-5d163e59292c ] in child containers [ 
bf8acb4d-41db-49d5-9548-ba37e82b1d1c.10fc31f1-e35a-4236-9a7b-39f62570289e, 
bf8acb4d-41db-49d5-9548-ba37e82b1d1c.79bf6b5e-3c03-4452-b327-339cb0058608 ]
I0912 23:17:52.034749 13096 default_executor.cpp:697] Waiting for child 
container 
bf8acb4d-41db-49d5-9548-ba37e82b1d1c.10fc31f1-e35a-4236-9a7b-39f62570289e of 
task 'ba34a8c9-cc79-49fe-8472-bf3b271d9ad9'
I0912 23:17:52.035748 13096 default_executor.cpp:697] Waiting for child 
container 
bf8acb4d-41db-49d5-9548-ba37e82b1d1c.79bf6b5e-3c03-4452-b327-339cb0058608 of 
task '4ca81226-60ab-4cea-a946-5d163e59292c'
I0912 23:17:52.139567  5240 default_executor.cpp:185] Received ACKNOWLEDGED 
event
I0912 23:17:52.142751 12788 default_executor.cpp:185] Received ACKNOWLEDGED 
event
I0912 23:17:52.563763 13136 

Re: Review Request 58048: Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.

2017-09-12 Thread Jie Yu

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




src/common/resources.cpp
Lines 185 (patched)


hum, i think we also need to check? wondering if we need to fix the rest.
```
if (left.has_id() != right.has_id()) {
  return false;
}
```



src/common/resources.cpp
Lines 372 (patched)


I think we should treat BLOCK the same as MOUNT.



src/v1/resources.cpp
Lines 353-358 (original), 386-391 (patched)


Do you still need this?



src/v1/resources.cpp
Lines 487 (patched)


Ditto. Block should be treated the same as MOUNT



src/v1/resources.cpp
Lines 491-493 (patched)


I think for RAW and if id are equal, we should return true (same as we did 
for MOUNT)


- Jie Yu


On Sept. 12, 2017, 1:14 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated Sept. 12, 2017, 1:14 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type. We also add a metadata field which can be used to expose
> additional disk information.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto fa475dcc3aa0b15ee4b2b740a24ab3c0b9a3274c 
>   include/mesos/v1/mesos.proto dfba4181203e23f7eedf67c19379d031e0993fd5 
>   src/common/resources.cpp 14b600ca1577be4910164396c75b866b53439ade 
>   src/tests/resources_tests.cpp 8a86efe60024d9e36294f0acb3f43bdd3f52f5bc 
>   src/v1/resources.cpp a5cc15591b7611274148139c43465adae44c1dbb 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/5/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 52064: Support for multiple versions of docs.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 52064`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/52064

Relevant logs:

- 
[apply-review-52064-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/52064/logs/apply-review-52064-stdout.log):
```error: patch failed: site/Gemfile:8
error: site/Gemfile: patch does not apply
error: patch failed: site/Gemfile.lock:1
error: site/Gemfile.lock: patch does not apply
error: patch failed: site/data/releases.yml:112
error: site/data/releases.yml: patch does not apply
```

- Mesos Reviewbot Windows


On May 5, 2017, 9:23 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated May 5, 2017, 9:23 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for multiple versions of docs.
> 
> 
> Diffs
> -
> 
>   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
>   site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/data/releases.yml 8bdc3ff11a821f5e1be04ac871c05b16403a82ac 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
> 
> 
> Diff: https://reviews.apache.org/r/52064/diff/5/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>



Re: Review Request 62197: Added new overloads for the `createExecutorInfo` test helper method.

2017-09-12 Thread Gastón Kleiman

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

(Updated Sept. 12, 2017, 11:49 p.m.)


Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.


Changes
---

Cleaned up parameters.


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


Repository: mesos


Description
---

These new overloads make it possible to specify a framework ID, executor
resources as a proto, and the executor ID also as a proto.


Diffs (updated)
-

  src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 


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

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


Testing
---

`make check` still passes on GNU/Linux.


Thanks,

Gastón Kleiman



Re: Review Request 62197: Added new overloads for the `createExecutorInfo` test helper method.

2017-09-12 Thread Greg Mann

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


Fix it, then Ship it!





src/tests/mesos.hpp
Lines 590 (patched)


I think that this parameter can be a `const std::string&` instead of an 
`Option`, which would allow us to get rid of the conditional in the function 
body.


- Greg Mann


On Sept. 12, 2017, 7:47 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62197/
> ---
> 
> (Updated Sept. 12, 2017, 7:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.
> 
> 
> Bugs: MESOS-7877
> https://issues.apache.org/jira/browse/MESOS-7877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These new overloads make it possible to specify a framework ID, executor
> resources as a proto, and the executor ID also as a proto.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 
> 
> 
> Diff: https://reviews.apache.org/r/62197/diff/5/
> 
> 
> Testing
> ---
> 
> `make check` still passes on GNU/Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 62253: Ignored cgroups v2 hierarchy when parsing /proc/self/cgroups.

2017-09-12 Thread Alexander Rukletsov

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


Ship it!




Ship It!

- Alexander Rukletsov


On Sept. 12, 2017, 6:28 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62253/
> ---
> 
> (Updated Sept. 12, 2017, 6:28 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7969
> https://issues.apache.org/jira/browse/MESOS-7969
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cgroups v2 hierarchies don't list the "controllers" field (e.g., 
> "0::/user.slice/user-1000.slice/session-5.scope) is empty and hence the 
> cgroup parser failes. We should simply skip over fields with empty controller 
> field.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 95c81d90f956e11d23e965dd0d0b83a66220d858 
> 
> 
> Diff: https://reviews.apache.org/r/62253/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Fedora 26.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 62254: Ignored /proc/self/ns/pid_for_children when listing namespaces.

2017-09-12 Thread Alexander Rukletsov

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


Ship it!




Ship It!

- Alexander Rukletsov


On Sept. 12, 2017, 6:27 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62254/
> ---
> 
> (Updated Sept. 12, 2017, 6:27 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7968
> https://issues.apache.org/jira/browse/MESOS-7968
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since Linux 4.12, /proc/self/ns/pid_for_children is a handle for the PID 
> namespace of child processes created by this process. Since this is not a 
> namespace type in its own, we should ignore this file when listing namespaces 
> via `ls /proc/self/ns`.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp f1792e854e3fb40321a6ad3f7d1f0f9fbd03761a 
> 
> 
> Diff: https://reviews.apache.org/r/62254/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Fedora 26.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 62230: Avoid GC pruning events from blocking other processes.

2017-09-12 Thread Chun-Hung Hsiao

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

(Updated Sept. 12, 2017, 8:52 p.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, and Jiang Yan Xu.


Changes
---

Used the new `process::Executor::execute()` interface.


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


Repository: mesos


Description
---

This patch dispatches `rmdirs` to a single executor instead of multiple
`AsyncExecutor`s such that heavy-duty pruning events won't occupy all
worker threads and block other actors.


Diffs (updated)
-

  src/slave/gc.cpp 83e4e2f3aba5c0d9900cf0beeea6e92320f889e7 
  src/slave/gc_process.hpp c383ce28411622692e42401c80e9443e7b1f5099 


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

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


Testing
---

sudo make test


Thanks,

Chun-Hung Hsiao



Re: Review Request 62252: Added `process::Executor::execute()`.

2017-09-12 Thread Chun-Hung Hsiao

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

(Updated Sept. 12, 2017, 8:12 p.m.)


Review request for mesos, Benjamin Hindman and Benjamin Mahler.


Changes
---

Moved `ExecutorProcess` to `Executor::Process` to avoid naming conflicts in 
Mesos.


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


Repository: mesos


Description
---

This patch adds a convenient interface to `process::Executor` to
asynchronously execute arbitrary functions.


Diffs (updated)
-

  3rdparty/libprocess/include/process/executor.hpp 
cd2f2f03cba8a435f142b31def9f89a6cd61af73 
  3rdparty/libprocess/src/tests/process_tests.cpp 
82efb2f8449e4cb13620cae1a49321fc42e2db60 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao



Re: Review Request 62197: Added new overloads for the `createExecutorInfo` test helper method.

2017-09-12 Thread Gastón Kleiman


> On Sept. 12, 2017, 5:21 p.m., Greg Mann wrote:
> > src/tests/mesos.hpp
> > Lines 512-522 (original), 620-638 (patched)
> > 
> >
> > I'm not sure why this overload was added originally, since `const 
> > char*` is implicitly convertible to `std::string`. I tried removing this 
> > overload as well and it compiled OK, so perhaps we can eliminate it?

Good idea, removed it.


> On Sept. 12, 2017, 5:21 p.m., Greg Mann wrote:
> > src/tests/mesos.hpp
> > Lines 641-658 (patched)
> > 
> >
> > I suspected that this overload wasn't strictly necessary, so I removed 
> > it and indeed compilation succeeded. Did you add this proactively to avoid 
> > the need to add it in the future?
> > 
> > I think we should only add overloads which are actually used in the 
> > current codebase. Could you verify which overloads in this patch are 
> > actually used?

I added it for consistency with the other method, but we're removing it, so I'm 
also removing this one.

The other overloads are being used, even though we could probably remove a few 
ones if we do a sweeping change to make all call sites pass the command as a 
`CommandInfo` instead of as a string.


- Gastón


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


On Sept. 12, 2017, 7:47 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62197/
> ---
> 
> (Updated Sept. 12, 2017, 7:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.
> 
> 
> Bugs: MESOS-7877
> https://issues.apache.org/jira/browse/MESOS-7877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These new overloads make it possible to specify a framework ID, executor
> resources as a proto, and the executor ID also as a proto.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 
> 
> 
> Diff: https://reviews.apache.org/r/62197/diff/5/
> 
> 
> Testing
> ---
> 
> `make check` still passes on GNU/Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 62197: Added new overloads for the `createExecutorInfo` test helper method.

2017-09-12 Thread Gastón Kleiman

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

(Updated Sept. 12, 2017, 7:47 p.m.)


Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.


Changes
---

Removed unused methods + used `createCommandInfo` helper.


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


Repository: mesos


Description
---

These new overloads make it possible to specify a framework ID, executor
resources as a proto, and the executor ID also as a proto.


Diffs (updated)
-

  src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 


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

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


Testing
---

`make check` still passes on GNU/Linux.


Thanks,

Gastón Kleiman



Re: Review Request 52064: Support for multiple versions of docs.

2017-09-12 Thread Vinod Kone
Great. I'll be there too!

On Tue, Sep 12, 2017 at 11:59 AM, Tim Anderegg 
wrote:

>
>
> > On May 9, 2017, 4:30 p.m., haosdent huang wrote:
> > > Hi, @tim Thanks a lot for your update. I am still reading you patch
> and have not finished. Could return my comments if it works at my side.
> Thanks a lot for your contributions.
> >
> > Tim Anderegg wrote:
> > Thanks @haosdent, please let me know if you have any questions or
> need clarification on anything!
> >
> > Vinod Kone wrote:
> > Any further updates on this? @haosdent can you commit this now?
>
> @haosdent @vinodkone This probably at least needs a rebase, but
> functionally speaking everything should be in working order.  I'll be at
> the hackathon at MesosCon tomorrow, if folks want to work on closing this
> out!
>
>
> - Tim
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/#review174335
> ---
>
>
> On May 5, 2017, 9:23 p.m., Tim Anderegg wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/52064/
> > ---
> >
> > (Updated May 5, 2017, 9:23 p.m.)
> >
> >
> > Review request for mesos, haosdent huang and Vinod Kone.
> >
> >
> > Bugs: MESOS-3011
> > https://issues.apache.org/jira/browse/MESOS-3011
> >
> >
> > Repository: mesos
> >
> >
> > Description
> > ---
> >
> > Support for multiple versions of docs.
> >
> >
> > Diffs
> > -
> >
> >   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807
> >   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf
> >   site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b
> >   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7
> >   site/data/releases.yml 8bdc3ff11a821f5e1be04ac871c05b16403a82ac
> >   site/source/assets/js/versions.js PRE-CREATION
> >   site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6
> >
> >
> > Diff: https://reviews.apache.org/r/52064/diff/5/
> >
> >
> > Testing
> > ---
> >
> > Testing was done manually to verify that the documentation was built for
> each version of Mesos that is supported (some older versions do not have
> compatible documentation).
> >
> >
> > Thanks,
> >
> > Tim Anderegg
> >
> >
>
>


Re: Review Request 52064: Support for multiple versions of docs.

2017-09-12 Thread Tim Anderegg


> On May 9, 2017, 4:30 p.m., haosdent huang wrote:
> > Hi, @tim Thanks a lot for your update. I am still reading you patch and 
> > have not finished. Could return my comments if it works at my side. Thanks 
> > a lot for your contributions.
> 
> Tim Anderegg wrote:
> Thanks @haosdent, please let me know if you have any questions or need 
> clarification on anything!
> 
> Vinod Kone wrote:
> Any further updates on this? @haosdent can you commit this now?

@haosdent @vinodkone This probably at least needs a rebase, but functionally 
speaking everything should be in working order.  I'll be at the hackathon at 
MesosCon tomorrow, if folks want to work on closing this out!


- Tim


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


On May 5, 2017, 9:23 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated May 5, 2017, 9:23 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for multiple versions of docs.
> 
> 
> Diffs
> -
> 
>   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
>   site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/data/releases.yml 8bdc3ff11a821f5e1be04ac871c05b16403a82ac 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
> 
> 
> Diff: https://reviews.apache.org/r/52064/diff/5/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>



Re: Review Request 61982: Cleaned up DefaultExecutor tests.

2017-09-12 Thread Greg Mann

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


Ship it!




- Greg Mann


On Sept. 9, 2017, 12:03 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61982/
> ---
> 
> (Updated Sept. 9, 2017, 12:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-7916
> https://issues.apache.org/jira/browse/MESOS-7916
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update the DefaultExecutor tests to use test helpers where possible.
> Also make the boilerplate initialization code consistent across tests.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 186b8333c02ba3b9257e19437c6d689761085362 
> 
> 
> Diff: https://reviews.apache.org/r/61982/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*DefaultExecutor*"` succeeds on 
> GNU/Linux
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Review Request 62253: Ignored cgroups v2 hierarchy when parsing /proc/self/cgroups.

2017-09-12 Thread Kapil Arya

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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Cgroups v2 hierarchies don't list the "controllers" field (e.g., 
"0::/user.slice/user-1000.slice/session-5.scope) is empty and hence the cgroup 
parser failes. We should simply skip over fields with empty controller field.


Diffs
-

  src/linux/cgroups.cpp 95c81d90f956e11d23e965dd0d0b83a66220d858 


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


Testing
---

make check on Fedora 26.


Thanks,

Kapil Arya



Review Request 62254: Ignored /proc/self/ns/pid_for_children when listing namespaces.

2017-09-12 Thread Kapil Arya

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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Since Linux 4.12, /proc/self/ns/pid_for_children is a handle for the PID 
namespace of child processes created by this process. Since this is not a 
namespace type in its own, we should ignore this file when listing namespaces 
via `ls /proc/self/ns`.


Diffs
-

  src/linux/ns.hpp f1792e854e3fb40321a6ad3f7d1f0f9fbd03761a 


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


Testing
---

make check on Fedora 26.


Thanks,

Kapil Arya



Re: Review Request 52064: Support for multiple versions of docs.

2017-09-12 Thread Vinod Kone


> On May 9, 2017, 4:30 p.m., haosdent huang wrote:
> > Hi, @tim Thanks a lot for your update. I am still reading you patch and 
> > have not finished. Could return my comments if it works at my side. Thanks 
> > a lot for your contributions.
> 
> Tim Anderegg wrote:
> Thanks @haosdent, please let me know if you have any questions or need 
> clarification on anything!

Any further updates on this? @haosdent can you commit this now?


- Vinod


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


On May 5, 2017, 9:23 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated May 5, 2017, 9:23 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for multiple versions of docs.
> 
> 
> Diffs
> -
> 
>   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
>   site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/data/releases.yml 8bdc3ff11a821f5e1be04ac871c05b16403a82ac 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
> 
> 
> Diff: https://reviews.apache.org/r/52064/diff/5/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>



Review Request 62252: Added `process::Executor::execute()`.

2017-09-12 Thread Chun-Hung Hsiao

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

Review request for mesos, Benjamin Hindman and Benjamin Mahler.


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


Repository: mesos


Description
---

This patch adds a convenient interface to `process::Executor` to
asynchronously execute arbitrary functions.


Diffs
-

  3rdparty/libprocess/include/process/executor.hpp 
cd2f2f03cba8a435f142b31def9f89a6cd61af73 
  3rdparty/libprocess/src/tests/process_tests.cpp 
82efb2f8449e4cb13620cae1a49321fc42e2db60 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao



Re: Review Request 62197: Added new overloads for the `createExecutorInfo` test helper method.

2017-09-12 Thread Greg Mann

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




src/tests/mesos.hpp
Lines 512-522 (original), 620-638 (patched)


I'm not sure why this overload was added originally, since `const char*` is 
implicitly convertible to `std::string`. I tried removing this overload as well 
and it compiled OK, so perhaps we can eliminate it?



src/tests/mesos.hpp
Lines 641-658 (patched)


I suspected that this overload wasn't strictly necessary, so I removed it 
and indeed compilation succeeded. Did you add this proactively to avoid the 
need to add it in the future?

I think we should only add overloads which are actually used in the current 
codebase. Could you verify which overloads in this patch are actually used?


- Greg Mann


On Sept. 9, 2017, 6:52 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62197/
> ---
> 
> (Updated Sept. 9, 2017, 6:52 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Joerg Schad.
> 
> 
> Bugs: MESOS-7877
> https://issues.apache.org/jira/browse/MESOS-7877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These new overloads make it possible to specify a framework ID, executor
> resources as a proto, and the executor ID also as a proto.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554 
> 
> 
> Diff: https://reviews.apache.org/r/62197/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` still passes on GNU/Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 62246: Added workaround for a docker bug in docker build helper.

2017-09-12 Thread Mesos Reviewbot Windows

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



Bad review!

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

- Mesos Reviewbot Windows


On Sept. 12, 2017, 2:33 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62246/
> ---
> 
> (Updated Sept. 12, 2017, 2:33 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds workaround for an incorrect docker file execution on
> Apache CI due to a docker bug, see:
> https://github.com/moby/moby/issues/9547
> Quote from the docker issue:
> The execution of `chmod +x b.sh` just prior to trying to running b.sh
> is leaving it in the 'Text file busy' state for long enough that then
> running it fails.
> 
> 
> Diffs
> -
> 
>   support/docker-build.sh 6443e56be8bf1421b29afef059196a838100b8af 
> 
> 
> Diff: https://reviews.apache.org/r/62246/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Review Request 62246: Added workaround for a docker bug in docker build helper.

2017-09-12 Thread Andrei Budnik

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

Review request for mesos.


Repository: mesos


Description
---

This patch adds workaround for an incorrect docker file execution on
Apache CI due to a docker bug, see:
https://github.com/moby/moby/issues/9547
Quote from the docker issue:
The execution of `chmod +x b.sh` just prior to trying to running b.sh
is leaving it in the 'Text file busy' state for long enough that then
running it fails.


Diffs
-

  support/docker-build.sh 6443e56be8bf1421b29afef059196a838100b8af 


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


Testing
---


Thanks,

Andrei Budnik



Re: Review Request 62241: Added SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.

2017-09-12 Thread Ilya Pronin


> On Sept. 12, 2017, 3:16 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Mesos failed to build.
> > 
> > Reviews applied: [62240, 62241]
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241
> > 
> > Relevant logs:
> > 
> >  - mesos-cmake-build.log:
> > 
> > ??-tion is MSVC 19.10.25019.0

> > -- The CXX compiler identification is 
> > MSVC 19.10.25019.0

> > -- Check for working C compiler: 
> > C:/Program Files (x86)/Microsoft 
> > Visual 
> > Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe

> > -- Check for working C compiler: 
> > C:/Program Files (x86)/Microsoft 
> > Visual 
> > Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
> >  -- works

> > -- Detecting C compiler ABI info

> > -- Detecting C compiler ABI info - 
> > done

> > -- Check for working CXX compiler: 
> > C:/Program Files (x86)/Microsoft 
> > Visual 
> > Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe

> > -- Check for working CXX compiler: 
> > C:/Program Files (x86)/Microsoft 
> > Visual 
> > Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
> >  -- works

> > -- Detecting CXX compiler ABI info

> > -- Detecting CXX compiler ABI info - 
> > done

> > -- Detecting CXX compile features

> > -- Detecting CXX compile features - 
> > done

> > -- cotire 1.7.10 loaded.

> > -- 
> > ************************************************************

> > -- ********* Beginning Mesos CMake 
> > configuration step *********

> > -- 
> > ************************************************************

> > -- INSTALLATION PREFIX: C:/Program 
> > Files/Mesos

> > -- MACHINE SPECS:

> > --     Hostname: 

> > --     OS:       WINDOWS(10.0.14393)

> > --     Arch:     AMD64

> > --     BitMode:  

> > --     BuildID:  

> > -- 
> > ************************************************************

> > 
> > Full log available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241/logs/mesos-cmake-build.log

Build logs are weird:
```
Building Mesos
-- The C compiler identification is MSVC 19.10.25019.0
-- The CXX compiler identification is MSVC 19.10.25019.0
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- cotire 1.7.10 loaded.
-- 
-- * Beginning Mesos CMake configuration step *
-- 
-- INSTALLATION PREFIX: C:/Program Files/Mesos
-- MACHINE SPECS:
-- Hostname: 
-- OS:   WINDOWS(10.0.14393)
-- Arch: AMD64
-- BitMode:  
-- BuildID:  
-- 
Mesos failed to build.
```


- Ilya


---
This is an 

Re: Review Request 62241: Added SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Mesos failed to build.

Reviews applied: [62240, 62241]

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241

Relevant logs:

 - mesos-cmake-build.log:

��-- The C compiler identification is 
MSVC 19.10.25019.0

-- The CXX compiler identification is 
MSVC 19.10.25019.0

-- Check for working C compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe

-- Check for working C compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
 -- works

-- Detecting C compiler ABI info

-- Detecting C compiler ABI info - done

-- Check for working CXX compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe

-- Check for working CXX compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
 -- works

-- Detecting CXX compiler ABI info

-- Detecting CXX compiler ABI info - 
done

-- Detecting CXX compile features

-- Detecting CXX compile features - 
done

-- cotire 1.7.10 loaded.

-- 
************************************************************

-- ********* Beginning Mesos CMake 
configuration step *********

-- 
************************************************************

-- INSTALLATION PREFIX: C:/Program 
Files/Mesos

-- MACHINE SPECS:

--     Hostname: 

--     OS:       WINDOWS(10.0.14393)

--     Arch:     AMD64

--     BitMode:  

--     BuildID:  

-- 
************************************************************


Full log available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241/logs/mesos-cmake-build.log

- Mesos Reviewbot Windows


On Sept. 12, 2017, 12:59 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> ---
> 
> (Updated Sept. 12, 2017, 12:59 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_http_api_tests.cpp 
> cc03be0d2110f08023bf0ce181da37658737bd7c 
> 
> 
> Diff: https://reviews.apache.org/r/62241/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`. Verified that the test fails without the previous patch.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>



Re: Review Request 62213: Fix unit tests that were broken by the additional TASK_STARTING update.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Mesos failed to build.

Reviews applied: [62212, 62213]

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62213

Relevant logs:

 - mesos-cmake-build.log:

��-- The C compiler identification is 
MSVC 19.10.25019.0

-- The CXX compiler identification is 
MSVC 19.10.25019.0

-- Check for working C compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe

-- Check for working C compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
 -- works

-- Detecting C compiler ABI info

-- Detecting C compiler ABI info - done

-- Check for working CXX compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe

-- Check for working CXX compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
 -- works

-- Detecting CXX compiler ABI info

-- Detecting CXX compiler ABI info - 
done

-- Detecting CXX compile features

-- Detecting CXX compile features - 
done

-- cotire 1.7.10 loaded.

-- 
************************************************************

-- ********* Beginning Mesos CMake 
configuration step *********

-- 
************************************************************

-- INSTALLATION PREFIX: C:/Program 
Files/Mesos

-- MACHINE SPECS:

--     Hostname: 

--     OS:       WINDOWS(10.0.14393)

--     Arch:     AMD64

--     BitMode:  

--     BuildID:  

-- 
************************************************************


Full log available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62213/logs/mesos-cmake-build.log

- Mesos Reviewbot Windows


On Sept. 12, 2017, 9:58 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62213/
> ---
> 
> (Updated Sept. 12, 2017, 9:58 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix unit tests that were broken by the additional TASK_STARTING update.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 0902a66772d178505e83b81878b7878a45c7bb25 
>   src/tests/check_tests.cpp cc7be317030d147c5617e784a50e42b0365b9df6 
>   src/tests/command_executor_tests.cpp 
> f706f55b5bfab824268498d95d775b216561cd66 
>   src/tests/container_logger_tests.cpp 
> 97e79792d3ea8023890ad2a705db47f2aeb419cf 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> 38fef2d5f677f768a0533d1ac085b1197b3b764d 
>   src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
> 0030cd1e1f73422bef806bbc0453134e3d7840d8 
>   src/tests/default_executor_tests.cpp 
> 186b8333c02ba3b9257e19437c6d689761085362 
>   src/tests/disk_quota_tests.cpp 3bf0508238a228d86737d6cc899fa68e2046f2e2 
>   src/tests/fault_tolerance_tests.cpp 
> 5ac38a897fefb6f40d69ca6e27a6f23176e42d36 
>   src/tests/health_check_tests.cpp 2c43241a5cc245a1797ea16c68c94638cec6803d 
>   src/tests/master_tests.cpp 

Re: Review Request 62230: Avoid GC pruning events from blocking other processes.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Mesos failed to build.

Reviews applied: [62230]

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62230

Relevant logs:

 - mesos-cmake-build.log:

��-- The C compiler identification is 
MSVC 19.10.25019.0

-- The CXX compiler identification is 
MSVC 19.10.25019.0

-- Check for working C compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe

-- Check for working C compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
 -- works

-- Detecting C compiler ABI info

-- Detecting C compiler ABI info - done

-- Check for working CXX compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe

-- Check for working CXX compiler: 
C:/Program Files (x86)/Microsoft Visual 
Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
 -- works

-- Detecting CXX compiler ABI info

-- Detecting CXX compiler ABI info - 
done

-- Detecting CXX compile features

-- Detecting CXX compile features - 
done

-- cotire 1.7.10 loaded.

-- 
************************************************************

-- ********* Beginning Mesos CMake 
configuration step *********

-- 
************************************************************

-- INSTALLATION PREFIX: C:/Program 
Files/Mesos

-- MACHINE SPECS:

--     Hostname: 

--     OS:       WINDOWS(10.0.14393)

--     Arch:     AMD64

--     BitMode:  

--     BuildID:  

-- 
************************************************************


Full log available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62230/logs/mesos-cmake-build.log

- Mesos Reviewbot Windows


On Sept. 12, 2017, 10:37 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62230/
> ---
> 
> (Updated Sept. 12, 2017, 10:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7964
> https://issues.apache.org/jira/browse/MESOS-7964
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch dispatches `rmdirs` to a single executor instead of multiple
> `AsyncExecutor`s such that heavy-duty pruning events won't occupy all
> worker threads and block other actors.
> 
> 
> Diffs
> -
> 
>   src/slave/gc.cpp 83e4e2f3aba5c0d9900cf0beeea6e92320f889e7 
>   src/slave/gc_process.hpp c383ce28411622692e42401c80e9443e7b1f5099 
> 
> 
> Diff: https://reviews.apache.org/r/62230/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make test
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 58048: Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.

2017-09-12 Thread Benjamin Bannier


> On Sept. 6, 2017, 7:15 p.m., Jie Yu wrote:
> > include/mesos/mesos.proto
> > Lines 1044 (patched)
> > 
> >
> > Also, I'd suggest we put `id` into `source`. Given that the CSI spec 
> > change, we'll use string for `id` and `Labels` for metadata:
> > 
> > ```
> > message Source {
> >   optional string id;
> >   optional Labels metadata;
> > }
> > ```

Done, see above.


- Benjamin


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


On Sept. 12, 2017, 3:14 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated Sept. 12, 2017, 3:14 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type. We also add a metadata field which can be used to expose
> additional disk information.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto fa475dcc3aa0b15ee4b2b740a24ab3c0b9a3274c 
>   include/mesos/v1/mesos.proto dfba4181203e23f7eedf67c19379d031e0993fd5 
>   src/common/resources.cpp 14b600ca1577be4910164396c75b866b53439ade 
>   src/tests/resources_tests.cpp 8a86efe60024d9e36294f0acb3f43bdd3f52f5bc 
>   src/v1/resources.cpp a5cc15591b7611274148139c43465adae44c1dbb 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/5/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 58048: Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.

2017-09-12 Thread Benjamin Bannier


> On April 17, 2017, 11:49 a.m., Jie Yu wrote:
> > src/tests/resources_tests.cpp
> > Lines 2255 (patched)
> > 
> >
> > instead of relying on 'count', let's use `size()` instead.
> 
> Qian Zhang wrote:
> I think here the intention is to count how many `disks` are in `(r1 + 
> r2)`, so `count()` should be the right method to call.
> 
> Benjamin Bannier wrote:
> Yes, exactly. Note that we could use `size` and explicit value checks 
> here, but this would introduce dependencies to the exact disk values which in 
> my opinion makes the code harder to understand.

I removed the code here, but `count` has incorrect semantics (number of users 
of shared resource instead of number of appearances of a specific `Resource`). 
I use `size` in newly added code now (an alternative possibility would have 
been to first filter for a `Resource` and then check the `size` of the result).


- Benjamin


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


On Sept. 12, 2017, 3:14 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated Sept. 12, 2017, 3:14 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type. We also add a metadata field which can be used to expose
> additional disk information.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto fa475dcc3aa0b15ee4b2b740a24ab3c0b9a3274c 
>   include/mesos/v1/mesos.proto dfba4181203e23f7eedf67c19379d031e0993fd5 
>   src/common/resources.cpp 14b600ca1577be4910164396c75b866b53439ade 
>   src/tests/resources_tests.cpp 8a86efe60024d9e36294f0acb3f43bdd3f52f5bc 
>   src/v1/resources.cpp a5cc15591b7611274148139c43465adae44c1dbb 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/5/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 58048: Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.

2017-09-12 Thread Benjamin Bannier


> On Sept. 2, 2017, 6:27 p.m., Jie Yu wrote:
> > This might need to be adjusted based on this:
> > https://github.com/container-storage-interface/spec/pull/97
> > 
> > so `id` should be string, and `metadata` is labels given the above PR goes 
> > through

Like you suggested below, I moved `id` to `Source`, and also added a `metadata` 
field.

While `id` is considered when evaluating identity in the matrix you gave above, 
`metadata` just participates via equality.


- Benjamin


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


On Sept. 12, 2017, 3:14 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated Sept. 12, 2017, 3:14 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type. We also add a metadata field which can be used to expose
> additional disk information.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto fa475dcc3aa0b15ee4b2b740a24ab3c0b9a3274c 
>   include/mesos/v1/mesos.proto dfba4181203e23f7eedf67c19379d031e0993fd5 
>   src/common/resources.cpp 14b600ca1577be4910164396c75b866b53439ade 
>   src/tests/resources_tests.cpp 8a86efe60024d9e36294f0acb3f43bdd3f52f5bc 
>   src/v1/resources.cpp a5cc15591b7611274148139c43465adae44c1dbb 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/5/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 58048: Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.

2017-09-12 Thread Benjamin Bannier


> On Aug. 28, 2017, 8:24 p.m., Jie Yu wrote:
> > src/common/resources.cpp
> > Line 187 (original), 187 (patched)
> > 
> >
> > We also need to update `addable` and `subtractable` similar to what we 
> > did for persistent volumes.
> > 
> > Depending on the "disk" source type, the semantics is different:
> > 1. RAW w/ id:
> >- Cannot be split into smaller pieces
> >- Cannot be merged even if they are equal
> > 2. RAW w/o id:
> >- Can be split into smaller pieces
> >- Can be merged if RAW parts equal
> > 3. PATH w/ id:
> >- Can be split into smaller pieces
> >- Can be merged if PATH parts equal
> > 4. PATH w/o id:
> >- Same as PATH w/ id
> > 5. MOUNT w/ id:
> >- Cannot be split into smaller pieces
> >- Cannot be merged even if they are equal
> > 6. MOUNT w/o id:
> >- Same as MOUNT w/ id

I updated the code to follow these semantics. You did not mention `BLOCK` 
sources above; I gave them the same semantics as `RAW` sources.


- Benjamin


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


On Sept. 12, 2017, 3:14 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated Sept. 12, 2017, 3:14 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type. We also add a metadata field which can be used to expose
> additional disk information.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto fa475dcc3aa0b15ee4b2b740a24ab3c0b9a3274c 
>   include/mesos/v1/mesos.proto dfba4181203e23f7eedf67c19379d031e0993fd5 
>   src/common/resources.cpp 14b600ca1577be4910164396c75b866b53439ade 
>   src/tests/resources_tests.cpp 8a86efe60024d9e36294f0acb3f43bdd3f52f5bc 
>   src/v1/resources.cpp a5cc15591b7611274148139c43465adae44c1dbb 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/5/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 58048: Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.

2017-09-12 Thread Benjamin Bannier

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

(Updated Sept. 12, 2017, 3:14 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed a number of comments from Jie.


Summary (updated)
-

Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.


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


Repository: mesos


Description (updated)
---

Ids will allow to create distinguishable resources, e.g., of RAW or
BLOCK type. We also add a metadata field which can be used to expose
additional disk information.


Diffs (updated)
-

  include/mesos/mesos.proto fa475dcc3aa0b15ee4b2b740a24ab3c0b9a3274c 
  include/mesos/v1/mesos.proto dfba4181203e23f7eedf67c19379d031e0993fd5 
  src/common/resources.cpp 14b600ca1577be4910164396c75b866b53439ade 
  src/tests/resources_tests.cpp 8a86efe60024d9e36294f0acb3f43bdd3f52f5bc 
  src/v1/resources.cpp a5cc15591b7611274148139c43465adae44c1dbb 


Diff: https://reviews.apache.org/r/58048/diff/5/

Changes: https://reviews.apache.org/r/58048/diff/4-5/


Testing
---

make check (OS X, Fedora25)


Thanks,

Benjamin Bannier



Review Request 62241: Added SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.

2017-09-12 Thread Ilya Pronin

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

This test verifies that we are able to upgrade from a PID based
framework to an HTTP framework and then downgrade back without
restarting the master.


Diffs
-

  src/tests/scheduler_http_api_tests.cpp 
cc03be0d2110f08023bf0ce181da37658737bd7c 


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


Testing
---

Ran `make check`. Verified that the test fails without the previous patch.


Thanks,

Ilya Pronin



Review Request 62240: Removed metrics removal from Master::failoverFramework().

2017-09-12 Thread Ilya Pronin

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

When a framework upgrades from a PID based driver to an HTTP based
driver, the master removes its per-principal metrics. When the same
framework downgrades back to a PID based driver, the master doesn't
reinstate those metrics. This causes a crash when the master receives a
message from the failed over framework and tries to increment its
metrics.

This patch fixes the issue by removing metrics removal from framework
failover handling code. Note that it doesn't handle the case when the
framework's principal change. This situation is being dealt with
separately in MESOS-2842.


Diffs
-

  src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 


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


Testing
---

Ran `make check`. A regression test that reproduces the issue is added in the 
subsequent patch.


Thanks,

Ilya Pronin



Re: Review Request 62212: Send TASK_STARTING from the built-in executors. [1/2]

2017-09-12 Thread Benno Evers


> On Sept. 12, 2017, 11:07 a.m., Alexander Rukletsov wrote:
> > src/launcher/default_executor.cpp
> > Lines 1317-1319 (original), 1333-1337 (patched)
> > 
> >
> > Instead of checking a flag, why not replacing `CHECK` with a condition?

I was assuming this `CHECK` was mostly intended to guard against some weird 
race conditions where this function might be called before `task_id` is 
inserted, even though we expect it to be. So it felt safer to keep the check. I 
don't have a strong opinion on this though, if you prefer I'll replace it.


- Benno


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


On Sept. 11, 2017, 9:16 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62212/
> ---
> 
> (Updated Sept. 11, 2017, 9:16 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives schedulers more information about a tasks status,
> in particular it gives a better estimate of a tasks start time
> and helps differentiating between tasks stuck in TASK_STAGING
> and tasks stuck in TASK_STARTING.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 73743aba31f9d0ca827d318e2ecb4752a91b1be0 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
> 
> 
> Diff: https://reviews.apache.org/r/62212/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 62212: Send TASK_STARTING from the built-in executors. [1/2]

2017-09-12 Thread Benno Evers


> On Sept. 12, 2017, 11:03 a.m., Andrei Budnik wrote:
> > src/docker/executor.cpp
> > Lines 141 (patched)
> > 
> >
> > Do we really need to send `TASK_STARTING`, if we know that right after 
> > sending `TASK_STARTING`, `launchTask` might send `TASK_FAILED`, e.g. in 
> > case of following checks:
> > ```c++
> > if (run.isSome()) {
> >  // ...
> >  status.set_state(TASK_FAILED);
> > }
> > ...
> > if (runOptions.isError()) {
> >  // ...
> >  status.set_state(TASK_FAILED);
> > }
> > ```
> > What is the semantics of `TASK_STARTING` - should an executor send it 
> > always and unconditionally, or can we omit `TASK_STARTING` when we are 
> > pretty sure that we won't be able to launch a task?
> > Omitting `TASK_STARTING` before sending `TASK_FAILED` might be a good 
> > optimization, if it's not important to send `TASK_STARTING` first.
> 
> Alexander Rukletsov wrote:
> Agree with Andrei: we should send task starting if we try to start the 
> task. If we know that starting will never succeed, we don't even try, hence 
> no task starting update : )

The previous documentation on `TASK_STARTING` was more of a loose guideline, so 
I think we're de-facto deciding on the `TASK_STARTING` semantics in this 
review. Intuitively, I find it most natural to have it mean "the executor is 
now aware of the task and will attempt to start it", and not to create any 
special cases.


- Benno


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


On Sept. 11, 2017, 9:16 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62212/
> ---
> 
> (Updated Sept. 11, 2017, 9:16 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives schedulers more information about a tasks status,
> in particular it gives a better estimate of a tasks start time
> and helps differentiating between tasks stuck in TASK_STAGING
> and tasks stuck in TASK_STARTING.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 73743aba31f9d0ca827d318e2ecb4752a91b1be0 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
> 
> 
> Diff: https://reviews.apache.org/r/62212/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-12 Thread Armand Grillet

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

(Updated Sept. 12, 2017, 11:33 a.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


Changes
---

Resolved issue.


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


Repository: mesos


Description
---

Thiw will be used to initialize logging in the main functions
that do not use mesos::internal::logging::Flags.


Diffs (updated)
-

  src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 


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

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


Testing (updated)
---

```
$ make check
```

Also tested manually to see if using `LOG()` without initializing GLOG gives 
the same result as using it `LOG()` after running 
`mesos::internal::logging::initialize` without flags.


Thanks,

Armand Grillet



Re: Review Request 62212: Send TASK_STARTING from the built-in executors. [1/2]

2017-09-12 Thread Alexander Rukletsov


> On Sept. 12, 2017, 11:03 a.m., Andrei Budnik wrote:
> > src/docker/executor.cpp
> > Lines 141 (patched)
> > 
> >
> > Do we really need to send `TASK_STARTING`, if we know that right after 
> > sending `TASK_STARTING`, `launchTask` might send `TASK_FAILED`, e.g. in 
> > case of following checks:
> > ```c++
> > if (run.isSome()) {
> >  // ...
> >  status.set_state(TASK_FAILED);
> > }
> > ...
> > if (runOptions.isError()) {
> >  // ...
> >  status.set_state(TASK_FAILED);
> > }
> > ```
> > What is the semantics of `TASK_STARTING` - should an executor send it 
> > always and unconditionally, or can we omit `TASK_STARTING` when we are 
> > pretty sure that we won't be able to launch a task?
> > Omitting `TASK_STARTING` before sending `TASK_FAILED` might be a good 
> > optimization, if it's not important to send `TASK_STARTING` first.

Agree with Andrei: we should send task starting if we try to start the task. If 
we know that starting will never succeed, we don't even try, hence no task 
starting update : )


> On Sept. 12, 2017, 11:03 a.m., Andrei Budnik wrote:
> > src/launcher/executor.cpp
> > Lines 512 (patched)
> > 
> >
> > What happens if a buggy scheduler will try to launch multiple tasks 
> > with same `task_id` using a single `command` executor? I guess, `command` 
> > executor will send `TASK_STARTING`, then `TASK_FAILED`. If so, then is it 
> > ok to have multiple `TASK_STARTING` status updates (having different 
> > timestamps)?

Multiple tasks for a single command executor are not allowed. We should send 
task starting only once, for the task that we actually try to start.


- Alexander


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


On Sept. 11, 2017, 9:16 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62212/
> ---
> 
> (Updated Sept. 11, 2017, 9:16 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives schedulers more information about a tasks status,
> in particular it gives a better estimate of a tasks start time
> and helps differentiating between tasks stuck in TASK_STAGING
> and tasks stuck in TASK_STARTING.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 73743aba31f9d0ca827d318e2ecb4752a91b1be0 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
> 
> 
> Diff: https://reviews.apache.org/r/62212/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 62212: Send TASK_STARTING from the built-in executors. [1/2]

2017-09-12 Thread Alexander Rukletsov

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



For the default executor, a cleaner approach would be to initialize the 
`containers` collection earlier, right before (or together with) sending 
`TASK_STARTING`. If we rely on the fact that `containers` include only launched 
containers, we can introduce `active` flag into the `Container` struct and set 
it where now an instance of the struct is created. (Speaking about flags, we 
should have a proper state machine with clear transitions per container instead 
of a bunch of flags). 

Moreover, from what I see in the code, if a launch nested container has failed, 
no task updates are sent by executor at all (which makes agent to create an 
implicit update). It seems reasonable to me to send an update from the executor 
at least for the task that has failed to launch. In the future we will likely 
allow some tasks in a group to fail without killing the complete group, hence 
keeping task info as early as possible seems reasonable to me.


src/docker/executor.cpp
Line 138 (original), 138-139 (patched)


Please add the todo here:
```
// TODO(alexr): Use `protobuf::createTaskStatus()`
// instead of manually setting fields.
```



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


Instead of short-circuiting here, I suggest to use 
`protobuf::createTaskStatus` directly, explaining in the comment at the call 
site why we do so. There is another alternative, more about it in the review 
header section.



src/launcher/default_executor.cpp
Lines 1317-1319 (original), 1333-1337 (patched)


Instead of checking a flag, why not replacing `CHECK` with a condition?


- Alexander Rukletsov


On Sept. 11, 2017, 9:16 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62212/
> ---
> 
> (Updated Sept. 11, 2017, 9:16 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives schedulers more information about a tasks status,
> in particular it gives a better estimate of a tasks start time
> and helps differentiating between tasks stuck in TASK_STAGING
> and tasks stuck in TASK_STARTING.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 73743aba31f9d0ca827d318e2ecb4752a91b1be0 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
> 
> 
> Diff: https://reviews.apache.org/r/62212/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 62212: Send TASK_STARTING from the built-in executors. [1/2]

2017-09-12 Thread Andrei Budnik

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




src/docker/executor.cpp
Lines 141 (patched)


Do we really need to send `TASK_STARTING`, if we know that right after 
sending `TASK_STARTING`, `launchTask` might send `TASK_FAILED`, e.g. in case of 
following checks:
```c++
if (run.isSome()) {
 // ...
 status.set_state(TASK_FAILED);
}
...
if (runOptions.isError()) {
 // ...
 status.set_state(TASK_FAILED);
}
```
What is the semantics of `TASK_STARTING` - should an executor send it 
always and unconditionally, or can we omit `TASK_STARTING` when we are pretty 
sure that we won't be able to launch a task?
Omitting `TASK_STARTING` before sending `TASK_FAILED` might be a good 
optimization, if it's not important to send `TASK_STARTING` first.



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


Why `DefaultExecutor::createTaskStatus` used instead of 
`protobuf::createTaskStatus`?
Do we really need to set `executor_id` for `TASK_STARTING` status update? 
If so, then why don't we set `executor_id` in other built-in executors for 
consistency?



src/launcher/executor.cpp
Lines 512 (patched)


What happens if a buggy scheduler will try to launch multiple tasks with 
same `task_id` using a single `command` executor? I guess, `command` executor 
will send `TASK_STARTING`, then `TASK_FAILED`. If so, then is it ok to have 
multiple `TASK_STARTING` status updates (having different timestamps)?


- Andrei Budnik


On Sept. 11, 2017, 9:16 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62212/
> ---
> 
> (Updated Sept. 11, 2017, 9:16 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives schedulers more information about a tasks status,
> in particular it gives a better estimate of a tasks start time
> and helps differentiating between tasks stuck in TASK_STAGING
> and tasks stuck in TASK_STARTING.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 73743aba31f9d0ca827d318e2ecb4752a91b1be0 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
> 
> 
> Diff: https://reviews.apache.org/r/62212/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 62237: Fixed a memory leak in composing containerizer.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Mesos stdout-tests failed to build.

Reviews applied: [62237]

All the build artifacts available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62237

Relevant logs:

 - stout-tests-cmake-build.log:

  
C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\user\win32\groupinfo.c(53):
 warning C4133: 'function': 
incompatible types - from 'char [256]' 
to 'LPWSTR' 
[C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build\libapr-1.vcxproj]
 
[C:\mesos\3rdparty\libapr-1.5.2.vcxproj]

  
C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\user\win32\groupinfo.c(59):
 warning C4133: 'function': 
incompatible types - from 'char *' to 
'LPCWSTR' 
[C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build\libapr-1.vcxproj]
 
[C:\mesos\3rdparty\libapr-1.5.2.vcxproj]

  
C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\user\win32\groupinfo.c(59):
 warning C4133: 'function': 
incompatible types - from 'const char 
*' to 'LPCWSTR' 
[C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build\libapr-1.vcxproj]
 
[C:\mesos\3rdparty\libapr-1.5.2.vcxproj]

  
C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\user\win32\groupinfo.c(60):
 warning C4133: 'function': 
incompatible types - from 'char [256]' 
to 'LPWSTR' 
[C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build\libapr-1.vcxproj]
 
[C:\mesos\3rdparty\libapr-1.5.2.vcxproj]

  
C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\user\win32\groupinfo.c(79):
 warning C4133: 'function': 
incompatible types - from 'char [260]' 
to 'LPWSTR' 
[C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build\libapr-1.vcxproj]
 
[C:\mesos\3rdparty\libapr-1.5.2.vcxproj]

  
C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\user\win32\userinfo.c(101):
 warning C4133: 'function': 
incompatible types - from 'char [520]' 
to 'LPCWSTR' 
[C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build\libapr-1.vcxproj]
 
[C:\mesos\3rdparty\libapr-1.5.2.vcxproj]

  
C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\user\win32\userinfo.c(227):
 warning C4133: 'function': 
incompatible types - from 'char *' to 
'LPCWSTR' 
[C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build\libapr-1.vcxproj]
 
[C:\mesos\3rdparty\libapr-1.5.2.vcxproj]

  
C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2\user\win32\userinfo.c(227):
 warning C4133: 'function': 
incompatible types - from 'const char 
*' to 'LPCWSTR' 
[C:\mesos\3rdparty\libapr-1.5.2\src\libapr-1.5.2-build\libapr-1.vcxproj]
 
[C:\mesos\3rdparty\libapr-1.5.2.vcxproj]

  

Re: Review Request 62213: Fix unit tests that were broken by the additional TASK_STARTING update.

2017-09-12 Thread Benno Evers

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

(Updated Sept. 12, 2017, 9:58 a.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


Changes
---

Fix DefaultExecutorTest.ROOT_ContainerStatusForTas


Summary (updated)
-

Fix unit tests that were broken by the additional TASK_STARTING update.


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


Repository: mesos


Description (updated)
---

Fix unit tests that were broken by the additional TASK_STARTING update.


Diffs (updated)
-

  src/tests/api_tests.cpp 0902a66772d178505e83b81878b7878a45c7bb25 
  src/tests/check_tests.cpp cc7be317030d147c5617e784a50e42b0365b9df6 
  src/tests/command_executor_tests.cpp f706f55b5bfab824268498d95d775b216561cd66 
  src/tests/container_logger_tests.cpp 97e79792d3ea8023890ad2a705db47f2aeb419cf 
  src/tests/containerizer/environment_secret_isolator_tests.cpp 
38fef2d5f677f768a0533d1ac085b1197b3b764d 
  src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
0030cd1e1f73422bef806bbc0453134e3d7840d8 
  src/tests/default_executor_tests.cpp 186b8333c02ba3b9257e19437c6d689761085362 
  src/tests/disk_quota_tests.cpp 3bf0508238a228d86737d6cc899fa68e2046f2e2 
  src/tests/fault_tolerance_tests.cpp 5ac38a897fefb6f40d69ca6e27a6f23176e42d36 
  src/tests/health_check_tests.cpp 2c43241a5cc245a1797ea16c68c94638cec6803d 
  src/tests/master_tests.cpp 59fbad468ae671a8fd39457632262015989f26c2 
  src/tests/master_validation_tests.cpp 
710b25c1afdff4de7b2eb9b8a38f6856c373fb3c 
  src/tests/oversubscription_tests.cpp a4c4a6083a2cb55e2d69eba1719555a7a13ee8fb 
  src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
  src/tests/persistent_volume_endpoints_tests.cpp 
7a24bf48e9d84bf31ad85397ac3e1b4e566cbc2c 
  src/tests/persistent_volume_tests.cpp 
3e1d1fe468298186347b07b5882973c5a16231a3 
  src/tests/reconciliation_tests.cpp 728961874239c7e5ce8accbaeb2a1c86a73a4f0f 
  src/tests/reservation_endpoints_tests.cpp 
3278732cb130c73be0f20c0204eddaee05123ff9 
  src/tests/role_tests.cpp fc4c017dc59ab44a0ce0ea46f02eb6e1706ffb72 
  src/tests/scheduler_tests.cpp 21f8825c214bb0c331f66bfb9b12a354d6673a43 
  src/tests/slave_authorization_tests.cpp 
30eceae0920351cffc9c3393c6d08917c4041c1a 
  src/tests/slave_recovery_tests.cpp 0e46748be266809c413fb10fc447516c51504fce 
  src/tests/slave_tests.cpp 1bdadce4c50cbff958f2be2a4261e130b414acfd 
  src/tests/teardown_tests.cpp 8834333bbcdb6a04a95a6ed9632ad4ead0791f76 


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

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


Testing
---


Thanks,

Benno Evers



Re: Review Request 62047: Allowed look up latest executor directory by virtual path.

2017-09-12 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos tests failed.

Reviews applied: [62040, 62174, 62047]

All the build artifacts available here: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62047

Relevant logs:

 - mesos-tests-stdout.log:

[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (229 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (226 ms)
[--] 30 tests from ContentType/SchedulerTest (24710 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (906 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 (963 
ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (1941 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (135 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (152 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (344 ms 
total)

[--] Global test environment tear-down
[==] 628 tests from 66 test cases ran. (336810 ms total)
[  PASSED  ] 626 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] ContentType/MasterAPITest.EventAuthorizationFiltering/1, where 
GetParam() = application/json
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.SigkillExecutor/0, where 
GetParam() = "mesos"

 2 FAILED TESTS
  YOU HAVE 174 DISABLED TESTS


Full log available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62047/logs/mesos-tests-stdout.log

 - mesos-tests-stderr.log:

I0912 09:42:54.638357 10592 master.cpp:8418] Removing framework 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- (default)
I0912 09:42:54.643355 10592 master.cpp:3267] Deactivating framework 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- (default)
I0912 09:42:54.643355  6672 hierarchical.cpp:412] Deactivated framework 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa-
I0912 09:42:54.643355 12124 slave.cpp:3245] Shutting down framework 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa-
I0912 09:42:54.643355 10592 master.cpp:8993] Updating the state of task 
916869c6-e96d-4bd9-a8b8-b02eb3e588a4 of framework 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0912 09:42:54.644357 12124 slave.cpp:5752] Shutting down executor 'default' of 
framework 5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- (via HTTP)
I0912 09:42:54.648355 10592 master.cpp:9087] Removing task 
916869c6-e96d-4bd9-a8b8-b02eb3e588a4 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework 5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- on agent 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa-S0 at slave(255)@10.3.1.7:55320 
(mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0912 09:42:54.669356 10592 master.cpp:9116] Removing executor 'default' with 
resources [] of framework 5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- on agent 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa-S0 at slave(255)@10.3.1.7:55320 
(mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0912 09:42:54.673357 11076 hierarchical.cpp:355] Removed framework 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa-
E0912 09:42:54.673357  7624 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0912 09:42:54.688356  7624 scheduler.cpp:444] Re-detecting master
I0912 09:42:54.691357  6672 scheduler.cpp:470] New master detected at 
master@10.3.1.7:55320
I0912 09:42:54.701359  9500 slave.cpp:5417] Executor 'default' of framework 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- exited with status 0
I0912 09:42:54.702358  9500 slave.cpp:5521] Cleaning up executor 'default' of 
framework 5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- (via HTTP)
W0912 09:42:54.702358 12124 master.cpp:7021] Ignoring unknown exited executor 
'default' of framework 5889f9ef-0acf-4698-ac73-4bf5c47e2dfa- on agent 
5889f9ef-0acf-4698-ac73-4bf5c47e2dfa-S0 at slave(255)@10.3.1.7:55320 

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-12 Thread Armand Grillet


> On Sept. 4, 2017, 1:39 p.m., Alexander Rukletsov wrote:
> > src/logging/logging.cpp
> > Lines 135-139 (patched)
> > 
> >
> > This approach changes defaults for clients, who start calling 
> > `logging::initialize(argv0)` now. For example, according to 
> > https://github.com/google/glog/blob/v0.3.3/src/logging.cc#L140,
> > the default for `FLAGS_logbufsecs` will change from `30` to `0`. More 
> > importantly, the default for `FLAGS_logtostderr` is `false` (see 
> > https://github.com/google/glog/blob/v0.3.3/src/logging.cc#L101), while with 
> > your change it will become `true`.
> > 
> > It is hard to say whether it is an issue or not (probably not). If you 
> > want to code defensively, you should guard parts of this function with `if 
> > (_flags.isSome()) {...}`. However, I think it's fine to keep it as is, 
> > extending the comment here saying that glog defaults can be overridden.

The new patch adds guard parts.


- Armand


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


On Sept. 6, 2017, 9:48 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 6, 2017, 9:48 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Thiw will be used to initialize logging in the main functions
> that do not use mesos::internal::logging::Flags.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



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

2017-09-12 Thread Qian Zhang


> On Sept. 8, 2017, 3:43 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Line 112 (original), 112 (patched)
> > 
> >
> > Why do we need a `for` loop like this? I think the previous `while` 
> > loop is good enough.
> 
> James Peach wrote:
> If we want to depend on `errno`, we need to guarantee that it is `0` 
> before we call the `readddir`. Since there is non-trivial logic inside the 
> loop, it is difficult to be confident that nothing we call is going to change 
> it, particularly if this code is ever updated.

OK, then what about a `while` loop and setting `errno` to 0 in the end of each 
iteration? Setting `errno` to 0 in the `for` loop seems a bit strange to me.


- Qian


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


On Sept. 8, 2017, 5:50 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60495/
> ---
> 
> (Updated Sept. 8, 2017, 5:50 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> 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
> -
> 
>   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/15/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Review Request 62237: Fixed a memory leak in composing containerizer.

2017-09-12 Thread Qian Zhang

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

Review request for mesos, Anand Mazumdar and Jie Yu.


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


Repository: mesos


Description
---

Fixed a memory leak in composing containerizer.


Diffs
-

  src/slave/containerizer/composing.cpp 
c076c74bb09e5d528420cece38e583a1e1ae07c9 


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


Testing
---

sudo make check


Thanks,

Qian Zhang