Re: Review Request 65045: Tested correct operation handling during master failover.

[Greg Mann]

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




src/tests/master_tests.cpp
Lines 8772 (patched)


Is this necessary?



src/tests/master_tests.cpp
Lines 8812 (patched)


Is this necessary?



src/tests/master_tests.cpp
Lines 8893 (patched)


Is this necessary?



src/tests/master_tests.cpp
Lines 8899 (patched)


Is this necessary?



src/tests/master_tests.cpp
Lines 8903 (patched)


Do we need this, or does framework registration after master failover 
prompt an offer?



src/tests/master_tests.cpp
Lines 8912-8914 (patched)


Please put each condition on a separate line:
```
if (resource.has_provider_id() &&
resource.has_disk() &&
resource.disk().has_source() &&
resource.disk().source().type() == 
Resource::DiskInfo::Source::MOUNT) {
```


- Greg Mann


On Jan. 25, 2018, 1:10 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65045/
> ---
> 
> (Updated Jan. 25, 2018, 1:10 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8424
> https://issues.apache.org/jira/browse/MESOS-8424
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tested correct operation handling during master failover.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp d01f3fbdd688ddd31fb0c777f973928f5b5fa5e7 
> 
> 
> Diff: https://reviews.apache.org/r/65045/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 65044: Added the v1 API 'GET_OPERATIONS' call for master and agent.

[Greg Mann]

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



Looks great, thanks Jan! I left some comments below on the agent API test, but 
the same comments also apply to the master API test; please make changes where 
appropriate in both locations.


src/tests/api_tests.cpp
Line 247 (original)


Nit: could you split this change out into a separate patch?



src/tests/api_tests.cpp
Lines 6280 (patched)


Could you leave a comment here saying that we do this simply to make it 
easier to handle resource provider communication?



src/tests/api_tests.cpp
Lines 6296 (patched)


Is this settle necessary? I suspect the AWAIT following this may be 
sufficient.



src/tests/api_tests.cpp
Lines 6349-6352 (patched)


Are these necessary? I think that framework registration may be sufficient 
to produce an offer?



src/tests/api_tests.cpp
Lines 6361-6363 (patched)


Could you place all the conditions on separate lines? i.e.
```
if (resource.has_provider_id() &&
resource.has_disk() &&
resource.disk().has_source() &&
resource.disk().source().type() == Resource::DiskInfo::Source::RAW) 
{
```



src/tests/api_tests.cpp
Lines 6387-6388 (patched)


Could we also validate the contents of the operation here?


- Greg Mann


On Jan. 25, 2018, 10:36 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65044/
> ---
> 
> (Updated Jan. 25, 2018, 10:36 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, Jie 
> Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8424
> https://issues.apache.org/jira/browse/MESOS-8424
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'GET_OPERATIONS' call lists all operations known to master or agent.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 315820041990d28e5cdae71c8385d4551d56558c 
>   include/mesos/master/master.proto 3e34634f3b864222d07374936c0d9a18269c2fbd 
>   include/mesos/v1/agent/agent.proto 9e8b49defb83ffd4dd240ebb3a09b2dd3610ab2a 
>   include/mesos/v1/master/master.proto 
> 6759c3004e7e04c0044360b0c54f4438c2f6ec8a 
>   src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990 
>   src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2 
>   src/master/validation.cpp f0b86775b7919ba6aa4a73038edb78a0adca68f4 
>   src/slave/http.hpp 1619bb77e3e7d5a6d387e4d4bb071d83ce914967 
>   src/slave/http.cpp 77e711ceeb0e2613d629b5e21fd686f85dfad11a 
>   src/slave/validation.cpp 0c2ccda177734cf3c47c0346ed34d20d58e7d932 
>   src/tests/api_tests.cpp bdacc30be4dc8656a41a0c47d0e350d48e59ad94 
> 
> 
> Diff: https://reviews.apache.org/r/65044/diff/8/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 65044: Added the v1 API 'GET_OPERATIONS' call for master and agent.

[Greg Mann]

> On Jan. 26, 2018, 9:20 p.m., Vinod Kone wrote:
> > src/slave/http.cpp
> > Lines 1685 (patched)
> > 
> >
> > Not yours, but can we make the logging consistent here and in master 
> > for operator api handlers? Looks like master doesn't even log here?

Looks like we do log in the master's top-level API handler. So logging is done 
in both cases, but we could be more consistent in how/where the logging is done 
precisely.


- Greg


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


On Jan. 25, 2018, 10:36 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65044/
> ---
> 
> (Updated Jan. 25, 2018, 10:36 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, Jie 
> Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8424
> https://issues.apache.org/jira/browse/MESOS-8424
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 'GET_OPERATIONS' call lists all operations known to master or agent.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 315820041990d28e5cdae71c8385d4551d56558c 
>   include/mesos/master/master.proto 3e34634f3b864222d07374936c0d9a18269c2fbd 
>   include/mesos/v1/agent/agent.proto 9e8b49defb83ffd4dd240ebb3a09b2dd3610ab2a 
>   include/mesos/v1/master/master.proto 
> 6759c3004e7e04c0044360b0c54f4438c2f6ec8a 
>   src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990 
>   src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2 
>   src/master/validation.cpp f0b86775b7919ba6aa4a73038edb78a0adca68f4 
>   src/slave/http.hpp 1619bb77e3e7d5a6d387e4d4bb071d83ce914967 
>   src/slave/http.cpp 77e711ceeb0e2613d629b5e21fd686f85dfad11a 
>   src/slave/validation.cpp 0c2ccda177734cf3c47c0346ed34d20d58e7d932 
>   src/tests/api_tests.cpp bdacc30be4dc8656a41a0c47d0e350d48e59ad94 
> 
> 
> Diff: https://reviews.apache.org/r/65044/diff/8/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 65427: Used unique CSI plugin names for SLRP tests.

[Gaston Kleiman]

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


Ship it!




Ship It!

- Gaston Kleiman


On Jan. 30, 2018, 5 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65427/
> ---
> 
> (Updated Jan. 30, 2018, 5 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-8399
> https://issues.apache.org/jira/browse/MESOS-8399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To provide better test isolation, the SLRP tests would generate random
> unique names in CSI plugin configs, so the plugin containers would use
> different container IDs.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> e0b089ac96de4762f2c5ddae9716a38d9837d1bf 
> 
> 
> Diff: https://reviews.apache.org/r/65427/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65427: Used unique CSI plugin names for SLRP tests.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Jan. 31, 2018, 1 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65427/
> ---
> 
> (Updated Jan. 31, 2018, 1 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-8399
> https://issues.apache.org/jira/browse/MESOS-8399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To provide better test isolation, the SLRP tests would generate random
> unique names in CSI plugin configs, so the plugin containers would use
> different container IDs.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> e0b089ac96de4762f2c5ddae9716a38d9837d1bf 
> 
> 
> Diff: https://reviews.apache.org/r/65427/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65448: Added a test to ensure master removes executors that never launched.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65448 was successfully built and tested.

Reviews applied: `['65445', '65446', '65447', '65449', '65448']`

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

- Mesos Reviewbot Windows


On Feb. 1, 2018, 10:04 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65448/
> ---
> 
> (Updated Feb. 1, 2018, 10:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.
> 
> 
> Bugs: MESOS-1720
> https://issues.apache.org/jira/browse/MESOS-1720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that agent send exited executor message when the
> executor is never launched. So that master's executor bookkeeping
> entry is removed. See MESOS-1720.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 
>   src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 
> 
> 
> Diff: https://reviews.apache.org/r/65448/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65445: Added new protobuf field `launch_executor` in RunTask(Group)Message.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65445 was successfully built and tested.

Reviews applied: `['65445']`

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

- Mesos Reviewbot Windows


On Feb. 1, 2018, 2 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65445/
> ---
> 
> (Updated Feb. 1, 2018, 2 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.
> 
> 
> Bugs: MESOS-1720
> https://issues.apache.org/jira/browse/MESOS-1720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This boolean flag is used for the master to specify whether a
> new executor should be launched for the task or taskGroup (with
> the exception of command executor). This will let the master
> to control executor creation on the agent.
> 
> Also updated the relevant message handlers and mock functions.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto 0db44a24979cfdc748cd7fa3acd9e0346b14cfd3 
>   src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
>   src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
>   src/tests/mock_slave.hpp 29ce7140501888d95d5f2d6c26b752ad276b484a 
>   src/tests/mock_slave.cpp 8357edc7b3a35624c813eccb9ecca9d3b5dbe07c 
>   src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 
> 
> 
> Diff: https://reviews.apache.org/r/65445/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65451: Fixed `contributors.yaml`.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Jan. 31, 2018, 11:28 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65451/
> ---
> 
> (Updated Jan. 31, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed `contributors.yaml`.
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml e368372fdc3715f1e755a502bf0ecc9d1d1bc880 
> 
> 
> Diff: https://reviews.apache.org/r/65451/diff/1/
> 
> 
> Testing
> ---
> 
> Used a python script to load the yaml.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 65448: Added a test to ensure master removes executors that never launched.

[Meng Zhu]

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

Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.


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


Repository: mesos


Description
---

This test ensures that agent send exited executor message when the
executor is never launched. So that master's executor bookkeeping
entry is removed. See MESOS-1720.


Diffs
-

  src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 
  src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 65449: Fixed an issue where executor info linger on master if failed to launch.

[Meng Zhu]

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

Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.


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


Repository: mesos


Description
---

Master relies on `ExitedExecutorMessage` from the agent to recycle
executor entry. However, this message won't be sent if the executor
never actually launched (due to transient error), leaving executor
info on the master lingering and resource claimed.
See MESOS-1720.

This patch fixes this issue by sending the `ExitedExecutorMessage`
from the agent if the executor is never launched. And by
setting a new field `launch_executor` in the RunTask(Group)Message,
the master is able to control the executor creation on the agent.


Diffs
-

  src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2 
  src/master/master.cpp b97ebae6ebfd8ae0f73e617d0c55e140b9c3fce7 
  src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
  src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
  src/tests/mock_slave.hpp 29ce7140501888d95d5f2d6c26b752ad276b484a 
  src/tests/mock_slave.cpp 8357edc7b3a35624c813eccb9ecca9d3b5dbe07c 
  src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 


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


Testing
---


Thanks,

Meng Zhu

Review Request 65447: Refactored couple of launch task sanity checks into a single code path.

[Meng Zhu]

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

Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.


Repository: mesos


Description
---

Initial steps common to `_run()` and `__run()` on the task launch
code path are refactored.


Diffs
-

  src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
  src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 65446: Added helper function for the agent to send `ExitedExecutorMessage`.

[Meng Zhu]

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

Review request for mesos.


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


Repository: mesos


Description
---

Added helper function for the agent to send `ExitedExecutorMessage`.


Diffs
-

  src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
  src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 65445: Added new protobuf field `launch_executor` in RunTask(Group)Message.

[Meng Zhu]

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

Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.


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


Repository: mesos


Description
---

This boolean flag is used for the master to specify whether a
new executor should be launched for the task or taskGroup (with
the exception of command executor). This will let the master
to control executor creation on the agent.

Also updated the relevant message handlers and mock functions.


Diffs
-

  src/messages/messages.proto 0db44a24979cfdc748cd7fa3acd9e0346b14cfd3 
  src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
  src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
  src/tests/mock_slave.hpp 29ce7140501888d95d5f2d6c26b752ad276b484a 
  src/tests/mock_slave.cpp 8357edc7b3a35624c813eccb9ecca9d3b5dbe07c 
  src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 65451: Fixed `contributors.yaml`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65451 was successfully built and tested.

Reviews applied: `['65451']`

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

- Mesos Reviewbot Windows


On Jan. 31, 2018, 11:28 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65451/
> ---
> 
> (Updated Jan. 31, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed `contributors.yaml`.
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml e368372fdc3715f1e755a502bf0ecc9d1d1bc880 
> 
> 
> Diff: https://reviews.apache.org/r/65451/diff/1/
> 
> 
> Testing
> ---
> 
> Used a python script to load the yaml.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 65451: Fixed `contributors.yaml`.

[Gaston Kleiman]

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

Review request for mesos and Greg Mann.


Repository: mesos


Description
---

Fixed `contributors.yaml`.


Diffs
-

  docs/contributors.yaml e368372fdc3715f1e755a502bf0ecc9d1d1bc880 


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


Testing
---

Used a python script to load the yaml.


Thanks,

Gaston Kleiman

Re: Review Request 65438: Fixed flaky SlaveTest.RegisteredAgentReregisterAfterFailover.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Jan. 31, 2018, 5:58 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65438/
> ---
> 
> (Updated Jan. 31, 2018, 5:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and James Peach.
> 
> 
> Bugs: MESOS-8232
> https://issues.apache.org/jira/browse/MESOS-8232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously due to a race condition a registration retry could be held
> in the authorizer and forwarded back to the master only after the
> agent's disconnection is detected, which leads to an remove registrar
> operation when the test asserts there's none.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 20b874481d3818574731fc30ba9df1fc2bcbe900 
> 
> 
> Diff: https://reviews.apache.org/r/65438/diff/1/
> 
> 
> Testing
> ---
> 
> Ran the test with 1000 iterations.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 65442: Made `ns::setns` overloads consistent.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65442 was successfully built and tested.

Reviews applied: `['65442']`

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

- Mesos Reviewbot Windows


On Jan. 31, 2018, 1 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65442/
> ---
> 
> (Updated Jan. 31, 2018, 1 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only the path-based overload of `ns::setns` accepted the
> `checkMultithreaded` flag. For API consistency, we now also
> accept this flag in the PID-based overload.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp e24d79a41eefcade343b2825b5a758d7d30a5f91 
>   src/linux/ns.cpp 446640705b7c060776ed87655fb17509865cd335 
> 
> 
> Diff: https://reviews.apache.org/r/65442/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 65441: Deleted deprecated Windows build script.

[Andrew Schwartzmeyer]

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

That confirms the review bot doesn't use `verify-reviews.py`.


- Andrew


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


On Jan. 31, 2018, 10:59 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65441/
> ---
> 
> (Updated Jan. 31, 2018, 10:59 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script has been unmaintained and is no longer used. The build
> process for Windows has improved to the point that scripting it has
> become unnecessary. So we delete it to avoid confusion over a likely
> broken script.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 49732c4baae3bdcdf8254ded03e7dab961b74b2e 
> 
> 
> Diff: https://reviews.apache.org/r/65441/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65442: Made `ns::setns` overloads consistent.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Jan. 31, 2018, 9 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65442/
> ---
> 
> (Updated Jan. 31, 2018, 9 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only the path-based overload of `ns::setns` accepted the
> `checkMultithreaded` flag. For API consistency, we now also
> accept this flag in the PID-based overload.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp e24d79a41eefcade343b2825b5a758d7d30a5f91 
>   src/linux/ns.cpp 446640705b7c060776ed87655fb17509865cd335 
> 
> 
> Diff: https://reviews.apache.org/r/65442/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 65442: Made `ns::setns` overloads consistent.

[James Peach]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Only the path-based overload of `ns::setns` accepted the
`checkMultithreaded` flag. For API consistency, we now also
accept this flag in the PID-based overload.


Diffs
-

  src/linux/ns.hpp e24d79a41eefcade343b2825b5a758d7d30a5f91 
  src/linux/ns.cpp 446640705b7c060776ed87655fb17509865cd335 


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


Testing
---

make check (Fedora 27)


Thanks,

James Peach

Re: Review Request 65361: Added special handling for devolving ACKNOWLEDGE_OPERATION_STATUS calls.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Jan. 29, 2018, 10:09 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65361/
> ---
> 
> (Updated Jan. 29, 2018, 10:09 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8184
> https://issues.apache.org/jira/browse/MESOS-8184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added special handling for devolving ACKNOWLEDGE_OPERATION_STATUS calls.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.cpp 1cfbdf2ab58cf64ca0947c2dbc8e7f244c912fbe 
> 
> 
> Diff: https://reviews.apache.org/r/65361/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65424: Improved task status acknowledgement logging messages.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Jan. 30, 2018, 9:35 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65424/
> ---
> 
> (Updated Jan. 30, 2018, 9:35 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8184
> https://issues.apache.org/jira/browse/MESOS-8184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved task status acknowledgement logging messages.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp b97ebae6ebfd8ae0f73e617d0c55e140b9c3fce7 
> 
> 
> Diff: https://reviews.apache.org/r/65424/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65360: Made `master::receive()` drop `ACKNOWLEDGE_OPERATION_STATUS` calls.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Jan. 26, 2018, 10:08 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65360/
> ---
> 
> (Updated Jan. 26, 2018, 10:08 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8184
> https://issues.apache.org/jira/browse/MESOS-8184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `Master::receive()` handles scheduler API calls sent by v0 API clients.
> `ACKNOWLEDGE_OPERATION_STATUS` is a v1-only API call, so
> `Master::receive()` should drop it.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp b97ebae6ebfd8ae0f73e617d0c55e140b9c3fce7 
> 
> 
> Diff: https://reviews.apache.org/r/65360/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65441: Deleted deprecated Windows build script.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65441 was successfully built and tested.

Reviews applied: `['65441']`

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

- Mesos Reviewbot Windows


On Jan. 31, 2018, 6:59 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65441/
> ---
> 
> (Updated Jan. 31, 2018, 6:59 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script has been unmaintained and is no longer used. The build
> process for Windows has improved to the point that scripting it has
> become unnecessary. So we delete it to avoid confusion over a likely
> broken script.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 49732c4baae3bdcdf8254ded03e7dab961b74b2e 
> 
> 
> Diff: https://reviews.apache.org/r/65441/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65439: Made cleanliness checks in post-reviews.py less strict.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65439 was successfully built and tested.

Reviews applied: `['65439']`

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

- Mesos Reviewbot Windows


On Jan. 31, 2018, 10:15 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65439/
> ---
> 
> (Updated Jan. 31, 2018, 10:15 a.m.)
> 
> 
> Review request for mesos and Armand Grillet.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These checks generate a high amount of unnecessary work for developers
> who want to submit part of an ongoing refactoring for review, and
> since most developers are advanced git users anyways the amount of
> hand-holding seems unnecessary.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 231a3ac641d1ddf2f1788829f4fa8c36811bed90 
> 
> 
> Diff: https://reviews.apache.org/r/65439/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65426: Made gRPC calls wait for the channel to be ready.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Jan. 31, 2018, 7:20 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65426/
> ---
> 
> (Updated Jan. 31, 2018, 7:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8514
> https://issues.apache.org/jira/browse/MESOS-8514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It takes long in gRPC 1.8 to prepare a domain socket, and as a result,
> SLRP cannot talk to a CSI plugin immediately after the socket file is
> created. This patch fixes this problem by asking gRPC calls to wait for
> the socket to be ready.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 0f63e576f04af4df8894626eebbb1a0e01eadf22 
> 
> 
> Diff: https://reviews.apache.org/r/65426/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65426: Made gRPC calls wait for the channel to be ready.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Jan. 31, 2018, 11:20 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65426/
> ---
> 
> (Updated Jan. 31, 2018, 11:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8514
> https://issues.apache.org/jira/browse/MESOS-8514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It takes long in gRPC 1.8 to prepare a domain socket, and as a result,
> SLRP cannot talk to a CSI plugin immediately after the socket file is
> created. This patch fixes this problem by asking gRPC calls to wait for
> the socket to be ready.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 0f63e576f04af4df8894626eebbb1a0e01eadf22 
> 
> 
> Diff: https://reviews.apache.org/r/65426/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65426: Made gRPC calls wait for the channel to be ready.

[Chun-Hung Hsiao]

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

(Updated Jan. 31, 2018, 7:20 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, and Jie Yu.


Changes
---

Addressed Gilbert's comments.


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


Repository: mesos


Description
---

It takes long in gRPC 1.8 to prepare a domain socket, and as a result,
SLRP cannot talk to a CSI plugin immediately after the socket file is
created. This patch fixes this problem by asking gRPC calls to wait for
the socket to be ready.


Diffs (updated)
-

  3rdparty/libprocess/include/process/grpc.hpp 
0f63e576f04af4df8894626eebbb1a0e01eadf22 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65426: Made gRPC calls wait for the channel to be ready.

[Gilbert Song]

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




3rdparty/libprocess/include/process/grpc.hpp
Lines 142 (patched)


Newline above or move this line above `set_deadline()`. Also, should we add 
comments to explain `wait_for_ready` represents the re-try in gRPC?


- Gilbert Song


On Jan. 30, 2018, 4:59 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65426/
> ---
> 
> (Updated Jan. 30, 2018, 4:59 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Jie Yu.
> 
> 
> Bugs: MESOS-8514
> https://issues.apache.org/jira/browse/MESOS-8514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It takes long in gRPC 1.8 to prepare a domain socket, and as a result,
> SLRP cannot talk to a CSI plugin immediately after the socket file is
> created. This patch fixes this problem by asking gRPC calls to wait for
> the socket to be ready.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 0f63e576f04af4df8894626eebbb1a0e01eadf22 
> 
> 
> Diff: https://reviews.apache.org/r/65426/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65426: Made gRPC calls wait for the channel to be ready.

[Jie Yu]

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


Ship it!




LGTM. Long term, let's expose those knobs to the caller and let caller decides

- Jie Yu


On Jan. 31, 2018, 12:59 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65426/
> ---
> 
> (Updated Jan. 31, 2018, 12:59 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Jie Yu.
> 
> 
> Bugs: MESOS-8514
> https://issues.apache.org/jira/browse/MESOS-8514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It takes long in gRPC 1.8 to prepare a domain socket, and as a result,
> SLRP cannot talk to a CSI plugin immediately after the socket file is
> created. This patch fixes this problem by asking gRPC calls to wait for
> the socket to be ready.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 0f63e576f04af4df8894626eebbb1a0e01eadf22 
> 
> 
> Diff: https://reviews.apache.org/r/65426/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65441: Deleted deprecated Windows build script.

[Andrew Schwartzmeyer]

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



Before we commit this, we need to look at these two references:

```
support/verify-reviews.py
178:command = "support\windows-build.bat"

support/jenkins/windows-buildbot.bat
38:"support/windows-build.bat"
```

- Andrew Schwartzmeyer


On Jan. 31, 2018, 10:59 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65441/
> ---
> 
> (Updated Jan. 31, 2018, 10:59 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script has been unmaintained and is no longer used. The build
> process for Windows has improved to the point that scripting it has
> become unnecessary. So we delete it to avoid confusion over a likely
> broken script.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 49732c4baae3bdcdf8254ded03e7dab961b74b2e 
> 
> 
> Diff: https://reviews.apache.org/r/65441/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 65441: Deleted deprecated Windows build script.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

This script has been unmaintained and is no longer used. The build
process for Windows has improved to the point that scripting it has
become unnecessary. So we delete it to avoid confusion over a likely
broken script.


Diffs
-

  support/windows-build.bat 49732c4baae3bdcdf8254ded03e7dab961b74b2e 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 65437: Added documentation for fault domains.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65437 was successfully built and tested.

Reviews applied: `['65437']`

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

- Mesos Reviewbot Windows


On Jan. 31, 2018, 5:50 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65437/
> ---
> 
> (Updated Jan. 31, 2018, 5:50 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-8483
> https://issues.apache.org/jira/browse/MESOS-8483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fault domains are a new feature in 1.5 which did not yet have
> a corresponding description in the documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master-and-agent.md 
> f247498ead43a16bbef5afb49d453073dd9ab6ef 
>   docs/fault-domains.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65437/diff/2/
> 
> 
> Testing
> ---
> 
> No.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65438: Fixed flaky SlaveTest.RegisteredAgentReregisterAfterFailover.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65438 was successfully built and tested.

Reviews applied: `['65438']`

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

- Mesos Reviewbot Windows


On Jan. 31, 2018, 5:58 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65438/
> ---
> 
> (Updated Jan. 31, 2018, 5:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and James Peach.
> 
> 
> Bugs: MESOS-8232
> https://issues.apache.org/jira/browse/MESOS-8232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously due to a race condition a registration retry could be held
> in the authorizer and forwarded back to the master only after the
> agent's disconnection is detected, which leads to an remove registrar
> operation when the test asserts there's none.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 20b874481d3818574731fc30ba9df1fc2bcbe900 
> 
> 
> Diff: https://reviews.apache.org/r/65438/diff/1/
> 
> 
> Testing
> ---
> 
> Ran the test with 1000 iterations.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 65439: Made cleanliness checks in post-reviews.py less strict.

[Armand Grillet]

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


Ship it!




Can you please add tillt and/or bbannier as reviewers so that they can shepherd 
this patch?

- Armand Grillet


On Jan. 31, 2018, 6:15 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65439/
> ---
> 
> (Updated Jan. 31, 2018, 6:15 p.m.)
> 
> 
> Review request for mesos and Armand Grillet.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These checks generate a high amount of unnecessary work for developers
> who want to submit part of an ongoing refactoring for review, and
> since most developers are advanced git users anyways the amount of
> hand-holding seems unnecessary.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 231a3ac641d1ddf2f1788829f4fa8c36811bed90 
> 
> 
> Diff: https://reviews.apache.org/r/65439/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65409: Fixed `SlaveRecoveryTest.ReconcileTasksMissingFromSlave`.

[Andrew Schwartzmeyer]

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

(Updated Jan. 31, 2018, 10:33 a.m.)


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


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


Repository: mesos


Description
---

Because it is not possible to delete a file (or a folder recursively)
with open handles on Windows, we have to explicitly `reset()` the agent
before removing the framework meta directory. Otherwise, the task status
update manager will be destructed too late, and so an open handle for
`task.updates` will cause the `os::rmdir` to fail.

This is safe because we previously destructed the agent anyway, just
later in the test when it was reassigned.


Diffs
-

  src/tests/slave_recovery_tests.cpp 6dcbedb94a2c7b3d65770fc89920c0cad1298d1f 


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


Testing (updated)
---

make check on CentOS 7, all passed
ctest on Windows, all passed including new SlaveRecoveryTests

Note that while this chain enables recovery of Docker tasks on Windows, it 
explicitly does not fix MESOS-8519 (recovery of job object tasks).


Thanks,

Andrew Schwartzmeyer

Re: Review Request 65437: Added documentation for fault domains.

[Benno Evers]

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




docs/fault-domains.md
Lines 74 (patched)


This example actually differs from the description in the first section, 
but it seems to me that placing all masters in the same data center somewhat 
defeats the goal of achieving high availability, and  that most people would 
prefer running jobs in a remote data center rather than having the "main" 
datacenter overloaded. But maybe someone with more insight into the design of 
fault domains can comment on this?


- Benno Evers


On Jan. 31, 2018, 5:50 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65437/
> ---
> 
> (Updated Jan. 31, 2018, 5:50 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-8483
> https://issues.apache.org/jira/browse/MESOS-8483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fault domains are a new feature in 1.5 which did not yet have
> a corresponding description in the documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master-and-agent.md 
> f247498ead43a16bbef5afb49d453073dd9ab6ef 
>   docs/fault-domains.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65437/diff/2/
> 
> 
> Testing
> ---
> 
> No.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65202: Adopted `DEFAULT_TEST_TIMEOUT` in Mesos tests.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['65201', '65343', '65298', '65202']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
   Actual: never called - unsatisfied and active
[  FAILED  ] ExceptionTest.DisallowSchedulerActionsOnAbort (142 ms)
[ RUN  ] ExceptionTest.DisallowSchedulerCallbacksOnAbort
D:\DCOS\mesos\mesos\src\tests\exception_tests.cpp(163): error: Failed to wait 
0ns for offers
D:\DCOS\mesos\mesos\src\tests\exception_tests.cpp(154): error: Actual function 
call count doesn't match EXPECT_CALL(sched, resourceOffers(, _))...
 Expected: to be called at least once
   Actual: never called - unsatisfied and active
D:\DCOS\mesos\mesos\src\tests\exception_tests.cpp(151): error: Actual function 
call count doesn't match EXPECT_CALL(sched, registered(, _, _))...
 Expected: to be called once
   Actual: never called - unsatisfied and active
D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/gmock.hpp(467): error: 
Actual function call count doesn't match EXPECT_CALL(filter->mock, 
filter(testing::A()))...
Expected args: message matcher (40-byte object <10-A4 FE-10 E6-01 00-00 
60-82 40-10 E6-01 00-00 CC-CC CC-CC CC-CC CC-CC 29-00 00-00 00-00 00-00 2F-00 
00-00 00-00 00-00>, 1-byte object , 1-byte object )
 Expected: to be called once
   Actual: never called - unsatisfied and active
[  FAILED  ] ExceptionTest.DisallowSchedulerCallbacksOnAbort (143 ms)
[--] 3 tests from ExceptionTest (453 ms total)

[--] 4 tests from ExecutorHttpApiTest
[ RUN  ] ExecutorHttpApiTest.NoContentType
D:\DCOS\mesos\mesos\src\tests\executor_http_api_tests.cpp(138): error: Failed 
to wait 0ns for __recover
[  FAILED  ] ExecutorHttpApiTest.NoContentType (130 ms)
[ RUN  ] ExecutorHttpApiTest.ValidJsonButInvalidProtobuf
D:\DCOS\mesos\mesos\src\tests\executor_http_api_tests.cpp(178): error: Failed 
to wait 0ns for __recover
[  FAILED  ] ExecutorHttpApiTest.ValidJsonButInvalidProtobuf (134 ms)
[ RUN  ] ExecutorHttpApiTest.GetRequest
D:\DCOS\mesos\mesos\src\tests\executor_http_api_tests.cpp(346): error: Failed 
to wait 0ns for __recover
[  FAILED  ] ExecutorHttpApiTest.GetRequest (140 ms)
[ RUN  ] ExecutorHttpApiTest.SubscribeBeforeContainerizerRecovery
D:\DCOS\mesos\mesos\src\tests\executor_http_api_tests.cpp(815): error: Failed 
to wait 0ns for recover
```

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

```
I0131 18:01:06.610699   212 slave.cpp:931] Agent terminating
I0131 18:01:06.632688 10036 master.cpp:1149] Master terminating
I0131 18:01:06.664726 10036 cluster.cpp:172] Creating default 'local' authorizer
I0131 18:01:06.671723  7300 master.cpp:458] Master 
91d5fd91-b661-428d-95aa-235e0c4900b7 
(build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) started on 
10.3.1.5:65368
I0131 18:01:06.672722  7300 master.cpp:460] Flags at startup: --acls="" 
--agent_ping_timeout="15secs" --agent_reregister_timeout="10mins" 
--allocation_interval="1secs" --allocator="HierarchicalDRF" 
--authenticate_agents="true" --authenticate_frameworks="true" 
--authenticate_http_frameworks="true" --authenticate_http_readonly="true" 
--authenticate_http_readwrite="true" --authenticators="crammd5" 
--authorizers="local" 
--credentials="C:\Users\mesos\AppData\Local\Temp\5XOMBJ\credentials" 
--filter_gpu_resources="true" --framework_sorter="drf" --help="false" 
--hostname_lookup="true" --http_authenticators="basic" 
--http_framework_authenticators="basic" --initialize_driver_logging="true" 
--log_auto_initialize="true" --logbufsecs="0" --logging_level="INFO" 
--max_agent_ping_timeouts="5" --max_completed_frameworks="50" 
--max_completed_tasks_per_framework="1000" 
--max_unreachable_tasks_per_framework="1000" --port="5050" --quiet="false" 
--recovery_agent_removal_limit="100%" --registry="in_memory"
  --registry_fetch_timeout="1mins" --registry_gc_interval="15mins" 
--registry_max_agent_age="2weeks" --registry_max_agent_count="102400" 
--registry_store_timeout="100secs" --registry_strict="false" 
--require_agent_domain="false" --root_submissions="true" --user_sorter="drf" 
--version="false" --webui_dir="/webui" 
--work_dir="C:\Users\mesos\AppData\Local\Temp\5XOMBJ\master" 
--zk_session_timeout="10secs"
I0131 18:01:06.673723  7300 master.cpp:509] Master only allowing authenticated 
frameworks to register
I0131 18:01:06.673723  7300 master.cpp:515] Master 

Review Request 65438: Fixed flaky SlaveTest.RegisteredAgentReregisterAfterFailover.

[Jiang Yan Xu]

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

Review request for mesos, Alexander Rukletsov and James Peach.


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


Repository: mesos


Description
---

Previously due to a race condition a registration retry could be held
in the authorizer and forwarded back to the master only after the
agent's disconnection is detected, which leads to an remove registrar
operation when the test asserts there's none.


Diffs
-

  src/tests/slave_tests.cpp 20b874481d3818574731fc30ba9df1fc2bcbe900 


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


Testing
---

Ran the test with 1000 iterations.


Thanks,

Jiang Yan Xu

Review Request 65439: Made cleanliness checks in post-reviews.py less strict.

[Benno Evers]

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

Review request for mesos.


Repository: mesos


Description
---

These checks generate a high amount of unnecessary work for developers
who want to submit part of an ongoing refactoring for review, and
since most developers are advanced git users anyways the amount of
hand-holding seems unnecessary.


Diffs
-

  support/post-reviews.py 231a3ac641d1ddf2f1788829f4fa8c36811bed90 


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


Testing
---


Thanks,

Benno Evers

Review Request 65437: Added documentation for fault domains.

[Benno Evers]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Fault domains are a new feature in 1.5 which did not yet have
a corresponding description in the documentation.


Diffs
-

  3rdparty/CMakeLists.txt da605707b89bbe9b3db9e60bc0b0a26dac46e56e 
  3rdparty/boost-1.53.0.tar.gz PRE-CREATION 
  3rdparty/boost-1.65.0.tar.gz 25973f3ec5d3ecb09d82774780726ee7dfd50518 
  3rdparty/cmake/Versions.cmake 94b0d8048412e00e2480f45e7ce4e6494da4bd5d 
  3rdparty/libprocess/Makefile.am 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
  3rdparty/libprocess/configure.ac 69ec9b9683199f580405e38ec16a581a01f919c4 
  3rdparty/libprocess/include/process/process.hpp 
8661706cb058efb26f5bfbcc84972f9930d3670f 
  3rdparty/libprocess/include/process/queue.hpp 
c364b7dc831050bc1b23f669dc1ccc1f1255a9c8 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
521b0cfbccd3599524b1407ef70880f4538941df 
  3rdparty/libprocess/src/process.cpp ba9bc291bb6741e32b3a066fe90771311d21852a 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
48b5f5cf38d8fb4d77025f9723031075ca8ab677 
  3rdparty/libprocess/src/tests/http_tests.cpp 
a60f6b132aff00da42e733f5c5f387dff2428ed2 
  3rdparty/libprocess/src/tests/queue_tests.cpp 
cc6b3d82ce5eb5ca16e2b9378dd5b1f28ac62fe8 
  3rdparty/stout/Makefile.am ef22a02a8a11326c6af19eb11d79eb82ff7861da 
  3rdparty/stout/configure.ac 5a8c023de63e6b96b128cdfb472859a6f3e6ede5 
  3rdparty/stout/include/stout/duration.hpp 
42c43cda21c75fc3bef962af67c4a09df68a95af 
  3rdparty/stout/include/stout/json.hpp 
7484f4aab0025a1b6e5d51484f92b0aef96fe433 
  3rdparty/stout/include/stout/numify.hpp 
ecf4fcf9f87d372bac6737d8c3a7ac9885bc581d 
  3rdparty/stout/include/stout/posix/os.hpp 
dfee0faa084932bf60c93c7aae63665225cd4ab1 
  3rdparty/stout/include/stout/protobuf.hpp 
7e7659814a5d95c41c4dfabb5669fd061dff4716 
  3rdparty/stout/include/stout/windows/os.hpp 
ec935209fc1532ebe087ac20933c99eed393506d 
  3rdparty/stout/tests/json_tests.cpp af00e425961daab6989339165045da3aad0513ba 
  3rdparty/stout/tests/os/env_tests.cpp 
b5b124dc6316e661af6dd90335ade5283c26d9f2 
  3rdparty/versions.am 3e008d5a7bdc029de23950463c295b968b3280c8 
  CHANGELOG 7bf7124bb5de5bd2db58f64382f5ac57c5687460 
  configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 
  docs/cmake-examples.md 74abfcc4ab97cf9ee91d99293b6731c1247a65e8 
  docs/configuration/master-and-agent.md 
f247498ead43a16bbef5afb49d453073dd9ab6ef 
  docs/fault-domains.md PRE-CREATION 
  docs/modules.md d3e3017fc98929baa4cb5bc29531a368dda0db8c 
  m4/libnl3.m4 e64ba492554d38f98b4d7cf8bd4bd1443e4d0a10 
  src/Makefile.am 155338673e1c23a2cfeab39bdf3ebe14568bfebc 
  src/executor/executor.cpp 945936883afba0b52c95242cd410c335424b5768 
  src/logging/logging.cpp 8e03ac66ea27ce1cc3a3fb5f2489ca201e1f8e92 
  src/slave/containerizer/docker.cpp f1d7d3e6afa119a6a24b054dcaa5ee68dbea965d 
  src/slave/containerizer/mesos/io/switchboard.cpp 
aeb0b3e4cf75b19efd9b3922cc4707d3c5cb 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
697f998713f8bbb306f5186dcad7e5e92c2dd415 
  src/slave/containerizer/mesos/paths.hpp 
b9f0f454ada3ee4e6648916a2c582bcfeebe1732 
  src/slave/containerizer/mesos/paths.cpp 
186048f159d6ce20d9c5dc65c00012eb5911520b 
  src/tests/api_tests.cpp b639a4bafbf69d3ea53aa0573e9f0489c3a1a57f 
  src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
  src/tests/containerizer/cni_isolator_tests.cpp 
b8eb80fb671646cd7551f1ee6cfad6f4a81ebc8f 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
4fce8af7c89fd3b9166e1784203f99853624faef 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
796bc401d4f922e7a6bd9e5391003cddd4331c95 
  src/tests/default_executor_tests.cpp cc97e0d1fea7f4d0bc544d850593d8d91921b552 
  src/tests/disk_quota_tests.cpp 31cf4c44b1494afd3501046abffd303c7a1307f8 
  src/tests/fault_tolerance_tests.cpp 64a865339cc0dad1bc43aea40c6491d0ece6e48a 
  src/tests/hierarchical_allocator_tests.cpp 
42dc6ac5e281bb0cdb938106fd3f12324e9e6330 
  src/tests/master_slave_reconciliation_tests.cpp 
0541aa56a414fbec872e7e3ec6ad50206766054a 
  src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 
  src/tests/master_validation_tests.cpp 
1260357e42f58934b55cc9a1544b4953844c6966 
  src/tests/oversubscription_tests.cpp 41c1178ec4514ef6a1fb369734d4f9d73710d883 
  src/tests/reconciliation_tests.cpp ad6c99306a82ef3da54a9d6d3c9d6f11ba145d6c 
  src/tests/slave_recovery_tests.cpp 6dcbedb94a2c7b3d65770fc89920c0cad1298d1f 
  src/tests/slave_tests.cpp 20b874481d3818574731fc30ba9df1fc2bcbe900 
  src/tests/sorter_tests.cpp da4e0f64a565af1d9458ff256ae0eafddd0a6b68 
  src/tests/storage_local_resource_provider_tests.cpp 
e0b089ac96de4762f2c5ddae9716a38d9837d1bf 
  

Re: Review Request 65432: Added `examples/flags.hpp` to `src/Makefile.am`.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On Jan. 31, 2018, 9:20 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65432/
> ---
> 
> (Updated Jan. 31, 2018, 9:20 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `examples/flags.hpp` to `src/Makefile.am`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 155338673e1c23a2cfeab39bdf3ebe14568bfebc 
> 
> 
> Diff: https://reviews.apache.org/r/65432/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 65432: Added `examples/flags.hpp` to `src/Makefile.am`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65432 was successfully built and tested.

Reviews applied: `['65432']`

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

- Mesos Reviewbot Windows


On Jan. 31, 2018, 9:20 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65432/
> ---
> 
> (Updated Jan. 31, 2018, 9:20 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `examples/flags.hpp` to `src/Makefile.am`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 155338673e1c23a2cfeab39bdf3ebe14568bfebc 
> 
> 
> Diff: https://reviews.apache.org/r/65432/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 65432: Added `examples/flags.hpp` to `src/Makefile.am`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65432 was successfully built and tested.

Reviews applied: `['65432']`

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

- Mesos Reviewbot Windows


On Jan. 31, 2018, 9:20 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65432/
> ---
> 
> (Updated Jan. 31, 2018, 9:20 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `examples/flags.hpp` to `src/Makefile.am`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 155338673e1c23a2cfeab39bdf3ebe14568bfebc 
> 
> 
> Diff: https://reviews.apache.org/r/65432/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 65432: Added `examples/flags.hpp` to `src/Makefile.am`.

[Michael Park]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Added `examples/flags.hpp` to `src/Makefile.am`.


Diffs
-

  src/Makefile.am 155338673e1c23a2cfeab39bdf3ebe14568bfebc 


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


Testing
---


Thanks,

Michael Park