Re: Review Request 66696: Updated documentation for operation feedback.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66458', '66459', '66461', '66489', '66462', '66463', 
'66464', '66465', '66466', '66467', '66460', '66468', '66696']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] ContentType/SchedulerTest.Suppress/0 (255 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (280 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (272 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (277 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 (317 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1 (326 
ms)
[ RUN  ] ContentType/SchedulerTest.Message/0
[   OK ] ContentType/SchedulerTest.Message/0 (327 ms)
[ RUN  ] ContentType/SchedulerTest.Message/1
[   OK ] ContentType/SchedulerTest.Message/1 (348 ms)
[ RUN  ] ContentType/SchedulerTest.Request/0
[   OK ] ContentType/SchedulerTest.Request/0 (107 ms)
[ RUN  ] ContentType/SchedulerTest.Request/1
[   OK ] ContentType/SchedulerTest.Request/1 (109 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (85 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (99 ms)
[--] 38 tests from ContentType/SchedulerTest (15102 ms total)

[--] 4 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 
(3024 ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
```

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

```
I0421 05:06:00.164638  8196 slave.cpp:8877] Launching executor 'default' of 
framework cc818463-3626-476c-b312-80b34c67e891- with resources [] in work 
directory 
'C:\Users\mesos\AppData\Local\Temp\2fdNvb\slaves\cc818463-3626-476c-b312-80b34c67e891-S0\frameworks\cc818463-3626-476c-b312-80b34c67e891-\executors\default\runs\e0af3181-83e7-473c-8d02-aade3308a4ef'
I0421 05:06:00.168326  8196 slave.cpp:3028] Queued task 
'89f8c42f-bcba-40aa-8751-291a8053f1b4' for executor 'default' of framework 
cc818463-3626-476c-b312-80b34c67e891-
I0421 05:06:00.169327  8196 slave.cpp:3507] Launching container 
e0af3181-83e7-473c-8d02-aade3308a4ef for executor 'default' of framework 
cc818463-3626-476c-b312-80b34c67e891-
I0421 05:06:00.191339 31300 executor.cpp:192] Version: 1.6.0
I0421 05:06:00.241346 17992 http.cpp:1099] HTTP POST for 
/slave(423)/api/v1/executor from 10.3.1.8:62545
I0421 05:06:00.241346 17992 slave.cpp:4581] Received Subscribe request for HTTP 
executor 'default' of framework cc818463-3626-476c-b312-80b34c67e891-
I0421 05:06:00.247342 31300 slave.cpp:3239] Sending queued task 
'89f8c42f-bcba-40aa-8751-291a8053f1b4' to executor 'default' of framework 
cc818463-3626-476c-b312-80b34c67e891- (via HTTP)
I0421 05:06:00.265344 32064 http.cpp:1099] HTTP POST for 
/slave(423)/api/v1/executor from 10.3.1.8:62546
I0421 05:06:00.266338 32064 slave.cpp:5243] Handling status update TASK_RUNNING 
(Status UUID: d43d485d-3571-4991-92a5-26dd10a50893) for task 
89f8c42f-bcba-40aa-8751-291a8053f1b4 of framework 
cc818463-3626-476c-b312-80b34c67e891-
I0421 05:06:00.268343 31548 task_status_update_manager.cpp:328] Received task 
status update TASK_RUNNING (Status UUID: d43d485d-3571-4991-92a5-26dd10a50893) 
for task 89f8c42f-bcba-40aa-8751-291a8053f1b4 of framework 
cc818463-3626-476c-b312-80b34c67e891-
I0421 05:06:00.270331 32468 slave.cpp:5735] Forwarding the update TASK_RUNNING 
(Status UUID: d43d485d-3571-4991-92a5-26dd10a50893) for task 
89f8c42f-bcba-40aa-8751-291a8053f1b4 of framework 
cc818463-3626-476c-b312-80b34c67e891- to master@10.3.1.8:62542
I0421 05:06:00.271348 26460 master.cpp:8082] Status update TASK_RUNNING (Status 
UUID: 

Re: Review Request 66733: Added a new `RESIZE_VOLUME` agent capability.

[Chun-Hung Hsiao]

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


Fix it, then Ship it!





src/tests/master_tests.cpp
Lines 5047-5051 (original), 5047-5052 (patched)


Let's update the comment and use string literals 
(http://en.cppreference.com/w/cpp/language/string_literal):
```
// Agents should have the following capabilities in the current
// implementation.
Try expectedCapabilities = JSON::parse(
R"~(
[
  "MULTI_ROLE",
  "HIERARCHICAL_ROLE",
  "RESERVATION_REFINEMENT",
  "RESOURCE_PROVIDER",
  "RESIZE_VOLUME"
]
)~");
```



src/tests/slave_tests.cpp
Lines 1614-1619 (original), 1614-1619 (patched)


Ditto.


- Chun-Hung Hsiao


On April 20, 2018, 6:38 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66733/
> ---
> 
> (Updated April 20, 2018, 6:38 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used as a feature flag to gate the new volume resize
> feature. This feature will be turn on by default once released.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9e24d3ea46edc21572e9226e2e76c7d55618db24 
>   include/mesos/v1/mesos.proto 0f3fd8a2608b5edabc21f5fe5df9b70fc0fa8dc2 
>   src/common/protobuf_utils.hpp ae060f3a7946ac5862faeca15330e9642f934137 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/slave/constants.cpp 51de71bc45ff4fb862690efd9428b25d92055391 
>   src/tests/master_tests.cpp be7a3cc6126d2fe7c56c49b3da5f6f4bf29657f5 
>   src/tests/slave_tests.cpp 04f7acad47aa980d507bbe29b29db5eb050c02f3 
> 
> 
> Diff: https://reviews.apache.org/r/66733/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66753: Windows: Fixed build with OpenSSL due to missing header.

[Andrew Schwartzmeyer]

> On April 20, 2018, 6:42 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> > 
> > Reviews applied: `['66753']`
> > 
> > Failed command: `Start-MesosCITesting`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66753
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66753/logs/mesos-tests-stdout.log):
> > 
> > ```
> > [   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (128 ms)
> > [--] 9 tests from Endpoint/SlaveEndpointTest (1114 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 
> > (41 ms)
> > [--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (80 
> > ms total)
> > 
> > [--] 1 test from IsolationFlag/CpuIsolatorTest
> > [ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
> > [   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (742 ms)
> > [--] 1 test from IsolationFlag/CpuIsolatorTest (768 ms total)
> > 
> > [--] 1 test from IsolationFlag/MemoryIsolatorTest
> > [ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
> > [   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (726 ms)
> > [--] 1 test from IsolationFlag/MemoryIsolatorTest (750 ms total)
> > 
> > [--] Global test environment tear-down
> > [==] 956 tests from 94 test cases ran. (432286 ms total)
> > [  PASSED  ] 955 tests.
> > [  FAILED  ] 1 test, listed below:
> > [  FAILED  ] MesosContainerizer/DefaultExecutorTest.SigkillExecutor/0, 
> > where GetParam() = "mesos"
> > 
> >  1 FAILED TEST
> >   YOU HAVE 214 DISABLED TESTS
> > 
> > ```
> > 
> > - 
> > [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66753/logs/mesos-tests-stderr.log):
> > 
> > ```
> > I0421 01:42:33.695284 26280 executor.cpp:177] Received SUBSCRIBED event
> > I0421 01:42:33.700284 26280 executor.cpp:181] Subscribed executor on 
> > winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
> > I0421 01:42:33.701284 26280 executor.cpp:177] Received LAUNCH event
> > I0421 01:42:33.705279 26280 executor.cpp:649] Starting task 
> > 43ad30bf-4c67-4f55-ad50-d35b462ae163
> > I0421 01:42:33.792289 26280 executor.cpp:484] Running 
> > 'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
> > I0421 01:42:33.806294 26280 executor.cpp:662] Forked command at 22416
> > I0421 01:42:33.840289 12860 exec.cpp:445] Executor asked to shutdown
> > I0421 01:42:33.841274 32472 executor.cpp:177] Received SHUTDOWN event
> > I0421 01:42:33.841274 32472 executor.cpp:759] Shutting down
> > I0421 01:42:33.841274 32472 executor.cpp:869] Sending SIGTERM to process 
> > tree at pid ] Deactivating framework 
> > b973dca4-a467-4996-964e-a07bce194ca9- (default) at 
> > scheduler-154e8bae-7b67-469a-886e-86711daae66f@10.3.1.8:57994
> > I0421 01:42:33.838289 30064 hierarchical.cpp:405] Deactivated framework 
> > b973dca4-a467-4996-964e-a07bce194ca9-
> > I0421 01:42:33.838289 31004 master.cpp:10453] Updating the state of task 
> > 43ad30bf-4c67-4f55-ad50-d35b462ae163 of framework 
> > b973dca4-a467-4996-964e-a07bce194ca9- (latest state: TASK_KILLED, 
> > status update state: TASK_KILLED)
> > I0421 01:42:33.838289 31852 slave.cpp:3923] Shutting down framework 
> > b973dca4-a467-4996-964e-a07bce194ca9-
> > I0421 01:42:33.838289 31852 slave.cpp:6620] Shutting down executor 
> > '43ad30bf-4c67-4f55-ad50-d35b462ae163' of framework 
> > b973dca4-a467-4996-964e-a07bce194ca9- at executor(1)@10.3.1.8:58015
> > I0421 01:42:33.839288 31852 slave.cpp:923] Agent terminating
> > W0421 01:42:33.839288 31852 slave.cpp:3919] Ignoring shutdown framework 
> > b973dca4-a467-4996-964e-a07bce194ca9- because it is terminating
> > I0421 01:42:33.841274 31004 master.cpp:10552] Removing task 
> > 43ad30bf-4c67-4f55-ad50-d35b462ae163 with resources cpus(allocated: *):4; 
> > mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
> > *):[31000-32000] of framework b973dca4-a467-4996-964e-a07bce194ca9- on 
> > agent b973dca4-a467-4996-964e-a07bce194ca9-S0 at slave(427)@10.3.1.8:57994 
> > (winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
> > I0421 01:42:33.844292 30628 containerizer.cpp:2375] Destroying container 
> > 0d2190d1-7e8c-4d17-a3dd-7823cddff36e in RUNNING state
> > I0421 01:42:33.844292 30628 containerizer.cpp:2989] Transitioning the state 
> > of container 0d2190d1-7e8c-4d17-a3dd-7823cddff36e from RUNNING to DESTROYING
> > I0421 01:42:33.845290 30628 

Re: Review Request 66753: Windows: Fixed build with OpenSSL due to missing header.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66753']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (128 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1114 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 (41 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (80 ms 
total)

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

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

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

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

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

```
I0421 01:42:33.695284 26280 executor.cpp:177] Received SUBSCRIBED event
I0421 01:42:33.700284 26280 executor.cpp:181] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0421 01:42:33.701284 26280 executor.cpp:177] Received LAUNCH event
I0421 01:42:33.705279 26280 executor.cpp:649] Starting task 
43ad30bf-4c67-4f55-ad50-d35b462ae163
I0421 01:42:33.792289 26280 executor.cpp:484] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0421 01:42:33.806294 26280 executor.cpp:662] Forked command at 22416
I0421 01:42:33.840289 12860 exec.cpp:445] Executor asked to shutdown
I0421 01:42:33.841274 32472 executor.cpp:177] Received SHUTDOWN event
I0421 01:42:33.841274 32472 executor.cpp:759] Shutting down
I0421 01:42:33.841274 32472 executor.cpp:869] Sending SIGTERM to process tree 
at pid ] Deactivating framework b973dca4-a467-4996-964e-a07bce194ca9- 
(default) at scheduler-154e8bae-7b67-469a-886e-86711daae66f@10.3.1.8:57994
I0421 01:42:33.838289 30064 hierarchical.cpp:405] Deactivated framework 
b973dca4-a467-4996-964e-a07bce194ca9-
I0421 01:42:33.838289 31004 master.cpp:10453] Updating the state of task 
43ad30bf-4c67-4f55-ad50-d35b462ae163 of framework 
b973dca4-a467-4996-964e-a07bce194ca9- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0421 01:42:33.838289 31852 slave.cpp:3923] Shutting down framework 
b973dca4-a467-4996-964e-a07bce194ca9-
I0421 01:42:33.838289 31852 slave.cpp:6620] Shutting down executor 
'43ad30bf-4c67-4f55-ad50-d35b462ae163' of framework 
b973dca4-a467-4996-964e-a07bce194ca9- at executor(1)@10.3.1.8:58015
I0421 01:42:33.839288 31852 slave.cpp:923] Agent terminating
W0421 01:42:33.839288 31852 slave.cpp:3919] Ignoring shutdown framework 
b973dca4-a467-4996-964e-a07bce194ca9- because it is terminating
I0421 01:42:33.841274 31004 master.cpp:10552] Removing task 
43ad30bf-4c67-4f55-ad50-d35b462ae163 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework b973dca4-a467-4996-964e-a07bce194ca9- on 
agent b973dca4-a467-4996-964e-a07bce194ca9-S0 at slave(427)@10.3.1.8:57994 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0421 01:42:33.844292 30628 containerizer.cpp:2375] Destroying container 
0d2190d1-7e8c-4d17-a3dd-7823cddff36e in RUNNING state
I0421 01:42:33.844292 30628 containerizer.cpp:2989] Transitioning the state of 
container 0d2190d1-7e8c-4d17-a3dd-7823cddff36e from RUNNING to DESTROYING
I0421 01:42:33.845290 30628 launcher.cpp:156] Asked to destroy container 
0d2190d1-7e8c-4d17-a3dd-7823cddff36e
I0421 01:42:33.845290 31004 master.cpp:1296] Agent 
b973dca4-a467-4996-964e-a07bce194ca9-S0 at slave(427)@10.3.1.8:57994 

Re: Review Request 66753: Windows: Fixed build with OpenSSL due to missing header.

[Andrew Schwartzmeyer]

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

(Updated April 20, 2018, 6:32 p.m.)


Review request for mesos, Akash Gupta, Chun-Hung Hsiao, and Joseph Wu.


Repository: mesos


Description (updated)
---

When `-DENABLE_SSL=TRUE`, then `USE_SSL_SOCKET` is defined, and in
this `src/local/local.cpp` in a section guarded by said definition,
the construct `os::Permissions` is used. However, this is defined in
`stout/os/permissions.hpp`, which is implicitly included on POSIX, but
not on Windows. The fix is to IWYU and explicitly include it.


Diffs (updated)
-

  src/local/local.cpp 9c5b45b7c4e7705861f1e7926847711dc16f1264 


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

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


Testing
---

Built on Windows with `-DENABLE_SSL`. Previously it failed due to 
`os::Permissions` being undefined.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66753: Windows: Fixed build with OpenSSL due to missing header.

[Chun-Hung Hsiao]

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


Fix it, then Ship it!




Could you update the commit message to make "this file" more specific?


src/local/local.cpp
Lines 60 (patched)


Grouping a public header and a private header with this condition looks a 
litter weird ;) How about having two pairs of `#ifdef`?


- Chun-Hung Hsiao


On April 20, 2018, 11:52 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66753/
> ---
> 
> (Updated April 20, 2018, 11:52 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Chun-Hung Hsiao, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When `-DENABLE_SSL=TRUE`, then `USE_SSL_SOCKET` is defined, and in
> this file in a section guarded by the definition, the construct
> `os::Permissions` is used. However, this is defined in
> `stout/os/permissions.hpp`, which is implicitly included on POSIX, but
> not on Windows. The fix is to IWYU and explicitly include it.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp 9c5b45b7c4e7705861f1e7926847711dc16f1264 
> 
> 
> Diff: https://reviews.apache.org/r/66753/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows with `-DENABLE_SSL`. Previously it failed due to 
> `os::Permissions` being undefined.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66679: Made the master send operation status updates when dropping operations.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66679 was successfully built and tested.

Reviews applied: `['66679']`

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

- Mesos Reviewbot Windows


On April 20, 2018, 10:47 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66679/
> ---
> 
> (Updated April 20, 2018, 10:47 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8781
> https://issues.apache.org/jira/browse/MESOS-8781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the master send an operation status update to the
> framework with  status `OPERATION_ERROR` when an operation with an
> operation ID is dropped.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
> 
> 
> Diff: https://reviews.apache.org/r/66679/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66696: Updated documentation for operation feedback.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66458', '66459', '66461', '66489', '66462', '66463', 
'66464', '66465', '66466', '66467', '66468', '66696']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(543):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(548):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(569):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]


   "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
   (ClCompile target) -> 
 d:\dcos\mesos\mesos\src\tests\operation_reconciliation_tests.cpp(141): 
error C2039: 'call': is 

Re: Review Request 66460: Added a `call()` method to the v1 scheduler library.

[Gaston Kleiman]

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



Changed the return type to the following proto message:

```
/**
 * This message is used ty the C++ Scheduler HTTP API library as the return
 * type of the `call()` method. The message includes the HTTP status code with
 * which the master responded, and optionally a `scheduler::Response` message.
 *
 * There are three cases to consider depending on the HTTP response status code:
 *
 *  (1) '202 ACCEPTED': Indicates the call was accepted for processing and
 *neither `response` nor `error` will be set.
 *
 *  (2) '200 OK': Indicates the call completed successfully, and the `response`
 *field will be set if the `scheduler::Call::Type` has a corresponding
 *`scheduler::Response::Type`; `error` will not be set.
 *
 *  (3) For all other HTTP status codes, the `response` field will not be set
 *  and the `error` field may be set to provide more information.
 *
 * NOTE: This message is used by the C++ Scheduler HTTP API library and not part
 * of the API specification.
 */
message APIResult {
  // HTTP status code with which the master responded.
  required uint32 status_code = 1;

  // This field will only be set if the call completed successfully and the
  // master responded with `200 OK` and a non-empty body.
  optional Response response = 2;

  // This field will only be set if the call did not complete successfully and
  // the master responded with a status other than `202 Accepted` or `200 OK`,
  // and with a non-empty body.
  optional string error = 3;
}
```

I tried to reply to BenM's issue, but Review Board doesn't let me publish the 
draft ¯_(?)_/¯.

- Gaston Kleiman


On April 20, 2018, 5:03 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66460/
> ---
> 
> (Updated April 20, 2018, 5:03 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-8192
> https://issues.apache.org/jira/browse/MESOS-8192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a `call()` method to the scheduler library that allows
> clients to send a `v1::scheduler::Call` to the master and receive a
> `v1::scheduler::Response`.
> 
> It is going to be used to test operation state reconciliation.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
>   include/mesos/v1/scheduler/scheduler.proto 
> 25ebcfcb07cb4fb928dcfdc242bb6509c9080491 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
> 60b17b9be74132c81532d22eba681feb899b22a3 
>   src/scheduler/scheduler.cpp ecef916ffd0797a13552525ff2cda0f99ca57e74 
> 
> 
> Diff: https://reviews.apache.org/r/66460/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66460: Added a `call()` method to the v1 scheduler library.

[Gaston Kleiman]

> On April 6, 2018, 2:13 p.m., Vinod Kone wrote:
> > include/mesos/v1/scheduler.hpp
> > Line 50 (original), 54-55 (patched)
> > 
> >
> > Since we don't guarantee backwards compat for this library, can we just 
> > update the signature of `send`?
> 
> Gaston Kleiman wrote:
> Would the libmesos binary then be compatible? Or would frameworks then 
> have to be recompiled using this new header in order to use new libmesos 
> binaries?
> 
> Greg Mann wrote:
> So, it sounds like if we broke the existing scheduler library interface, 
> that would mean that Java frameworks using the V0->V1 adapter would need to 
> simultaneously upgrade their Mesos JAR and libmesos. This is probably fine, 
> but we're checking with some framework developers to confirm.

Updating the signature of `send` would break people who have installed 
schedulers from packages that don't bundle the Mesos JAR and libmesos together, 
e.g., people who installed Marathon/Chronos and Mesos from rpm or deb packages.


- Gaston


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


On April 20, 2018, 5:03 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66460/
> ---
> 
> (Updated April 20, 2018, 5:03 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-8192
> https://issues.apache.org/jira/browse/MESOS-8192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a `call()` method to the scheduler library that allows
> clients to send a `v1::scheduler::Call` to the master and receive a
> `v1::scheduler::Response`.
> 
> It is going to be used to test operation state reconciliation.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
>   include/mesos/v1/scheduler/scheduler.proto 
> 25ebcfcb07cb4fb928dcfdc242bb6509c9080491 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
> 60b17b9be74132c81532d22eba681feb899b22a3 
>   src/scheduler/scheduler.cpp ecef916ffd0797a13552525ff2cda0f99ca57e74 
> 
> 
> Diff: https://reviews.apache.org/r/66460/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66468: Added tests for operation status reconciliation.

[Gaston Kleiman]

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

(Updated April 20, 2018, 5:03 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased + updates due to having changed the signature of the Scheduler library 
`call()` method.


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


Repository: mesos


Description
---

Added tests for operation status reconciliation.


Diffs (updated)
-

  src/Makefile.am 9d610bbe8108e5caa4a22977caa9dff07c8bb665 
  src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
  src/tests/mesos.hpp 7356523a2a37858bbe866527425f35502ff5ad82 
  src/tests/operation_reconciliation_tests.cpp PRE-CREATION 
  src/tests/storage_local_resource_provider_tests.cpp 
2872f1aec1a7b94fc302a533f5ae9e1be9658087 


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

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


Testing
---

The new tests passed 1000 iterations on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 66460: Added a `call()` method to the v1 scheduler library.

[Gaston Kleiman]

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

(Updated April 20, 2018, 5:03 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


Changes
---

Changed the signature of the new method.


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


Repository: mesos


Description
---

This patch adds a `call()` method to the scheduler library that allows
clients to send a `v1::scheduler::Call` to the master and receive a
`v1::scheduler::Response`.

It is going to be used to test operation state reconciliation.


Diffs (updated)
-

  include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661 
  include/mesos/v1/scheduler/scheduler.proto 
25ebcfcb07cb4fb928dcfdc242bb6509c9080491 
  src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
60b17b9be74132c81532d22eba681feb899b22a3 
  src/scheduler/scheduler.cpp ecef916ffd0797a13552525ff2cda0f99ca57e74 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux


Thanks,

Gaston Kleiman

Review Request 66753: Windows: Fixed build with OpenSSL due to missing header.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Chun-Hung Hsiao, and Joseph Wu.


Repository: mesos


Description
---

When `-DENABLE_SSL=TRUE`, then `USE_SSL_SOCKET` is defined, and in
this file in a section guarded by the definition, the construct
`os::Permissions` is used. However, this is defined in
`stout/os/permissions.hpp`, which is implicitly included on POSIX, but
not on Windows. The fix is to IWYU and explicitly include it.


Diffs
-

  src/local/local.cpp 9c5b45b7c4e7705861f1e7926847711dc16f1264 


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


Testing
---

Built on Windows with `-DENABLE_SSL`. Previously it failed due to 
`os::Permissions` being undefined.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66733: Added a new `RESIZE_VOLUME` agent capability.

[Greg Mann]

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




include/mesos/mesos.proto
Lines 997 (patched)


s/express/expresses/

Here and below.



include/mesos/mesos.proto
Line 997 (original), 1001 (patched)


As discussed in chat, perhaps we want some agent flag validation which will 
require that the RESOURCE_PROVIDER capability is set if the operator sets the 
RESIZE_VOLUME capability?

It's not necessary yet, but will be once we switch to non-speculative 
operations, so perhaps we want to set that constraint now.


- Greg Mann


On April 20, 2018, 6:38 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66733/
> ---
> 
> (Updated April 20, 2018, 6:38 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used as a feature flag to gate the new volume resize
> feature. This feature will be turn on by default once released.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9e24d3ea46edc21572e9226e2e76c7d55618db24 
>   include/mesos/v1/mesos.proto 0f3fd8a2608b5edabc21f5fe5df9b70fc0fa8dc2 
>   src/common/protobuf_utils.hpp ae060f3a7946ac5862faeca15330e9642f934137 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/slave/constants.cpp 51de71bc45ff4fb862690efd9428b25d92055391 
>   src/tests/master_tests.cpp be7a3cc6126d2fe7c56c49b3da5f6f4bf29657f5 
>   src/tests/slave_tests.cpp 04f7acad47aa980d507bbe29b29db5eb050c02f3 
> 
> 
> Diff: https://reviews.apache.org/r/66733/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66577: Enabled CSI proto compilation by default.

[Chun-Hung Hsiao]

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

(Updated April 20, 2018, 11:34 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, and Jie Yu.


Changes
---

Addressed Benjamin's comment.


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


Repository: mesos


Description
---

CSI proto compilation is disabled due to MESOS-8749, which is resolved
by bumping CSI to v0.2. This patch enables the compilation again.


Diffs (updated)
-

  src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
  src/Makefile.am 9d610bbe8108e5caa4a22977caa9dff07c8bb665 
  src/slave/flags.cpp e330e5fa0b49ad1d1765e492bc44a262a7856db5 
  src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 66577: Enabled CSI proto compilation by default.

[Chun-Hung Hsiao]

> On April 20, 2018, 7:53 a.m., Benjamin Bannier wrote:
> > src/Makefile.am
> > Lines 1529-1536 (original), 1521-1525 (patched)
> > 
> >
> > We can merge these two blocks now.
> 
> Chun-Hung Hsiao wrote:
> I was following the convention to split apart implementation files and 
> header files ;) But will do.

Oh I guess you mean to just remove the blank line. I'll modify my code this way.


- Chun-Hung


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


On April 17, 2018, 3:03 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66577/
> ---
> 
> (Updated April 17, 2018, 3:03 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, and Jie Yu.
> 
> 
> Bugs: MESOS-8724
> https://issues.apache.org/jira/browse/MESOS-8724
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CSI proto compilation is disabled due to MESOS-8749, which is resolved
> by bumping CSI to v0.2. This patch enables the compilation again.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/Makefile.am 07eb13851967b8a28b46afa3304a9d4e298a5016 
>   src/slave/flags.cpp 4ea2286a6a874e6e7f473f97fb0431e17b19c1f8 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
> 
> 
> Diff: https://reviews.apache.org/r/66577/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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

[Mesos Reviewbot Windows]

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



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 April 20, 2018, 10:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66749/
> ---
> 
> (Updated April 20, 2018, 10:32 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 
> def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> af34a856e092a880a0809da34ead9d8588b0ac8f 
>   src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 
> 
> 
> Diff: https://reviews.apache.org/r/66749/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66577: Enabled CSI proto compilation by default.

[Chun-Hung Hsiao]

> On April 20, 2018, 7:53 a.m., Benjamin Bannier wrote:
> > src/Makefile.am
> > Lines 1529-1536 (original), 1521-1525 (patched)
> > 
> >
> > We can merge these two blocks now.

I was following the convention to split apart implementation files and header 
files ;) But will do.


- Chun-Hung


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


On April 17, 2018, 3:03 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66577/
> ---
> 
> (Updated April 17, 2018, 3:03 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, and Jie Yu.
> 
> 
> Bugs: MESOS-8724
> https://issues.apache.org/jira/browse/MESOS-8724
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CSI proto compilation is disabled due to MESOS-8749, which is resolved
> by bumping CSI to v0.2. This patch enables the compilation again.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/Makefile.am 07eb13851967b8a28b46afa3304a9d4e298a5016 
>   src/slave/flags.cpp 4ea2286a6a874e6e7f473f97fb0431e17b19c1f8 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
> 
> 
> Diff: https://reviews.apache.org/r/66577/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66727: Cherry-picked gRPC PR #15128 for Windows compilation.

[Andrew Schwartzmeyer]

> On April 20, 2018, 4:08 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/grpc-1.10.0.patch
> > Lines 9-19 (patched)
> > 
> >
> > We discussed this a bit, and it's just... weird. gRPC should not be 
> > _requiring_ `_WIN32_WINNT` to be defined.
> > 
> > Chun, I think, perhaps, could we just change `#error` to `#warning`? 
> > Changing the ordering also ensure it gets defined  before the check, but 
> > really just solves our problem by happenstance.

Actually, I'm dropping this. This approach is reasonable as `windows.h` will 
include `sdkddkver.h`, which has the following code:

```
#if !defined(_WIN32_WINNT) && !defined(_CHICAGO_)
#define  _WIN32_WINNT   0x0A00
#endif
```

Thus by moving `#include ` up, the SDK will define `_WIN32_WINNT` 
for us if and only if we hadn't previously defined it, and gRPC's version 
checking logic will still work as intended.


- Andrew


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


On April 19, 2018, 7:24 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66727/
> ---
> 
> (Updated April 19, 2018, 7:24 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-7881
> https://issues.apache.org/jira/browse/MESOS-7881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We added a gRPC patch file that includes
> https://github.com/grpc/grpc/pull/15128 to fix Windows compilation, and
> updated `grpc.md` to match the format of `protobuf.md`.
> 
> 
> Diffs
> -
> 
>   3rdparty/grpc-1.10.0.patch PRE-CREATION 
>   3rdparty/grpc-1.10.0.readme d33a80881f9660e95cf3c2b032b3053d5ed9c7ff 
>   3rdparty/grpc.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66727/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

[Zhitao Li]

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

(Updated April 20, 2018, 4:18 p.m.)


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


Changes
---

Renamed Authorization.


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


Repository: mesos


Description
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/13/

Changes: https://reviews.apache.org/r/66051/diff/12-13/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66532: Added test for authorization actions for `RESIZE_VOLUME`.

[Zhitao Li]

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

(Updated April 20, 2018, 4:17 p.m.)


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


Changes
---

Renamed to `ResizeVolume`.


Summary (updated)
-

Added test for authorization actions for `RESIZE_VOLUME`.


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


Repository: mesos


Description (updated)
---

Added test for authorization actions for `RESIZE_VOLUME`.


Diffs (updated)
-

  src/tests/authorization_tests.cpp a76ad18a54ec232e6d6c92ec6a45b445a83f174c 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66531: Added new authorization for `ResizeVolume`.

[Zhitao Li]

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

(Updated April 20, 2018, 4:16 p.m.)


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


Changes
---

Rename to `ResizeVolume`.


Summary (updated)
-

Added new authorization for `ResizeVolume`.


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


Repository: mesos


Description
---

The new authorization action is modelled after `CreateVolume`, and will
be shared by both `GROW_VOLUME` and `SHRINK_VOLUME` operations and
corresponding operator APIs.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto 8ef33210644e7d2924b402a3158b1b38c1fdb464 
  include/mesos/authorizer/authorizer.proto 
1508c01130013d74ed2b2574a2428f38e3d2c064 
  src/authorizer/local/authorizer.cpp c098ba9ded1b29a2e079cf09ab80b61f6fa4af05 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 


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

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


Testing
---

Manually created ACL and verified that untrusted principals will not allow to 
grow/shrink volumes.
Also created unit test in next patch.


Thanks,

Zhitao Li

Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.

[Zhitao Li]

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

(Updated April 20, 2018, 4:15 p.m.)


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


Changes
---

Review comments.


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


Repository: mesos


Description
---

These two operations are intended to be non-speculative eventually but
are implemented as speculative right now. To avoid frameworks opt-in to
dangerous behavior, we require that accept can only contain one
`GROW_VOLUME` or `SHRINK_VOLUME` and no other operations.

This is implemented by reuse existing code which already drops `LAUNCH`
or `LAUNCH_GROUP` with proper reason and message.


Diffs (updated)
-

  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 


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

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


Testing
---

Behavior tested in https://reviews.apache.org/r/66569/.


Thanks,

Zhitao Li

Re: Review Request 66727: Cherry-picked gRPC PR #15128 for Windows compilation.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On April 19, 2018, 7:24 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66727/
> ---
> 
> (Updated April 19, 2018, 7:24 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-7881
> https://issues.apache.org/jira/browse/MESOS-7881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We added a gRPC patch file that includes
> https://github.com/grpc/grpc/pull/15128 to fix Windows compilation, and
> updated `grpc.md` to match the format of `protobuf.md`.
> 
> 
> Diffs
> -
> 
>   3rdparty/grpc-1.10.0.patch PRE-CREATION 
>   3rdparty/grpc-1.10.0.readme d33a80881f9660e95cf3c2b032b3053d5ed9c7ff 
>   3rdparty/grpc.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66727/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On April 19, 2018, 7:34 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 19, 2018, 7:34 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 173089a1ebf80a6288431075770a807235524ac1 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/5/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61118: Building gRPC support in libprocess with CMake.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On April 19, 2018, 7:31 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61118/
> ---
> 
> (Updated April 19, 2018, 7:31 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-7881
> https://issues.apache.org/jira/browse/MESOS-7881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables CMake to build code for gRPC support in libprocess
> along with tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 1eca50a4d513b2fead111e453565edff0a3ee497 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> d624402bc9efb43a130a2afdf27cfc1aa69010e4 
> 
> 
> Diff: https://reviews.apache.org/r/61118/diff/7/
> 
> 
> Testing
> ---
> 
> `cmake .. -DENABLE_GRPC=1` and then `cmake --build . --target check` on Linux 
> and Windows.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61096: Building gRPC with CMake.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On April 20, 2018, 3:18 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61096/
> ---
> 
> (Updated April 20, 2018, 3:18 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-7881
> https://issues.apache.org/jira/browse/MESOS-7881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an `ENABLE_GRPC` option and builds the bundled gRPC
> 3rdparty library in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt e6bb4076e630d78e73a82a93b1f64b3bc5d859b9 
>   3rdparty/cmake/Versions.cmake 33577cc86d3ef31e129b8ea55d44d3fee02d4a30 
>   cmake/CompilationConfigure.cmake 173089a1ebf80a6288431075770a807235524ac1 
>   src/cmake/MesosProtobuf.cmake dde034fe4d4251dd7c7397d422136bfa3c9c3ebc 
> 
> 
> Diff: https://reviews.apache.org/r/61096/diff/12/
> 
> 
> Testing
> ---
> 
> `cmake .. -DENABLE_GRPC=1` then `cmake --build .` on both Linux and Windows.
> 
> NOTE: libprocess-tests will be failed to build on Windows with VS. This is 
> fixed in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66727: Cherry-picked gRPC PR #15128 for Windows compilation.

[Andrew Schwartzmeyer]

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




3rdparty/grpc-1.10.0.patch
Lines 9-19 (patched)


We discussed this a bit, and it's just... weird. gRPC should not be 
_requiring_ `_WIN32_WINNT` to be defined.

Chun, I think, perhaps, could we just change `#error` to `#warning`? 
Changing the ordering also ensure it gets defined  before the check, but really 
just solves our problem by happenstance.


- Andrew Schwartzmeyer


On April 19, 2018, 7:24 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66727/
> ---
> 
> (Updated April 19, 2018, 7:24 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-7881
> https://issues.apache.org/jira/browse/MESOS-7881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We added a gRPC patch file that includes
> https://github.com/grpc/grpc/pull/15128 to fix Windows compilation, and
> updated `grpc.md` to match the format of `protobuf.md`.
> 
> 
> Diffs
> -
> 
>   3rdparty/grpc-1.10.0.patch PRE-CREATION 
>   3rdparty/grpc-1.10.0.readme d33a80881f9660e95cf3c2b032b3053d5ed9c7ff 
>   3rdparty/grpc.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66727/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66696: Updated documentation for operation feedback.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:54 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


Repository: mesos


Description
---

Updated documentation for operation feedback.


Diffs (updated)
-

  docs/csi.md 2d96ee754b3fdcfb18a7516f30abf0086fa75137 
  docs/scheduler-http-api.md 3929c33781a152428338c4cdaf7dbc47da7c875e 


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

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


Testing
---


Thanks,

Gaston Kleiman

Re: Review Request 66466: Updated `RESERVE()` helper to allow specifying an operation ID.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:54 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Updated `RESERVE()` helper to allow specifying an operation ID.


Diffs (updated)
-

  src/tests/mesos.hpp 7356523a2a37858bbe866527425f35502ff5ad82 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux

The tests added by https://reviews.apache.org/r/66468/ use this helper with an 
`OperationID`.


Thanks,

Gaston Kleiman

Re: Review Request 66467: Added a test helper for creating `RECONCILE_OPERATIONS` v1 calls.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:54 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added a test helper for creating `RECONCILE_OPERATIONS` v1 calls.


Diffs (updated)
-

  src/tests/mesos.hpp 7356523a2a37858bbe866527425f35502ff5ad82 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux

The tests added by https://reviews.apache.org/r/66468/ use this helper.


Thanks,

Gaston Kleiman

Re: Review Request 66465: Updated `using` statements in `tests/mesos.hpp`.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:54 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Updated `using` statements in `tests/mesos.hpp`.


Diffs (updated)
-

  src/tests/mesos.hpp 7356523a2a37858bbe866527425f35502ff5ad82 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux

The tests added by https://reviews.apache.org/r/66468/ use these statements.


Thanks,

Gaston Kleiman

Re: Review Request 66464: Implemented operation status reconciliation.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:53 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Addressed feedback + rebased.


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


Repository: mesos


Description
---

Implemented operation status reconciliation.


Diffs (updated)
-

  include/mesos/v1/scheduler/scheduler.proto 
25ebcfcb07cb4fb928dcfdc242bb6509c9080491 
  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
  src/messages/messages.proto 556801d9ece3ada6d8e3cec51882ad50c27e24b2 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux

https://reviews.apache.org/r/66468/ adds new tests.


Thanks,

Gaston Kleiman

Re: Review Request 66462: Added new operation states to be used for status reconciliation.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:53 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added new operation states to be used for status reconciliation.


Diffs (updated)
-

  include/mesos/mesos.proto 9e24d3ea46edc21572e9226e2e76c7d55618db24 
  include/mesos/v1/mesos.proto 0f3fd8a2608b5edabc21f5fe5df9b70fc0fa8dc2 
  src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
  src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 66463: Added a master metric for operations reconciliation messages.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:53 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added a master metric for operations reconciliation messages.


Diffs (updated)
-

  docs/monitoring.md 12e2103eb041e3e1b99bddafafcf4c615205fb0c 
  docs/operator-http-api.md 10dcac83fa70da5760a5c231665d7498cc168622 
  src/master/metrics.hpp 5699c64bac19cb735a4c72f9a067b1338c2ac3c2 
  src/master/metrics.cpp 894c04173d218ffd891d203254035fe4756cc8bb 
  src/tests/master_tests.cpp be7a3cc6126d2fe7c56c49b3da5f6f4bf29657f5 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 66489: Cleaned up internal evolve functions.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:47 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch removes the `mesos::` prefix from declarations where
possible, uses a consistent number of empty lines in the header,
and reorders some declarations.


Diffs (updated)
-

  src/internal/evolve.hpp e00ac7175438f758b10c0aa9485fc29ffa4efa29 
  src/internal/evolve.cpp f598ea11b64bdbface5bf1f124a02ea876196b67 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 66459: Fixed bug in `Master::updateSlave()`.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:47 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


Repository: mesos


Description
---

A part of `Master::updateSlave()` doesn't account for operations created
via the operator API; this patch fixes that.


Diffs (updated)
-

  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 66461: Added an evolve function for `v1::scheduler::Response`.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:47 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added an evolve function for `v1::scheduler::Response`.


Diffs (updated)
-

  src/internal/evolve.hpp e00ac7175438f758b10c0aa9485fc29ffa4efa29 
  src/internal/evolve.cpp f598ea11b64bdbface5bf1f124a02ea876196b67 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 66458: Fixed handling of operations in `master::recoverFramework()`.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:47 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


Repository: mesos


Description
---

`Master::recoverFramework()` only recovers operations affecting agent
default resources. This patch makes it also recover operations affecting
resources managed by resource providers.

It also fixes a bug in which not just the corresponding operations, but
all the ones affecting agent default resources will be added to the
framework.


Diffs (updated)
-

  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 66679: Made the master send operation status updates when dropping operations.

[Gaston Kleiman]

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

(Updated April 20, 2018, 3:47 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch makes the master send an operation status update to the
framework with  status `OPERATION_ERROR` when an operation with an
operation ID is dropped.


Diffs (updated)
-

  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 


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

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


Testing
---

`sudo bin/mesos-tests.sh` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 66464: Implemented operation status reconciliation.

[Greg Mann]

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




src/master/master.cpp
Lines 8929-8933 (patched)


I think this case is actually "Operation is unknown, slave is unknown" 
since the scheduler specified an agent ID, but we didn't locate it in any of 
the agent maps.

Maybe the message here should be something like "Reconciliation: Both 
operation and agent are unknown"?



src/master/master.cpp
Lines 8939 (patched)


I think this message should be "Reconciliation: Operation is unknown".

Maybe we should elaborate a bit and say "Reconciliation: Operation is 
unknown and no 'agent_id' was provided"? I mean, they should know they didn't 
provide an agent ID :) But this might be useful? Not sure.


- Greg Mann


On April 18, 2018, 9:36 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66464/
> ---
> 
> (Updated April 18, 2018, 9:36 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8191
> https://issues.apache.org/jira/browse/MESOS-8191
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operation status reconciliation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
> 
> 
> Diff: https://reviews.apache.org/r/66464/diff/5/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests` on GNU/Linux
> 
> https://reviews.apache.org/r/66468/ adds new tests.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 61096: Building gRPC with CMake.

[Chun-Hung Hsiao]

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

(Updated April 20, 2018, 10:18 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Addressed a VS builed problem.


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


Repository: mesos


Description
---

This patch adds an `ENABLE_GRPC` option and builds the bundled gRPC
3rdparty library in CMake.


Diffs (updated)
-

  3rdparty/CMakeLists.txt e6bb4076e630d78e73a82a93b1f64b3bc5d859b9 
  3rdparty/cmake/Versions.cmake 33577cc86d3ef31e129b8ea55d44d3fee02d4a30 
  cmake/CompilationConfigure.cmake 173089a1ebf80a6288431075770a807235524ac1 
  src/cmake/MesosProtobuf.cmake dde034fe4d4251dd7c7397d422136bfa3c9c3ebc 


Diff: https://reviews.apache.org/r/61096/diff/12/

Changes: https://reviews.apache.org/r/61096/diff/11-12/


Testing
---

`cmake .. -DENABLE_GRPC=1` then `cmake --build .` on both Linux and Windows.

NOTE: libprocess-tests will be failed to build on Windows with VS. This is 
fixed in the next patch.


Thanks,

Chun-Hung Hsiao

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

[Meng Zhu]

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

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


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 
def09f1104213bf73d9f95cb5ad2fb80e3bdb04a 
  src/slave/containerizer/mesos/linux_launcher.cpp 
af34a856e092a880a0809da34ead9d8588b0ac8f 
  src/slave/slave.cpp 9d2d1928b231044988f1855eb518448db38ff04f 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 61096: Building gRPC with CMake.

[Chun-Hung Hsiao]

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

(Updated April 20, 2018, 9:51 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Unified the build command for gRPC.


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


Repository: mesos


Description
---

This patch adds an `ENABLE_GRPC` option and builds the bundled gRPC
3rdparty library in CMake.


Diffs (updated)
-

  3rdparty/CMakeLists.txt e6bb4076e630d78e73a82a93b1f64b3bc5d859b9 
  3rdparty/cmake/Versions.cmake 33577cc86d3ef31e129b8ea55d44d3fee02d4a30 
  cmake/CompilationConfigure.cmake 173089a1ebf80a6288431075770a807235524ac1 
  src/cmake/MesosProtobuf.cmake dde034fe4d4251dd7c7397d422136bfa3c9c3ebc 


Diff: https://reviews.apache.org/r/61096/diff/11/

Changes: https://reviews.apache.org/r/61096/diff/10-11/


Testing
---

`cmake .. -DENABLE_GRPC=1` then `cmake --build .` on both Linux and Windows.

NOTE: libprocess-tests will be failed to build on Windows with VS. This is 
fixed in the next patch.


Thanks,

Chun-Hung Hsiao

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

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66230 was successfully built and tested.

Reviews applied: `['66746', '66228', '66229', '66230']`

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

- Mesos Reviewbot Windows


On April 20, 2018, 8:19 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> ---
> 
> (Updated April 20, 2018, 8:19 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/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 61096: Building gRPC with CMake.

[Chun-Hung Hsiao]

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

(Updated April 20, 2018, 9:22 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Fixed a Ninja build problem.


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


Repository: mesos


Description
---

This patch adds an `ENABLE_GRPC` option and builds the bundled gRPC
3rdparty library in CMake.


Diffs (updated)
-

  3rdparty/CMakeLists.txt e6bb4076e630d78e73a82a93b1f64b3bc5d859b9 
  3rdparty/cmake/Versions.cmake 33577cc86d3ef31e129b8ea55d44d3fee02d4a30 
  cmake/CompilationConfigure.cmake 173089a1ebf80a6288431075770a807235524ac1 
  src/cmake/MesosProtobuf.cmake dde034fe4d4251dd7c7397d422136bfa3c9c3ebc 


Diff: https://reviews.apache.org/r/61096/diff/10/

Changes: https://reviews.apache.org/r/61096/diff/9-10/


Testing
---

`cmake .. -DENABLE_GRPC=1` then `cmake --build .` on both Linux and Windows.

NOTE: libprocess-tests will be failed to build on Windows with VS. This is 
fixed in the next patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66726: Made CMake's `FindZLIB` module be able to find Zlib on Windows.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On April 19, 2018, 7:23 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66726/
> ---
> 
> (Updated April 19, 2018, 7:23 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-7881
> https://issues.apache.org/jira/browse/MESOS-7881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch "install" ZLib in `build/3rdparty/zlib-1.2.8/src/zlib-1.2.8`,
> so we can set `ZLIB_ROOT` to the directory to make `FindZLIB` be able to
> find the include directory containing both `zlib.h` and `zconf.h`, as
> well as to find the library directory containing the library files.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt e6bb4076e630d78e73a82a93b1f64b3bc5d859b9 
> 
> 
> Diff: https://reviews.apache.org/r/66726/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build .
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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

[Kapil Arya]

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

(Updated April 20, 2018, 4:19 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 (updated)
-

  src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 66229: Implemented UPDATE_FRAMEWORK call.

[Kapil Arya]

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

(Updated April 20, 2018, 4:19 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
---

Implemented UPDATE_FRAMEWORK call.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/master.cpp f7da675e8fe96159e5335c9e83b65b67ed90eda8 
  src/master/metrics.hpp 5699c64bac19cb735a4c72f9a067b1338c2ac3c2 
  src/master/metrics.cpp 894c04173d218ffd891d203254035fe4756cc8bb 
  src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 66228: Added Call for updating framework info.

[Kapil Arya]

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

(Updated April 20, 2018, 4:19 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 Call for updating framework info.


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
f6a780a7b75878b9e74402a28a25bb868f7ac36f 
  include/mesos/v1/scheduler/scheduler.proto 
25ebcfcb07cb4fb928dcfdc242bb6509c9080491 


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

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


Testing
---


Thanks,

Kapil Arya

Review Request 66746: Replaced protobuf-specific comparators with MessageDifferencer.

[Kapil Arya]

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

Review request for mesos, Benjamin Mahler and Till Toenshoff.


Repository: mesos


Description
---

The MessageDifferencer utility has been available since protobuf 3.0.0.
When comparing most repeated fields, we treat them as sets so the order
of elements doesn't matter. In some exceptional cases, we treat them as
list so the order becomes important. Further, fields with default
purposes are considered to be set for comparison purposes.


Diffs
-

  include/mesos/type_utils.hpp 19ea81716496bcc0117a1b0ff157a0374f38bbfa 
  include/mesos/v1/mesos.hpp fda3eb42061f820869a2d8da939fccadc4e5ddfb 
  src/common/type_utils.cpp 33d63809b61a18e03ff745c88f024c71dd221ca2 
  src/tests/master_allocator_tests.cpp e1aef8a9625a805e7ad2dfad37bfeedee82f160d 
  src/v1/mesos.cpp 9b2df2dd798dff24a91a2604ab53c7fbb5ecfbcf 


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


Testing
---

`make check` currently fails with:
DefaultExecutorCheckTest.CommandCheckSharesWorkDirWithTask
DefaultExecutorCheckTest.MultipleTasksWithChecks


Thanks,

Kapil Arya

Re: Review Request 66173: Add test for new `disk/xfs` kill functionality.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66001', '66173']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3426):
 warning C4996: 'strerror': This function or variable may be unsafe. Consider 
using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500):
 warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define 
_WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170):
 warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 

Re: Review Request 66725: Improved error message in the fetcher.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66725]

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 19, 2018, 4:08 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66725/
> ---
> 
> (Updated April 19, 2018, 4:08 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included error message from `mkdir` when
> returning error.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 8a8d7c3311c6d282648940bc4440f4c6d9d5e918 
> 
> 
> Diff: https://reviews.apache.org/r/66725/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

[Chun-Hung Hsiao]

> On April 20, 2018, 12:57 p.m., Benjamin Bannier wrote:
> > src/resource_provider/storage/provider.cpp
> > Line 1010 (original), 1011 (patched)
> > 
> >
> > Nit: _pending operations_ should be specific enough as this list is 
> > neither exhaustive today nor will remain in the future.

I'm specifically talking about `CREATE_VOLUME` and `CREATE_BLOCK` here. 
`DESTROY_VOLUME` and `DESTROY_BLOCK` don't need the profile info.


> On April 20, 2018, 12:57 p.m., Benjamin Bannier wrote:
> > src/resource_provider/storage/provider.cpp
> > Line 1021 (original), 1014 (patched)
> > 
> >
> > Both `disk` and `profile` are `optional` fields. We should check 
> > explicitly whether they are set (otherwise we might at some point e.g., hit 
> > a failure where the profile `` (empty string) was not found.

The implicit invariant here is that each resource either has it's id or profile 
set. I'll add a check to make it more explicit.


- Chun-Hung


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


On April 12, 2018, 3:35 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65594/
> ---
> 
> (Updated April 12, 2018, 3:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SLRP now checkpoints profiles associated with storage pools, and does
> not depend on the `DiskProfileAdaptor` module to return the set of
> previously-known profiles during recovery.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65594/diff/8/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66173: Add test for new `disk/xfs` kill functionality.

[Harold Dost]

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

(Updated April 20, 2018, 6:49 p.m.)


Review request for mesos and James Peach.


Changes
---

Update based on changes.


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


Repository: mesos


Description
---

MESOS-6575


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp 
64c3e1c3f0bc435897626cb0a13bc19c7cb1a4fe 


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

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


Testing
---


Thanks,

Harold Dost

Re: Review Request 66001: MESOS-6575: Add soft limit and kill to disk/xfs.

[Harold Dost]

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

(Updated April 20, 2018, 6:48 p.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

New Flags for disk/xfs isolator:
- xfs_kill_containers - This will create a 10 MB buffer between the soft
and hard limits, and when the soft limit is exceeded it will
subsequently be killed.

Functionality
- This by default disabled behavior allows for the `disk/xfs` isolator
to kill containers which surpass their limit.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
07e68a777aefba4dd35066f2eb207bba7f199d83 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
8d9f8f846866f9de377c59cb7fb311041283ba70 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
e034133629a9c1cf58b776f8da2a93421332cee0 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
2708524add1ff693b616d4fb241c4a0a3070520b 
  src/slave/flags.hpp 4195d3b4170c5fa0348b63ed19e18dd208593975 
  src/slave/flags.cpp e330e5fa0b49ad1d1765e492bc44a262a7856db5 
  src/tests/containerizer/xfs_quota_tests.cpp 
64c3e1c3f0bc435897626cb0a13bc19c7cb1a4fe 


Diff: https://reviews.apache.org/r/66001/diff/14/

Changes: https://reviews.apache.org/r/66001/diff/13-14/


Testing
---


Thanks,

Harold Dost

Re: Review Request 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.

[Andrew Schwartzmeyer]

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

(Updated April 20, 2018, 11:44 a.m.)


Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Changes
---

Removed old comment.


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


Repository: mesos


Description
---

Instead of using the CRT implementation of `_wopen()` for the
`os::open()` API, we now use the Windows API `CreateFileW()`, mapping
each of the Linux `open()` flags to their semantic equivalents. This
will make implementing overlapped I/O possible, and is a step toward
removing the use of integer file descriptors on Windows.

Note that instead of redefining the C flags like `O_RDONLY`, we just
use them directly in our mapping logic, and set the used but
unsupported flags to zero.

This change uncovered several bugs such as incorrect access flags, and
used-but-not-included headers.

We currently ignore creation permissions as they will be handled in a
broader project to map permissions to Windows correctly.


Diffs (updated)
-

  3rdparty/stout/include/stout/net.hpp d2992c05b221ea90dae1c06d27753932f7411925 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
  3rdparty/stout/include/stout/os/windows/mktemp.hpp 
5c775c45c415d9ddd6a80ab814fb55475e9f871e 
  3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 
  3rdparty/stout/include/stout/windows.hpp 
1bfcdf4a5c097cc6d2293396ce39c8ad2c9ec993 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Zhitao Li]

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

(Updated April 20, 2018, 11:40 a.m.)


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


Changes
---

Rebase and check resize volume capability.


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


Repository: mesos


Description
---

The new offer operations are implemented as speculative operations, but
we will use validation to make them non-speculative on API level so that
we can transition later without a breaking change.


Diffs (updated)
-

  src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
  src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
  src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66050/diff/12/

Changes: https://reviews.apache.org/r/66050/diff/11-12/


Testing
---


Thanks,

Zhitao Li

Review Request 66733: Added a new `RESIZE_VOLUME` agent capability.

[Zhitao Li]

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

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


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


Repository: mesos


Description
---

This will be used as a feature flag to gate the new volume resize
feature. This feature will be turn on by default once released.


Diffs
-

  include/mesos/mesos.proto 9e24d3ea46edc21572e9226e2e76c7d55618db24 
  include/mesos/v1/mesos.proto 0f3fd8a2608b5edabc21f5fe5df9b70fc0fa8dc2 
  src/common/protobuf_utils.hpp ae060f3a7946ac5862faeca15330e9642f934137 
  src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
  src/slave/constants.cpp 51de71bc45ff4fb862690efd9428b25d92055391 
  src/tests/master_tests.cpp be7a3cc6126d2fe7c56c49b3da5f6f4bf29657f5 
  src/tests/slave_tests.cpp 04f7acad47aa980d507bbe29b29db5eb050c02f3 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

[Zhitao Li]

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

(Updated April 20, 2018, 11:38 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added offer operation to grow and shrink persistent volumes.


Diffs (updated)
-

  include/mesos/mesos.proto 9e24d3ea46edc21572e9226e2e76c7d55618db24 
  include/mesos/v1/mesos.proto 0f3fd8a2608b5edabc21f5fe5df9b70fc0fa8dc2 
  src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
  src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
  src/tests/mesos.hpp 7356523a2a37858bbe866527425f35502ff5ad82 
  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 
  src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 


Diff: https://reviews.apache.org/r/66049/diff/10/

Changes: https://reviews.apache.org/r/66049/diff/9-10/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66423: Split `stout/os/open.hpp` into Windows and POSIX files.

[Andrew Schwartzmeyer]

> On April 18, 2018, 11:09 a.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/os/windows/open.hpp
> > Lines 29-32 (patched)
> > 
> >
> > We won't need this note anymore (on the Windows header).

That's true, but it should stay in this patch as it's a straight split. I'll 
remove it in the next patch.


- Andrew


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


On April 3, 2018, 10:47 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66423/
> ---
> 
> (Updated April 3, 2018, 10:47 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8673
> https://issues.apache.org/jira/browse/MESOS-8673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The logic remained the same, just with the Windows code removed from
> the POSIX code, and vice versa.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 742bfc44d68d978dd2249ece500d6f64e4d7f02a 
>   3rdparty/stout/include/stout/os/open.hpp 
> 4dc5b087abf40ac9b81f0fd611aca192e5d33ce7 
>   3rdparty/stout/include/stout/os/posix/open.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66423/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 62472: Fixed the ordering of Mesos containerizer isolators.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On April 19, 2018, 2:22 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62472/
> ---
> 
> (Updated April 19, 2018, 2:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Kevin 
> Klues.
> 
> 
> Bugs: MESOS-7643
> https://issues.apache.org/jira/browse/MESOS-7643
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Historically, isolators were invoked in the order listed by
> the operator in the `--isolation` flag. This changed to an
> internal ordering in 6fb9c024, back to the operator ordering
> in af474a6fa, and then to an undefined ordering in 9642d3c67b.
> 
> This commit switches back to an internal ordering for all the built-in
> isolators. Custom isolators loaded in modules are run in operator order
> after all the built-in ones. The rationale for an internal ordering is
> expressed in MESOS-5581; basically we should not burden the operator
> with having to figure out how to make the order correct. In the case of
> custom isolators there's no way for us to know the correct ordering so
> we make an arbitrary choice.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6568126252a0a8ff5f6095bedae7828a9a1ba0fd 
> 
> 
> Diff: https://reviews.apache.org/r/62472/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> Posting this as an RFC. If reviewers are OK with the approach, I'll float it 
> on users@ and dev@ and update the documentation.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.

[Benjamin Bannier]

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



LGTM, but I wonder whether it would make sense to keep the assertions and 
reject outdated resources more generally based on e.g., resource versions.

- Benjamin Bannier


On April 12, 2018, 5:37 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65976/
> ---
> 
> (Updated April 12, 2018, 5:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed profiles to be missing from `DiskProfileAdaptor`.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
> 
> 
> Diff: https://reviews.apache.org/r/65976/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65975: Sequentialize reconciliations of storage pools in SLRP.

[Benjamin Bannier]

> On April 20, 2018, 3:27 p.m., Benjamin Bannier wrote:
> > First round of reviews.
> > 
> > I am not a big fan on how reconcilations are modelled here. The counting 
> > seems to lead to an incomplete encapsulation of correct behavior. I'd much 
> > rather see standard `libprocess` actor semantics and better use of data 
> > structures instead. Right now the code appears too complicated and 
> > potentially brittle to the touch to me.

I am nto sure what a good example of a solution would, but have a look at e.g., 
the master registrar in how it pushes updates (operations) into a simple 
`deque` which it then works on sequentially. While it triggers work on the 
updates manually the control flow is easier to see (still not easy enough 
though). Maybe we have other examples as well.


- Benjamin


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


On April 12, 2018, 5:36 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65975/
> ---
> 
> (Updated April 12, 2018, 5:36 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The storage pools needs to be reconciled in the following two scenarios:
> 
> 1. When there is a change in the set of known profiles.
> 2. When a volume/block of an unknown profile is destroyed, because the
>disk space being freed up may belong to a known profile.
> 
> This patch adds a sequence to coordinate the reconciliations for the
> above two cases.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65975/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65975: Sequentialize reconciliations of storage pools in SLRP.

[Benjamin Bannier]

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



First round of reviews.

I am not a big fan on how reconcilations are modelled here. The counting seems 
to lead to an incomplete encapsulation of correct behavior. I'd much rather see 
standard `libprocess` actor semantics and better use of data structures 
instead. Right now the code appears too complicated and potentially brittle to 
the touch to me.


src/resource_provider/storage/provider.cpp
Lines 1069-1109 (patched)


Could we move this to some named function/lambda? I feel with this change 
the code here becomes too hard to follow.



src/resource_provider/storage/provider.cpp
Lines 1092 (patched)


Could you add a comment why this and the assertion in the continuation 
below are valid? The guaranteed execution paths of this `loop` with below 
`defer`s are not obvious to me.

Ideally we would not need this variable at all to ensure correct push-pop 
semantics, but defer to actor behavior and queues. This is hidden under very 
thick continuation layers at the moment.



src/resource_provider/storage/provider.cpp
Lines 1105 (patched)


Please use a temporary, this does not help readability.



src/resource_provider/storage/provider.cpp
Lines 1284 (patched)


It would be great if this function would either return a value, or trigger 
`fatal()` to make sure we do our part to keep the agent up-to-date.


- Benjamin Bannier


On April 12, 2018, 5:36 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65975/
> ---
> 
> (Updated April 12, 2018, 5:36 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The storage pools needs to be reconciled in the following two scenarios:
> 
> 1. When there is a change in the set of known profiles.
> 2. When a volume/block of an unknown profile is destroyed, because the
>disk space being freed up may belong to a known profile.
> 
> This patch adds a sequence to coordinate the reconciliations for the
> above two cases.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65975/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

[Benjamin Bannier]

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




src/resource_provider/storage/provider.cpp
Lines 1003 (patched)


Even thought an `auto` might make sense in such a place _in general_, due 
to the interface of `MapInfo` it leads to pretty dense code and maybe repeating 
a type here might make this easier to read. How about

// NOTE: this form requires a `using` decl above.
using Profile =
  MapPair;
  
foreach (const Profile& profile, storage.profiles()) {
  profileInfos.put(
  profile.first,
  {profile.second.capability(), profile.second.parameters()});
}



src/resource_provider/storage/provider.cpp
Line 1010 (original), 1011 (patched)


Nit: _pending operations_ should be specific enough as this list is neither 
exhaustive today nor will remain in the future.



src/resource_provider/storage/provider.cpp
Line 1020 (original), 1013 (patched)


Let's add a comment pointing out that this matches what is done and 
documented in some more detail in `checkpointResourceProviderState`.



src/resource_provider/storage/provider.cpp
Line 1021 (original), 1014 (patched)


Both `disk` and `profile` are `optional` fields. We should check explicitly 
whether they are set (otherwise we might at some point e.g., hit a failure 
where the profile `` (empty string) was not found.



src/resource_provider/storage/provider.cpp
Lines 3271 (patched)


This invariant seems to be pretty non-local :(



src/resource_provider/storage/provider.cpp
Lines 3276 (patched)


Should we capture this?

ProfileInfo& profile_ = *(storage->mutable_profiles())[profile];

No strong opinion, it seems hard to make interacting with proto maps nice.


- Benjamin Bannier


On April 12, 2018, 5:35 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65594/
> ---
> 
> (Updated April 12, 2018, 5:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SLRP now checkpoints profiles associated with storage pools, and does
> not depend on the `DiskProfileAdaptor` module to return the set of
> previously-known profiles during recovery.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65594/diff/8/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65974: Added comments and made some renaming in SLRP.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/resource_provider/storage/provider.cpp
Lines 352 (patched)


`s/reconfile/reconcile/`?


- Benjamin Bannier


On April 12, 2018, 5:33 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65974/
> ---
> 
> (Updated April 12, 2018, 5:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added comments and made some renaming in SLRP.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65974/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65995: Declined unwanted offers in `RetryOperationStatusUpdate*` SLRP tests.

[Benjamin Bannier]

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




src/tests/storage_local_resource_provider_tests.cpp
Lines 2573-2574 (patched)


I feel this leads to slightly involved state transitions below (e.g., 
requiring explicit clock manipulations to restore sane state).

How about introducing a

auto isNotRaw = [isRaw](const Resource& r) { return !isRaw(r); };

and then making dedicated declines,

EXPECT_CALL(sched, resourceOffers(, OffersHaveAnyResource(
std::bind(isNotRaw, lambda::_1
  .WillRepeatedly(DeclineOffers());
  
We should be able to remove the new clock manipulations then.



src/tests/storage_local_resource_provider_tests.cpp
Lines 2737-2738 (patched)


Ditto.


- Benjamin Bannier


On April 12, 2018, 5:31 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> ---
> 
> (Updated April 12, 2018, 5:31 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 2872f1aec1a7b94fc302a533f5ae9e1be9658087 
> 
> 
> Diff: https://reviews.apache.org/r/65995/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check
> Ran the two tests in repitition.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61096: Building gRPC with CMake.

[Benjamin Bannier]

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



This does currently not configure for me with `ENABLE_GRPC` (for the Ninja 
generator):

CMake Error at 3rdparty/CMakeLists.txt:177 (get_target_property):
   INTERFACE_LIBRARY targets may only have whitelisted properties.  The
   property "IMPORTED_LOCATION" is not allowed.
 Call Stack (most recent call first):
   3rdparty/CMakeLists.txt:1112 (GET_BYPRODUCTS)

- Benjamin Bannier


On April 20, 2018, 4:28 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61096/
> ---
> 
> (Updated April 20, 2018, 4:28 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-7881
> https://issues.apache.org/jira/browse/MESOS-7881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an `ENABLE_GRPC` option and builds the bundled gRPC
> 3rdparty library in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt e6bb4076e630d78e73a82a93b1f64b3bc5d859b9 
>   3rdparty/cmake/Versions.cmake 33577cc86d3ef31e129b8ea55d44d3fee02d4a30 
>   cmake/CompilationConfigure.cmake 173089a1ebf80a6288431075770a807235524ac1 
>   src/cmake/MesosProtobuf.cmake dde034fe4d4251dd7c7397d422136bfa3c9c3ebc 
> 
> 
> Diff: https://reviews.apache.org/r/61096/diff/9/
> 
> 
> Testing
> ---
> 
> `cmake .. -DENABLE_GRPC=1` then `cmake --build .` on both Linux and Windows.
> 
> NOTE: libprocess-tests will be failed to build on Windows with VS. This is 
> fixed in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66311: Set up recovery code paths of resource provider manager.

[Benjamin Bannier]

> On April 19, 2018, 8 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/registrar.cpp
> > Lines 193-203 (original), 194-203 (patched)
> > 
> >
> > How about moving this into `GenericRegistrarProcess::initialize()` to 
> > start recovery early?
> > 
> > If we do this and keep `recovered` (See below) then this function 
> > should return
> > ```
> > recovered->then([=] { return registry.get(); })
> > ```
> 
> Benjamin Bannier wrote:
> We already require users to `recover` registrars before being able to 
> perform operations against them (like for other registrars), so I am not 
> really sure I understand how what you suggest would help. Could you explain?
> 
> Chun-Hung Hsiao wrote:
> Oh what I mean is doing the following:
> ```
> virtual void initialize() override {
>   registry = ... // start the recovery.
> }
> 
> Future recover() {
>   return registry;
> }
> ```
> 
> The second part of my comment above is just illustrating the code if we 
> keep `registry` as an `Option` and `recovered` as a `Future`.

Thank you, that makes sense. I adjusted the code to track recovery status 
explicitly (`Future recovered`) which allows us a simple 
`Option registry`. I adjusted the code like that.


> On April 19, 2018, 8 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/registrar.cpp
> > Line 205 (original), 205 (patched)
> > 
> >
> > Why do we need the `undiscardable` here? Could you add some comments?
> > 
> > Also, should we prevent the recovery being discarded due to a caller 
> > discarding an `apply` call? If yes then we should do
> > ```
> > return undiscardable(registry.get()).then(... ::_apply ...);
> > ```
> > in the following `apply()` function.
> 
> Benjamin Bannier wrote:
> I added a comment and also fixed below `apply` to use `recover()` which 
> would return the cached result, already guarded by `undiscarable`.
> 
> Chun-Hung Hsiao wrote:
> I was condering that we could do `undiscardable` in `apply` so that the 
> caller can actually discard the recovery if they want to. Not sure if this is 
> a valid use case though. Please drop it if you don't think so.

I think we should allow users to cancel an already pending recovery. I believe 
this currently not only uninteresting to them, but would also complicate 
reasoning about internal state and invariants.

Marking as fixed.


> On April 19, 2018, 8 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/registrar.cpp
> > Line 216 (original), 216 (patched)
> > 
> >
> > The overall logic is correct, but it takes a bit of inference to know 
> > that overwriting `registry` with a new `Future` in an earlier `_apply` does 
> > not affect a later `_apply` that is chained with the overwritten `Future`. 
> > So it seems more readible to me if we keep the original 
> > `Option recovered` (or make it just a `Promise`), 
> > and chain `_apply` with `recovered` here.
> 
> Benjamin Bannier wrote:
> I replaced the direct access to `registry` with `recover` here which once 
> recovered would just serve a cached result. Does that look more reasonable to 
> you?
> 
> Chun-Hung Hsiao wrote:
> The thing that seems unintuitive to me is that `recover()` would return 
> different futures at different times. May I ask what the reason to make 
> `registry` a `Future` instead of an `Option`?

That's a very good point. I changed the code to always return the same registry 
now (we'll hold an `Option` instead of a `Future` now).


- Benjamin


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


On April 20, 2018, 12:23 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66311/
> ---
> 
> (Updated April 20, 2018, 12:23 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adjusts the control flow of the resource provider manager
> so that we can in the future make use of persisted resource provider
> information. While this patch sets up the needed flow, it does not
> implement recovery logic, yet.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.hpp 
> 

Re: Review Request 66311: Set up recovery code paths of resource provider manager.

[Benjamin Bannier]

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

(Updated April 20, 2018, 12:23 p.m.)


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


Changes
---

Addressed remaining issues from Chun.


Summary (updated)
-

Set up recovery code paths of resource provider manager.


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


Repository: mesos


Description (updated)
---

This patch adjusts the control flow of the resource provider manager
so that we can in the future make use of persisted resource provider
information. While this patch sets up the needed flow, it does not
implement recovery logic, yet.


Diffs (updated)
-

  src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
  src/resource_provider/registrar.hpp 39f45b0a2a7c35bfe72a9305f5248409e4a3ef45 
  src/resource_provider/registrar.cpp 92ef9aecb1e93d10f46e53984391558f9901a60b 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

[Benjamin Bannier]

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

(Updated April 20, 2018, 12:23 p.m.)


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


Changes
---

Added a comment as suggested by Chun.


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


Repository: mesos


Description
---

By delaying the construction of the agent's resource provider manager
we prepare for a following patch introducing a dependency on the
resource provider manager on the agent's ID.

Depending on whether the agent was able to recover an agent ID from
its log or still needs to obtain on in a first registration with the
master, we can only construct the resource provider manager after
different points in the initialization of the agent. To capture the
common code we introduce a helper function performing the necessary
steps.


Diffs (updated)
-

  src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
  src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66694: Updated the 1.6.0 CHANGELOG.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On April 18, 2018, 8:45 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66694/
> ---
> 
> (Updated April 18, 2018, 8:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, 
> Alexander Rojas, Benjamin Bannier, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the 1.6.0 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 09fff1d50504de158b6c77f51bd7968746e31371 
> 
> 
> Diff: https://reviews.apache.org/r/66694/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66577: Enabled CSI proto compilation by default.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/Makefile.am
Lines 1529-1536 (original), 1521-1525 (patched)


We can merge these two blocks now.


- Benjamin Bannier


On April 17, 2018, 5:03 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66577/
> ---
> 
> (Updated April 17, 2018, 5:03 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, and Jie Yu.
> 
> 
> Bugs: MESOS-8724
> https://issues.apache.org/jira/browse/MESOS-8724
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CSI proto compilation is disabled due to MESOS-8749, which is resolved
> by bumping CSI to v0.2. This patch enables the compilation again.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/Makefile.am 07eb13851967b8a28b46afa3304a9d4e298a5016 
>   src/slave/flags.cpp 4ea2286a6a874e6e7f473f97fb0431e17b19c1f8 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
> 
> 
> Diff: https://reviews.apache.org/r/66577/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66621: Add support for alg RS256 to JWT library.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66621]

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 19, 2018, 11:10 p.m., Clément Michaud wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
> 
> (Updated April 19, 2018, 11:10 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Bugs: MESOS-8788
> https://issues.apache.org/jira/browse/MESOS-8788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for alg RS256 to JWT library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/jwt.hpp 
> 768cbf6fa91537ff9f45f236f4033097c5cea959 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> b7cc31c33fd35c93754407f8b350eeb993177f1d 
>   3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 4d3727daf53ec62a19255da5a9804d342e770ec2 
>   3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> eb36a9aed3b11208c7cdc6f20b5347f46821a207 
> 
> 
> Diff: https://reviews.apache.org/r/66621/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> I added the same tests than the ones for HS256 (i.e., validation in following 
> cases: bad header, bad payload, unknown alg, unsupported alg, valid token 
> etc.. and creation of a valid token). I also added a test to verify that the 
> validation of a RS256 token fails when using the wrong public key.
> 
> 
> Thanks,
> 
> Clément Michaud
> 
>