date:20180504

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

2018-05-04 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66938]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 4, 2018, 4:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 4, 2018, 4:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a check to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
>   src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66938 was successfully built and tested.

Reviews applied: `['66938']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 11:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 4, 2018, 11:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a check to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
>   src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66924: Made the master include the operation ID in OPERATION_DROPPED updates.

2018-05-04 Thread Mesos Reviewbot Windows


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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66924']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (112 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1078 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (31 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (38 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (72 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (694 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (717 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (718 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (742 ms total)

[--] Global test environment tear-down
[==] 979 tests from 95 test cases ran. (462086 ms total)
[  PASSED  ] 978 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SlaveTest.RestartSlaveRequireExecutorAuthentication

 1 FAILED TEST
  YOU HAVE 219 DISABLED TESTS

```

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

```
I0505 02:57:27.794875 17052 slave.cpp:3935] Shutting down framework 
969dced1-813a-48ea-be45-24a8a2cbb9b2-
I0505 02:57:27.794875  5632 master.cpp:10831] Updating the state of task 
7a1fa960-8267-418b-8978-39ac35d37287 of framework 
969dced1-813a-48ea-be45-24a8a2cbb9b2- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0505 02:57:27.794875 17052 slave.cpp:6656] ShuttI0505 02:57:27.633867 15408 
exec.cpp:162] Version: 1.6.0
I0505 02:57:27.659853  2712 exec.cpp:236] Executor registered on agent 
969dced1-813a-48ea-be45-24a8a2cbb9b2-S0
I0505 02:57:27.663861 15328 executor.cpp:178] Received SUBSCRIBED event
I0505 02:57:27.667856 15328 executor.cpp:182] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0505 02:57:27.668855 15328 executor.cpp:178] Received LAUNCH event
I0505 02:57:27.672873 15328 executor.cpp:665] Starting task 
7a1fa960-8267-418b-8978-39ac35d37287
I0505 02:57:27.752871 15328 executor.cpp:485] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0505 02:57:27.764875 15328 executor.cpp:678] Forked command at 10752
I0505 02:57:27.796874 16336 exec.cpp:445] Executor asked to shutdown
I0505 02:57:27.797874 12124 executor.cpp:178] Received SHUTDOWN event
I0505 02:57:27.797874 12124 executor.cpp:781] Shutting down
I0505 02:57:27.797874 12124 executor.cpp:894] Sending SIGTERM to process tree 
at pid ing down executor '7a1fa960-8267-418b-8978-39ac35d37287' of framework 
969dced1-813a-48ea-be45-24a8a2cbb9b2- at executor(1)@10.3.1.8:58856
I0505 02:57:27.796874 17052 slave.cpp:929] Agent terminating
W0505 02:57:27.796874 17052 slave.cpp:3931] Ignoring shutdown framework 
969dced1-813a-48ea-be45-24a8a2cbb9b2- because it is terminating
I0505 02:57:27.797874  5632 master.cpp:10930] Removing task 
7a1fa960-8267-418b-8978-39ac35d37287 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 969dced1-813a-48ea-be45-24a8a2cbb9b2- on 
agent 969dced1-813a-48ea-be45-24a8a2cbb9b2-S0 at slave(444)@10.3.1.8:58835 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0505 02:57:27.800869  5632 master.cpp:1293] Agent 
969dced1-813a-48ea-be45-24a8a2cbb9b2-S0 at slave(444)@10.3.1.8:58835 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0505 02:57:27.800869  5632 master.cpp:3299] Disconnecting agent 
969dced1-813a-48ea-be45-24a8a2cbb9b2-S0 at slave(444)@10.3.1.8:58835 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0505 02:57:27.801872 14452 hierarchical.cpp:344] Removed framework 
969dced1-813a-48ea-be45-24a8a2cbb9b2-
I0505 02:57:27.801872  5632 master.cpp:3318] Deactivating agent 
969dced1-813a-48ea-be45-24a8a2cbb9b2-S0 at slave(444)@10.3.1.8:58835

Re: Review Request 66939: Improved validation messages for some operations.

2018-05-04 Thread Mesos Reviewbot Windows


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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66939']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (121 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1106 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (36 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (38 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (76 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (756 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (783 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (728 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (755 ms total)

[--] Global test environment tear-down
[==] 978 tests from 95 test cases ran. (442853 ms total)
[  PASSED  ] 977 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] 
ROOT_DOCKER_DockerAndMesosContainerizers/DefaultExecutorTest.SigkillExecutor/0, 
where GetParam() = "docker,mesos"

 1 FAILED TEST
  YOU HAVE 219 DISABLED TESTS

```

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

```
I0505 02:24:29.888206 12916 master.cpp:10820] Updating the state of task 
b1f57839-7595-497a-b1b6-01815b07ef24 of framework 
40e763f7-4f68-4143-afd0-0f12a8e4775c- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0505 02:24:29.888206 11332 slave.cpp:3935] Shutting down framework 
40e763f7-4f68-4143-afd0-0f12a8e4775c-
I0505 02:24:29.888206 11332 sI0505 02:24:29.718236  6800 exec.cpp:162] Version: 
1.6.0
I0505 02:24:29.744251 10512 exec.cpp:236] Executor registered on agent 
40e763f7-4f68-4143-afd0-0f12a8e4775c-S0
I0505 02:24:29.748239  9220 executor.cpp:178] Received SUBSCRIBED event
I0505 02:24:29.753237  9220 executor.cpp:182] Subscribed executor on 
winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0505 02:24:29.753237  9220 executor.cpp:178] Received LAUNCH event
I0505 02:24:29.759203  9220 executor.cpp:665] Starting task 
b1f57839-7595-497a-b1b6-01815b07ef24
I0505 02:24:29.849251  9220 executor.cpp:485] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0505 02:24:29.863219  9220 executor.cpp:678] Forked command at 12556
I0505 02:24:29.890205 15192 exec.cpp:445] Executor asked to shutdown
I0505 02:24:29.891204  7876 executor.cpp:178] Received SHUTDOWN event
I0505 02:24:29.891204  7876 executor.cpp:781] Shutting down
I0505 02:24:29.891204  7876 executor.cpp:894] Sending SIGTERM to process tree 
at pid lave.cpp:6656] Shutting down executor 
'b1f57839-7595-497a-b1b6-01815b07ef24' of framework 
40e763f7-4f68-4143-afd0-0f12a8e4775c- at executor(1)@10.3.1.5:56470
I0505 02:24:29.890205 11332 slave.cpp:929] Agent terminating
I0505 02:24:29.890205 12916 master.cpp:10919] Removing task 
b1f57839-7595-497a-b1b6-01815b07ef24 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 40e763f7-4f68-4143-afd0-0f12a8e4775c- on 
agent 40e763f7-4f68-4143-afd0-0f12a8e4775c-S0 at slave(443)@10.3.1.5:56449 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
W0505 02:24:29.890205 11332 slave.cpp:3931] Ignoring shutdown framework 
40e763f7-4f68-4143-afd0-0f12a8e4775c- because it is terminating
I0505 02:24:29.894217  9376 master.cpp:1293] Agent 
40e763f7-4f68-4143-afd0-0f12a8e4775c-S0 at slave(443)@10.3.1.5:56449 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0505 02:24:29.894217  9376 master.cpp:3299] Disconnecting agent 
40e763f7-4f68-4143-afd0-0f12a8e4775c-S0 at slave(443)@10.3.1.5:56449 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0505 02:24:29.895225 10312 hierarchical.cpp:344] Removed framework 
40e763f7-4f68-4143-afd0-0f12a8e4775c-
I0505 02:24:29.895225  9376 master.cpp:3318] Deactivating agent

Re: Review Request 66964: Ported (some) Python 2 support scripts to Python 3.

2018-05-04 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66964]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 4, 2018, 1:44 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66964/
> ---
> 
> (Updated May 4, 2018, 1:44 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Mahler, Eric Chung, 
> Clement Michaud, Gaston Kleiman, Joseph Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7658
> https://issues.apache.org/jira/browse/MESOS-7658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to https://docs.python.org/3/howto/pyporting.html, we can
> make our support scripts compatible with both Python 2 and 3 provided
> we only worry about supporting Python 2.7.
> 
> Using the Python-Future quick start guide
> http://python-future.org/quickstart.html, I installed a Python 3.6.5
> environment, ran `pip install future`, and then ran `futurize -w` on
> the modified scripts.
> 
> In my unchanged system environment, attempting to use the scripts
> resulted in a traceback with the error `ImportError: No module named
> future`, since the `future` package is sadly not installed by default.
> On Ubuntu 17.10, I was able to `apt install python-future` to add it
> to my system's Python 2 installation, so would be a new requirement
> despite the Python 2 support.
> 
> The ported scripts were chosen in order to support using Python 3 on
> the Windows build bot, so that author names with non-ASCII characters
> can be used. The rest of the scripts should follow, but are not
> necessary, and require more extensive testing and fixes.
> 
> A manual change was made to `post-reviews` to deal with the stdout of
> child processes. In `execute` we explicitly decode the stdout to a
> string to match the existing expectation (in Python 2, it was already
> a string, but in Python 3 bytes and strings are different types). When
> printing the stdout of a child process, we use `sys.stdout.buffer` to
> print bytes instead of a string in order to preserve ANSI color
> escapes in the output. This also fixes an existing bug on Windows
> where the script would not correctly display colors.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py b6801b926d9fc32172f102586d49e0464da71359 
>   support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
>   support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
>   support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
>   support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 
> 
> 
> Diff: https://reviews.apache.org/r/66964/diff/1/
> 
> 
> Testing
> ---
> 
> This is a work-in-progress. I used the automatic porting script `futurize` on 
> some of our support scripts (I tried on all of them, but some (like 
> `cpplint.py`) are going to take more work). So I'd prefer to start with the 
> set of scripts necessary to change the Windows ReviewBot over to Python 3. 
> Also, `mesos-style.py` _does not_ like these changes; it throws a bunch of 
> errors and warnings when linting these files:
> 
> ```
> W: 39, 0: Redefining built-in 'str' (redefined-builtin)
> E: 37, 0: Unable to import 'future' (import-error)
> E: 39, 0: Unable to import 'builtins' (import-error)
> C: 39, 0: Import "from builtins import str" should be placed at the top of 
> the module (wrong-import-position)
> C: 40, 0: Import "import atexit" should be placed at the top of the module 
> (wrong-import-position)
> C: 41, 0: Import "import json" should be placed at the top of the module 
> (wrong-import-position)
> C: 42, 0: Import "import os" should be placed at the top of the module 
> (wrong-import-position)
> C: 43, 0: Import "import platform" should be placed at the top of the module 
> (wrong-import-position)
> C: 44, 0: Import "import subprocess" should be placed at the top of the 
> module (wrong-import-position)
> C: 45, 0: Import "import sys" should be placed at the top of the module 
> (wrong-import-position)
> E: 46, 0: No name 'request' in module 'urllib' (no-name-in-module)
> E: 46, 0: Unable to import 'urllib.request' (import-error)
> C: 46, 0: Import "import urllib.request" should be placed at the top of the 
> module (wrong-import-position)
> E: 47, 0: No name 'parse' in module 'urllib' (no-name-in-module)
> E: 47, 0: Unable to import 'urllib.parse' (import-error)
> C: 47, 0: Import "import urllib.parse" should be placed at the top of the 
> module (wrong-import-position)
>

Re: Review Request 66969: Added unresolved critical issues to the 1.6 CHANGELOG.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66969 was successfully built and tested.

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

- Mesos Reviewbot Windows


On May 4, 2018, 10:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66969/
> ---
> 
> (Updated May 4, 2018, 10:34 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unresolved critical issues to the 1.6 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 02c7d6f82e0bb24c23143f696f29c57076456fa0 
> 
> 
> Diff: https://reviews.apache.org/r/66969/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66939: Improved validation messages for some operations.

2018-05-04 Thread Gaston Kleiman


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

(Updated May 4, 2018, 4:41 p.m.)


Review request for mesos, Greg Mann and Jan Schlicht.


Changes
---

Fixed tests.


Repository: mesos


Description
---

Improved validation messages for some operations.


Diffs (updated)
-

  src/master/validation.cpp 0c1c9249a0c1ca5ca52e5d7f32a1f02f6e2078d1 
  src/tests/master_validation_tests.cpp 
fb1d8bdf9ba09cace3ceb202dc70c0d201f2784e 


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

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


Testing
---


Thanks,

Gaston Kleiman

Re: Review Request 66924: Made the master include the operation ID in OPERATION_DROPPED updates.

2018-05-04 Thread Gaston Kleiman


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

(Updated May 4, 2018, 4:37 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Greg Mann.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

Made the master include the operation ID in OPERATION_DROPPED updates.


Diffs (updated)
-

  src/master/master.hpp 76e7763a99972f686af9d7eed08b6b6b1db23b0f 
  src/master/master.cpp 3b5d2eba3f602f68a6bb1e00444b01fb58a1bfc2 
  src/tests/master_slave_reconciliation_tests.cpp 
71e22af7ac3b7bc4c72340274961db16d7355e7d 
  src/tests/mesos.hpp 8da3b021874097c5b66e5bc7d9fdafcc8fc377ef 


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

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


Testing
---

`make check` on GNU/Linux.


The test in this patch fails without the corresponding master change, but 
succeeds with it applied.


Thanks,

Gaston Kleiman

Re: Review Request 66962: Windows: Added tests for async IO functions.

2018-05-04 Thread Radhika Jandhyala via Review Board


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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> ---
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.

2018-05-04 Thread Radhika Jandhyala via Review Board


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


Ship it!




Ship It!


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


Would this mean that on windows process launched by the containerizer would 
not be able to perform async io?


- Radhika Jandhyala


On May 4, 2018, 5:19 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66959/
> ---
> 
> (Updated May 4, 2018, 5:19 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8674
> https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Mesos containerizer needs an inheritable pipe that is not used for
> stdio of the child process. It is used for signaling when the child
> process should start running, so the pipe needs to be inheritable and
> not overlapped.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 
> 
> 
> Diff: https://reviews.apache.org/r/66959/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66956: Windows: Added overlapped support to `os::write`.

2018-05-04 Thread Radhika Jandhyala via Review Board


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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:21 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66956/
> ---
> 
> (Updated May 4, 2018, 5:21 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added overlapped handle support to `os::write`. Now, `os::write` will do
> a blocking write no matter the handle type. Also, `os::write_async` will
> do an overlapped write on an overlapped handle.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 295c031c5824d13d74e2c9006e62c391d5020f69 
> 
> 
> Diff: https://reviews.apache.org/r/66956/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

2018-05-04 Thread Greg Mann



> On May 4, 2018, 2:45 p.m., Benjamin Bannier wrote:
> > src/sched/sched.cpp
> > Lines 1349 (patched)
> > 
> >
> > Do we really need to `return` here? It seems just dropping this 
> > particular operation would be enough (in addition to calling `error` with 
> > all its side-effects). I am especially wondering about the tracking of 
> > operations.
> > 
> > (With a `CHECK` the expected behavior would be simpler, not saying we 
> > should prefer it).

I switched back to aborting the process directly rather than calling error() - 
while this means we must eliminate the relevant test, it seems to make a bit 
more sense for the framework developer I think. It also matches what we do in a 
similar case, where the framework attempts to acknowledge a status update 
explicitly after implicit acknowledgement has been enabled.


- Greg


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


On May 4, 2018, 11:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 4, 2018, 11:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a check to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
>   src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

2018-05-04 Thread Greg Mann


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

(Updated May 4, 2018, 11:20 p.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
Vinod Kone.


Repository: mesos


Description
---

Since the 'SchedulerDriver' does not support operation status updates,
this patch adds a check to the driver which will abort the scheduler
if the 'id' field is set in an offer operation.


Diffs (updated)
-

  src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
  src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 66952: Windows: Added overlapped field to WindowsFD.

2018-05-04 Thread Radhika Jandhyala via Review Board


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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:15 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66952/
> ---
> 
> (Updated May 4, 2018, 5:15 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670 and MESOS-8674
> https://issues.apache.org/jira/browse/MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an field to WindowsFD for functions that need to know if a
> Windows file is opened in overlapped mode, such as `os::read`, since
> Windows doesn't provide a Win32 API for it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> bab16e869e69c214e18de584d1615311316e001a 
>   3rdparty/stout/include/stout/os/windows/open.hpp 
> 701dcec7c4c1162d38d8f25596af29ed3b32691d 
> 
> 
> Diff: https://reviews.apache.org/r/66952/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66953: Windows: Fixed dup to properly copy the overlapped WindowsFD.

2018-05-04 Thread Radhika Jandhyala via Review Board


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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:16 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66953/
> ---
> 
> (Updated May 4, 2018, 5:16 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After the previous commit,`os::dup` was not copying the overlapped
> field, so it has been updated to copy it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> af98054f1bd9c8e55c52b246fda8734e3ca96e21 
> 
> 
> Diff: https://reviews.apache.org/r/66953/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66964: Ported (some) Python 2 support scripts to Python 3.

2018-05-04 Thread Andrew Schwartzmeyer



> On May 4, 2018, 3:44 p.m., Mesos Reviewbot Windows wrote:
> > PASS: Mesos patch 66964 was successfully built and tested.
> > 
> > Reviews applied: `['66964']`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66964
> 
> Andrew Schwartzmeyer wrote:
> Grr. It looks like the review bot uses an out-of-tree script instead of 
> `verify-reviews.py`:
> 
> ```
> Successfully executed: python.exe 
> C:\Jenkins\workspace\mesos-reviewbot-testing\Mesos\utils\get-review-ids.py -r 
> 66964 -o C:\Users\mesos\AppData\Local\Temp\mesos_dependent_review_ids
> ```
> 
> They live 
> [here](https://github.com/Microsoft/mesos-jenkins/tree/master/Mesos/utils). 
> I'll ask Ionut to start upstreaming these...

[Issue #66](https://github.com/Microsoft/mesos-jenkins/issues/66).


- Andrew


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


On May 4, 2018, 1:44 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66964/
> ---
> 
> (Updated May 4, 2018, 1:44 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Mahler, Eric Chung, 
> Clement Michaud, Gaston Kleiman, Joseph Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7658
> https://issues.apache.org/jira/browse/MESOS-7658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to https://docs.python.org/3/howto/pyporting.html, we can
> make our support scripts compatible with both Python 2 and 3 provided
> we only worry about supporting Python 2.7.
> 
> Using the Python-Future quick start guide
> http://python-future.org/quickstart.html, I installed a Python 3.6.5
> environment, ran `pip install future`, and then ran `futurize -w` on
> the modified scripts.
> 
> In my unchanged system environment, attempting to use the scripts
> resulted in a traceback with the error `ImportError: No module named
> future`, since the `future` package is sadly not installed by default.
> On Ubuntu 17.10, I was able to `apt install python-future` to add it
> to my system's Python 2 installation, so would be a new requirement
> despite the Python 2 support.
> 
> The ported scripts were chosen in order to support using Python 3 on
> the Windows build bot, so that author names with non-ASCII characters
> can be used. The rest of the scripts should follow, but are not
> necessary, and require more extensive testing and fixes.
> 
> A manual change was made to `post-reviews` to deal with the stdout of
> child processes. In `execute` we explicitly decode the stdout to a
> string to match the existing expectation (in Python 2, it was already
> a string, but in Python 3 bytes and strings are different types). When
> printing the stdout of a child process, we use `sys.stdout.buffer` to
> print bytes instead of a string in order to preserve ANSI color
> escapes in the output. This also fixes an existing bug on Windows
> where the script would not correctly display colors.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py b6801b926d9fc32172f102586d49e0464da71359 
>   support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
>   support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
>   support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
>   support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 
> 
> 
> Diff: https://reviews.apache.org/r/66964/diff/1/
> 
> 
> Testing
> ---
> 
> This is a work-in-progress. I used the automatic porting script `futurize` on 
> some of our support scripts (I tried on all of them, but some (like 
> `cpplint.py`) are going to take more work). So I'd prefer to start with the 
> set of scripts necessary to change the Windows ReviewBot over to Python 3. 
> Also, `mesos-style.py` _does not_ like these changes; it throws a bunch of 
> errors and warnings when linting these files:
> 
> ```
> W: 39, 0: Redefining built-in 'str' (redefined-builtin)
> E: 37, 0: Unable to import 'future' (import-error)
> E: 39, 0: Unable to import 'builtins' (import-error)
> C: 39, 0: Import "from builtins import str" should be placed at the top of 
> the module (wrong-import-position)
> C: 40, 0: Import "import atexit" should be placed at the top of the module 
> (wrong-import-position)
> C: 41, 0: Import "import json" should be placed at the top of the module 
> (wrong-import-position)
> C: 42, 0: Import "import os" should be placed at the top of the module 
> (wrong-import-position)
> C: 43, 0: Import "import platform" should be placed at the top of the module 
> (wrong-import-position)
> C: 44, 0: Import "import subprocess" should be placed at the top of the 
> module

Re: Review Request 66964: Ported (some) Python 2 support scripts to Python 3.

2018-05-04 Thread Andrew Schwartzmeyer



> On May 4, 2018, 3:44 p.m., Mesos Reviewbot Windows wrote:
> > PASS: Mesos patch 66964 was successfully built and tested.
> > 
> > Reviews applied: `['66964']`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66964

Grr. It looks like the review bot uses an out-of-tree script instead of 
`verify-reviews.py`:

```
Successfully executed: python.exe 
C:\Jenkins\workspace\mesos-reviewbot-testing\Mesos\utils\get-review-ids.py -r 
66964 -o C:\Users\mesos\AppData\Local\Temp\mesos_dependent_review_ids
```

They live 
[here](https://github.com/Microsoft/mesos-jenkins/tree/master/Mesos/utils). 
I'll ask Ionut to start upstreaming these...


- Andrew


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


On May 4, 2018, 1:44 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66964/
> ---
> 
> (Updated May 4, 2018, 1:44 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Mahler, Eric Chung, 
> Clement Michaud, Gaston Kleiman, Joseph Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7658
> https://issues.apache.org/jira/browse/MESOS-7658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to https://docs.python.org/3/howto/pyporting.html, we can
> make our support scripts compatible with both Python 2 and 3 provided
> we only worry about supporting Python 2.7.
> 
> Using the Python-Future quick start guide
> http://python-future.org/quickstart.html, I installed a Python 3.6.5
> environment, ran `pip install future`, and then ran `futurize -w` on
> the modified scripts.
> 
> In my unchanged system environment, attempting to use the scripts
> resulted in a traceback with the error `ImportError: No module named
> future`, since the `future` package is sadly not installed by default.
> On Ubuntu 17.10, I was able to `apt install python-future` to add it
> to my system's Python 2 installation, so would be a new requirement
> despite the Python 2 support.
> 
> The ported scripts were chosen in order to support using Python 3 on
> the Windows build bot, so that author names with non-ASCII characters
> can be used. The rest of the scripts should follow, but are not
> necessary, and require more extensive testing and fixes.
> 
> A manual change was made to `post-reviews` to deal with the stdout of
> child processes. In `execute` we explicitly decode the stdout to a
> string to match the existing expectation (in Python 2, it was already
> a string, but in Python 3 bytes and strings are different types). When
> printing the stdout of a child process, we use `sys.stdout.buffer` to
> print bytes instead of a string in order to preserve ANSI color
> escapes in the output. This also fixes an existing bug on Windows
> where the script would not correctly display colors.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py b6801b926d9fc32172f102586d49e0464da71359 
>   support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
>   support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
>   support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
>   support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 
> 
> 
> Diff: https://reviews.apache.org/r/66964/diff/1/
> 
> 
> Testing
> ---
> 
> This is a work-in-progress. I used the automatic porting script `futurize` on 
> some of our support scripts (I tried on all of them, but some (like 
> `cpplint.py`) are going to take more work). So I'd prefer to start with the 
> set of scripts necessary to change the Windows ReviewBot over to Python 3. 
> Also, `mesos-style.py` _does not_ like these changes; it throws a bunch of 
> errors and warnings when linting these files:
> 
> ```
> W: 39, 0: Redefining built-in 'str' (redefined-builtin)
> E: 37, 0: Unable to import 'future' (import-error)
> E: 39, 0: Unable to import 'builtins' (import-error)
> C: 39, 0: Import "from builtins import str" should be placed at the top of 
> the module (wrong-import-position)
> C: 40, 0: Import "import atexit" should be placed at the top of the module 
> (wrong-import-position)
> C: 41, 0: Import "import json" should be placed at the top of the module 
> (wrong-import-position)
> C: 42, 0: Import "import os" should be placed at the top of the module 
> (wrong-import-position)
> C: 43, 0: Import "import platform" should be placed at the top of the module 
> (wrong-import-position)
> C: 44, 0: Import "import subprocess" should be placed at the top of the 
> module (wrong-import-position)
> C: 45, 0: Import "import sys" should be placed at the top of the module 
> (wrong-import-position)
> E: 46, 0: No name 'request'

Re: Review Request 66969: Added unresolved critical issues to the 1.6 CHANGELOG.

2018-05-04 Thread Greg Mann



> On May 4, 2018, 10:46 p.m., Chun-Hung Hsiao wrote:
> > CHANGELOG
> > Lines 62 (patched)
> > 
> >
> > Did you sort it in the reversed order? ;)

Whoops, thanks Chun-Hung :)

I fixed this with another commit.


- Greg


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


On May 4, 2018, 10:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66969/
> ---
> 
> (Updated May 4, 2018, 10:34 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unresolved critical issues to the 1.6 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 02c7d6f82e0bb24c23143f696f29c57076456fa0 
> 
> 
> Diff: https://reviews.apache.org/r/66969/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66230: Added test for adding/removing framework roles.

2018-05-04 Thread Mesos Reviewbot


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



Bad patch!

Reviews applied: [66230, 66229, 66228, 66872]

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

Error:
2018-05-04 22:50:30 URL:https://reviews.apache.org/r/66229/diff/raw/ 
[12365/12365] -> "66229.patch" [1]
error: patch failed: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp:684
error: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp: patch does not 
apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/22373/console

- Mesos Reviewbot


On May 4, 2018, 5:33 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> ---
> 
> (Updated May 4, 2018, 5:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for adding/removing framework roles.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 
> 
> 
> Diff: https://reviews.apache.org/r/66230/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 66969: Added unresolved critical issues to the 1.6 CHANGELOG.

2018-05-04 Thread Chun-Hung Hsiao


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


Fix it, then Ship it!





CHANGELOG
Lines 62 (patched)


Did you sort it in the reversed order? ;)


- Chun-Hung Hsiao


On May 4, 2018, 10:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66969/
> ---
> 
> (Updated May 4, 2018, 10:34 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unresolved critical issues to the 1.6 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 02c7d6f82e0bb24c23143f696f29c57076456fa0 
> 
> 
> Diff: https://reviews.apache.org/r/66969/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66964: Ported (some) Python 2 support scripts to Python 3.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66964 was successfully built and tested.

Reviews applied: `['66964']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 8:44 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66964/
> ---
> 
> (Updated May 4, 2018, 8:44 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Mahler, Eric Chung, 
> Clement Michaud, Gaston Kleiman, Joseph Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7658
> https://issues.apache.org/jira/browse/MESOS-7658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to https://docs.python.org/3/howto/pyporting.html, we can
> make our support scripts compatible with both Python 2 and 3 provided
> we only worry about supporting Python 2.7.
> 
> Using the Python-Future quick start guide
> http://python-future.org/quickstart.html, I installed a Python 3.6.5
> environment, ran `pip install future`, and then ran `futurize -w` on
> the modified scripts.
> 
> In my unchanged system environment, attempting to use the scripts
> resulted in a traceback with the error `ImportError: No module named
> future`, since the `future` package is sadly not installed by default.
> On Ubuntu 17.10, I was able to `apt install python-future` to add it
> to my system's Python 2 installation, so would be a new requirement
> despite the Python 2 support.
> 
> The ported scripts were chosen in order to support using Python 3 on
> the Windows build bot, so that author names with non-ASCII characters
> can be used. The rest of the scripts should follow, but are not
> necessary, and require more extensive testing and fixes.
> 
> A manual change was made to `post-reviews` to deal with the stdout of
> child processes. In `execute` we explicitly decode the stdout to a
> string to match the existing expectation (in Python 2, it was already
> a string, but in Python 3 bytes and strings are different types). When
> printing the stdout of a child process, we use `sys.stdout.buffer` to
> print bytes instead of a string in order to preserve ANSI color
> escapes in the output. This also fixes an existing bug on Windows
> where the script would not correctly display colors.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py b6801b926d9fc32172f102586d49e0464da71359 
>   support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
>   support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
>   support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
>   support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 
> 
> 
> Diff: https://reviews.apache.org/r/66964/diff/1/
> 
> 
> Testing
> ---
> 
> This is a work-in-progress. I used the automatic porting script `futurize` on 
> some of our support scripts (I tried on all of them, but some (like 
> `cpplint.py`) are going to take more work). So I'd prefer to start with the 
> set of scripts necessary to change the Windows ReviewBot over to Python 3. 
> Also, `mesos-style.py` _does not_ like these changes; it throws a bunch of 
> errors and warnings when linting these files:
> 
> ```
> W: 39, 0: Redefining built-in 'str' (redefined-builtin)
> E: 37, 0: Unable to import 'future' (import-error)
> E: 39, 0: Unable to import 'builtins' (import-error)
> C: 39, 0: Import "from builtins import str" should be placed at the top of 
> the module (wrong-import-position)
> C: 40, 0: Import "import atexit" should be placed at the top of the module 
> (wrong-import-position)
> C: 41, 0: Import "import json" should be placed at the top of the module 
> (wrong-import-position)
> C: 42, 0: Import "import os" should be placed at the top of the module 
> (wrong-import-position)
> C: 43, 0: Import "import platform" should be placed at the top of the module 
> (wrong-import-position)
> C: 44, 0: Import "import subprocess" should be placed at the top of the 
> module (wrong-import-position)
> C: 45, 0: Import "import sys" should be placed at the top of the module 
> (wrong-import-position)
> E: 46, 0: No name 'request' in module 'urllib' (no-name-in-module)
> E: 46, 0: Unable to import 'urllib.request' (import-error)
> C: 46, 0: Import "import urllib.request" should be placed at the top of the 
> module (wrong-import-position)
> E: 47, 0: No name 'parse' in module 'urllib' (no-name-in-module)
> E: 47, 0: Unable to import 'urllib.parse' (import-error)
> C: 47, 0: Import "import urllib.parse" should be placed at the top of the 
> module (wrong-import-position)
> E: 48, 0: No name 'error' in module

Re: Review Request 66919: Failure to update registry should abort the master process.

2018-05-04 Thread Jiang Yan Xu


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



The "process aborting" logic is a bit hard to write tests for but we can test 
it manually without checking in the code.

e.g., I changed one test a bit to show the check failure: 
https://github.com/xujyan/mesos/commit/68051320a87431f6d2f3fbad6b0b97814200a731

Could you update the "Testing Done" section with the link? That should cover 
it. :)

- Jiang Yan Xu


On May 4, 2018, 12:46 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66919/
> ---
> 
> (Updated May 4, 2018, 12:46 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8630
> https://issues.apache.org/jira/browse/MESOS-8630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the registrar fails to update the registry it would abort the
> actor and fail all future operations,However when the registrar
> updates is requested by an operator API such as maintenance update,
> the master process doesn't shut down (a 500 error is returned to the
> client instead)and all subsequent operations will fail.
> 
> Review: https://reviews.apache.org/r/66919
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
> 
> 
> Diff: https://reviews.apache.org/r/66919/diff/1/
> 
> 
> Testing
> ---
> 
> No error found by running all unit tests(make check)
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 66965: Added MESOS-8054 to the 1.6 CHANGELOG.

2018-05-04 Thread Gaston Kleiman


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


Ship it!




Ship It!

- Gaston Kleiman


On May 4, 2018, 3:35 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66965/
> ---
> 
> (Updated May 4, 2018, 3:35 p.m.)
> 
> 
> Review request for mesos and Gaston Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-8054 to the 1.6 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 02c7d6f82e0bb24c23143f696f29c57076456fa0 
> 
> 
> Diff: https://reviews.apache.org/r/66965/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66965: Added MESOS-8054 to the 1.6 CHANGELOG.

2018-05-04 Thread Greg Mann


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

(Updated May 4, 2018, 10:35 p.m.)


Review request for mesos and Gaston Kleiman.


Repository: mesos


Description
---

Added MESOS-8054 to the 1.6 CHANGELOG.


Diffs (updated)
-

  CHANGELOG 02c7d6f82e0bb24c23143f696f29c57076456fa0 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66969: Added unresolved critical issues to the 1.6 CHANGELOG.

2018-05-04 Thread Greg Mann


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

(Updated May 4, 2018, 10:34 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Vinod Kone.


Changes
---

Rebase.


Repository: mesos


Description
---

Added unresolved critical issues to the 1.6 CHANGELOG.


Diffs (updated)
-

  CHANGELOG 02c7d6f82e0bb24c23143f696f29c57076456fa0 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66969: Added unresolved critical issues to the 1.6 CHANGELOG.

2018-05-04 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the dependent review: 66965.

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

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

Relevant logs:

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

```
error: patch failed: CHANGELOG:29
error: CHANGELOG: patch does not apply
```

- Mesos Reviewbot Windows


On May 4, 2018, 9:13 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66969/
> ---
> 
> (Updated May 4, 2018, 9:13 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unresolved critical issues to the 1.6 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 
> 
> 
> Diff: https://reviews.apache.org/r/66969/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66970: Added MESOS-8682 and MESOS-8659 to the 1.6.0 CHANGELOG.

2018-05-04 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: patch failed: CHANGELOG:20
error: CHANGELOG: patch does not apply
```

- Mesos Reviewbot Windows


On May 4, 2018, 9:18 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66970/
> ---
> 
> (Updated May 4, 2018, 9:18 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-8682 and MESOS-8659 to the 1.6.0 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 
> 
> 
> Diff: https://reviews.apache.org/r/66970/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66965: Added MESOS-8054 to the 1.6 CHANGELOG.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66965 was successfully built and tested.

Reviews applied: `['66963', '66965']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 11:35 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66965/
> ---
> 
> (Updated May 4, 2018, 11:35 a.m.)
> 
> 
> Review request for mesos and Gaston Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-8054 to the 1.6 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 
> 
> 
> Diff: https://reviews.apache.org/r/66965/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 66970: Added MESOS-8682 and MESOS-8659 to the 1.6.0 CHANGELOG.

2018-05-04 Thread Andrew Schwartzmeyer


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

Review request for mesos and Greg Mann.


Repository: mesos


Description
---

Added MESOS-8682 and MESOS-8659 to the 1.6.0 CHANGELOG.


Diffs
-

  CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66968: Added MESOS-8649, MESOS-8793 and MESOS-8874 to the 1.6.0 CHANGELOG.

2018-05-04 Thread Greg Mann


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


Ship it!




Ship It!

- Greg Mann


On May 4, 2018, 8:37 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66968/
> ---
> 
> (Updated May 4, 2018, 8:37 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-8649, MESOS-8793 and MESOS-8874 to the 1.6.0 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 
> 
> 
> Diff: https://reviews.apache.org/r/66968/diff/3/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66923: Added documentation on volume resize support.

2018-05-04 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66218, 66049, 66733, 66050, 66219, 66858, 66220, 66531, 
66532, 66052, 66051, 66227, 66923]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 4, 2018, 4:49 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66923/
> ---
> 
> (Updated May 4, 2018, 4:49 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on volume resize support.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md fdbef770c38ab38ab748b4f943b331d4442a2ce1 
>   docs/operator-http-api.md 9be1e2db60562b369bdc6ee2c732a74fad049580 
>   docs/persistent-volume.md 1a5799b8d8e5302e0b01da6b9a16a3b9fb75c898 
> 
> 
> Diff: https://reviews.apache.org/r/66923/diff/2/
> 
> 
> Testing
> ---
> 
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/persistent-volume.md
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/operator-http-api.md
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/authorization.md
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66969: Added unresolved critical issues to the 1.6 CHANGELOG.

2018-05-04 Thread Greg Mann


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

(Updated May 4, 2018, 9:13 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Vinod Kone.


Repository: mesos


Description
---

Added unresolved critical issues to the 1.6 CHANGELOG.


Diffs (updated)
-

  CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 


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

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


Testing
---


Thanks,

Greg Mann

Review Request 66969: Added unresolved critical issues to the 1.6 CHANGELOG.

2018-05-04 Thread Greg Mann


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

Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Vinod Kone.


Repository: mesos


Description
---

Added unresolved critical issues to the 1.6 CHANGELOG.


Diffs
-

  CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66900: Avoid copying of re-register framework messages in the master.

2018-05-04 Thread Benjamin Mahler


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


Ship it!




Ship It!

- Benjamin Mahler


On May 4, 2018, 12:58 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66900/
> ---
> 
> (Updated May 4, 2018, 12:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8628
> https://issues.apache.org/jira/browse/MESOS-8628
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this patch, the re-register framework message was copied
> into a subscribe call. This updates the handler to perform moves
> instead.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
> 
> 
> Diff: https://reviews.apache.org/r/66900/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66860: Avoid copying of register framework messages in the master.

2018-05-04 Thread Benjamin Mahler


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


Ship it!




Ship It!

- Benjamin Mahler


On May 4, 2018, 12:58 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66860/
> ---
> 
> (Updated May 4, 2018, 12:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8628
> https://issues.apache.org/jira/browse/MESOS-8628
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this patch, the register framework message was copied
> into a subscribe call. This updates the handler to perform moves
> instead.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
> 
> 
> Diff: https://reviews.apache.org/r/66860/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66923: Added documentation on volume resize support.

2018-05-04 Thread Greg Mann


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


Ship it!




- Greg Mann


On May 4, 2018, 4:49 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66923/
> ---
> 
> (Updated May 4, 2018, 4:49 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on volume resize support.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md fdbef770c38ab38ab748b4f943b331d4442a2ce1 
>   docs/operator-http-api.md 9be1e2db60562b369bdc6ee2c732a74fad049580 
>   docs/persistent-volume.md 1a5799b8d8e5302e0b01da6b9a16a3b9fb75c898 
> 
> 
> Diff: https://reviews.apache.org/r/66923/diff/2/
> 
> 
> Testing
> ---
> 
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/persistent-volume.md
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/operator-http-api.md
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/authorization.md
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 66964: Ported (some) Python 2 support scripts to Python 3.

2018-05-04 Thread Andrew Schwartzmeyer


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

Review request for mesos, Armand Grillet, Benjamin Mahler, Eric Chung, Clement 
Michaud, Gaston Kleiman, Joseph Wu, and Kevin Klues.


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


Repository: mesos


Description
---

According to https://docs.python.org/3/howto/pyporting.html, we can
make our support scripts compatible with both Python 2 and 3 provided
we only worry about supporting Python 2.7.

Using the Python-Future quick start guide
http://python-future.org/quickstart.html, I installed a Python 3.6.5
environment, ran `pip install future`, and then ran `futurize -w` on
the modified scripts.

In my unchanged system environment, attempting to use the scripts
resulted in a traceback with the error `ImportError: No module named
future`, since the `future` package is sadly not installed by default.
On Ubuntu 17.10, I was able to `apt install python-future` to add it
to my system's Python 2 installation, so would be a new requirement
despite the Python 2 support.

The ported scripts were chosen in order to support using Python 3 on
the Windows build bot, so that author names with non-ASCII characters
can be used. The rest of the scripts should follow, but are not
necessary, and require more extensive testing and fixes.

A manual change was made to `post-reviews` to deal with the stdout of
child processes. In `execute` we explicitly decode the stdout to a
string to match the existing expectation (in Python 2, it was already
a string, but in Python 3 bytes and strings are different types). When
printing the stdout of a child process, we use `sys.stdout.buffer` to
print bytes instead of a string in order to preserve ANSI color
escapes in the output. This also fixes an existing bug on Windows
where the script would not correctly display colors.


Diffs
-

  support/apply-reviews.py b6801b926d9fc32172f102586d49e0464da71359 
  support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
  support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
  support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
  support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 


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


Testing
---

This is a work-in-progress. I used the automatic porting script `futurize` on 
some of our support scripts (I tried on all of them, but some (like 
`cpplint.py`) are going to take more work). So I'd prefer to start with the set 
of scripts necessary to change the Windows ReviewBot over to Python 3. Also, 
`mesos-style.py` _does not_ like these changes; it throws a bunch of errors and 
warnings when linting these files:

```
W: 39, 0: Redefining built-in 'str' (redefined-builtin)
E: 37, 0: Unable to import 'future' (import-error)
E: 39, 0: Unable to import 'builtins' (import-error)
C: 39, 0: Import "from builtins import str" should be placed at the top of the 
module (wrong-import-position)
C: 40, 0: Import "import atexit" should be placed at the top of the module 
(wrong-import-position)
C: 41, 0: Import "import json" should be placed at the top of the module 
(wrong-import-position)
C: 42, 0: Import "import os" should be placed at the top of the module 
(wrong-import-position)
C: 43, 0: Import "import platform" should be placed at the top of the module 
(wrong-import-position)
C: 44, 0: Import "import subprocess" should be placed at the top of the module 
(wrong-import-position)
C: 45, 0: Import "import sys" should be placed at the top of the module 
(wrong-import-position)
E: 46, 0: No name 'request' in module 'urllib' (no-name-in-module)
E: 46, 0: Unable to import 'urllib.request' (import-error)
C: 46, 0: Import "import urllib.request" should be placed at the top of the 
module (wrong-import-position)
E: 47, 0: No name 'parse' in module 'urllib' (no-name-in-module)
E: 47, 0: Unable to import 'urllib.parse' (import-error)
C: 47, 0: Import "import urllib.parse" should be placed at the top of the 
module (wrong-import-position)
E: 48, 0: No name 'error' in module 'urllib' (no-name-in-module)
E: 48, 0: Unable to import 'urllib.error' (import-error)
C: 48, 0: Import "import urllib.error" should be placed at the top of the 
module (wrong-import-position)
C: 50, 0: Import "from datetime import datetime" should be placed at the top of 
the module (wrong-import-position)
```

It doesn't seem to handle the Python 2/3 compatibility layer.

Please help test these scripts, I'm testing by hand but we don't have any unit 
test coverage.

For Windows, installing `future` is easiest with `python -m pip install 
future`, and for Linux, the `python-future` package.

So far I tested:

`post-reviews.py` on Linux (for this patch) with Python 2 and 3
`apply-reviews.py` on Windows with Python 2 and 3
`mesos-split.py` on Linux

Re: Review Request 66968: Added MESOS-8649, MESOS-8793 and MESOS-8874 to the 1.6.0 CHANGELOG.

2018-05-04 Thread Chun-Hung Hsiao


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

(Updated May 4, 2018, 8:37 p.m.)


Review request for mesos and Greg Mann.


Repository: mesos


Description
---

Added MESOS-8649, MESOS-8793 and MESOS-8874 to the 1.6.0 CHANGELOG.


Diffs (updated)
-

  CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 


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

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


Testing
---

N/A


Thanks,

Chun-Hung Hsiao

Re: Review Request 66968: Added MESOS-8649, MESOS-8793 and MESOS-8874 to the 1.6.0 CHANGELOG.

2018-05-04 Thread Chun-Hung Hsiao


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

(Updated May 4, 2018, 8:35 p.m.)


Review request for mesos and Greg Mann.


Repository: mesos


Description
---

Added MESOS-8649, MESOS-8793 and MESOS-8874 to the 1.6.0 CHANGELOG.


Diffs (updated)
-

  CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 


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

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


Testing
---

N/A


Thanks,

Chun-Hung Hsiao

Re: Review Request 66962: Windows: Added tests for async IO functions.

2018-05-04 Thread Radhika Jandhyala via Review Board


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




3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 724-729 (patched)


I believe definitions for read_async/write_async might not be in master as 
yet. Would this break the build?



3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 769 (patched)


Does this really block? Then execution would not proceed to read_async?


- Radhika Jandhyala


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> ---
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Review Request 66968: Added MESOS-8649, MESOS-8793 and MESOS-8874 to the 1.6.0 CHANGELOG.

2018-05-04 Thread Chun-Hung Hsiao


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

Review request for mesos and Greg Mann.


Repository: mesos


Description
---

Added MESOS-8649, MESOS-8793 and MESOS-8874 to the 1.6.0 CHANGELOG.


Diffs
-

  CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 


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


Testing
---

N/A


Thanks,

Chun-Hung Hsiao

Re: Review Request 66919: Failure to update registry should abort the master process.

2018-05-04 Thread Jiang Yan Xu



> On May 4, 2018, 8:19 a.m., Jiang Yan Xu wrote:
> > src/master/http.cpp
> > Lines 4163-4165 (patched)
> > 
> >
> > The following will be more idiomatic.
> > 
> > ```
> >   .onAny([](const Future& result) {
> > CHECK_READY(result);
> >   })
> > ```
> > 
> > Note the spaces and const ref.
> > 
> > 
> > It'll be helpful to get familiarize with
> > 
> > https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md
> > 
> > which in turn references 
> > 
> > https://google.github.io/styleguide/cppguide.html
> 
> Jiang Yan Xu wrote:
> It's also helpful to attach a message to the check failure:
> 
> 
> ```
> CHECK_READY(result)
>   << "Failed to update maintenance schedule in the registry";
> ```

Also let's add a TODO above the `CHECK_READY`:

```
TODO(fiu): Consider changing/refactoring the registrar itself so the individual 
call sites don't need to handle this separately. All registrar failures that 
cause it to abort should instead abort the process.
```


- Jiang Yan


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


On May 4, 2018, 12:46 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66919/
> ---
> 
> (Updated May 4, 2018, 12:46 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8630
> https://issues.apache.org/jira/browse/MESOS-8630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the registrar fails to update the registry it would abort the
> actor and fail all future operations,However when the registrar
> updates is requested by an operator API such as maintenance update,
> the master process doesn't shut down (a 500 error is returned to the
> client instead)and all subsequent operations will fail.
> 
> Review: https://reviews.apache.org/r/66919
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
> 
> 
> Diff: https://reviews.apache.org/r/66919/diff/1/
> 
> 
> Testing
> ---
> 
> No error found by running all unit tests(make check)
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 66963: Updated the 1.6 CHANGELOG.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66963 was successfully built and tested.

Reviews applied: `['66963']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 11:34 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66963/
> ---
> 
> (Updated May 4, 2018, 11:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Chun-Hung Hsiao, Gaston 
> Kleiman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the 1.6 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 
> 
> 
> Diff: https://reviews.apache.org/r/66963/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66940: Documented the changes in the agent flags and modules.

2018-05-04 Thread Greg Mann


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


Ship it!




Ship It!

- Greg Mann


On May 3, 2018, 10:38 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66940/
> ---
> 
> (Updated May 3, 2018, 10:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the changes in the agent flags and modules.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 2de8900d4a7d192fad959da8a1b6929067a6bfc9 
> 
> 
> Diff: https://reviews.apache.org/r/66940/diff/1/
> 
> 
> Testing
> ---
> 
> Manually tested if the hyperlinks work ;) No test is needed.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

2018-05-04 Thread Akash Gupta


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



By the way, this patch set is roughly half of the changes for the IOCP backend. 
These focus mainly on the stout changes. I'm currently cleaning up some 
libprocess code so that I can post the remaining half.

- Akash Gupta


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> ---
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66962 was successfully built and tested.

Reviews applied: `['66952', '66953', '66954', '66955', '66956', '66957', 
'66958', '66959', '66960', '66961', '66962']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> ---
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66966: Fixed synchronization orders for destructing mock resource providers.

2018-05-04 Thread Benjamin Bannier


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


Ship it!




Thanks for the cleanup, and congrats on the  trophy!

- Benjamin Bannier


On May 4, 2018, 9:15 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66966/
> ---
> 
> (Updated May 4, 2018, 9:15 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed synchronization orders for destructing mock resource providers.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66966/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 66966: Fixed synchronization orders for destructing mock resource providers.

2018-05-04 Thread Chun-Hung Hsiao


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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

Fixed synchronization orders for destructing mock resource providers.


Diffs
-

  src/tests/resource_provider_manager_tests.cpp 
e8ca377fd0a927b99fdaf6a8ee0139025a41298e 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 66952: Windows: Added overlapped field to WindowsFD.

2018-05-04 Thread Radhika Jandhyala via Review Board


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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:15 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66952/
> ---
> 
> (Updated May 4, 2018, 5:15 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670 and MESOS-8674
> https://issues.apache.org/jira/browse/MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an field to WindowsFD for functions that need to know if a
> Windows file is opened in overlapped mode, such as `os::read`, since
> Windows doesn't provide a Win32 API for it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> bab16e869e69c214e18de584d1615311316e001a 
>   3rdparty/stout/include/stout/os/windows/open.hpp 
> 701dcec7c4c1162d38d8f25596af29ed3b32691d 
> 
> 
> Diff: https://reviews.apache.org/r/66952/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66952: Windows: Added overlapped field to WindowsFD.

2018-05-04 Thread Eric Mumau via Review Board


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




3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 183 (patched)


Shouldn't the overlapped_ member be constant?


- Eric Mumau


On May 4, 2018, 5:15 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66952/
> ---
> 
> (Updated May 4, 2018, 5:15 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670 and MESOS-8674
> https://issues.apache.org/jira/browse/MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an field to WindowsFD for functions that need to know if a
> Windows file is opened in overlapped mode, such as `os::read`, since
> Windows doesn't provide a Win32 API for it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> bab16e869e69c214e18de584d1615311316e001a 
>   3rdparty/stout/include/stout/os/windows/open.hpp 
> 701dcec7c4c1162d38d8f25596af29ed3b32691d 
> 
> 
> Diff: https://reviews.apache.org/r/66952/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66947: Increased the timeout for waiting for `reaped` to be invoked.

2018-05-04 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66947]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 4, 2018, 9:32 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66947/
> ---
> 
> (Updated May 4, 2018, 9:32 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-8876
> https://issues.apache.org/jira/browse/MESOS-8876
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously after the container process is reaped by the Docker
> executor, we will wait 3 seconds for `reaped` to be invoked.
> However in some cases (e.g., launch a Docker container to use
> an external rexray volume), there will be more than 3 seconds
> after the container process exits and before the `docker run`
> command returns (i.e., `reaped` invoked). So in this patch,
> the timeout is increased to 60 seconds.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 16509dae3acebc59635e925073372adfb3786edc 
> 
> 
> Diff: https://reviews.apache.org/r/66947/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 66965: Added MESOS-8054 to the 1.6 CHANGELOG.

2018-05-04 Thread Greg Mann


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

Review request for mesos and Gaston Kleiman.


Repository: mesos


Description
---

Added MESOS-8054 to the 1.6 CHANGELOG.


Diffs
-

  CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66963: Updated the 1.6 CHANGELOG.

2018-05-04 Thread Greg Mann


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

(Updated May 4, 2018, 6:34 p.m.)


Review request for mesos, Alexander Rukletsov, Chun-Hung Hsiao, Gaston Kleiman, 
and Vinod Kone.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated the 1.6 CHANGELOG.


Diffs (updated)
-

  CHANGELOG 03675609c6af1c706dd4dae9e9fff3d7e3889568 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66230: Added test for adding/removing framework roles.

2018-05-04 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the dependent review: 66229.

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

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

Relevant logs:

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

```
error: patch failed: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp:684
error: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp: patch does not 
apply
```

- Mesos Reviewbot Windows


On May 4, 2018, 5:33 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> ---
> 
> (Updated May 4, 2018, 5:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for adding/removing framework roles.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 
> 
> 
> Diff: https://reviews.apache.org/r/66230/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 66923: Added documentation on volume resize support.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66923 was successfully built and tested.

Reviews applied: `['66923']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 4:49 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66923/
> ---
> 
> (Updated May 4, 2018, 4:49 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on volume resize support.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md fdbef770c38ab38ab748b4f943b331d4442a2ce1 
>   docs/operator-http-api.md 9be1e2db60562b369bdc6ee2c732a74fad049580 
>   docs/persistent-volume.md 1a5799b8d8e5302e0b01da6b9a16a3b9fb75c898 
> 
> 
> Diff: https://reviews.apache.org/r/66923/diff/2/
> 
> 
> Testing
> ---
> 
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/persistent-volume.md
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/operator-http-api.md
> https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/authorization.md
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66635: Fixed potential races when interacting with cgroup subsystem isolators.

2018-05-04 Thread Gilbert Song



> On May 4, 2018, 1:53 a.m., Qian Zhang wrote:
> > Thanks for the patch!
> > 
> > The `doXxx()` functions seem a bit strange to me. I am wondering if we can 
> > follow the similar way of how Mesos containerizer calls isolators, e.g., we 
> > could rename `Subsystem` to `SubsystemProcess` and all the `XxxSubsystem` 
> > to `XxxSubsystemProcess` since they are all actually `Process`, and then 
> > introduce a new class `Subsystem` which internally dispatches calls to 
> > `SubsystemProcess`, and `CgroupsIsolatorProcess` always directly calls the 
> > functions of `Subsystem`.
> 
> Benjamin Bannier wrote:
> Like outlined in the ticket, that would be the other possibility. Note 
> that we not only require dedicated subsystem processes, but also wrappers 
> derived from `Subsystem` for each one (we could use a template here to remove 
> duplicating classes). This could still lead to a lot of duplication in terms 
> of e.g., functions to implement. I got the feeling that the change here was 
> less invasive. Let me know if you think the code is too strange and I can 
> reiterate.

Duplications are tolerable. Let's follow Qian's advice if possible.


- Gilbert


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


On April 16, 2018, 8:03 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66635/
> ---
> 
> (Updated April 16, 2018, 8:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Access to cgroups subsystem-specific isolators is modelled with actors
> which e.g., dispatch to themself on asynchronous code paths. We
> previously allowed callers to directly invoke functions on these
> actors which could e.g., introduce races on instance-internal data.
> 
> In this patch we refactor the `Subsystem` base class to provide a safe
> interface. The abstract base class and its concrete, derived classes
> are still libprocess processes, but we now use a template method
> pattern where the base class only provides a non-`virtual` `public`
> interface and all customization points are `protected`. With that we
> can inject the needed `dispatch` in the base class, rendering the
> class hierarchy safe to use in concurrent contexts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 9afa02b207e6272836e5a36d69fb48f1f4d02150 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp 
> ebddf4e9812c7e1eecec99215dfa5fd8e4e6e361 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> cba21ede59916b0a6e120ecd2b6348b8a946c507 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> 282a76189579d3ddd61f4aad210ce8e876ff0c25 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 75fab0f1fc7e7403855de0786b8c1155d7599b7f 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
> e4977da476a04a708a2e9b9dcc5a2f85da5867f4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
> 3be419ad477e9baa329b5388d4c12fa743c7f563 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> 4ab224e3b55a9d2b37d5e9abe94c069d89cd6d80 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> 4c43191251d7411d14ed8b11c79a64ff5d6ccd76 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> d79fda3cf852e50e75dbeeeaeae50b1f471a55a4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 50348d63f6fffa7662e6b91b5ce4ff730380e708 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
> bcaa16142c4e9882dea88e70095cfb7f223401ef 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
> f778a419e15940d92079d04884887615322791f5 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
> 14c0e79f045fb826d20e476772e017439977dded 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> d394d793c6bc38e249530c54ddb868ecff7f7865 
> 
> 
> Diff: https://reviews.apache.org/r/66635/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66230: Added test for adding/removing framework roles.

2018-05-04 Thread Kapil Arya


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

(Updated May 4, 2018, 1:33 p.m.)


Review request for mesos, Benjamin Mahler and Till Toenshoff.


Changes
---

Enhanced tests.


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


Repository: mesos


Description
---

Added test for adding/removing framework roles.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 


Diff: https://reviews.apache.org/r/66230/diff/8/

Changes: https://reviews.apache.org/r/66230/diff/7-8/


Testing
---


Thanks,

Kapil Arya

Review Request 66962: Windows: Added tests for async IO functions.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

New asynchronous functions were introduced to the Windows stout read
and write implementations, so eztra tests were added.


Diffs
-

  3rdparty/stout/tests/os/filesystem_tests.cpp 
b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 


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


Testing
---


Thanks,

Akash Gupta

Re: Review Request 66963: Updated the 1.6 CHANGELOG.

2018-05-04 Thread Vinod Kone


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


Ship it!




Ship It!

- Vinod Kone


On May 4, 2018, 5:18 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66963/
> ---
> 
> (Updated May 4, 2018, 5:18 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Chun-Hung Hsiao, Gaston 
> Kleiman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the 1.6 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG befaab123e0a8fc2a8bb659a0a0bb00ab30c74f5 
> 
> 
> Diff: https://reviews.apache.org/r/66963/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 66954: Windows: Added overlapped aware io.hpp file for `os::read/write`.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

Added a helper file for io, because `os::read/write` need to work on
both non-overlapped and overlapped files, so the logic has gotten
more complicated. The new file, `io.hpp`, has all the shared logic.


Diffs
-

  3rdparty/stout/include/Makefile.am f2e60224d9c729497bbcfffbec989c4f355d8f85 
  3rdparty/stout/include/stout/os/windows/io.hpp PRE-CREATION 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66956: Windows: Added overlapped support to `os::write`.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

Added overlapped handle support to `os::write`. Now, `os::write` will do
a blocking write no matter the handle type. Also, `os::write_async` will
do an overlapped write on an overlapped handle.


Diffs
-

  3rdparty/stout/include/stout/os/windows/write.hpp 
295c031c5824d13d74e2c9006e62c391d5020f69 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66955: Windows: Added overlapped support to `os::read`.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

Added overlapped handle support to `os::read`. Now, `os::read` will do
a blocking read no matter the handle type. Also, `os::read_async` will
do an overlapped read on an overlapped handle.


Diffs
-

  3rdparty/stout/include/stout/os/windows/read.hpp 
e957da81e55867b260d356f035d98918b85d1965 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66957: Windows: Enabled creating overlapped pipes with `os::pipe`.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

`os::pipe` was using `CreatePipe`, which does not support overlapped
io. Now, the implementation of `os::pipe` has been changed to use
`CreateNamedPipe`, which supports overlapped IO. The named pipe is
created with an unique random name, so it is effectively anonymous.


Diffs
-

  3rdparty/stout/include/stout/os/windows/pipe.hpp 
a3574fd6f2ff1608396b47cad8cbed88134a74ca 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66958: Windows: Fixed inheritance in subprocess_windows.cpp.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

subprocess_windows.cpp was creating inheritable pipes and files. On
Windows, our policy is to make everything not inheritable and then have
`CreateProcessW` set the the right pipes to be inheritable to avoid any
handle leaks. Now, subprocess_windows.cpp follows that inheritance
policy.


Diffs
-

  3rdparty/libprocess/src/subprocess_windows.cpp 
a1e6425faff01f816748f0b8b5307612b6dd8302 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

The Mesos containerizer needs an inheritable pipe that is not used for
stdio of the child process. It is used for signaling when the child
process should start running, so the pipe needs to be inheritable and
not overlapped.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66960: Windows: Added async version of `os::sendfile`.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

Broke the original `os::sendfile` implementation into an asynchronous
`os::sendfile_async`function and a synchronous `os::sendfile` function.
The synchronous version simply calls the asynchronous version and waits.


Diffs
-

  3rdparty/stout/include/stout/os/windows/sendfile.hpp 
594d9c7beeff2766f59f16ba25986314f2f7ceb8 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66963: Updated the 1.6 CHANGELOG.

2018-05-04 Thread Greg Mann


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

Review request for mesos, Alexander Rukletsov, Chun-Hung Hsiao, Gaston Kleiman, 
and Vinod Kone.


Repository: mesos


Description
---

Updated the 1.6 CHANGELOG.


Diffs
-

  CHANGELOG befaab123e0a8fc2a8bb659a0a0bb00ab30c74f5 


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


Testing
---


Thanks,

Greg Mann

Review Request 66961: Windows: Ported sendfile_tests.cpp.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

The sendfile tests were using `socketpair`, but the rest of the tests
were crossplatform. By providing a Windows `socketpair` implementations,
the tests are now ported.


Diffs
-

  3rdparty/stout/tests/CMakeLists.txt 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec 
  3rdparty/stout/tests/os/sendfile_tests.cpp 
05966ae067ae3972598da3370eb16fdce5736c21 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66953: Windows: Fixed dup to properly copy the overlapped WindowsFD.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

After the previous commit,`os::dup` was not copying the overlapped
field, so it has been updated to copy it.


Diffs
-

  3rdparty/stout/include/stout/os/windows/dup.hpp 
af98054f1bd9c8e55c52b246fda8734e3ca96e21 


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


Testing
---


Thanks,

Akash Gupta

Review Request 66952: Windows: Added overlapped field to WindowsFD.

2018-05-04 Thread Akash Gupta


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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
---

Added an field to WindowsFD for functions that need to know if a
Windows file is opened in overlapped mode, such as `os::read`, since
Windows doesn't provide a Win32 API for it.


Diffs
-

  3rdparty/stout/include/stout/os/windows/fd.hpp 
bab16e869e69c214e18de584d1615311316e001a 
  3rdparty/stout/include/stout/os/windows/open.hpp 
701dcec7c4c1162d38d8f25596af29ed3b32691d 


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


Testing
---


Thanks,

Akash Gupta

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

2018-05-04 Thread Chun-Hung Hsiao



> On May 4, 2018, 3:22 a.m., Chun-Hung Hsiao wrote:
> > src/tests/resource_provider_manager_tests.cpp
> > Lines 1132 (patched)
> > 
> >
> > This action will be invoked in the Driver's context. Could you explain 
> > why this could happen?
> 
> Benjamin Bannier wrote:
> The action will be invoked on the thread running the test body while the 
> `disconnected` callback is triggered from the driver. Since the `reset` tears 
> down the driver, the "spurious calls" here could in principle be multiple 
> invocations of `disconnected` before the driver is cleaned up -- looking at 
> the code this does not appear to be possible right now (thanks Jan for the 
> help!), but I think being a little more defensive here does not hurt.
> 
> What do you think?

IIUC the callback is not run on the caller's thread, not the test body thread. 
But we currently use `process::async` to call the callback, meaning that all 
calls to the callback will be run on different actors. So this defensive action 
LGTM.


- Chun-Hung


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


On May 4, 2018, 10:37 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 4, 2018, 10:37 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

2018-05-04 Thread Chun-Hung Hsiao


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


Fix it, then Ship it!





src/tests/resource_provider_manager_tests.cpp
Lines 1133 (patched)


Let's satisfy the future after the RP is destructed.


- Chun-Hung Hsiao


On May 4, 2018, 10:37 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 4, 2018, 10:37 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66924: Made the master include the operation ID in OPERATION_DROPPED updates.

2018-05-04 Thread Gaston Kleiman



> On May 4, 2018, 5:39 a.m., Benjamin Bannier wrote:
> > src/master/master.hpp
> > Line 442 (original), 442 (patched)
> > 
> >
> > The original pattern here seems to have been to always consitently 
> > break after the opening paren.
> > 
> > Let's not change that.

We seem to only break after the opening paren if the first parameter is a 
`const process::UPID&`, see these cases in which we don't insert a line break:

https://github.com/apache/mesos/blob/351bade6c28682daf821e88a40140e1364d69cb0/src/master/master.hpp#L523
https://github.com/apache/mesos/blob/351bade6c28682daf821e88a40140e1364d69cb0/src/master/master.hpp#L546
https://github.com/apache/mesos/blob/351bade6c28682daf821e88a40140e1364d69cb0/src/master/master.hpp#L569
https://github.com/apache/mesos/blob/351bade6c28682daf821e88a40140e1364d69cb0/src/master/master.hpp#L573


> On May 4, 2018, 5:39 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 3500 (patched)
> > 
> >
> > The behavior described in the comment above seems rather simple, but we 
> > use a pretty complicated setup below. This not only leads to us writing too 
> > much code unrelated to the thing under test, but also introduces unneeded 
> > dependencies on behavior we don't care about (here) -- e.g., I don't think 
> > this test depends on a SLRP. I would also much prefer a non-`ROOT` test so 
> > we run this as often as possible (I suspect developers do not regularly run 
> > `make check` as root on their development machines).
> > 
> > I would suggest to reimplement the test with a `MockResourceProvider` 
> > if we do really need a `CREATE_VOLUME` operation below -- but it seems we 
> > should be able to perform this test with even non-RP operations which would 
> > simplify things further.
> > 
> > See e.g., the test added in r/66908 which needs 60% less test code.

I had originally implemented  test like yours in r/66908, but then remembered 
that Mesos 1.6.0 will not support operation feedback on non-RP resources; I am 
going to add a validation rejecting operations that don't affect RP resources 
but have an operation ID, so we need a test that uses a resource provider.

I agree that there is too much boiler plate in the test, I also think we should 
make it easier to write tests using a SLRP.

We currently have no test that checks that the master correctly forwards the 
`OPERATION_DROPPED` updates generated by a SLRP when an operation ID is 
specified. This test checks that, but if I reimplement it to use a 
`MockResourceProvider`, then we should check in 
`StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation` whether the 
update is correctly forwarded to the scheduler.


- Gaston


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


On May 3, 2018, 5 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66924/
> ---
> 
> (Updated May 3, 2018, 5 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Greg Mann.
> 
> 
> Bugs: MESOS-8784
> https://issues.apache.org/jira/browse/MESOS-8784
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the master include the operation ID in OPERATION_DROPPED updates.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ccb114aacc904998dfeef4128f6d38dfb3c865c7 
> 
> 
> Diff: https://reviews.apache.org/r/66924/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux.
> 
> 
> The test in this patch fails without the corresponding master change, but 
> succeeds with it applied.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66923: Added documentation on volume resize support.

2018-05-04 Thread Zhitao Li


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

(Updated May 4, 2018, 9:49 a.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Changes
---

Review comments.


Repository: mesos


Description
---

Added documentation on volume resize support.


Diffs (updated)
-

  docs/authorization.md fdbef770c38ab38ab748b4f943b331d4442a2ce1 
  docs/operator-http-api.md 9be1e2db60562b369bdc6ee2c732a74fad049580 
  docs/persistent-volume.md 1a5799b8d8e5302e0b01da6b9a16a3b9fb75c898 


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

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


Testing
---

https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/persistent-volume.md
https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/operator-http-api.md
https://github.com/zhitaoli/mesos/blob/zhitao/public/volume_resize_mesos_4965_speculative/docs/authorization.md


Thanks,

Zhitao Li

Re: Review Request 66919: Failure to update registry should abort the master process.

2018-05-04 Thread Jiang Yan Xu


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



LGTM

It's hard to write a test for this but we can probably modify a test in 
master_maintenance_tests.cpp to observe the check failure. 

Let's also check Joseph with the comment here.
https://github.com/apache/mesos/blob/520b729857223aeade345cbdf61209ec4f395ad9/src/master/maintenance.hpp#L39


src/master/http.cpp
Lines 4163-4165 (patched)


The following will be more idiomatic.

```
  .onAny([](const Future& result) {
CHECK_READY(result);
  })
```

Note the spaces and const ref.

It'll be helpful to get familiarize with

https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md

which in turn references 

https://google.github.io/styleguide/cppguide.html


- Jiang Yan Xu


On May 3, 2018, 9:43 a.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66919/
> ---
> 
> (Updated May 3, 2018, 9:43 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8630
> https://issues.apache.org/jira/browse/MESOS-8630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the registrar fails to update the registry it would abort the
> actor and fail all future operations,However when the registrar
> updates is requested by an operator API such as maintenance update,
> the master process doesn't shut down (a 500 error is returned to the
> client instead)and all subsequent operations will fail.
> 
> Review: https://reviews.apache.org/r/66919
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
> 
> 
> Diff: https://reviews.apache.org/r/66919/diff/1/
> 
> 
> Testing
> ---
> 
> No error found by running all unit tests(make check)
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 66635: Fixed potential races when interacting with cgroup subsystem isolators.

2018-05-04 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66634, 66635]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 16, 2018, 3:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66635/
> ---
> 
> (Updated April 16, 2018, 3:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Access to cgroups subsystem-specific isolators is modelled with actors
> which e.g., dispatch to themself on asynchronous code paths. We
> previously allowed callers to directly invoke functions on these
> actors which could e.g., introduce races on instance-internal data.
> 
> In this patch we refactor the `Subsystem` base class to provide a safe
> interface. The abstract base class and its concrete, derived classes
> are still libprocess processes, but we now use a template method
> pattern where the base class only provides a non-`virtual` `public`
> interface and all customization points are `protected`. With that we
> can inject the needed `dispatch` in the base class, rendering the
> class hierarchy safe to use in concurrent contexts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 9afa02b207e6272836e5a36d69fb48f1f4d02150 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp 
> ebddf4e9812c7e1eecec99215dfa5fd8e4e6e361 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> cba21ede59916b0a6e120ecd2b6348b8a946c507 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> 282a76189579d3ddd61f4aad210ce8e876ff0c25 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 75fab0f1fc7e7403855de0786b8c1155d7599b7f 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
> e4977da476a04a708a2e9b9dcc5a2f85da5867f4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
> 3be419ad477e9baa329b5388d4c12fa743c7f563 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> 4ab224e3b55a9d2b37d5e9abe94c069d89cd6d80 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> 4c43191251d7411d14ed8b11c79a64ff5d6ccd76 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> d79fda3cf852e50e75dbeeeaeae50b1f471a55a4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 50348d63f6fffa7662e6b91b5ce4ff730380e708 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
> bcaa16142c4e9882dea88e70095cfb7f223401ef 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
> f778a419e15940d92079d04884887615322791f5 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
> 14c0e79f045fb826d20e476772e017439977dded 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> d394d793c6bc38e249530c54ddb868ecff7f7865 
> 
> 
> Diff: https://reviews.apache.org/r/66635/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66938: Made the 'SchedulerDriver' abort when operation's 'id' field is set.

2018-05-04 Thread Benjamin Bannier


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




src/sched/sched.cpp
Lines 1349 (patched)


Do we really need to `return` here? It seems just dropping this particular 
operation would be enough (in addition to calling `error` with all its 
side-effects). I am especially wondering about the tracking of operations.

(With a `CHECK` the expected behavior would be simpler, not saying we 
should prefer it).


- Benjamin Bannier


On May 4, 2018, 1:14 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66938/
> ---
> 
> (Updated May 4, 2018, 1:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jan Schlicht, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'SchedulerDriver' does not support operation status updates,
> this patch adds a check to the driver which will abort the scheduler
> if the 'id' field is set in an offer operation.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 620a3b26d8bf3487b6ce922b2280be7da291df06 
> 
> 
> Diff: https://reviews.apache.org/r/66938/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66924: Made the master include the operation ID in OPERATION_DROPPED updates.

2018-05-04 Thread Benjamin Bannier

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

src/master/master.hpp
Line 442 (original), 442 (patched)

The original pattern here seems to have been to always consitently break 
after the opening paren.

Let's not change that.

src/master/master.hpp
Line 506 (original), 505 (patched)

See above.

src/master/master.hpp
Line 513 (original), 511 (patched)

Break after `(`.

src/tests/storage_local_resource_provider_tests.cpp
Lines 3500 (patched)

The behavior described in the comment above seems rather simple, but we use 
a pretty complicated setup below. This not only leads to us writing too much 
code unrelated to the thing under test, but also introduces unneeded 
dependencies on behavior we don't care about (here) -- e.g., I don't think this 
test depends on a SLRP. I would also much prefer a non-`ROOT` test so we run 
this as often as possible (I suspect developers do not regularly run `make 
check` as root on their development machines).

I would suggest to reimplement the test with a `MockResourceProvider` if we 
do really need a `CREATE_VOLUME` operation below -- but it seems we should be 
able to perform this test with even non-RP operations which would simplify 
things further.

See e.g., the test added in r/66908 which needs 60% less test code.

- Benjamin Bannier

On May 4, 2018, 2 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66924/
> ---
> 
> (Updated May 4, 2018, 2 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Greg Mann.
> 
> 
> Bugs: MESOS-8784
> https://issues.apache.org/jira/browse/MESOS-8784
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the master include the operation ID in OPERATION_DROPPED updates.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ccb114aacc904998dfeef4128f6d38dfb3c865c7 
> 
> 
> Diff: https://reviews.apache.org/r/66924/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux.
> 
> 
> The test in this patch fails without the corresponding master change, but 
> succeeds with it applied.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66749: Added more logging to agent recovery path.

2018-05-04 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66749]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 4, 2018, 3:52 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66749/
> ---
> 
> (Updated May 4, 2018, 3:52 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-8793
> https://issues.apache.org/jira/browse/MESOS-8793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logging in some agent recovery continuations to
> make analyzing agent recovery related issue less painful.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> af34a856e092a880a0809da34ead9d8588b0ac8f 
>   src/slave/slave.cpp 6ca3d79fd38c800f258c571bb58164427db2ac7c 
> 
> 
> Diff: https://reviews.apache.org/r/66749/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66931 was successfully built and tested.

Reviews applied: `['66931']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 10:37 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 4, 2018, 10:37 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66947: Increased the timeout for waiting for `reaped` to be invoked.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66947 was successfully built and tested.

Reviews applied: `['66947']`

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

- Mesos Reviewbot Windows


On May 4, 2018, 9:32 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66947/
> ---
> 
> (Updated May 4, 2018, 9:32 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-8876
> https://issues.apache.org/jira/browse/MESOS-8876
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously after the container process is reaped by the Docker
> executor, we will wait 3 seconds for `reaped` to be invoked.
> However in some cases (e.g., launch a Docker container to use
> an external rexray volume), there will be more than 3 seconds
> after the container process exits and before the `docker run`
> command returns (i.e., `reaped` invoked). So in this patch,
> the timeout is increased to 60 seconds.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 16509dae3acebc59635e925073372adfb3786edc 
> 
> 
> Diff: https://reviews.apache.org/r/66947/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66900: Avoid copying of re-register framework messages in the master.

2018-05-04 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66860, 66900]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 4, 2018, 2:58 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66900/
> ---
> 
> (Updated May 4, 2018, 2:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8628
> https://issues.apache.org/jira/browse/MESOS-8628
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this patch, the re-register framework message was copied
> into a subscribe call. This updates the handler to perform moves
> instead.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
> 
> 
> Diff: https://reviews.apache.org/r/66900/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

2018-05-04 Thread Benjamin Bannier


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

(Updated May 4, 2018, 12:37 p.m.)


Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

We previously did not make ensure that after the simulated agent
failover in
`ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
mock resource provider created as part of the test did not reconnect
to the restarted agent (as opposed to the newly initialized resource
provider). This lead to unmet test expectations.

With this patch we now explicitly tear down the mock resource provider
after we have detected that the agent went away to prevent the race.


Diffs (updated)
-

  src/tests/resource_provider_manager_tests.cpp 
e8ca377fd0a927b99fdaf6a8ee0139025a41298e 


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

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


Testing
---

`make check`

Ran the test repeatedly under high system load without triggering the issue 
again with this patch.


Thanks,

Benjamin Bannier

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

2018-05-04 Thread Benjamin Bannier



> On May 4, 2018, 5:22 a.m., Chun-Hung Hsiao wrote:
> > src/tests/resource_provider_manager_tests.cpp
> > Lines 1132 (patched)
> > 
> >
> > This action will be invoked in the Driver's context. Could you explain 
> > why this could happen?

The action will be invoked on the thread running the test body while the 
`disconnected` callback is triggered from the driver. Since the `reset` tears 
down the driver, the "spurious calls" here could in principle be multiple 
invocations of `disconnected` before the driver is cleaned up -- looking at the 
code this does not appear to be possible right now (thanks Jan for the help!), 
but I think being a little more defensive here does not hurt.

What do you think?


> On May 4, 2018, 5:22 a.m., Chun-Hung Hsiao wrote:
> > src/tests/resource_provider_manager_tests.cpp
> > Line 1140 (original), 1146 (patched)
> > 
> >
> > Instead of creating a new mock resource provider, could we restart the 
> > agent with the same pid, and let the same mock resource provider to 
> > subscribe to the new agent?
> > 
> > Feel free to drop this if you have concerns with this approach.

There are two reasons for this. First, the resource provider's driver will 
automatically try to resubscribe and explicitly restarting it here will allow 
us to more clearly manage test expectations. Second, in non-test code the 
agent's lifetime always exceeds the RP's and we are approximating that setup 
here.

Please let me know if you think this part should be rewritten, dropping for now.


- Benjamin


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


On May 4, 2018, 12:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 4, 2018, 12:37 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66931: Fixed a race in resource provider resubscription test.

2018-05-04 Thread Benjamin Bannier



> On May 3, 2018, 3:52 p.m., Jan Schlicht wrote:
> > src/tests/resource_provider_manager_tests.cpp
> > Lines 1130-1131 (patched)
> > 
> >
> > Nit: `disconnected` is called asynchronously. The `Clock::settle()` 
> > below will make sure that it is called. Not having the settle would 
> > introduce another flakyness though. To be extra sure, we could set a future 
> > and await that one before we reset `resourceProvider` with a new instance.
> 
> Chun-Hung Hsiao wrote:
> +1 for awaiting a future instead of using `Clock::settle()`. This would 
> make the intention more clear.

Added an explicition expectation on disconnected before restarting the agent.

@chun The `Clock::settle` is still needed for the agent registration.


- Benjamin


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


On May 4, 2018, 12:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66931/
> ---
> 
> (Updated May 4, 2018, 12:37 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8874
> https://issues.apache.org/jira/browse/MESOS-8874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously did not make ensure that after the simulated agent
> failover in
> `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the
> mock resource provider created as part of the test did not reconnect
> to the restarted agent (as opposed to the newly initialized resource
> provider). This lead to unmet test expectations.
> 
> With this patch we now explicitly tear down the mock resource provider
> after we have detected that the agent went away to prevent the race.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> e8ca377fd0a927b99fdaf6a8ee0139025a41298e 
> 
> 
> Diff: https://reviews.apache.org/r/66931/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran the test repeatedly under high system load without triggering the issue 
> again with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66908: Correctly reconciled dropped operation after agent failover.

2018-05-04 Thread Benjamin Bannier


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

(Updated May 4, 2018, 12:37 p.m.)


Review request for mesos, Gaston Kleiman and Greg Mann.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

When the master receives an `UpdateSlaveMessage` after agent failover
it previously did not correctly detect dropped operations (operations
known to the master, but unknown to the agent) and did not trigger
reconciliation for such operations.

This patch fixes the handler in the master so that such dropped
operations are reconciled.


Diffs (updated)
-

  src/master/master.cpp 7a2f69c1fe2508e16c2685cd3490d5b1f59d6ac4 
  src/tests/master_slave_reconciliation_tests.cpp 
6bb4263323bcfd191c8e3c1ccba10a240e9ddd83 


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

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


Testing
---

`make check`

The test in this patch fails without the corresponding master change, but 
succeeds with it applied.


Thanks,

Benjamin Bannier

Re: Review Request 66908: Correctly reconciled dropped operation after agent failover.

2018-05-04 Thread Benjamin Bannier



> On May 3, 2018, 7 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 7569 (patched)
> > 
> >
> > I think the following is appropriate?
> > 
> > s/after a master failover/after a master or agent failover/

I think not? This paragraph describes two different scenarios (master failover 
or agent failover), and the two parts are explained separately.

How about the following -- note the different punctation,

// Below we loop over all received operations and check whether
// they are known to the master; operations can be unknown to the
// master after a master failover. To handle dropped operations on
// agent failover we explicitly track the received operations and
// compare them against the operations known to the master.


- Benjamin


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


On May 4, 2018, 12:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66908/
> ---
> 
> (Updated May 4, 2018, 12:37 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-8870
> https://issues.apache.org/jira/browse/MESOS-8870
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the master receives an `UpdateSlaveMessage` after agent failover
> it previously did not correctly detect dropped operations (operations
> known to the master, but unknown to the agent) and did not trigger
> reconciliation for such operations.
> 
> This patch fixes the handler in the master so that such dropped
> operations are reconciled.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 7a2f69c1fe2508e16c2685cd3490d5b1f59d6ac4 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 6bb4263323bcfd191c8e3c1ccba10a240e9ddd83 
> 
> 
> Diff: https://reviews.apache.org/r/66908/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> The test in this patch fails without the corresponding master change, but 
> succeeds with it applied.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 66947: Increased the timeout for waiting for `reaped` to be invoked.

2018-05-04 Thread Qian Zhang


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

Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description
---

Previously after the container process is reaped by the Docker
executor, we will wait 3 seconds for `reaped` to be invoked.
However in some cases (e.g., launch a Docker container to use
an external rexray volume), there will be more than 3 seconds
after the container process exits and before the `docker run`
command returns (i.e., `reaped` invoked). So in this patch,
the timeout is increased to 60 seconds.


Diffs
-

  src/docker/executor.cpp 16509dae3acebc59635e925073372adfb3786edc 


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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 66635: Fixed potential races when interacting with cgroup subsystem isolators.

2018-05-04 Thread Benjamin Bannier



> On May 4, 2018, 10:53 a.m., Qian Zhang wrote:
> > Thanks for the patch!
> > 
> > The `doXxx()` functions seem a bit strange to me. I am wondering if we can 
> > follow the similar way of how Mesos containerizer calls isolators, e.g., we 
> > could rename `Subsystem` to `SubsystemProcess` and all the `XxxSubsystem` 
> > to `XxxSubsystemProcess` since they are all actually `Process`, and then 
> > introduce a new class `Subsystem` which internally dispatches calls to 
> > `SubsystemProcess`, and `CgroupsIsolatorProcess` always directly calls the 
> > functions of `Subsystem`.

Like outlined in the ticket, that would be the other possibility. Note that we 
not only require dedicated subsystem processes, but also wrappers derived from 
`Subsystem` for each one (we could use a template here to remove duplicating 
classes). This could still lead to a lot of duplication in terms of e.g., 
functions to implement. I got the feeling that the change here was less 
invasive. Let me know if you think the code is too strange and I can reiterate.


- Benjamin


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


On April 16, 2018, 5:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66635/
> ---
> 
> (Updated April 16, 2018, 5:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Access to cgroups subsystem-specific isolators is modelled with actors
> which e.g., dispatch to themself on asynchronous code paths. We
> previously allowed callers to directly invoke functions on these
> actors which could e.g., introduce races on instance-internal data.
> 
> In this patch we refactor the `Subsystem` base class to provide a safe
> interface. The abstract base class and its concrete, derived classes
> are still libprocess processes, but we now use a template method
> pattern where the base class only provides a non-`virtual` `public`
> interface and all customization points are `protected`. With that we
> can inject the needed `dispatch` in the base class, rendering the
> class hierarchy safe to use in concurrent contexts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 9afa02b207e6272836e5a36d69fb48f1f4d02150 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp 
> ebddf4e9812c7e1eecec99215dfa5fd8e4e6e361 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> cba21ede59916b0a6e120ecd2b6348b8a946c507 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> 282a76189579d3ddd61f4aad210ce8e876ff0c25 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 75fab0f1fc7e7403855de0786b8c1155d7599b7f 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
> e4977da476a04a708a2e9b9dcc5a2f85da5867f4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
> 3be419ad477e9baa329b5388d4c12fa743c7f563 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> 4ab224e3b55a9d2b37d5e9abe94c069d89cd6d80 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> 4c43191251d7411d14ed8b11c79a64ff5d6ccd76 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> d79fda3cf852e50e75dbeeeaeae50b1f471a55a4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 50348d63f6fffa7662e6b91b5ce4ff730380e708 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
> bcaa16142c4e9882dea88e70095cfb7f223401ef 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
> f778a419e15940d92079d04884887615322791f5 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
> 14c0e79f045fb826d20e476772e017439977dded 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> d394d793c6bc38e249530c54ddb868ecff7f7865 
> 
> 
> Diff: https://reviews.apache.org/r/66635/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66635: Fixed potential races when interacting with cgroup subsystem isolators.

2018-05-04 Thread Qian Zhang


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



Thanks for the patch!

The `doXxx()` functions seem a bit strange to me. I am wondering if we can 
follow the similar way of how Mesos containerizer calls isolators, e.g., we 
could rename `Subsystem` to `SubsystemProcess` and all the `XxxSubsystem` to 
`XxxSubsystemProcess` since they are all actually `Process`, and then introduce 
a new class `Subsystem` which internally dispatches calls to 
`SubsystemProcess`, and `CgroupsIsolatorProcess` always directly calls the 
functions of `Subsystem`.

- Qian Zhang


On April 16, 2018, 11:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66635/
> ---
> 
> (Updated April 16, 2018, 11:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Access to cgroups subsystem-specific isolators is modelled with actors
> which e.g., dispatch to themself on asynchronous code paths. We
> previously allowed callers to directly invoke functions on these
> actors which could e.g., introduce races on instance-internal data.
> 
> In this patch we refactor the `Subsystem` base class to provide a safe
> interface. The abstract base class and its concrete, derived classes
> are still libprocess processes, but we now use a template method
> pattern where the base class only provides a non-`virtual` `public`
> interface and all customization points are `protected`. With that we
> can inject the needed `dispatch` in the base class, rendering the
> class hierarchy safe to use in concurrent contexts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 9afa02b207e6272836e5a36d69fb48f1f4d02150 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp 
> ebddf4e9812c7e1eecec99215dfa5fd8e4e6e361 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> cba21ede59916b0a6e120ecd2b6348b8a946c507 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> 282a76189579d3ddd61f4aad210ce8e876ff0c25 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 75fab0f1fc7e7403855de0786b8c1155d7599b7f 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
> e4977da476a04a708a2e9b9dcc5a2f85da5867f4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
> 3be419ad477e9baa329b5388d4c12fa743c7f563 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> 4ab224e3b55a9d2b37d5e9abe94c069d89cd6d80 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> 4c43191251d7411d14ed8b11c79a64ff5d6ccd76 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> d79fda3cf852e50e75dbeeeaeae50b1f471a55a4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 50348d63f6fffa7662e6b91b5ce4ff730380e708 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
> bcaa16142c4e9882dea88e70095cfb7f223401ef 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
> f778a419e15940d92079d04884887615322791f5 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
> 14c0e79f045fb826d20e476772e017439977dded 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> d394d793c6bc38e249530c54ddb868ecff7f7865 
> 
> 
> Diff: https://reviews.apache.org/r/66635/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66634: Explicitly marked functions `override` in cgroup subsystems.

2018-05-04 Thread Gilbert Song


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


Ship it!




Ship It!

- Gilbert Song


On April 16, 2018, 8:03 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66634/
> ---
> 
> (Updated April 16, 2018, 8:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds `override` keywords to functions overriding `virtual`
> base class functions. This explicitness helps in a subsequent patch in
> refactoring the `Subsystem` base class interface.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp 
> ebddf4e9812c7e1eecec99215dfa5fd8e4e6e361 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> 282a76189579d3ddd61f4aad210ce8e876ff0c25 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
> e4977da476a04a708a2e9b9dcc5a2f85da5867f4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuset.hpp 
> b5d712a8eb8ba74092184024e3b40ad9ba1b7584 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> 4ab224e3b55a9d2b37d5e9abe94c069d89cd6d80 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/hugetlb.hpp 
> 27407133e7315dccf1efe2440cc1bf79c51e7dca 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> d79fda3cf852e50e75dbeeeaeae50b1f471a55a4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
> bcaa16142c4e9882dea88e70095cfb7f223401ef 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_prio.hpp 
> 002c689503d45622cba437c851561f5ec7dc8a12 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
> 14c0e79f045fb826d20e476772e017439977dded 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/pids.hpp 
> cb6c91920355d3d5599f8a3cf0ce2ac5eee18d37 
> 
> 
> Diff: https://reviews.apache.org/r/66634/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66635: Fixed potential races when interacting with cgroup subsystem isolators.

2018-05-04 Thread Gilbert Song


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


Ship it!




Ship It!

- Gilbert Song


On April 16, 2018, 8:03 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66635/
> ---
> 
> (Updated April 16, 2018, 8:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Access to cgroups subsystem-specific isolators is modelled with actors
> which e.g., dispatch to themself on asynchronous code paths. We
> previously allowed callers to directly invoke functions on these
> actors which could e.g., introduce races on instance-internal data.
> 
> In this patch we refactor the `Subsystem` base class to provide a safe
> interface. The abstract base class and its concrete, derived classes
> are still libprocess processes, but we now use a template method
> pattern where the base class only provides a non-`virtual` `public`
> interface and all customization points are `protected`. With that we
> can inject the needed `dispatch` in the base class, rendering the
> class hierarchy safe to use in concurrent contexts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 9afa02b207e6272836e5a36d69fb48f1f4d02150 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp 
> ebddf4e9812c7e1eecec99215dfa5fd8e4e6e361 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> cba21ede59916b0a6e120ecd2b6348b8a946c507 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> 282a76189579d3ddd61f4aad210ce8e876ff0c25 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 75fab0f1fc7e7403855de0786b8c1155d7599b7f 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
> e4977da476a04a708a2e9b9dcc5a2f85da5867f4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
> 3be419ad477e9baa329b5388d4c12fa743c7f563 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> 4ab224e3b55a9d2b37d5e9abe94c069d89cd6d80 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> 4c43191251d7411d14ed8b11c79a64ff5d6ccd76 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> d79fda3cf852e50e75dbeeeaeae50b1f471a55a4 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 50348d63f6fffa7662e6b91b5ce4ff730380e708 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
> bcaa16142c4e9882dea88e70095cfb7f223401ef 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
> f778a419e15940d92079d04884887615322791f5 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
> 14c0e79f045fb826d20e476772e017439977dded 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> d394d793c6bc38e249530c54ddb868ecff7f7865 
> 
> 
> Diff: https://reviews.apache.org/r/66635/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66924: Made the master include the operation ID in OPERATION_DROPPED updates.

2018-05-04 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66908, 66924]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 3, 2018, 5 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66924/
> ---
> 
> (Updated May 3, 2018, 5 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Greg Mann.
> 
> 
> Bugs: MESOS-8784
> https://issues.apache.org/jira/browse/MESOS-8784
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the master include the operation ID in OPERATION_DROPPED updates.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ccb114aacc904998dfeef4128f6d38dfb3c865c7 
> 
> 
> Diff: https://reviews.apache.org/r/66924/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux.
> 
> 
> The test in this patch fails without the corresponding master change, but 
> succeeds with it applied.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66749: Added more logging to agent recovery path.

2018-05-04 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66749 was successfully built and tested.

Reviews applied: `['66749']`

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

- Mesos Reviewbot Windows


On May 3, 2018, 8:52 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66749/
> ---
> 
> (Updated May 3, 2018, 8:52 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-8793
> https://issues.apache.org/jira/browse/MESOS-8793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logging in some agent recovery continuations to
> make analyzing agent recovery related issue less painful.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
>   src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> af34a856e092a880a0809da34ead9d8588b0ac8f 
>   src/slave/slave.cpp 6ca3d79fd38c800f258c571bb58164427db2ac7c 
> 
> 
> Diff: https://reviews.apache.org/r/66749/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

94 matches

Mail list logo