Re: Review Request 65146: Windows: Enabled internet tests.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['65144', '65145', '65146']`

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

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

Relevant logs:

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

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

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

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

[--] Global test environment tear-down
[==] 856 tests from 86 test cases ran. (313684 ms total)
[  PASSED  ] 849 tests.
[  FAILED  ] 7 tests, listed below:
[  FAILED  ] 
HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaHTTPWithContainerImage
[  FAILED  ] 
HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaHTTPSWithContainerImage
[  FAILED  ] 
HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaTCPWithContainerImage
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchManifest
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchBlob
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 7 FAILED TESTS
  YOU HAVE 214 DISABLED TESTS

```

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

```
I0113 07:31:15.445353 10872 master.cpp:10161] Updating the state of task 
c71d6266-2180-42b3-a146-d555eadb593e of framework 
5bc91b29-d0e2-4f30-b31e-1ce1f239d52c- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0113 07:31:15.44I0113 07:31:14.777302  8796 exec.cpp:162] Version: 1.6.0
I0113 07:31:14.802307  9684 exec.cpp:236] Executor registered on agent 
5bc91b29-d0e2-4f30-b31e-1ce1f239d52c-S0
I0113 07:31:14.80530596 executor.cpp:171] Received SUBSCRIBED event
I0113 07:31:14.80930996 executor.cpp:175] Subscribed executor on 
build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0113 07:31:14.81030896 executor.cpp:171] Received LAUNCH event
I0113 07:31:14.81330896 executor.cpp:638] Starting task 
c71d6266-2180-42b3-a146-d555eadb593e
I0113 07:31:14.89131596 executor.cpp:478] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0113 07:31:15.42034796 executor.cpp:651] Forked command at 14972
I0113 07:31:15.447351 11288 exec.cpp:445] Executor asked to shutdown
I0113 07:31:15.44935096 executor.cpp:171] Received SHUTDOWN event
I0113 07:31:15.44935096 executor.cpp:748] Shutting down
I0113 07:31:15.44935096 executor.cpp:863] Sending SIGTERM to process tree 
at pid 5353 15208 slave.cpp:3388] Shutting down framework 
5bc91b29-d0e2-4f30-b31e-1ce1f239d52c-
I0113 07:31:15.446352 15208 slave.cpp:6066] Shutting down executor 
'c71d6266-2180-42b3-a146-d555eadb593e' of framework 
5bc91b29-d0e2-4f30-b31e-1ce1f239d52c- at executor(1)@10.3.1.11:55644
I0113 07:31:15.447351 15208 slave.cpp:931] Agent terminating
W0113 07:31:15.447351 15208 slave.cpp:3384] Ignoring shutdown framework 
5bc91b29-d0e2-4f30-b31e-1ce1f239d52c- because it is terminating
I0113 07:31:15.448351 10872 master.cpp:10265] Removing task 
c71d6266-2180-42b3-a146-d555eadb593e with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 5bc91b29-d0e2-4f30-b31e-1ce1f239d52c- on 
agent 5bc91b29-d0e2-4f30-b31e-1ce1f239d52c-S0 at slave(329)@10.3.1.11:55623 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0113 07:31:15.451349 10872 master.cpp:1306] Agent 
5bc91b29-d0e2-4f30-b31e-1ce1f239d52c-S0 at slave(329)@10.3.1.11:55623 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0113 07:31:15.451349  1532 containerizer.cpp:2352] Destroying container 
c1cba775-ed81-4651-9253-7f220b32eb2f in RUNNING state
I0113 07:31:15.451349 12296 hierarchical.cpp:344] Removed framework 
5bc91b29-d0e2-4f30-b31e-1ce1f239d52c-
I0113 07:31:15.451349 10872 master.cpp:3287] Disconnecting agent 
5bc91b29-d0e2-4f30-b31e-1ce1f239d52c-S0 at 

Re: Review Request 65147: Windows: Fixed docker executor `PATH` variable.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65147 was successfully built and tested.

Reviews applied: `['65144', '65145', '65147']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 10:31 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65147/
> ---
> 
> (Updated Jan. 12, 2018, 10:31 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8443
> https://issues.apache.org/jira/browse/MESOS-8443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `docker` isn't always installed in `os::host_default_path()` on Windows,
> so the docker executor cannot find it. Now, before launching the
> executor, the mesos agent finds `docker`'s parent directory and appends
> that to `PATH`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp b42fe1f4b7151ee5cb1d7c680c7c0237882bd4bb 
> 
> 
> Diff: https://reviews.apache.org/r/65147/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Windows 10, all passed.
> 
> Integration tested (courtsey of Akash) that Docker containers can be launched 
> without manually fixing the path.
> 
> `make check` on CentOS 7, all passed.
> 
> NOTE: This is Windows-only, and replaces 
> [#64570](https://reviews.apache.org/r/64570/). Joseph and I worked out this 
> approach to prevent a regression for Mesos customers moving from `1.4` to 
> `1.5`; see MESOS-8443 for details.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65149: MesosTidy: Enabled `mesos-this-capture`.

[Benjamin Bannier]

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


Fix it, then Ship it!





support/clang-tidy
Line 26 (original), 26 (patched)


We can just remove this line as `mesos-this-capture` is included in 
`mesos-*`.


- Benjamin Bannier


On Jan. 13, 2018, 6:32 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65149/
> ---
> 
> (Updated Jan. 13, 2018, 6:32 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MesosTidy: Enabled `mesos-this-capture`.
> 
> 
> Diffs
> -
> 
>   support/clang-tidy d68ddab6c796f1f583fb72ec06d6a91d845d1da9 
> 
> 
> Diff: https://reviews.apache.org/r/65149/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `support/mesos-tidy.sh` and confirmed that the false positives have 
> disappeared.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 65149: MesosTidy: Enabled `mesos-this-capture`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65149 was successfully built and tested.

Reviews applied: `['65149']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 9:32 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65149/
> ---
> 
> (Updated Jan. 12, 2018, 9:32 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MesosTidy: Enabled `mesos-this-capture`.
> 
> 
> Diffs
> -
> 
>   support/clang-tidy d68ddab6c796f1f583fb72ec06d6a91d845d1da9 
> 
> 
> Diff: https://reviews.apache.org/r/65149/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `support/mesos-tidy.sh` and confirmed that the false positives have 
> disappeared.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 65147: Windows: Fixed docker executor `PATH` variable.

[Andrew Schwartzmeyer]

> On Jan. 12, 2018, 6:51 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some Mesos tests failed.
> > 
> > Reviews applied: `['65144', '65145', '65146', '65147']`
> > 
> > Failed command: `D:\DCOS\mesos\src\mesos-tests.exe --verbose`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65147
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65147/logs/mesos-tests-stdout.log):
> > 
> > ```
> > [ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
> > [   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 
> > (36 ms)
> > [--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (68 
> > ms total)
> > 
> > [--] 1 test from IsolationFlag/CpuIsolatorTest
> > [ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
> > [   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2481 ms)
> > [--] 1 test from IsolationFlag/CpuIsolatorTest (2505 ms total)
> > 
> > [--] 1 test from IsolationFlag/MemoryIsolatorTest
> > [ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
> > [   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2379 ms)
> > [--] 1 test from IsolationFlag/MemoryIsolatorTest (2402 ms total)
> > 
> > [--] Global test environment tear-down
> > [==] 856 tests from 86 test cases ran. (309199 ms total)
> > [  PASSED  ] 849 tests.
> > [  FAILED  ] 7 tests, listed below:
> > [  FAILED  ] 
> > HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaHTTPWithContainerImage
> > [  FAILED  ] 
> > HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaHTTPSWithContainerImage
> > [  FAILED  ] 
> > HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaTCPWithContainerImage
> > [  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchManifest
> > [  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchBlob
> > [  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
> > [  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName
> > 
> >  7 FAILED TESTS
> >   YOU HAVE 214 DISABLED TESTS
> > 
> > ```
> > 
> > - 
> > [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65147/logs/mesos-tests-stderr.log):
> > 
> > ```
> > I0113 02:51:12.640440 10348 slave.cpp:3388] Shutting down frameI0113 
> > 02:51:11.954365  5936 exec.cpp:162] Version: 1.6.0
> > I0113 02:51:11.985371 15076 exec.cpp:236] Executor registered on agent 
> > e7aa8781-54a9-412d-bea4-00dcd5b00201-S0
> > I0113 02:51:11.988370 13828 executor.cpp:171] Received SUBSCRIBED event
> > I0113 02:51:11.993367 13828 executor.cpp:175] Subscribed executor on 
> > build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
> > I0113 02:51:11.993367 13828 executor.cpp:171] Received LAUNCH event
> > I0113 02:51:11.997367 13828 executor.cpp:638] Starting task 
> > ca210858-1bd1-442c-9903-2675f2028a00
> > I0113 02:51:12.076376 13828 executor.cpp:478] Running 
> > 'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
> > I0113 02:51:12.616439 13828 executor.cpp:651] Forked command at 10492
> > I0113 02:51:12.642441 10516 exec.cpp:445] Executor asked to shutdown
> > I0113 02:51:12.643440 13828 executor.cpp:171] Received SHUTDOWN event
> > I0113 02:51:12.643440 13828 executor.cpp:748] Shutting down
> > I0113 02:51:12.643440 13828 executor.cpp:863] Sending SIGTERM to process 
> > tree at pid work e7aa8781-54a9-412d-bea4-00dcd5b00201-
> > I0113 02:51:12.640440  1248 master.cpp:10161] Updating the state of task 
> > ca210858-1bd1-442c-9903-2675f2028a00 of framework 
> > e7aa8781-54a9-412d-bea4-00dcd5b00201- (latest state: TASK_KILLED, 
> > status update state: TASK_KILLED)
> > I0113 02:51:12.641441 10348 slave.cpp:6066] Shutting down executor 
> > 'ca210858-1bd1-442c-9903-2675f2028a00' of framework 
> > e7aa8781-54a9-412d-bea4-00dcd5b00201- at executor(1)@10.3.1.11:50065
> > I0113 02:51:12.642441 10348 slave.cpp:931] Agent terminating
> > W0113 02:51:12.642441 10348 slave.cpp:3384] Ignoring shutdown framework 
> > e7aa8781-54a9-412d-bea4-00dcd5b00201- because it is terminating
> > I0113 02:51:12.643440  1248 master.cpp:10265] Removing task 
> > ca210858-1bd1-442c-9903-2675f2028a00 with resources cpus(allocated: *):4; 
> > mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
> > *):[31000-32000] of framework e7aa8781-54a9-412d-bea4-00dcd5b00201- on 
> > agent e7aa8781-54a9-412d-bea4-00dcd5b00201-S0 at slave(329)@10.3.1.11:50044 
> > (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
> > I0113 02:51:12.645442 10348 containerizer.cpp:2352] Destroying container 
> > 5e1f835c-6470-42e4-98cb-6a4beaf21b3b in RUNNING state
> > I0113 02:51:12.645442 10348 containerizer.cpp:2966] Transitioning the state 
> > of container 5e1f835c-6470-42e4-98cb-6a4beaf21b3b from RUNNING to DESTROYING
> > I0113 02:51:12.646443  1248 master.cpp:1306] Agent 
> > 

Re: Review Request 65147: Windows: Fixed docker executor `PATH` variable.

[Andrew Schwartzmeyer]

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

(Updated Jan. 12, 2018, 10:31 p.m.)


Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


Changes
---

Skip the enable `INTERNET` filter tests because they fail on the bot, and 
aren't necessary.


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


Repository: mesos


Description
---

`docker` isn't always installed in `os::host_default_path()` on Windows,
so the docker executor cannot find it. Now, before launching the
executor, the mesos agent finds `docker`'s parent directory and appends
that to `PATH`.


Diffs
-

  src/slave/containerizer/docker.cpp b42fe1f4b7151ee5cb1d7c680c7c0237882bd4bb 


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


Testing
---

Tested on Windows 10, all passed.

Integration tested (courtsey of Akash) that Docker containers can be launched 
without manually fixing the path.

`make check` on CentOS 7, all passed.

NOTE: This is Windows-only, and replaces 
[#64570](https://reviews.apache.org/r/64570/). Joseph and I worked out this 
approach to prevent a regression for Mesos customers moving from `1.4` to 
`1.5`; see MESOS-8443 for details.


Thanks,

Andrew Schwartzmeyer

Review Request 65149: MesosTidy: Enabled `mesos-this-capture`.

[Michael Park]

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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

MesosTidy: Enabled `mesos-this-capture`.


Diffs
-

  support/clang-tidy d68ddab6c796f1f583fb72ec06d6a91d845d1da9 


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


Testing
---

Ran `support/mesos-tidy.sh` and confirmed that the false positives have 
disappeared.


Thanks,

Michael Park

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65137 was successfully built and tested.

Reviews applied: `['65137']`

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

- Mesos Reviewbot Windows


On Jan. 13, 2018, 2:29 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65137/
> ---
> 
> (Updated Jan. 13, 2018, 2:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds some logic to deduplicate the status updates that are
> added to `Operation::statuses`. The logic is consistent with the
> handling of task status updates, but we should revisit it in MESOS-8441.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 
> 
> 
> Diff: https://reviews.apache.org/r/65137/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh` on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65147: Windows: Fixed docker executor `PATH` variable.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['65144', '65145', '65146', '65147']`

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

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

Relevant logs:

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

```
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (36 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (68 ms 
total)

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

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

[--] Global test environment tear-down
[==] 856 tests from 86 test cases ran. (309199 ms total)
[  PASSED  ] 849 tests.
[  FAILED  ] 7 tests, listed below:
[  FAILED  ] 
HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaHTTPWithContainerImage
[  FAILED  ] 
HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaHTTPSWithContainerImage
[  FAILED  ] 
HealthCheckTest.ROOT_INTERNET_CURL_HealthyTaskViaTCPWithContainerImage
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchManifest
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchBlob
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 7 FAILED TESTS
  YOU HAVE 214 DISABLED TESTS

```

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

```
I0113 02:51:12.640440 10348 slave.cpp:3388] Shutting down frameI0113 
02:51:11.954365  5936 exec.cpp:162] Version: 1.6.0
I0113 02:51:11.985371 15076 exec.cpp:236] Executor registered on agent 
e7aa8781-54a9-412d-bea4-00dcd5b00201-S0
I0113 02:51:11.988370 13828 executor.cpp:171] Received SUBSCRIBED event
I0113 02:51:11.993367 13828 executor.cpp:175] Subscribed executor on 
build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0113 02:51:11.993367 13828 executor.cpp:171] Received LAUNCH event
I0113 02:51:11.997367 13828 executor.cpp:638] Starting task 
ca210858-1bd1-442c-9903-2675f2028a00
I0113 02:51:12.076376 13828 executor.cpp:478] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0113 02:51:12.616439 13828 executor.cpp:651] Forked command at 10492
I0113 02:51:12.642441 10516 exec.cpp:445] Executor asked to shutdown
I0113 02:51:12.643440 13828 executor.cpp:171] Received SHUTDOWN event
I0113 02:51:12.643440 13828 executor.cpp:748] Shutting down
I0113 02:51:12.643440 13828 executor.cpp:863] Sending SIGTERM to process tree 
at pid work e7aa8781-54a9-412d-bea4-00dcd5b00201-
I0113 02:51:12.640440  1248 master.cpp:10161] Updating the state of task 
ca210858-1bd1-442c-9903-2675f2028a00 of framework 
e7aa8781-54a9-412d-bea4-00dcd5b00201- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0113 02:51:12.641441 10348 slave.cpp:6066] Shutting down executor 
'ca210858-1bd1-442c-9903-2675f2028a00' of framework 
e7aa8781-54a9-412d-bea4-00dcd5b00201- at executor(1)@10.3.1.11:50065
I0113 02:51:12.642441 10348 slave.cpp:931] Agent terminating
W0113 02:51:12.642441 10348 slave.cpp:3384] Ignoring shutdown framework 
e7aa8781-54a9-412d-bea4-00dcd5b00201- because it is terminating
I0113 02:51:12.643440  1248 master.cpp:10265] Removing task 
ca210858-1bd1-442c-9903-2675f2028a00 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework e7aa8781-54a9-412d-bea4-00dcd5b00201- on 
agent e7aa8781-54a9-412d-bea4-00dcd5b00201-S0 at slave(329)@10.3.1.11:50044 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0113 02:51:12.645442 10348 containerizer.cpp:2352] Destroying container 
5e1f835c-6470-42e4-98cb-6a4beaf21b3b in RUNNING state
I0113 02:51:12.645442 10348 containerizer.cpp:2966] Transitioning the state of 
container 5e1f835c-6470-42e4-98cb-6a4beaf21b3b from RUNNING to DESTROYING
I0113 02:51:12.646443  1248 master.cpp:1306] Agent 
e7aa8781-54a9-412d-bea4-00dcd5b00201-S0 at slave(329)@10.3.1.11:50044 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0113 02:51:12.646443  1248 master.cpp:3287] Disconnecting agent 

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65056 was successfully built and tested.

Reviews applied: `['65056']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 5:56 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65056/
> ---
> 
> (Updated Jan. 12, 2018, 5:56 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8078
> https://issues.apache.org/jira/browse/MESOS-8078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Several fields were present in the old master '/state' endpoint,
> which are now not accessible via the V1 operator API. This patch
> adds these fields to the response for the GET_MASTER call.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
>   include/mesos/v1/master/master.proto 
> 5cb64df12459b1529306cd33691c003172e4b3ec 
>   src/master/http.cpp 0c2cd554124ee400c05f3256af36974a62e2303c 
>   src/tests/api_tests.cpp 28d46436b9c2fe85bde5bc8def7a840b72d29de3 
> 
> 
> Diff: https://reviews.apache.org/r/65056/diff/4/
> 
> 
> Testing
> ---
> 
> tested manually using curl
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

[Gaston Kleiman]

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

(Updated Jan. 12, 2018, 6:29 p.m.)


Review request for mesos, Benjamin Bannier and Greg Mann.


Changes
---

Addressed comment.


Repository: mesos


Description
---

This patch adds some logic to deduplicate the status updates that are
added to `Operation::statuses`. The logic is consistent with the
handling of task status updates, but we should revisit it in MESOS-8441.


Diffs (updated)
-

  src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 


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

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


Testing
---

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


Thanks,

Gaston Kleiman

Re: Review Request 61158: Introduced http::Server in process.cpp.

[Benjamin Mahler]

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


Ship it!




Can we clarify why we ifdef guarded it? Specifically only to ensure that 
there's no performance regression before removing the HttpProxy, right?


3rdparty/libprocess/src/process.cpp
Lines 1239 (patched)


s/incomping/incoming/



3rdparty/libprocess/src/process.cpp
Lines 1244 (patched)


CHECK_EQ?



3rdparty/libprocess/src/process.cpp
Lines 1262 (patched)


http server?


- Benjamin Mahler


On Nov. 26, 2017, 10:28 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61158/
> ---
> 
> (Updated Nov. 26, 2017, 10:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Avinash sridharan, and 
> Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Using http::Server can be compile time configured via USE_HTTP_SERVER.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 75cf1d3b6d3d257ba9bc81c68017a74a6511cebf 
> 
> 
> Diff: https://reviews.apache.org/r/61158/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 13, 2018, 1:56 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65056/
> ---
> 
> (Updated Jan. 13, 2018, 1:56 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8078
> https://issues.apache.org/jira/browse/MESOS-8078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Several fields were present in the old master '/state' endpoint,
> which are now not accessible via the V1 operator API. This patch
> adds these fields to the response for the GET_MASTER call.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
>   include/mesos/v1/master/master.proto 
> 5cb64df12459b1529306cd33691c003172e4b3ec 
>   src/master/http.cpp 0c2cd554124ee400c05f3256af36974a62e2303c 
>   src/tests/api_tests.cpp 28d46436b9c2fe85bde5bc8def7a840b72d29de3 
> 
> 
> Diff: https://reviews.apache.org/r/65056/diff/4/
> 
> 
> Testing
> ---
> 
> tested manually using curl
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

[Greg Mann]

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




src/master/master.cpp
Lines 10358-10359 (patched)


Whoops! Looks like we won't actually push the first status onto the list as 
written. Should probably be:
```
  if (operation->statuses().empty() ||
  *(operation->statuses().rbegin()) != status) {
```


- Greg Mann


On Jan. 13, 2018, 12:28 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65137/
> ---
> 
> (Updated Jan. 13, 2018, 12:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds some logic to deduplicate the status updates that are
> added to `Operation::statuses`. The logic is consistent with the
> handling of task status updates, but we should revisit it in MESOS-8441.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 
> 
> 
> Diff: https://reviews.apache.org/r/65137/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh` on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Greg Mann]

> On Jan. 13, 2018, 1:14 a.m., Vinod Kone wrote:
> > Can you update the `GetMaster` test in api_tests.cpp?

Yep, shoulda done this in the first place - thx! Test updated.


- Greg


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


On Jan. 13, 2018, 1:56 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65056/
> ---
> 
> (Updated Jan. 13, 2018, 1:56 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8078
> https://issues.apache.org/jira/browse/MESOS-8078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Several fields were present in the old master '/state' endpoint,
> which are now not accessible via the V1 operator API. This patch
> adds these fields to the response for the GET_MASTER call.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
>   include/mesos/v1/master/master.proto 
> 5cb64df12459b1529306cd33691c003172e4b3ec 
>   src/master/http.cpp 0c2cd554124ee400c05f3256af36974a62e2303c 
>   src/tests/api_tests.cpp 28d46436b9c2fe85bde5bc8def7a840b72d29de3 
> 
> 
> Diff: https://reviews.apache.org/r/65056/diff/4/
> 
> 
> Testing
> ---
> 
> tested manually using curl
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Greg Mann]

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

(Updated Jan. 13, 2018, 1:56 a.m.)


Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.


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


Repository: mesos


Description
---

Several fields were present in the old master '/state' endpoint,
which are now not accessible via the V1 operator API. This patch
adds these fields to the response for the GET_MASTER call.


Diffs (updated)
-

  include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
  include/mesos/v1/master/master.proto 5cb64df12459b1529306cd33691c003172e4b3ec 
  src/master/http.cpp 0c2cd554124ee400c05f3256af36974a62e2303c 
  src/tests/api_tests.cpp 28d46436b9c2fe85bde5bc8def7a840b72d29de3 


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

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


Testing
---

tested manually using curl


Thanks,

Greg Mann

Re: Review Request 65063: Replace `stringify()` with `jsonify()` in http serialization.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 9, 2018, 11:06 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65063/
> ---
> 
> (Updated Jan. 9, 2018, 11:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace `stringify()` with `jsonify()` in http serialization.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 3ada1f064b4bd001cd4d7dccc186641f475011a0 
> 
> 
> Diff: https://reviews.apache.org/r/65063/diff/1/
> 
> 
> Testing
> ---
> 
> Running on MacBook Pro with 3.3 GHz Intel Core i7, built with 
> `--enable-optimize`:
> 
> With `stringify()`:
> 
> Test setup: 2000 agents with a total of 10 running tasks and 10 
> completed tasks.
> Getstate application/json query response took 2.71121739318333mins
> 
> Test setup: 2000 agents with a total of 20 running tasks and 0 completed 
> tasks.
> Getstate application/json query response took 5.3197060575mins
> 
> Test setup: 2 agents with a total of 10 running tasks and 0 completed 
> tasks.
> Getstate application/json query response took 3.18027318681667mins
> 
> ---
> 
> With `jsonify()`:
> 
> Test setup: 2000 agents with a total of 10 running tasks and 10 
> completed tasks.
> Getstate application/json query response took 2.10034958736667mins
> 
> Test setup: 2000 agents with a total of 20 running tasks and 0 completed 
> tasks.
> Getstate application/json query response took 3.38100704925mins
> 
> Test setup: 2 agents with a total of 10 running tasks and 0 completed 
> tasks.
> Getstate application/json query response took 2.2140585963mins
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['65093', '65095', '65096', '65072', '65137']`

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

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

Relevant logs:

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

```
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/0 (4 
ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/1 (4 
ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/0
[   OK ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/0 (10 
ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/1
[   OK ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/1 (14 
ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/0
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/0 (8 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/1 (11 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/0
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/0 (16 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/1 (15 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/0
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/0 (11 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/1 (16 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/0
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/0 
(9 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/1 
(13 ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/0
[   OK ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/0 
(152 ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/1
[   OK ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/1 
(153 ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.ConvertResources/0
D:\DCOS\mesos\mesos\src\tests\resource_provider_manager_tests.cpp(1041): error: 
Failed to wait 15secs for offers2
```

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

```
@   7FF649D1CC31  google::LogMessageFatal::~LogMessageFatal
@   7FF6474E829B  mesos::internal::master::Master::updateOperationStatus
@   7FF6476501DF  
ProtobufProcess::_handlerM
@   7FF6475DAD59  __cdecl*& __ptr64)(mesos::internal::master::Master * 
__ptr64,void (__cdecl 
mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage
 const & __ptr64) __ptr64,process::UPID const & 
__ptr64,std::basic_string

Re: Review Request 64909: Removed unnecessary `SocketManager::dispose` data structure.

[Benjamin Mahler]

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



Looks good other than the looping over map values.


3rdparty/libprocess/src/process.cpp
Lines 1919 (patched)


Yikes, this is scary. Why did you need this guard?

It looks like we can do:

```
Option address = addresses.get(s);
CHECK_SOME(address);

temps.erase(address.get()); // Might be no-op, but ok.
// Or:
// if (temps.contains(address.get())) {
//   temps.erase(address.get());
// }
```

Now that we only have the "outbound" case, `addresses` should contain `s` 
iff `sockets` contains s, no?


- Benjamin Mahler


On Jan. 3, 2018, 7:21 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64909/
> ---
> 
> (Updated Jan. 3, 2018, 7:21 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We used `dispose` to capture the sockets that `HttpProxy` did not want
> to persist but now that `HttpProxy` does not use `SocketManager` we no
> longer need to use `dispose` because `temps` is sufficient for knowing
> which sockets are temporary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 75cf1d3b6d3d257ba9bc81c68017a74a6511cebf 
>   3rdparty/libprocess/src/socket_manager.hpp 
> dd4d111c4ae579420060e547dd12f8f0711c 
> 
> 
> Diff: https://reviews.apache.org/r/64909/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 64908: Refactored `HttpProxy` to not rely on `SocketManager`.

[Benjamin Mahler]

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


Fix it, then Ship it!




Looks good, just looks like the comments at the bottom of the socket manager 
need to be updated to reflect the removal of the "inbound" socket tracking.


3rdparty/libprocess/src/http_proxy.hpp
Lines 86-88 (patched)


None is EOF?



3rdparty/libprocess/src/http_proxy.cpp
Lines 57 (patched)


It took me awhile to figure out where this was coming from, wonder if it 
should be within an `encoder` namespace.



3rdparty/libprocess/src/http_proxy.cpp
Lines 63-67 (patched)


I wonder if we should log the failure case here, although not sure if we 
have a sufficient message in the future.



3rdparty/libprocess/src/http_proxy.cpp
Lines 75-85 (patched)


We can do this later, but just FYI this message has thrown off users for 
some time, in the case that it comes out from a socket already being closed. I 
wish we could check for that case and ignore it. We already have `SocketError`, 
would just need to wire it through.



3rdparty/libprocess/src/http_proxy.cpp
Lines 330 (patched)


Should this take an Owned to clarify that the ownership is passed in?



3rdparty/libprocess/src/http_proxy.cpp
Lines 339 (patched)


Can we move the response in? Looks like we copy one unnecessary time from 
the caller into the encoder here?



3rdparty/libprocess/src/http_proxy.cpp
Lines 346 (patched)


You can use .at here if you like, although what I don't like about it is it 
throws an exception rather than aborting:

```
if (response.headers.at("Connection") == "close") {
```



3rdparty/libprocess/src/process.cpp
Lines 1268-1274 (original), 1274-1279 (patched)


Huh.. I guess the "inbound" sockets get closed now via the HttpProxies 
being terminated above in the process manager finalization, nice.



3rdparty/libprocess/src/socket_manager.hpp
Lines 124-147 (original), 100-123 (patched)


My understanding here is that before we stored both "inbound" (client 
connected to our server) and "outbound" (we connected to the client's server) 
scokets, and the comments here reflect that.

Now, we only store the "outbound" case, so it seems like these comments 
need an update to reflect that?


- Benjamin Mahler


On Jan. 3, 2018, 7:20 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64908/
> ---
> 
> (Updated Jan. 3, 2018, 7:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This simplifies the introduction of `http::Server` so that it's easier
> to reason about and in the future will make removing the `HttpProxy`
> implementation much easier.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http_proxy.hpp 
> 5b6e7e8786ed9eab50cd4c2cfdec455c92d72eca 
>   3rdparty/libprocess/src/http_proxy.cpp 
> f584238aadd86875d7c87736653f394e716397de 
>   3rdparty/libprocess/src/process.cpp 
> 75cf1d3b6d3d257ba9bc81c68017a74a6511cebf 
>   3rdparty/libprocess/src/socket_manager.hpp 
> dd4d111c4ae579420060e547dd12f8f0711c 
> 
> 
> Diff: https://reviews.apache.org/r/64908/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Review Request 65147: Windows: Fixed docker executor `PATH` variable.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

`docker` isn't always installed in `os::host_default_path()` on Windows,
so the docker executor cannot find it. Now, before launching the
executor, the mesos agent finds `docker`'s parent directory and appends
that to `PATH`.


Diffs
-

  src/slave/containerizer/docker.cpp b42fe1f4b7151ee5cb1d7c680c7c0237882bd4bb 


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


Testing
---

Tested on Windows 10, all passed.

Integration tested (courtsey of Akash) that Docker containers can be launched 
without manually fixing the path.

`make check` on CentOS 7 (results pending, prior build with `os::which` changes 
passed tests)


Thanks,

Andrew Schwartzmeyer

Review Request 65146: Windows: Enabled internet tests.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

The `ping` command's interface is different on Windows, but the
operation is the same: one ping, with a one second timeout.


Diffs
-

  src/tests/environment.cpp 72bd621f02f97ea5fd553f3dc0bd52adb8ddee8f 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 65144: Ported `os::which` to Windows.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Because `os::which` still lived in `posix/os.hpp`, it was refactored
into its own `os/which.hpp` (which the respective `os/posix/which.hpp`
and `os/windows/which.hpp`. Consumers of this will need additionally
include the new header, instead of just `os.hpp`.

The differences in implementation from POSIX to Windows are:

* Split the `PATH` on `;` not `:`.
* Also loop over `PATHEXT` because executables on Windows end with one
  of a set of extensions.
* Removed the permissions check because it is not applicable on
  Windows (instead, an executable ends with an extension in `PATHEXT`,
  see above).


Diffs
-

  3rdparty/stout/include/Makefile.am e5f11048af4ced8e8312d5c77fc3dde94f8e7012 
  3rdparty/stout/include/stout/os/posix/which.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/which.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/which.hpp PRE-CREATION 
  3rdparty/stout/include/stout/posix/os.hpp 
7427bd792368fbacafb23593dfb9213618fdcddf 
  3rdparty/stout/tests/os_tests.cpp de4107718cfa45f3275693a7dd59b2f52b0ced7d 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 65145: Fixed use of `os::which`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Because `os::which` was added to its own header, all uses of it now need
to include said header.

In `tests/environment.cpp`, instead of using `os::system("which foo")`,
we now use `os::which("foo")` to be compatible with Windows.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
ba9e20c16841bfaa2a5c72d449a2da1a637b71df 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
61de16ba7f1b0ba80663a8544baa98fe36302df0 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 de64d6572c59da22dff528b308b4a4b0e9be9d2f 
  src/slave/containerizer/mesos/launch.cpp 
c45a038f191d7ddc536bb1ffc58532df90aff153 
  src/tests/environment.cpp 72bd621f02f97ea5fd553f3dc0bd52adb8ddee8f 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Vinod Kone]

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



Can you update the `GetMaster` test in api_tests.cpp?


include/mesos/v1/master/master.proto
Lines 468-472 (patched)


I would get rid of these for now. I would like this to be inferred from 
GetAgents instead of having it at 2 different places.



src/master/http.cpp
Lines 2274-2281 (patched)


i would put group these as:

```
  getMaster->mutable_master_info()->CopyFrom(master->info());
  
  getMaster->set_start_time(master->startTime.secs());
  if (master->electedTime.isSome()) {
getMaster->set_elected_time(master->electedTime.get().secs());
  }
  
  getMaster->set_activated_agents(master->_slaves_active());
  getMaster->set_deactivated_agents(master->_slaves_inactive());
  getMaster->set_unreachable_agents(master->_slaves_unreachable());
  

```

if you kill the `slaves_*` per comment above, i'll leave it upto you 
whether you want the space or not.


- Vinod Kone


On Jan. 13, 2018, 12:11 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65056/
> ---
> 
> (Updated Jan. 13, 2018, 12:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8078
> https://issues.apache.org/jira/browse/MESOS-8078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Several fields were present in the old master '/state' endpoint,
> which are now not accessible via the V1 operator API. This patch
> adds these fields to the response for the GET_MASTER call.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
>   include/mesos/v1/master/master.proto 
> 5cb64df12459b1529306cd33691c003172e4b3ec 
>   src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 
> 
> 
> Diff: https://reviews.apache.org/r/65056/diff/3/
> 
> 
> Testing
> ---
> 
> tested manually using curl
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Jan. 13, 2018, 12:28 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65137/
> ---
> 
> (Updated Jan. 13, 2018, 12:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds some logic to deduplicate the status updates that are
> added to `Operation::statuses`. The logic is consistent with the
> handling of task status updates, but we should revisit it in MESOS-8441.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 
> 
> 
> Diff: https://reviews.apache.org/r/65137/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh` on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65143: Logged some additional information on a master CHECK.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Jan. 12, 2018, 4:44 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65143/
> ---
> 
> (Updated Jan. 12, 2018, 4:44 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the master attempts to remove an unreachable task on a registered
> agent, log some additional debugging information in the CHECK to help
> track down what happened.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 
> 
> 
> Diff: https://reviews.apache.org/r/65143/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 65143: Logged some additional information on a master CHECK.

[James Peach]

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

Review request for mesos and Andrew Schwartzmeyer.


Repository: mesos


Description
---

If the master attempts to remove an unreachable task on a registered
agent, log some additional debugging information in the CHECK to help
track down what happened.


Diffs
-

  src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 


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


Testing
---

make check (Fedora 27)


Thanks,

James Peach

Re: Review Request 65141: Fixed the default executor flaky testes in tests/cluster.cpp.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 13, 2018, 12:15 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65141/
> ---
> 
> (Updated Jan. 13, 2018, 12:15 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin 
> Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7506
> https://issues.apache.org/jira/browse/MESOS-7506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes some flaky tests listed below:
> 1. DefaultExecutorTest.KillTask/0
> 2. DefaultExecutorTest.TaskWithFileURI/0
> 3. DefaultExecutorTest.ResourceLimitation/0
> 4. DefaultExecutorTest.KillMultipleTasks/0
> 
> The root cause is that either docker containerizer or mesos
> containerizer have wait() and destroy() rely on the same
> future `ContainerTermination` which means these two methods
> become ready simultaneously, but this is not true for the
> composing containerizer because wait() may finish before
> destroy in which case the `containers_` hasshmap is not
> cleaned up yet in destroy()'s `.onAny` callback.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 066dd31a7b5b49823ca642dfcc930f00e920feb3 
> 
> 
> Diff: https://reviews.apache.org/r/65141/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 65140: Reverted "Updated composing containerizer tests.".

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 13, 2018, 12:15 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65140/
> ---
> 
> (Updated Jan. 13, 2018, 12:15 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin 
> Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8391
> https://issues.apache.org/jira/browse/MESOS-8391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reverts commit 84365a140c3730e2d6579ad500118d6749d2f87f.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> 7c22f162b128c3fdf8d4b20cac73fdf442449d79 
> 
> 
> Diff: https://reviews.apache.org/r/65140/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 65139: Reverted "Fixed `wait()` and `destroy()` in composing containerizer.".

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 13, 2018, 12:15 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65139/
> ---
> 
> (Updated Jan. 13, 2018, 12:15 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin 
> Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8391
> https://issues.apache.org/jira/browse/MESOS-8391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reverts commit 95decd404438abd422794524e01d72a889821566.
> 
> There are two reasons to revert this commit:
>   1. After the agent recovers, if the nested containers that are
>  launched beforehand are killed, they will no longer be updated
>  with new status, because the `WAIT_NESTED_CONTAINER` call from
>  the default executor will end up with a future forever. Please
>  see MESOS-8391 for details.
>   2. The original commit makes the composing containerizer wait()
>  and destroy() rely on the same future of a ContainerTermination
>  promise. This would get into the bug that composing containerizer
>  destroy() may fail due to the wait() future got discarded.
>  Need to protect it by using `undiscardable()`. Please see
>  MESOS-7926 for details.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 9ace70d9fbd78182715c5ef13fcaf7ad45f76f97 
> 
> 
> Diff: https://reviews.apache.org/r/65139/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 64907: Added abandonment and process exit support to `loop()`.

[Benjamin Mahler]

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



Thanks for fixing this:

(1) There seems to be an issue with the Waiter not terminating in the case that 
the caller passes their own Process for the loop's execution context?

(2) Longer term, the way abandonement needed to be implemented looks 
unfortunate. Ideally we could have something like "onAny" that includes 
abandonement so that the new code here would have been alongside the other 
isReady/isFailed/isDiscarded handling rather than having to be far away from it.


3rdparty/libprocess/include/process/loop.hpp
Lines 363 (patched)


Need to terminate the waiter here? Ditto other reset cases.



3rdparty/libprocess/include/process/loop.hpp
Lines 360-375 (original), 370-390 (patched)


The existing and newly added `if (self) {` blocks seem to contain a lot of 
code and add nesting. Maybe this would all be a bit flatter and easier to read 
if we flipped it?

```
if (!self) { return; }

...
```



3rdparty/libprocess/include/process/loop.hpp
Lines 378-381 (original), 393-415 (patched)


Well, does a ternary work within the onAny?
Or can you pull the onAbandoned cases into a single one after the if 
condition? Ditto below.



3rdparty/libprocess/include/process/loop.hpp
Lines 466-471 (patched)


This is where an updated "onAny" that has abandonment would simplify 
things, no need for all these extra .onAbandoned cases here and instead they 
would go alongside the isReady/isFailed/isDiscarded cases.



3rdparty/libprocess/include/process/loop.hpp
Lines 496 (patched)


Maybe a little comment here saying that the waiter will ensure the loop is 
destroyed if the execution process is terminated?

Also, looking at the code, won't we leak a Waiter if the caller provided 
their own process for the execution context of the loop? Seems like we need to 
terminate the waiter whenever we Break from the loop? (i.e. all the 
`self->reset()` cases above).


- Benjamin Mahler


On Jan. 3, 2018, 7:20 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64907/
> ---
> 
> (Updated Jan. 3, 2018, 7:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current semantics for `loop()` mean that we'll leak resources
> under two different circumstances:
> 
>   (1) If either the "iteration" or "body" functions return future's
>   that are abandoned.
> 
>   (2) If the loop is using a process that exits.
> 
> This patch adds support to make sure that abandoned futures are
> properly propagated in the event that either of these situations
> occurs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/loop.hpp 
> 69d90f3e1b16189e0b1a6f1981e8509c18b38465 
>   3rdparty/libprocess/src/tests/loop_tests.cpp 
> 8d1837a0baedc12591f92c8f0f8ea83d0aa44ab0 
> 
> 
> Diff: https://reviews.apache.org/r/64907/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 65104: Updated documentation for `d_type` check regarding `xfs` on `overlay`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Jan. 11, 2018, 5:13 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65104/
> ---
> 
> (Updated Jan. 11, 2018, 5:13 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated documentation for `d_type` check regarding `xfs` on `overlay`.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 1112bd7c1759e0afacf7d9536749f2268be4c348 
> 
> 
> Diff: https://reviews.apache.org/r/65104/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 65141: Fixed the default executor flaky testes in tests/cluster.cpp.

[Gilbert Song]

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

Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin Mahler, 
Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

This patch fixes some flaky tests listed below:
1. DefaultExecutorTest.KillTask/0
2. DefaultExecutorTest.TaskWithFileURI/0
3. DefaultExecutorTest.ResourceLimitation/0
4. DefaultExecutorTest.KillMultipleTasks/0

The root cause is that either docker containerizer or mesos
containerizer have wait() and destroy() rely on the same
future `ContainerTermination` which means these two methods
become ready simultaneously, but this is not true for the
composing containerizer because wait() may finish before
destroy in which case the `containers_` hasshmap is not
cleaned up yet in destroy()'s `.onAny` callback.


Diffs
-

  src/tests/cluster.cpp 066dd31a7b5b49823ca642dfcc930f00e920feb3 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 65139: Reverted "Fixed `wait()` and `destroy()` in composing containerizer.".

[Gilbert Song]

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

Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin Mahler, 
Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

This reverts commit 95decd404438abd422794524e01d72a889821566.

There are two reasons to revert this commit:
  1. After the agent recovers, if the nested containers that are
 launched beforehand are killed, they will no longer be updated
 with new status, because the `WAIT_NESTED_CONTAINER` call from
 the default executor will end up with a future forever. Please
 see MESOS-8391 for details.
  2. The original commit makes the composing containerizer wait()
 and destroy() rely on the same future of a ContainerTermination
 promise. This would get into the bug that composing containerizer
 destroy() may fail due to the wait() future got discarded.
 Need to protect it by using `undiscardable()`. Please see
 MESOS-7926 for details.


Diffs
-

  src/slave/containerizer/composing.cpp 
9ace70d9fbd78182715c5ef13fcaf7ad45f76f97 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 65140: Reverted "Updated composing containerizer tests.".

[Gilbert Song]

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

Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin Mahler, 
Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

This reverts commit 84365a140c3730e2d6579ad500118d6749d2f87f.


Diffs
-

  src/tests/containerizer/composing_containerizer_tests.cpp 
7c22f162b128c3fdf8d4b20cac73fdf442449d79 


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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Greg Mann]

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

(Updated Jan. 13, 2018, 12:11 a.m.)


Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Several fields were present in the old master '/state' endpoint,
which are now not accessible via the V1 operator API. This patch
adds these fields to the response for the GET_MASTER call.


Diffs
-

  include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
  include/mesos/v1/master/master.proto 5cb64df12459b1529306cd33691c003172e4b3ec 
  src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 


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


Testing
---

tested manually using curl


Thanks,

Greg Mann

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65056 was successfully built and tested.

Reviews applied: `['65056']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 11:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65056/
> ---
> 
> (Updated Jan. 12, 2018, 11:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8078
> https://issues.apache.org/jira/browse/MESOS-8078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added missing fields to the GET_MASTER operator API call.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
>   include/mesos/v1/master/master.proto 
> 5cb64df12459b1529306cd33691c003172e4b3ec 
>   src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 
> 
> 
> Diff: https://reviews.apache.org/r/65056/diff/3/
> 
> 
> Testing
> ---
> 
> tested manually using curl
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

[Gaston Kleiman]

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

Review request for mesos, Benjamin Bannier and Greg Mann.


Repository: mesos


Description
---

This patch adds some logic to deduplicate the status updates that are
added to `Operation::statuses`. The logic is consistent with the
handling of task status updates, but we should revisit it in MESOS-8441.


Diffs
-

  src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 


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


Testing
---

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


Thanks,

Gaston Kleiman

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Greg Mann]

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

(Updated Jan. 12, 2018, 11:07 p.m.)


Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Added missing fields to the GET_MASTER operator API call.


Diffs (updated)
-

  include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
  include/mesos/v1/master/master.proto 5cb64df12459b1529306cd33691c003172e4b3ec 
  src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 


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

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


Testing
---

tested manually using curl


Thanks,

Greg Mann

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Greg Mann]

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

(Updated Jan. 12, 2018, 11:07 p.m.)


Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Added missing fields to the GET_MASTER operator API call.


Diffs (updated)
-

  include/mesos/master/master.proto f4506fe82ee7c1db77cfb7e94933d5a7f4b024df 
  include/mesos/v1/master/master.proto 5cb64df12459b1529306cd33691c003172e4b3ec 
  src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 


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

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


Testing
---

tested manually using curl


Thanks,

Greg Mann

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

[Vinod Kone]

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




include/mesos/v1/mesos.proto
Lines 901-909 (patched)


lets move this to `GetMaster` response instead of adding it to MasterInfo.


- Vinod Kone


On Jan. 10, 2018, 11:11 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65056/
> ---
> 
> (Updated Jan. 10, 2018, 11:11 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gaston Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8078
> https://issues.apache.org/jira/browse/MESOS-8078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Several fields were present in the old master '/state' endpoint,
> which are now not accessible via the V1 operator API. This patch
> adds these fields to the response for the GET_MASTER call.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2c5ae4cd8d9cfaabd42a9e94ce94cd73bfacc6eb 
>   include/mesos/v1/mesos.proto b8e016e60322879e53ffa3bef23481015b0a6d2d 
>   src/internal/evolve.cpp 7758c9b93ad41b45b941c0de8c2b1008fbc4e50d 
>   src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 
>   src/master/master.hpp 5e6ba53c075174a1e514a395ceb17c26201ec470 
> 
> 
> Diff: https://reviews.apache.org/r/65056/diff/2/
> 
> 
> Testing
> ---
> 
> tested manually using curl
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 62965: Added doc for cgroups devices isolator.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Jan. 12, 2018, 8:24 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62965/
> ---
> 
> (Updated Jan. 12, 2018, 8:24 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ilya Pronin, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added doc for cgroups devices isolator.
> 
> 
> Diffs
> -
> 
>   docs/isolators/cgroups-devices.md PRE-CREATION 
>   docs/mesos-containerizer.md 28d5ccd1fb760485a92ec532c301958224e39526 
> 
> 
> Diff: https://reviews.apache.org/r/62965/diff/4/
> 
> 
> Testing
> ---
> 
> https://github.com/jieyu/mesos/blob/doc/docs/isolators/cgroups-devices.md
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

[Jiang Yan Xu]

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


Ship it!




Will commit with a few touches like the following.


src/master/master.cpp
Lines 10937-10941 (original), 10935-10939 (patched)


Given our latest conclusion let's revert this back to 

```
count += framework->unreachableTasks.size();
```


- Jiang Yan Xu


On Jan. 12, 2018, 11:24 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64940/
> ---
> 
> (Updated Jan. 12, 2018, 11:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Jie Yu, Vinod 
> Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8337
> https://issues.apache.org/jira/browse/MESOS-8337
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If an agent is lost, we try to remove all the tasks that might have
> been lost. If a task is already terminal but has unacknowleged status
> updates, it is expected that we track it in the unreachable tasks list
> so we should remove the CHECK that prevents this. This also backs out
> changes to how unreachable tasks are presented in the HTTP endpoints to
> restore compatibility with previous Mesos releases.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 
>   src/master/master.hpp f915c6f611eaffd5c2cb6cba6c43def679e8f92d 
>   src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 
>   src/tests/mesos.hpp 93913f2e01898c73e09de58a975aa467e714d882 
>   src/tests/partition_tests.cpp 3813139f576ea01db0197f0fe8a73597db1bb69a 
> 
> 
> Diff: https://reviews.apache.org/r/64940/diff/7/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 65106: Removed the misleading `isRemovable` helper in the master.

[Benjamin Mahler]

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



Will need to rebase off of https://reviews.apache.org/r/64940/ once that's 
committed.

- Benjamin Mahler


On Jan. 12, 2018, 1:34 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65106/
> ---
> 
> (Updated Jan. 12, 2018, 1:34 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8389
> https://issues.apache.org/jira/browse/MESOS-8389
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the past, the notion of a "removable" task meant: the task is
> terminal and acknowledged. However, the `isRemovable` helper only
> defines removability by the task state (terminal or unreachable)
> but not whether the terminal update is acknowledged.
> 
> As a result, the code that is calling `isRemovable` is unintuitive.
> One example of a confusing piece of code is within updateTask. Here,
> we have logic which says, if the task is removable, recover the
> resources but don't remove it. This seems more intuitive if directly
> described as: "if the task is no longer consuming resources (e.g.
> transitioned to terminal or unreachable) then recover the resources".
> 
> If one looks up the documentation of `isRemovable`, it says "When a
> task becomes removable, it is erased from the master's primary task
> data structures", but that isn't accurate since this function doesn't
> say whether the terminal task has been acknowledged, which is
> required for a task to be removable.
> 
> To make an immediate improvement, this patch removes `isRemovable`
> (no pun intended) in favor of directly checking the states we care
> about. In the future, we may want to introduce some helpers like
> `isAllocatedResources` to describe whether the task's resources are
> considered allocated.
> 
> If we do introduce a notion of `isRemovable` in the future, it seems
> it should take the `Task*` so that it can say whether the task could
> be "removed" from the master, which includes checking that terminal
> tasks have been acknowledged. However, "remove" is somewhat of a
> confusing term, since what we're actually doing is "completing" the
> task.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp cdfd06ceb2a8eafa60d0af382b628b165e4ddcb5 
>   src/master/master.cpp 7ed15e4ba2a31c5fe4b8571f645cdca69a3e82f4 
> 
> 
> Diff: https://reviews.apache.org/r/65106/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 65095: Made it possible to update an operation without mutating resources.

[Greg Mann]

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


Ship it!





src/master/master.cpp
Line 10318 (original), 10319 (patched)


As we discussed in chat, while this isn't a huge deal at present (since 
updates are being acknowledged immediately by the master), we'll want to change 
this anyway once we have the framework API in place for operation feedback. If 
this helper is going to be called upon every receipt of an 
UpdateOperationStatusMessage, then we won't want to push the statuses from 
update retries onto the master's internal state. Once we're relying on 
frameworks to acknowledge, this could be an issue.

Gaston and I will follow up with a patch to implement some simple 
de-duplication for now, and we can revisit correct de-duplication semantics 
later on, when the operation update workflow becomes more complex.


- Greg Mann


On Jan. 12, 2018, 10:53 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65095/
> ---
> 
> (Updated Jan. 12, 2018, 10:53 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8422
> https://issues.apache.org/jira/browse/MESOS-8422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In certain situations it can make sense to update the state of an
> operation without also wanting to update resources. In this patch we
> modify the master's `updateOperation` function to take an additional
> parameter signifying whether resources should be updated, or whether
> we only care about updating the operation and tracking of used
> resources.
> 
> We will use this functionality in a subsequent patch to perform more
> contained updates to agent state when processing `UpdateSlaveMessages`
> which contain both resources and operations (and where any terminal
> operations were already applied to the agent's resources).
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp cdfd06ceb2a8eafa60d0af382b628b165e4ddcb5 
>   src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 
> 
> 
> Diff: https://reviews.apache.org/r/65095/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

[Vinod Kone]

> On Jan. 12, 2018, 6:19 p.m., James Peach wrote:
> > src/common/resources.cpp
> > Line 263 (original), 263 (patched)
> > 
> >
> > Nit: `static bool compareResourceMetadata(...)`

I'll fix this before committing.


- Vinod


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


On Jan. 12, 2018, 3:40 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65074/
> ---
> 
> (Updated Jan. 12, 2018, 3:40 p.m.)
> 
> 
> Review request for mesos, James Peach and Vinod Kone.
> 
> 
> Bugs: MESOS-8410
> https://issues.apache.org/jira/browse/MESOS-8410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The case where multiple resources have the same name
> was not handled correctly, and could result in false
> negatives.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 69bf823f800ac8bfc2bc211895b0967f573009b5 
>   src/common/resources.cpp 1291208b9f2a7c373ff7ec941d3e4310fec92207 
>   src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
>   src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
>   src/tests/slave_compatibility_tests.cpp 
> ab5ed29f43c593dbd7d90c235aa304b581d932a2 
> 
> 
> Diff: https://reviews.apache.org/r/65074/diff/2/
> 
> 
> Testing
> ---
> 
> * Several new unit tests added in the review
> * `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

[Vinod Kone]

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


Ship it!




LGTM.

I still see open issues though, can you please resolve them?

- Vinod Kone


On Jan. 12, 2018, 7:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64940/
> ---
> 
> (Updated Jan. 12, 2018, 7:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Jie Yu, Vinod 
> Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8337
> https://issues.apache.org/jira/browse/MESOS-8337
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If an agent is lost, we try to remove all the tasks that might have
> been lost. If a task is already terminal but has unacknowleged status
> updates, it is expected that we track it in the unreachable tasks list
> so we should remove the CHECK that prevents this. This also backs out
> changes to how unreachable tasks are presented in the HTTP endpoints to
> restore compatibility with previous Mesos releases.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 
>   src/master/master.hpp f915c6f611eaffd5c2cb6cba6c43def679e8f92d 
>   src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 
>   src/tests/mesos.hpp 93913f2e01898c73e09de58a975aa467e714d882 
>   src/tests/partition_tests.cpp 3813139f576ea01db0197f0fe8a73597db1bb69a 
> 
> 
> Diff: https://reviews.apache.org/r/64940/diff/7/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

[James Peach]

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

(Updated Jan. 12, 2018, 7:24 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Jie Yu, Vinod Kone, 
and Jiang Yan Xu.


Changes
---

Addressed nits.


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


Repository: mesos


Description
---

If an agent is lost, we try to remove all the tasks that might have
been lost. If a task is already terminal but has unacknowleged status
updates, it is expected that we track it in the unreachable tasks list
so we should remove the CHECK that prevents this. This also backs out
changes to how unreachable tasks are presented in the HTTP endpoints to
restore compatibility with previous Mesos releases.


Diffs (updated)
-

  src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 
  src/master/master.hpp f915c6f611eaffd5c2cb6cba6c43def679e8f92d 
  src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b 
  src/tests/mesos.hpp 93913f2e01898c73e09de58a975aa467e714d882 
  src/tests/partition_tests.cpp 3813139f576ea01db0197f0fe8a73597db1bb69a 


Diff: https://reviews.apache.org/r/64940/diff/7/

Changes: https://reviews.apache.org/r/64940/diff/6-7/


Testing
---

make check (Fedora 27)


Thanks,

James Peach

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

[Greg Mann]

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


Fix it, then Ship it!





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


Nit: s/operation/operator/

I can fix this while committing.


- Greg Mann


On Jan. 12, 2018, 12:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65096/
> ---
> 
> (Updated Jan. 12, 2018, 12:49 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Bugs: MESOS-8422
> https://issues.apache.org/jira/browse/MESOS-8422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes logging of master's `updateOperation` for operations
> without framework ID. We also add a `CHECK` before the part updating
> resources or the allocator for non-speculated operations; currently
> non-speculated operations can only be initiated from a framework, but
> not from e.g., the operation API, and additional work is needed to
> support this.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 
> 
> 
> Diff: https://reviews.apache.org/r/65096/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

[Vinod Kone]

> On Jan. 12, 2018, 6:19 p.m., James Peach wrote:
> > The bug we hit was where the resources didn't change but containered mount 
> > disks. Are we confident that the test here (where a mount disk is added) 
> > also covers the case where the resources don't change?

I think that's what the last line in the test is verifying? note that it's 
comparing `info2` to `info2`.


- Vinod


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


On Jan. 12, 2018, 3:40 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65074/
> ---
> 
> (Updated Jan. 12, 2018, 3:40 p.m.)
> 
> 
> Review request for mesos, James Peach and Vinod Kone.
> 
> 
> Bugs: MESOS-8410
> https://issues.apache.org/jira/browse/MESOS-8410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The case where multiple resources have the same name
> was not handled correctly, and could result in false
> negatives.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 69bf823f800ac8bfc2bc211895b0967f573009b5 
>   src/common/resources.cpp 1291208b9f2a7c373ff7ec941d3e4310fec92207 
>   src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
>   src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
>   src/tests/slave_compatibility_tests.cpp 
> ab5ed29f43c593dbd7d90c235aa304b581d932a2 
> 
> 
> Diff: https://reviews.apache.org/r/65074/diff/2/
> 
> 
> Testing
> ---
> 
> * Several new unit tests added in the review
> * `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 62965: Added doc for cgroups devices isolator.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62965 was successfully built and tested.

Reviews applied: `['62965']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 4:24 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62965/
> ---
> 
> (Updated Jan. 12, 2018, 4:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ilya Pronin, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added doc for cgroups devices isolator.
> 
> 
> Diffs
> -
> 
>   docs/isolators/cgroups-devices.md PRE-CREATION 
>   docs/mesos-containerizer.md 28d5ccd1fb760485a92ec532c301958224e39526 
> 
> 
> Diff: https://reviews.apache.org/r/62965/diff/4/
> 
> 
> Testing
> ---
> 
> https://github.com/jieyu/mesos/blob/doc/docs/isolators/cgroups-devices.md
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

[James Peach]

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


Fix it, then Ship it!




The bug we hit was where the resources didn't change but containered mount 
disks. Are we confident that the test here (where a mount disk is added) also 
covers the case where the resources don't change?


src/common/resources.cpp
Line 263 (original), 263 (patched)


Nit: `static bool compareResourceMetadata(...)`


- James Peach


On Jan. 12, 2018, 3:40 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65074/
> ---
> 
> (Updated Jan. 12, 2018, 3:40 p.m.)
> 
> 
> Review request for mesos, James Peach and Vinod Kone.
> 
> 
> Bugs: MESOS-8410
> https://issues.apache.org/jira/browse/MESOS-8410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The case where multiple resources have the same name
> was not handled correctly, and could result in false
> negatives.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 69bf823f800ac8bfc2bc211895b0967f573009b5 
>   src/common/resources.cpp 1291208b9f2a7c373ff7ec941d3e4310fec92207 
>   src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
>   src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
>   src/tests/slave_compatibility_tests.cpp 
> ab5ed29f43c593dbd7d90c235aa304b581d932a2 
> 
> 
> Diff: https://reviews.apache.org/r/65074/diff/2/
> 
> 
> Testing
> ---
> 
> * Several new unit tests added in the review
> * `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65074 was successfully built and tested.

Reviews applied: `['65074']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 3:40 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65074/
> ---
> 
> (Updated Jan. 12, 2018, 3:40 p.m.)
> 
> 
> Review request for mesos, James Peach and Vinod Kone.
> 
> 
> Bugs: MESOS-8410
> https://issues.apache.org/jira/browse/MESOS-8410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The case where multiple resources have the same name
> was not handled correctly, and could result in false
> negatives.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 69bf823f800ac8bfc2bc211895b0967f573009b5 
>   src/common/resources.cpp 1291208b9f2a7c373ff7ec941d3e4310fec92207 
>   src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
>   src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
>   src/tests/slave_compatibility_tests.cpp 
> ab5ed29f43c593dbd7d90c235aa304b581d932a2 
> 
> 
> Diff: https://reviews.apache.org/r/65074/diff/2/
> 
> 
> Testing
> ---
> 
> * Several new unit tests added in the review
> * `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65126: Added a resource provider test case.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65126 was successfully built and tested.

Reviews applied: `['65125', '65126']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 2:16 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65126/
> ---
> 
> (Updated Jan. 12, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a resource provider test case.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> d80823c47963e969113dae3623b18b7639c890fc 
> 
> 
> Diff: https://reviews.apache.org/r/65126/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 65127: Windows: Enabled docker health checks.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65127 was successfully built and tested.

Reviews applied: `['63859', '63860', '63861', '64570', '64386', '65127']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 2:37 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65127/
> ---
> 
> (Updated Jan. 12, 2018, 2:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The network health checks called curl and then executed setns to
> enter to container network namespace, which did not work on Windows.
> To do the equivalent, Windows nows calls docker run with powershell's
> curl equivalent (Invoke-WebRequest) and uses the
> network=container: flag to enter the container's namespace.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
> 
> 
> Diff: https://reviews.apache.org/r/65127/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

[Vinod Kone]

> On Jan. 12, 2018, 2:24 a.m., Vinod Kone wrote:
> > AFAICT, in 1.4.x we show unreachable terminal tasks on a removed agent as 
> > completed tasks. But now, we show them as unreachable tasks. If that's true 
> > it's an API change that needs to be called out. Is that really backwards 
> > compatible?
> 
> Jiang Yan Xu wrote:
> Yeah it's true. Despite it being a bug that if the unreachable terminal 
> tasks are considered completed and added to the completed list, it cannot be 
> later removed when the agent reregisters and duplicates are shown in the 
> webUI and APIs, it is indeed what 1.4.x gives you.
> 
> 1.5 fixes the duplication problem but we did the extra work (the 
> additional `if (task->state() != TASK_UNREACHABLE)` checks we added and this 
> revision removes) to make it look like the 1.4.x version, I guess it's fine 
> to keep it that way until we have a plan for an overhaul...
> 
> So, probably let's not revert the code that involves the http endpoints 
> (sorry for suggesting it earlier, there are small changes needed which I'll 
> comment on).

"unreachable terminal tasks are considered completed and added to the completed 
list, it cannot be later removed when the agent reregisters and duplicates are 
shown in the webUI and APIs" .

Duplicated tasks in 2 lists sounds like a bug and not intentional behavior in 
1.4.x. So, if possible we should fix it in 1.5.x to move the task from 
completed to active upon re-registration. Is that easy to do or do we have to 
live with the duplicates until the overhaul?


- Vinod


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


On Jan. 5, 2018, 7:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64940/
> ---
> 
> (Updated Jan. 5, 2018, 7:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Jie Yu, Vinod 
> Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8337
> https://issues.apache.org/jira/browse/MESOS-8337
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If an agent is lost, we try to remove all the tasks that might have
> been lost. If a task is already terminal but has unacknowleged status
> updates, it is expected that we track it in the unreachable tasks list
> so we should remove the CHECK that prevents this. This also backs out
> changes to how unreachable tasks are presented in the HTTP endpoints to
> restore compatibility with previous Mesos releases.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 
>   src/master/master.hpp 5e6ba53c075174a1e514a395ceb17c26201ec470 
>   src/master/master.cpp 6fc5de89e54ba0b9ae2c4fb475be9878910820d3 
>   src/tests/mesos.hpp 93913f2e01898c73e09de58a975aa467e714d882 
>   src/tests/partition_tests.cpp 3813139f576ea01db0197f0fe8a73597db1bb69a 
> 
> 
> Diff: https://reviews.apache.org/r/64940/diff/6/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 62965: Added doc for cgroups devices isolator.

[Jie Yu]

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

(Updated Jan. 12, 2018, 4:24 p.m.)


Review request for mesos, Gilbert Song, Ilya Pronin, and James Peach.


Repository: mesos


Description
---

Added doc for cgroups devices isolator.


Diffs (updated)
-

  docs/isolators/cgroups-devices.md PRE-CREATION 
  docs/mesos-containerizer.md 28d5ccd1fb760485a92ec532c301958224e39526 


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

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


Testing
---

https://github.com/jieyu/mesos/blob/doc/docs/isolators/cgroups-devices.md


Thanks,

Jie Yu

Re: Review Request 65072: Fixed handling of terminal operations in `updateSlave` handler.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65072 was successfully built and tested.

Reviews applied: `['65093', '65095', '65096', '65072']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 10:57 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65072/
> ---
> 
> (Updated Jan. 12, 2018, 10:57 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-8422
> https://issues.apache.org/jira/browse/MESOS-8422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An offer operation can be become terminal between any previously
> received non-terminal offer operation status update and receiving an
> `UpdateSlaveMessage` (e.g., if the agent failed over, or when the
> agent was partitioned from the master).
> 
> The master will in its offer operations status handler attempt
> to apply operations which became terminal since the last update. At
> the same time, the total resources in an `UpdateSlaveMessage` would
> already contain the result of applying the operation, and we need to
> prevent the master from attempting to apply the same operation twice.
> 
> This patch updates the master handler for `UpdateSlaveMessage` to
> transition pending operations which are reported as terminal without
> also updating the resources on the agent as any update would already
> be reflected in the new total from the `UpdateSlaveMessage.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 
> 
> 
> Diff: https://reviews.apache.org/r/65072/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65122 was successfully built and tested.

Reviews applied: `['65122']`

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

- Mesos Reviewbot Windows


On Jan. 12, 2018, 12:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65122/
> ---
> 
> (Updated Jan. 12, 2018, 12:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-7742
> https://issues.apache.org/jira/browse/MESOS-7742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, io switchboard terminated itself after io redirect is
> finished and before http response was sent to the agent for
> `ATTACH_CONTAINER_INPUT` request. This patch adds a check that an
> input connection exists in io redirect completion callback to postpone
> termination until `ATTACH_CONTAINER_INPUT` is completely processed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 89dd4988c7132d3c7aeacecad7a76961b0ec5a2c 
> 
> 
> Diff: https://reviews.apache.org/r/65122/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

[Benno Evers]

> On Jan. 12, 2018, 2:38 a.m., Vinod Kone wrote:
> > src/slave/compatibility.cpp
> > Line 156 (original), 135 (patched)
> > 
> >
> > s/by Mesos//. Seems redundant?

Without the suffix, the comment could be interpreted as saying that only the 
`additive`-policy doesn't support text resources.


- Benno


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


On Jan. 12, 2018, 3:40 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65074/
> ---
> 
> (Updated Jan. 12, 2018, 3:40 p.m.)
> 
> 
> Review request for mesos, James Peach and Vinod Kone.
> 
> 
> Bugs: MESOS-8410
> https://issues.apache.org/jira/browse/MESOS-8410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The case where multiple resources have the same name
> was not handled correctly, and could result in false
> negatives.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 69bf823f800ac8bfc2bc211895b0967f573009b5 
>   src/common/resources.cpp 1291208b9f2a7c373ff7ec941d3e4310fec92207 
>   src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
>   src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
>   src/tests/slave_compatibility_tests.cpp 
> ab5ed29f43c593dbd7d90c235aa304b581d932a2 
> 
> 
> Diff: https://reviews.apache.org/r/65074/diff/2/
> 
> 
> Testing
> ---
> 
> * Several new unit tests added in the review
> * `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

[Benno Evers]

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

(Updated Jan. 12, 2018, 3:40 p.m.)


Review request for mesos, James Peach and Vinod Kone.


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


Repository: mesos


Description (updated)
---

The case where multiple resources have the same name
was not handled correctly, and could result in false
negatives.


Diffs (updated)
-

  include/mesos/resources.hpp 69bf823f800ac8bfc2bc211895b0967f573009b5 
  src/common/resources.cpp 1291208b9f2a7c373ff7ec941d3e4310fec92207 
  src/slave/compatibility.hpp 78b421a01abe5d2178c93832577577a7ba282b38 
  src/slave/compatibility.cpp 4ead4a5b655f6f3d7812aa52d656830d7cff4598 
  src/tests/slave_compatibility_tests.cpp 
ab5ed29f43c593dbd7d90c235aa304b581d932a2 


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

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


Testing
---

* Several new unit tests added in the review
* `make check`


Thanks,

Benno Evers

Re: Review Request 65126: Added a resource provider test case.

[Mesos Reviewbot Windows]

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



FAIL: mesos-java failed to build.

Reviews applied: `['65125', '65126']`

Failed command: `cmake.exe --build . --target mesos-java`

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

Relevant logs:

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

```
  
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(2601):
 warning C4267: '=': conversion from 'size_t' to 'int', 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\zookeeper.c(3091):
 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\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(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\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]
  

Review Request 65127: Windows: Enabled docker health checks.

[Akash Gupta]

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

Review request for mesos, Alexander Rukletsov and Andrew Schwartzmeyer.


Repository: mesos


Description
---

The network health checks called curl and then executed setns to
enter to container network namespace, which did not work on Windows.
To do the equivalent, Windows nows calls docker run with powershell's
curl equivalent (Invoke-WebRequest) and uses the
network=container: flag to enter the container's namespace.


Diffs
-

  src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
  src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 


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


Testing
---


Thanks,

Akash Gupta

Re: Review Request 64386: Refactored health checks to take in executor information.

[Akash Gupta]

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

(Updated Jan. 12, 2018, 2:36 p.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
Kleiman, Jie Yu, Joseph Wu, and Michael Park.


Changes
---

Refactored health checks with Alex's feedback.


Summary (updated)
-

Refactored health checks to take in executor information.


Repository: mesos


Description (updated)
---

Added information about the calling executor to the health check
library. After the refactoring, the command health check code originally
in the docker executor is now in the health check library.


Diffs (updated)
-

  src/checks/checker.hpp 93502270f31e80c5f7c94b5b456625e9cdea1837 
  src/checks/checker.cpp fff0aac504b4283a210f936e00c977fa60d88b3d 
  src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
  src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
  src/checks/health_checker.hpp 019fbd791f250ecc28ff59d779f90e7ccbf0c685 
  src/checks/health_checker.cpp eaf9a18817eeeff7c29c7a4b9d1b183f398760a3 
  src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
  src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
  src/launcher/default_executor.cpp 6c88de413379d3b58f19a24ac0c2f43e38e85e54 
  src/launcher/executor.cpp 794bf7ca07e68c7d83993546c134f85cac5a68e3 


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

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


Testing
---

See https://reviews.apache.org/r/64387/


Thanks,

Akash Gupta

Re: Review Request 63861: Windows: Updated networking doc.

[Akash Gupta]

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

(Updated Jan. 12, 2018, 2:35 p.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
Kleiman, Jie Yu, John Kordich, and Michael Park.


Changes
---

Address Alex's feedback


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


Repository: mesos


Description
---

The networking docs now describe how the Docker network modes in the
`Network` enum work on Windows, since the enum only has Linux network
modes.


Diffs (updated)
-

  docs/networking.md bdd3a762435aae163fc659ccfea7da825983 


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

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


Testing
---


Thanks,

Akash Gupta

Re: Review Request 63860: Windows: Mapped the Docker network info types.

[Akash Gupta]

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

(Updated Jan. 12, 2018, 2:34 p.m.)


Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
Kleiman, Jie Yu, John Kordich, Joseph Wu, and Michael Park.


Changes
---

Address Alex's feedback


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


Repository: mesos


Description
---

The Network enum in DockerInfo is specific to Linux containers. `HOST`
doesn't exist on Windows and `BRIDGE` is `NAT` on Windows. The current
default docker network setting was always `HOST`, which broke the
Windows docker executor. Now, if a specific network isn't given, the
network mode will default to `HOST` on Linux agents and `NAT` on Windows
agents. Also, `BRIDGE` mode will be translated to `NAT` on Windows.


Diffs (updated)
-

  src/docker/docker.cpp 722a54ad113fc4e2bb22a8f08e307ab38d5fbfed 


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

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


Testing
---

See https://reviews.apache.org/r/63862/ for test results.


Thanks,

Akash Gupta

Re: Review Request 63859: Windows: Fixed mock signal values in stout.

[Akash Gupta]

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

(Updated Jan. 12, 2018, 2:33 p.m.)


Review request for mesos, Andrew Schwartzmeyer and John Kordich.


Changes
---

Removed SIGSTOP and SIGCONT


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


Repository: mesos


Description (updated)
---

Removed `SIGSTOP` and `SIGCONT` on Windows, since they are meaningless,
and never unused. Also, fixed the WEXITSTATUS macro to cast the exit
code instead of bit-masking it, since Windows exit codes are 32 bit
unsigned ints.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/kill.hpp 
b2a36b5c07df6642bb9b08b61ab3b678bf8a6b36 
  3rdparty/stout/include/stout/windows.hpp 
7aa0ba72c4cc3b688ce6374b3308945ea8cb7572 


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

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


Testing
---

See https://reviews.apache.org/r/63862/ for test results.


Thanks,

Akash Gupta

Review Request 65126: Added a resource provider test case.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added a resource provider test case.


Diffs
-

  src/tests/resource_provider_manager_tests.cpp 
d80823c47963e969113dae3623b18b7639c890fc 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 65125: Added a helper function for resource provider tests.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added a helper function for resource provider tests.


Diffs
-

  src/tests/resource_provider_manager_tests.cpp 
d80823c47963e969113dae3623b18b7639c890fc 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

[Mesos Reviewbot Windows]

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



FAIL: mesos-java failed to build.

Reviews applied: `['65122']`

Failed command: `cmake.exe --build . --target mesos-java`

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

Relevant logs:

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

```
  
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\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\java\mesos-java.vcxproj" (default target) (1) ->
"D:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj" (default target) (19) ->
(ClCompile target) -> 
  D:\DCOS\mesos\mesos\3rdparty\libprocess\src\poll_socket.cpp(279): fatal error 
C1088: Cannot flush 

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

[Benjamin Bannier]

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

(Updated Jan. 12, 2018, 1:49 p.m.)


Review request for mesos and Jan Schlicht.


Changes
---

Addressed comment from Jan.


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


Repository: mesos


Description
---

This patch fixes logging of master's `updateOperation` for operations
without framework ID. We also add a `CHECK` before the part updating
resources or the allocator for non-speculated operations; currently
non-speculated operations can only be initiated from a framework, but
not from e.g., the operation API, and additional work is needed to
support this.


Diffs (updated)
-

  src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

[Andrei Budnik]

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

Review request for mesos, Alexander Rukletsov, Greg Mann, and Kevin Klues.


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


Repository: mesos


Description
---

Previously, io switchboard terminated itself after io redirect is
finished and before http response was sent to the agent for
`ATTACH_CONTAINER_INPUT` request. This patch adds a check that an
input connection exists in io redirect completion callback to postpone
termination until `ATTACH_CONTAINER_INPUT` is completely processed.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
89dd4988c7132d3c7aeacecad7a76961b0ec5a2c 


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


Testing
---

sudo make check (Fedora 25)
internal CI


Thanks,

Andrei Budnik

Re: Review Request 65095: Made it possible to update an operation without mutating resources.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Jan. 12, 2018, 11:53 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65095/
> ---
> 
> (Updated Jan. 12, 2018, 11:53 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8422
> https://issues.apache.org/jira/browse/MESOS-8422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In certain situations it can make sense to update the state of an
> operation without also wanting to update resources. In this patch we
> modify the master's `updateOperation` function to take an additional
> parameter signifying whether resources should be updated, or whether
> we only care about updating the operation and tracking of used
> resources.
> 
> We will use this functionality in a subsequent patch to perform more
> contained updates to agent state when processing `UpdateSlaveMessages`
> which contain both resources and operations (and where any terminal
> operations were already applied to the agent's resources).
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp cdfd06ceb2a8eafa60d0af382b628b165e4ddcb5 
>   src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 
> 
> 
> Diff: https://reviews.apache.org/r/65095/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

[Jan Schlicht]

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


Fix it, then Ship it!





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


Let's not use an empty string but " of a operator API call" here, as it's 
done in `Master::updateOperationStatus`.


- Jan Schlicht


On Jan. 12, 2018, 11:53 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65096/
> ---
> 
> (Updated Jan. 12, 2018, 11:53 a.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Bugs: MESOS-8422
> https://issues.apache.org/jira/browse/MESOS-8422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes logging of master's `updateOperation` for operations
> without framework ID. We also add a `CHECK` before the part updating
> resources or the allocator for non-speculated operations; currently
> non-speculated operations can only be initiated from a framework, but
> not from e.g., the operation API, and additional work is needed to
> support this.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 
> 
> 
> Diff: https://reviews.apache.org/r/65096/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65072: Fixed handling of terminal operations in `updateSlave` handler.

[Mesos Reviewbot Windows]

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



FAIL: mesos-java failed to build.

Reviews applied: `['65093', '65095', '65096', '65072']`

Failed command: `cmake.exe --build . --target mesos-java`

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

Relevant logs:

- 
[mesos-java-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65072/logs/mesos-java-build-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\zookeeper.c(3091):
 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\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(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\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]
  

Re: Review Request 65072: Fixed handling of terminal operations in `updateSlave` handler.

[Benjamin Bannier]

> On Jan. 11, 2018, 3:07 a.m., Gaston Kleiman wrote:
> > src/master/master.cpp
> > Line 10419 (original), 10447 (patched)
> > 
> >
> > Shouldn't we skip this if the operation was already in a terminal state?

Why would we call `updateOperation` if we wanted no update? I'd prefer to keep 
updating here if asked since I cannot see how it would do harm.


- Benjamin


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


On Jan. 12, 2018, 11:57 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65072/
> ---
> 
> (Updated Jan. 12, 2018, 11:57 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-8422
> https://issues.apache.org/jira/browse/MESOS-8422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An offer operation can be become terminal between any previously
> received non-terminal offer operation status update and receiving an
> `UpdateSlaveMessage` (e.g., if the agent failed over, or when the
> agent was partitioned from the master).
> 
> The master will in its offer operations status handler attempt
> to apply operations which became terminal since the last update. At
> the same time, the total resources in an `UpdateSlaveMessage` would
> already contain the result of applying the operation, and we need to
> prevent the master from attempting to apply the same operation twice.
> 
> This patch updates the master handler for `UpdateSlaveMessage` to
> transition pending operations which are reported as terminal without
> also updating the resources on the agent as any update would already
> be reflected in the new total from the `UpdateSlaveMessage.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 
> 
> 
> Diff: https://reviews.apache.org/r/65072/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

[Benjamin Bannier]

> On Jan. 11, 2018, 10:22 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 10343-10344 (patched)
> > 
> >
> > What about speculative operations initiated via the operator API on 
> > resources offered by resource-provider-capable agents? I think we would hit 
> > this CHECK in that case?

Good catch.

I now preventing outputing a default `framework_id` value in the logging and 
moved the `CHECK` below the early exit for speculated operations.


- Benjamin


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


On Jan. 12, 2018, 11:53 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65096/
> ---
> 
> (Updated Jan. 12, 2018, 11:53 a.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Bugs: MESOS-8422
> https://issues.apache.org/jira/browse/MESOS-8422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes logging of master's `updateOperation` for operations
> without framework ID. We also add a `CHECK` before the part updating
> resources or the allocator for non-speculated operations; currently
> non-speculated operations can only be initiated from a framework, but
> not from e.g., the operation API, and additional work is needed to
> support this.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 
> 
> 
> Diff: https://reviews.apache.org/r/65096/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65095: Made it possible to update an operation without mutating resources.

[Benjamin Bannier]

> On Jan. 11, 2018, 10 p.m., Greg Mann wrote:
> > src/master/master.hpp
> > Lines 881-885 (original), 881-888 (patched)
> > 
> >
> > I'm thinking that `convertResources` might be a more intuitive name for 
> > the boolean parameter here? In all cases, resources associated with the 
> > operation are recovered. In only some cases, we must first convert the 
> > consumed resources to converted resources so that we can then recover them.
> > 
> > So, the variable behavior seems to be whether or not we are converting 
> > resources before we recover them. WDYT?
> > 
> > 
> > I think the comment here could be tweaked a bit to better reflect this 
> > as well. Perhaps:
> > 
> > "Transitions the operation and, if the operation becomes terminal, 
> > recovers the resources associate with the operation. If `convertResources` 
> > is false, then no conversion from consumed to converted resources is 
> > applied in the allocator prior to resource recovery."

Thank you for the suggestion, I changed the parameter name to 
`convertResources`.

I also update the comment slightly,

// Transitions the operation, and updates and recovers resources if
// the operation becomes terminal. If `convertResources` is `false`
// only the consumed resources of terminal operations are recovered,
// but no resources are converted.

I'd prefer a tone along these lines, so we do not just repeat the 
implementation incompletely, but instead reflect intention. The parameter 
`convertResources` e.g., does not just control resources updates in the 
allocator, but also in the master's agent state.


- Benjamin


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


On Jan. 12, 2018, 11:53 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65095/
> ---
> 
> (Updated Jan. 12, 2018, 11:53 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8422
> https://issues.apache.org/jira/browse/MESOS-8422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In certain situations it can make sense to update the state of an
> operation without also wanting to update resources. In this patch we
> modify the master's `updateOperation` function to take an additional
> parameter signifying whether resources should be updated, or whether
> we only care about updating the operation and tracking of used
> resources.
> 
> We will use this functionality in a subsequent patch to perform more
> contained updates to agent state when processing `UpdateSlaveMessages`
> which contain both resources and operations (and where any terminal
> operations were already applied to the agent's resources).
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp cdfd06ceb2a8eafa60d0af382b628b165e4ddcb5 
>   src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 
> 
> 
> Diff: https://reviews.apache.org/r/65095/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65072: Fixed handling of terminal operations in `updateSlave` handler.

[Benjamin Bannier]

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

(Updated Jan. 12, 2018, 11:57 a.m.)


Review request for mesos, Gaston Kleiman and Greg Mann.


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


Repository: mesos


Description
---

An offer operation can be become terminal between any previously
received non-terminal offer operation status update and receiving an
`UpdateSlaveMessage` (e.g., if the agent failed over, or when the
agent was partitioned from the master).

The master will in its offer operations status handler attempt
to apply operations which became terminal since the last update. At
the same time, the total resources in an `UpdateSlaveMessage` would
already contain the result of applying the operation, and we need to
prevent the master from attempting to apply the same operation twice.

This patch updates the master handler for `UpdateSlaveMessage` to
transition pending operations which are reported as terminal without
also updating the resources on the agent as any update would already
be reflected in the new total from the `UpdateSlaveMessage.


Diffs (updated)
-

  src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

[Benjamin Bannier]

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

(Updated Jan. 12, 2018, 11:53 a.m.)


Review request for mesos and Jan Schlicht.


Changes
---

Addressed issue identified by Greg.


Summary (updated)
-

Fixed master's `updateOperation` for operations without framework ID.


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


Repository: mesos


Description (updated)
---

This patch fixes logging of master's `updateOperation` for operations
without framework ID. We also add a `CHECK` before the part updating
resources or the allocator for non-speculated operations; currently
non-speculated operations can only be initiated from a framework, but
not from e.g., the operation API, and additional work is needed to
support this.


Diffs (updated)
-

  src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 65095: Made it possible to update an operation without mutating resources.

[Benjamin Bannier]

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

(Updated Jan. 12, 2018, 11:53 a.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jan Schlicht.


Changes
---

Implemented suggestions by Greg.


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


Repository: mesos


Description
---

In certain situations it can make sense to update the state of an
operation without also wanting to update resources. In this patch we
modify the master's `updateOperation` function to take an additional
parameter signifying whether resources should be updated, or whether
we only care about updating the operation and tracking of used
resources.

We will use this functionality in a subsequent patch to perform more
contained updates to agent state when processing `UpdateSlaveMessages`
which contain both resources and operations (and where any terminal
operations were already applied to the agent's resources).


Diffs (updated)
-

  src/master/master.hpp cdfd06ceb2a8eafa60d0af382b628b165e4ddcb5 
  src/master/master.cpp 8921964a3ed74538b8c831a2f827d07875c1a5ab 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

[Jiang Yan Xu]

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



LGTM modulo Vinod's comments. Sorry we have gone back and forth on this but 
this time I think we are close to reach what's shippable to unblock 1.5.


src/master/http.cpp
Lines 360-362 (original)


So, as reminded by Vinod, for backward compatibility let's keep the checks. 
However here we should change the condition to

```
if (task->state() == TASK_UNREACHABLE) {
  continue;
}
```

because other terminal tasks that are not TASK_LOST could also be in this 
map...



src/master/master.hpp
Lines 2326-2327 (original)


The following CHECK looks correct?

```
CHECK(Master::isRemovable(task->state()) << task->state();
```

Although failing CHECKs are annoying, it helped us uncover the 
inconsistency in our task state transitions so it's better to correct it?



src/master/master.cpp
Lines 11023-11036 (original), 11021-11034 (patched)


I was going to suggest that we remove the `if (task->state() == 
TASK_UNREACHABLE)` check but per Vinod's comment we shouldn't.



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


s/a/an/?



src/tests/mesos.hpp
Line 3511 (original), 3518 (patched)


s/a/an/?



src/tests/partition_tests.cpp
Lines 2401 (patched)


We have to s/unreachable/completed/ and s/but/and/ ?



src/tests/partition_tests.cpp
Lines 2402 (patched)


With all the discussions about teminology it seems  s/Completed/Terminal/ 
is more accurate?



src/tests/partition_tests.cpp
Lines 2440-2442 (patched)


Nit: declare them in the order we are expecting on them? i.e., starting -> 
running -> finished.



src/tests/partition_tests.cpp
Lines 2474-2475 (patched)


Without a paused clock, there is a chance of race here right? If the same 
status update is resent after a timeout before we acknowledge, it's possible 
that the resent update is going to be caught by the 2nd expectation?

Would it be possible to hoist the Clock::pause() statement?



src/tests/partition_tests.cpp
Lines 2527 (patched)


Remove this debugging log.



src/tests/partition_tests.cpp
Lines 2532-2534 (patched)


Sorry now you have to expect it to be in the completed_task section...


- Jiang Yan Xu


On Jan. 5, 2018, 11:37 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64940/
> ---
> 
> (Updated Jan. 5, 2018, 11:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Jie Yu, Vinod 
> Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8337
> https://issues.apache.org/jira/browse/MESOS-8337
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If an agent is lost, we try to remove all the tasks that might have
> been lost. If a task is already terminal but has unacknowleged status
> updates, it is expected that we track it in the unreachable tasks list
> so we should remove the CHECK that prevents this. This also backs out
> changes to how unreachable tasks are presented in the HTTP endpoints to
> restore compatibility with previous Mesos releases.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bc29fafb1f4b515aec3b77850f960c88a65c8362 
>   src/master/master.hpp 5e6ba53c075174a1e514a395ceb17c26201ec470 
>   src/master/master.cpp 6fc5de89e54ba0b9ae2c4fb475be9878910820d3 
>   src/tests/mesos.hpp 93913f2e01898c73e09de58a975aa467e714d882 
>   src/tests/partition_tests.cpp 3813139f576ea01db0197f0fe8a73597db1bb69a 
> 
> 
> Diff: https://reviews.apache.org/r/64940/diff/6/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>