Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

[Gilbert Song]

> On Dec. 22, 2017, 3:55 p.m., Jie Yu wrote:
> > @gilbert, @zhitao, is this a 1.5.0 blocker? Do we want to land this?

yes, but we still have one small issue to address. I will cherrypick it to the 
1.5.x branch. thanks!


- Gilbert


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


On Dec. 22, 2017, 12:10 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> ---
> 
> (Updated Dec. 22, 2017, 12:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> ---
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged 
> through new operator API call while active images (running or being pulled) 
> are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

[Gilbert Song]

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




src/slave/http.cpp
Lines 2452 (patched)


As we discussed offline, we should follow the semantic that if there are 
excluded images from the agent flag, they should have higher priority.


- Gilbert Song


On Dec. 22, 2017, 12:10 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> ---
> 
> (Updated Dec. 22, 2017, 12:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> ---
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged 
> through new operator API call while active images (running or being pulled) 
> are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 64823: Documented the change in the Protobuf requirement.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Qian Zhang.


Repository: mesos


Description
---

Documented the change in the Protobuf requirement.


Diffs
-

  CHANGELOG 179b53b0568e4d993553f5cdb13668d96aa400d0 
  docs/upgrades.md f2eb5a94d67b44a02d355981a673bd4ef5129653 


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


Testing
---

N/A

The requirement is checked in https://reviews.apache.org/r/64822/.


Thanks,

Chun-Hung Hsiao

Re: Review Request 64822: Made protobuf version 3+ a hard requirement for building Mesos.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Dec. 22, 2017, 11:56 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64822/
> ---
> 
> (Updated Dec. 22, 2017, 11:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made protobuf version 3+ a hard requirement for building Mesos.
> 
> 
> Diffs
> -
> 
>   configure.ac 5ffc33a6b9495db64f1ec164ed96f3faea620348 
> 
> 
> Diff: https://reviews.apache.org/r/64822/diff/1/
> 
> 
> Testing
> ---
> 
> Tested the following cases:
> `../configure --with-protobuf=/opt/protobuf-2.6.1 --disable-java 
> --disable-python` => Failure
> `../configure --with-protobuf=/opt/protobuf-3.3.0 --disable-java 
> --disable-python` => Success
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 64822: Made protobuf version 3+ a hard requirement for building Mesos.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Made protobuf version 3+ a hard requirement for building Mesos.


Diffs
-

  configure.ac 5ffc33a6b9495db64f1ec164ed96f3faea620348 


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


Testing
---

Tested the following cases:
`../configure --with-protobuf=/opt/protobuf-2.6.1 --disable-java 
--disable-python` => Failure
`../configure --with-protobuf=/opt/protobuf-3.3.0 --disable-java 
--disable-python` => Success


Thanks,

Chun-Hung Hsiao

Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

[Jie Yu]

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



@gilbert, @zhitao, is this a 1.5.0 blocker? Do we want to land this?

- Jie Yu


On Dec. 22, 2017, 8:10 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> ---
> 
> (Updated Dec. 22, 2017, 8:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> ---
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged 
> through new operator API call while active images (running or being pulled) 
> are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 64818: Fixed compile error for `make distcheck`.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Dec. 22, 2017, 11:35 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64818/
> ---
> 
> (Updated Dec. 22, 2017, 11:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed compile error for `make distcheck`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e587cff8c6d2c7f7189e2486feaefc9d20b3461c 
> 
> 
> Diff: https://reviews.apache.org/r/64818/diff/1/
> 
> 
> Testing
> ---
> 
> make distcheck with `--enable-grpc`
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 64818: Fixed compile error for `make distcheck`.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Fixed compile error for `make distcheck`.


Diffs
-

  src/Makefile.am e587cff8c6d2c7f7189e2486feaefc9d20b3461c 


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


Testing
---

make distcheck with `--enable-grpc`


Thanks,

Chun-Hung Hsiao

Re: Review Request 64814: Renamed isCommandExecutor to isGeneratedForCommandTask.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On Dec. 22, 2017, 9:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64814/
> ---
> 
> (Updated Dec. 22, 2017, 9:22 p.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The name `isCommandExecutor` is not really precise because it could also
> for Docker Executor. This patch renamed this method to be more accurate.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp e0f0c82981b735b2ae1d94309a9e84f673d20cce 
>   src/slave/slave.cpp dde3dac19b8fe32b9a08d89acca918be8d7f38c5 
> 
> 
> Diff: https://reviews.apache.org/r/64814/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 64793: Fixed a bug where resource over allocation may break quota headroom.

[Benjamin Mahler]

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


Fix it, then Ship it!




Hm.. technically the reservation might not be "making progress towards quota" 
if it does not involve any quota resources. I'll clarify that in the comments / 
description.


src/master/allocator/mesos/hierarchical.cpp
Line 1930 (original), 1946-1948 (patched)


Looks like this being guarded by `if (!unsatisfiedQuota.empty()) {` 
prevents this from adding the resources when just reservation(s) are being 
allocated, contrary to the comments?


- Benjamin Mahler


On Dec. 22, 2017, 4:17 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64793/
> ---
> 
> (Updated Dec. 22, 2017, 4:17 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8352
> https://issues.apache.org/jira/browse/MESOS-8352
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the quota role allocation stage, if a role gets some resources
> on an agent to meet its quota, it will also get all other resources
> on the same agent that it does not have quota for. This may starve
> roles behind it that have quotas set for those resources.
> 
> We fix this issue by enforcing that, in the quota role allocation
> stage, if a role has no quota set for a scalar resource, it will
> get that resource only when two conditions are both met:
> 
> (1) It got some other resources on the same agent to meet its quota;
> 
> (2) After allocating those resources, quota headroom is still above
> the required amount.
> 
> Also refactored the fine-grained quota allocation logic.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 78d7b2203532301ff26b7abb26f5d320831124a1 
> 
> 
> Diff: https://reviews.apache.org/r/64793/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64806: Fixed resource provider driver disconnection handling.

[Jie Yu]

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




src/resource_provider/http_connection.hpp
Lines 167 (patched)


This migh still trigger `detected` depending on how `detector->detect` is 
implemented. That means `detected` might be called multiple times?

I'd suggest we fix ConstantEndpointDetector to properly handle discard.


- Jie Yu


On Dec. 22, 2017, 1:36 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64806/
> ---
> 
> (Updated Dec. 22, 2017, 1:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8349
> https://issues.apache.org/jira/browse/MESOS-8349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The expectation for disconnection of the resource provider driver is
> that disconnection handlers of the resource provider would be invoked
> and a new connection would be detected.
> 
> Since the existing implementation did not account for the possibility of
> a remote endpoint already being detected, the code did now show the
> expected behavior. This is fixed in this patch by explicitly triggering
> an endpoint redetection if a disconnection was detected. We also modify
> the driver's 'start' method to implicitly discard any pending endpoint
> detections.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/http_connection.hpp 
> 3d5088dc1eeeb64a932a731d10e4c12564e79034 
>   src/tests/resource_provider_manager_tests.cpp 
> f58ab6b4c98eff92ae23acacefbcb9066b5ace91 
> 
> 
> Diff: https://reviews.apache.org/r/64806/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 64798: Added an utility to shrink scalar resource while keeping its meta-data.

[Benjamin Mahler]

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


Ship it!





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


We may want to avoid mentioning offers here and just say instead that they 
are indivisible?


- Benjamin Mahler


On Dec. 22, 2017, 4:08 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64798/
> ---
> 
> (Updated Dec. 22, 2017, 4:08 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an utility to shrink scalar resource while keeping its meta-data.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp eefe9eac23dd1eeb35df9ba1d109a446526ee0c3 
>   include/mesos/v1/resources.hpp 7a5b0e3cf156b04a232e014e87c49f8f1baa9133 
>   src/common/resources.cpp 919a03b6fa19fc9db4242a32fba3c617d1705413 
>   src/v1/resources.cpp 365ffa7b7f05d95321a8078a91f5bb17b2ee9419 
> 
> 
> Diff: https://reviews.apache.org/r/64798/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 64814: Renamed isCommandExecutor to isGeneratedForCommandTask.

[Jie Yu]

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

Review request for mesos and Chun-Hung Hsiao.


Repository: mesos


Description
---

The name `isCommandExecutor` is not really precise because it could also
for Docker Executor. This patch renamed this method to be more accurate.


Diffs
-

  src/slave/slave.hpp e0f0c82981b735b2ae1d94309a9e84f673d20cce 
  src/slave/slave.cpp dde3dac19b8fe32b9a08d89acca918be8d7f38c5 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 64811: Cleaned up ContainerInfo generation logic in the slave.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 64811 was successfully built and tested.

Reviews applied: `['63598', '64811']`

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

- Mesos Reviewbot Windows


On Dec. 22, 2017, 7:47 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64811/
> ---
> 
> (Updated Dec. 22, 2017, 7:47 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gilbert Song, and Julien Pepy.
> 
> 
> Bugs: MESOS-7007
> https://issues.apache.org/jira/browse/MESOS-7007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After this patch, the invariant becomes that for command tasks, we
> always set the ContainerInfo of the generated ExecutorInfo to be the
> same as that in TaskInfo (if exists). This simplifies the logic when we
> actually generate ContainerInfo for containerizer during launching
> phase.
> 
> This also made the logic introduced in the following patch more readable
> and maintainable: https://reviews.apache.org/r/63598
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 47d5632115bbfd2b729dc2388c01733f907cf938 
> 
> 
> Diff: https://reviews.apache.org/r/64811/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 64811: Cleaned up ContainerInfo generation logic in the slave.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On Dec. 22, 2017, 7:47 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64811/
> ---
> 
> (Updated Dec. 22, 2017, 7:47 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gilbert Song, and Julien Pepy.
> 
> 
> Bugs: MESOS-7007
> https://issues.apache.org/jira/browse/MESOS-7007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After this patch, the invariant becomes that for command tasks, we
> always set the ContainerInfo of the generated ExecutorInfo to be the
> same as that in TaskInfo (if exists). This simplifies the logic when we
> actually generate ContainerInfo for containerizer during launching
> phase.
> 
> This also made the logic introduced in the following patch more readable
> and maintainable: https://reviews.apache.org/r/63598
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 47d5632115bbfd2b729dc2388c01733f907cf938 
> 
> 
> Diff: https://reviews.apache.org/r/64811/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 64813: Documented new image gc support in Mesos containerizer.

[Zhitao Li]

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

(Updated Dec. 22, 2017, 8:21 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Documented new image gc support in Mesos containerizer.


Diffs
-

  docs/container-image.md 99f4f5c5b617f5deb618614fda1365d72c8685de 
  docs/operator-http-api.md f81e5bcb38455e6595e54892604773665140933d 


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


Testing (updated)
---

https://github.com/zhitaoli/mesos/blob/public/zhitao/image_gc_revision3/docs/container-image.md


Thanks,

Zhitao Li

Review Request 64813: Documented new image gc support in Mesos containerizer.

[Zhitao Li]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Documented new image gc support in Mesos containerizer.


Diffs
-

  docs/container-image.md 99f4f5c5b617f5deb618614fda1365d72c8685de 
  docs/operator-http-api.md f81e5bcb38455e6595e54892604773665140933d 


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


Testing
---


Thanks,

Zhitao Li

Review Request 64812: Added `excluded_images` parameter to `PRUNE_IMAGES` agent API.

[Zhitao Li]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added `excluded_images` parameter to `PRUNE_IMAGES` agent API.


Diffs
-

  include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
  include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
  src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

[Zhitao Li]

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

(Updated Dec. 22, 2017, 8:10 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added a new operator API for `PRUNE_IMAGES`.


Diffs (updated)
-

  include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
  include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
  src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
  src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
  src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 


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

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


Testing
---

1. New unit test to trigger the new call.
2. Manually tested that images previous pulled but not running can be purged 
through new operator API call while active images (running or being pulled) are 
not affected.


Thanks,

Zhitao Li

Re: Review Request 63598: Set container info from executor by default if available.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On Nov. 8, 2017, 1:03 a.m., Julien Pepy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63598/
> ---
> 
> (Updated Nov. 8, 2017, 1:03 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-7007
> https://issues.apache.org/jira/browse/MESOS-7007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current implementation only works for non-command executor
> instances. We still need to get the container info from the executor
> (if none has been defined in the task) in command mode to properly use
> some features (volumes for example).
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 
> 
> 
> Diff: https://reviews.apache.org/r/63598/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested manually with a single task on an agent configured with 
> `--default_container_info='{"type":"MESOS","volumes":[{"host_path":"tmp","container_path":"/tmp","mode":"RW"}]}'`:
> ```
> sudo -u hello mesos-execute --master='localhost:5050' --name=test 
> --command="echo hello >/tmp/test; df -hT /tmp" | tee ~/test.log
> framework_id=$(cat ~/test.log | grep Subscribed | awk '{print $4}')
> ls -l 
> /var/opt/mesos/slaves/*/frameworks/$framework_id/executors/test/runs/latest/tmp
> ```
> 
> Result output:
> ```
> Filesystem Type  Size  Used Avail Use% Mounted on
> /dev/sda3  ext4   37G  7.3G   28G  21% /tmp
> total 4
> -rw-r--r--. 1 hello users 6  6 nov.  15:21 test
> ```
> 
> 
> Thanks,
> 
> Julien Pepy
> 
>

Review Request 64811: Cleaned up ContainerInfo generation logic in the slave.

[Jie Yu]

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

Review request for mesos, Chun-Hung Hsiao and Gilbert Song.


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


Repository: mesos


Description
---

After this patch, the invariant becomes that for command tasks, we
always set the ContainerInfo of the generated ExecutorInfo to be the
same as that in TaskInfo (if exists). This simplifies the logic when we
actually generate ContainerInfo for containerizer during launching
phase.

This also made the logic introduced in the following patch more readable
and maintainable: https://reviews.apache.org/r/63598


Diffs
-

  src/slave/slave.cpp 47d5632115bbfd2b729dc2388c01733f907cf938 


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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 64686: Fixed the type-punned pointer and strict aliasing issue.

[Alexander Rukletsov]

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

(Updated Dec. 22, 2017, 6:41 p.m.)


Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.


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


Repository: mesos


Description (updated)
---

Dereferencing a pointer cast from a different type of pointer
violates the so-called "strict aliasing" rule, which is undefined
behaviour and might lead to bugs when compiler optimizations are
enabled.

For more information on this topic, see
https://blog.regehr.org/archives/959
http://alas.matf.bg.ac.rs/manuals/lspe/snode=153.html


Diffs
-

  src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 


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


Testing
---

make check on Mac OS 10.11.6 and various Linux distros


Thanks,

Alexander Rukletsov

Re: Review Request 64686: Fixed the type-punned pointer and strict aliasing issue.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Dec. 22, 2017, 5:42 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64686/
> ---
> 
> (Updated Dec. 22, 2017, 5:42 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.
> 
> 
> Bugs: MESOS-6616
> https://issues.apache.org/jira/browse/MESOS-6616
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Dereferencing a pointer cast from a different type of pointer
> violates the so-called “strict aliasing” rule, which is undefined
> behaviour and might lead to bugs when compiler optimizations are
> enabled.
> 
> For more information on this topic, see
> https://blog.regehr.org/archives/959
> http://alas.matf.bg.ac.rs/manuals/lspe/snode=153.html
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64686/diff/3/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.11.6 and various Linux distros
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64770: Fixed a potential race condition in the test infrastructure.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 64770 was successfully built and tested.

Reviews applied: `['64770']`

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

- Mesos Reviewbot Windows


On Dec. 21, 2017, 3:58 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64770/
> ---
> 
> (Updated Dec. 21, 2017, 3:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-7506
> https://issues.apache.org/jira/browse/MESOS-7506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There was a race condition leading to flaky
> `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` test.
> This test launches successively multiple agents, while reusing the same
> variable. After reassigning the value of the variable, agent's d'tor is
> called. If agent recovery is not yet completed, then some orphaned
> container might blink in the agent's d'tor as it is described in the
> comment to the code.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp f964bf0cd0cf22374877e5748ba142dcb5fee133 
> 
> 
> Diff: https://reviews.apache.org/r/64770/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check (fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 64810: Fixed clang build on e.g., ubuntu16.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Dec. 22, 2017, 3:21 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64810/
> ---
> 
> (Updated Dec. 22, 2017, 3:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, and Ilya 
> Pronin.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The clang version on e.g., ubuntu16 would emit a diagnostic message
> for the increment inside the expectation,
> 
> ../../src/tests/log_tests.cpp:2439:120:   \
>   error: expression with side effects has no effect
> 
> We reorder expressions to avoid above situation.
> 
> 
> Diffs
> -
> 
>   src/tests/log_tests.cpp 5dcb693928cc68ab135af5e289fe33a80cf2e69a 
> 
> 
> Diff: https://reviews.apache.org/r/64810/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`, ubuntu16 w/ clang.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 64810: Fixed clang build on e.g., ubuntu16.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov, Armand Grillet, and Ilya Pronin.


Repository: mesos


Description
---

The clang version on e.g., ubuntu16 would emit a diagnostic message
for the increment inside the expectation,

../../src/tests/log_tests.cpp:2439:120:   \
  error: expression with side effects has no effect

We reorder expressions to avoid above situation.


Diffs
-

  src/tests/log_tests.cpp 5dcb693928cc68ab135af5e289fe33a80cf2e69a 


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


Testing
---

`make check`, ubuntu16 w/ clang.


Thanks,

Benjamin Bannier

Re: Review Request 64770: Fixed a potential race condition in the test infrastructure.

[Andrei Budnik]

> On Dec. 22, 2017, 3:38 a.m., Greg Mann wrote:
> > This looks like a reasonable solution to me. However, it would be great if 
> > we could reproduce the bug and then verify the fix. Looking at the log of a 
> > failed test run in the JIRA, it seems to me that the problem occurs when 
> > cleanup of an orphaned container left over from a previous test is 
> > attempted by the agent destructor called during 
> > `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags`. To attempt a 
> > repro, I would suggest the following:
> > 1) Peg the CPU on the machine so that libprocess takes a long time to 
> > process messages in its queue
> > 2) Run `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` and one 
> > other (fast-running) test which creates a container, setting 
> > '--gtest_repeat=-1'
> > 
> > Hopefully, this may recreate the circumstances which led to the failure in 
> > CI?

That's a good idea! I'm going to try to reproduce the bug by running multiple 
tests simultaneously.


- Andrei


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


On Dec. 21, 2017, 3:58 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64770/
> ---
> 
> (Updated Dec. 21, 2017, 3:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-7506
> https://issues.apache.org/jira/browse/MESOS-7506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There was a race condition leading to flaky
> `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` test.
> This test launches successively multiple agents, while reusing the same
> variable. After reassigning the value of the variable, agent's d'tor is
> called. If agent recovery is not yet completed, then some orphaned
> container might blink in the agent's d'tor as it is described in the
> comment to the code.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp f964bf0cd0cf22374877e5748ba142dcb5fee133 
> 
> 
> Diff: https://reviews.apache.org/r/64770/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check (fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 64807: Removed redundant `struct` prefix in "linux/ns.cpp".

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 64686.

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

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

Relevant logs:

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

```
Traceback (most recent call last):
  File ".\support\apply-reviews.py", line 434, in 
main()
  File ".\support\apply-reviews.py", line 429, in main
reviewboard(options)
  File ".\support\apply-reviews.py", line 419, in reviewboard
apply_review(options)
  File ".\support\apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File ".\support\apply-reviews.py", line 271, in commit_patch
message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u201c' in position 
143: ordinal not in range(128)
```

- Mesos Reviewbot Windows


On Dec. 22, 2017, 8:42 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64807/
> ---
> 
> (Updated Dec. 22, 2017, 8:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64807/diff/1/
> 
> 
> Testing
> ---
> 
> make check on various Linux distros
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64806: Fixed resource provider driver disconnection handling.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 64806 was successfully built and tested.

Reviews applied: `['64805', '64806']`

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

- Mesos Reviewbot Windows


On Dec. 22, 2017, 1:36 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64806/
> ---
> 
> (Updated Dec. 22, 2017, 1:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8349
> https://issues.apache.org/jira/browse/MESOS-8349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The expectation for disconnection of the resource provider driver is
> that disconnection handlers of the resource provider would be invoked
> and a new connection would be detected.
> 
> Since the existing implementation did not account for the possibility of
> a remote endpoint already being detected, the code did now show the
> expected behavior. This is fixed in this patch by explicitly triggering
> an endpoint redetection if a disconnection was detected. We also modify
> the driver's 'start' method to implicitly discard any pending endpoint
> detections.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/http_connection.hpp 
> 3d5088dc1eeeb64a932a731d10e4c12564e79034 
>   src/tests/resource_provider_manager_tests.cpp 
> f58ab6b4c98eff92ae23acacefbcb9066b5ace91 
> 
> 
> Diff: https://reviews.apache.org/r/64806/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 64685: Improved the comment around the union in ns::clone().

[Alexander Rukletsov]

> On Dec. 22, 2017, 2:29 p.m., Benjamin Bannier wrote:
> > src/linux/ns.cpp
> > Line 320 (original), 320 (patched)
> > 
> >
> > Since you are at it, could you make the first part a full sentence, 
> > e.g.,
> > 
> > > _We_ need ...

Sure!


- Alexander


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


On Dec. 22, 2017, 1:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64685/
> ---
> 
> (Updated Dec. 22, 2017, 1:42 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64685/diff/2/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/64686/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64807: Removed redundant `struct` prefix in "linux/ns.cpp".

[Alexander Rukletsov]

> On Dec. 22, 2017, 2:29 p.m., Benjamin Bannier wrote:
> > src/linux/ns.cpp
> > Line 184 (original), 184 (patched)
> > 
> >
> > Kill this `struct` as well?

Not sure we can do this. There is also `stat()` POSIX function.


- Alexander


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


On Dec. 22, 2017, 1:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64807/
> ---
> 
> (Updated Dec. 22, 2017, 1:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64807/diff/1/
> 
> 
> Testing
> ---
> 
> make check on various Linux distros
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64807: Removed redundant `struct` prefix in "linux/ns.cpp".

[Benjamin Bannier]

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


Fix it, then Ship it!





src/linux/ns.cpp
Line 184 (original), 184 (patched)


Kill this `struct` as well?


- Benjamin Bannier


On Dec. 22, 2017, 2:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64807/
> ---
> 
> (Updated Dec. 22, 2017, 2:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64807/diff/1/
> 
> 
> Testing
> ---
> 
> make check on various Linux distros
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64685: Improved the comment around the union in ns::clone().

[Benjamin Bannier]

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


Fix it, then Ship it!





src/linux/ns.cpp
Line 320 (original), 320 (patched)


Since you are at it, could you make the first part a full sentence, e.g.,

> _We_ need ...


- Benjamin Bannier


On Dec. 22, 2017, 2:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64685/
> ---
> 
> (Updated Dec. 22, 2017, 2:42 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64685/diff/2/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/64686/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64686: Fixed the type-punned pointer and strict aliasing issue.

[Alexander Rukletsov]

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

(Updated Dec. 22, 2017, 1:42 p.m.)


Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.


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


Repository: mesos


Description
---

Dereferencing a pointer cast from a different type of pointer
violates the so-called “strict aliasing” rule, which is undefined
behaviour and might lead to bugs when compiler optimizations are
enabled.

For more information on this topic, see
https://blog.regehr.org/archives/959
http://alas.matf.bg.ac.rs/manuals/lspe/snode=153.html


Diffs (updated)
-

  src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 


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

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


Testing
---

make check on Mac OS 10.11.6 and various Linux distros


Thanks,

Alexander Rukletsov

Re: Review Request 64685: Improved the comment around the union in ns::clone().

[Alexander Rukletsov]

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

(Updated Dec. 22, 2017, 1:42 p.m.)


Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 


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

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


Testing
---

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


Thanks,

Alexander Rukletsov

Review Request 64807: Removed redundant `struct` prefix in "linux/ns.cpp".

[Alexander Rukletsov]

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

Review request for mesos, Benjamin Bannier and Michael Park.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 


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


Testing
---

make check on various Linux distros


Thanks,

Alexander Rukletsov

Re: Review Request 64686: Fixed the type-punned pointer and strict aliasing issue.

[Alexander Rukletsov]

> On Dec. 21, 2017, 8 p.m., Benjamin Bannier wrote:
> > src/linux/ns.cpp
> > Line 397 (original), 397-399 (patched)
> > 
> >
> > Why do we put `struct` here? This is C++ (and the `memcpy` fits on a 
> > single line w/o it).
> > 
> > Not yours, but we should probably also include `sys/socket.h` for 
> > `ucred` in this file.

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


> On Dec. 21, 2017, 8 p.m., Benjamin Bannier wrote:
> > src/linux/ns.cpp
> > Line 455 (original), 459 (patched)
> > 
> >
> > `struct`?

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


> On Dec. 21, 2017, 8 p.m., Benjamin Bannier wrote:
> > src/linux/ns.cpp
> > Lines 472 (patched)
> > 
> >
> > `struct`?

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


- Alexander


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


On Dec. 22, 2017, 1:39 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64686/
> ---
> 
> (Updated Dec. 22, 2017, 1:39 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.
> 
> 
> Bugs: MESOS-6616
> https://issues.apache.org/jira/browse/MESOS-6616
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Dereferencing a pointer cast from a different type of pointer
> violates the so-called “strict aliasing” rule, which is undefined
> behaviour and might lead to bugs when compiler optimizations are
> enabled.
> 
> For more information on this topic, see
> https://blog.regehr.org/archives/959
> http://alas.matf.bg.ac.rs/manuals/lspe/snode=153.html
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
> 
> 
> Diff: https://reviews.apache.org/r/64686/diff/2/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.11.6 and various Linux distros
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64686: Fixed the type-punned pointer and strict aliasing issue.

[Alexander Rukletsov]

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

(Updated Dec. 22, 2017, 1:39 p.m.)


Review request for mesos, Armand Grillet, Benjamin Bannier, and Michael Park.


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


Repository: mesos


Description (updated)
---

Dereferencing a pointer cast from a different type of pointer
violates the so-called “strict aliasing” rule, which is undefined
behaviour and might lead to bugs when compiler optimizations are
enabled.

For more information on this topic, see
https://blog.regehr.org/archives/959
http://alas.matf.bg.ac.rs/manuals/lspe/snode=153.html


Diffs
-

  src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 


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


Testing
---

make check on Mac OS 10.11.6 and various Linux distros


Thanks,

Alexander Rukletsov

Review Request 64806: Fixed resource provider driver disconnection handling.

[Benjamin Bannier]

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

Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

The expectation for disconnection of the resource provider driver is
that disconnection handlers of the resource provider would be invoked
and a new connection would be detected.

Since the existing implementation did not account for the possibility of
a remote endpoint already being detected, the code did now show the
expected behavior. This is fixed in this patch by explicitly triggering
an endpoint redetection if a disconnection was detected. We also modify
the driver's 'start' method to implicitly discard any pending endpoint
detections.


Diffs
-

  src/resource_provider/http_connection.hpp 
3d5088dc1eeeb64a932a731d10e4c12564e79034 
  src/tests/resource_provider_manager_tests.cpp 
f58ab6b4c98eff92ae23acacefbcb9066b5ace91 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 64805: Changed 'ConstantEndpointDetector' to have value semantics.

[Benjamin Bannier]

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

Review request for mesos, Jie Yu and Jan Schlicht.


Repository: mesos


Description
---

The class 'ConstantEndpointDetector' was holding a reference to one of
its constructor parameters which introduces unnecessary brittleness.

In this patch we adjust the class to just hold a copy of the
originally passed value instead.


Diffs
-

  src/resource_provider/detector.hpp 68ea8cf2de4f2decc3eede16cf59fa8a851502f4 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 64798: Added an utility to shrink scalar resource while keeping its meta-data.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 64798 was successfully built and tested.

Reviews applied: `['64798']`

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

- Mesos Reviewbot Windows


On Dec. 22, 2017, 4:08 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64798/
> ---
> 
> (Updated Dec. 22, 2017, 4:08 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an utility to shrink scalar resource while keeping its meta-data.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp eefe9eac23dd1eeb35df9ba1d109a446526ee0c3 
>   include/mesos/v1/resources.hpp 7a5b0e3cf156b04a232e014e87c49f8f1baa9133 
>   src/common/resources.cpp 919a03b6fa19fc9db4242a32fba3c617d1705413 
>   src/v1/resources.cpp 365ffa7b7f05d95321a8078a91f5bb17b2ee9419 
> 
> 
> Diff: https://reviews.apache.org/r/64798/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64765: Removed a stale TODO in the allocator.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 64765 was successfully built and tested.

Reviews applied: `['64765']`

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

- Mesos Reviewbot Windows


On Dec. 21, 2017, 3:49 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64765/
> ---
> 
> (Updated Dec. 21, 2017, 3:49 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This TODO was added when we used to skip roles with satisfied
> quota during the first phase of allocation. We no longer do
> this since we now allocate reservations for these roles.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7f7dae1950bebd22191189c2db0f6678d2e8fa3c 
> 
> 
> Diff: https://reviews.apache.org/r/64765/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 64070: Ensured executor adapter propagates error and shutdown messages.

[Alexander Rukletsov]

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

(Updated Dec. 22, 2017, 11:08 a.m.)


Review request for mesos, Andrei Budnik, Anand Mazumdar, Armand Grillet, and 
Vinod Kone.


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


Repository: mesos


Description
---

Prior to this patch, if an error, kill, or shutdown occurred during
subscription / registration with the agent, it was not propagated back
to the executor if the v0_v1 executor adapter was used. This happened
because the adapter did not call the `connected` callback until after
successful registration and hence the executor did not even try to
send the `SUBSCRIBE` call, without which the adapter did not send any
events to the executor.

A fix is to call the `connected` callback if an error occurred or
shutdown / kill event arrived before the executor had subscribed.


Diffs
-

  src/executor/v0_v1executor.cpp 61d591993e6388ba3b4d64a3bdb63c3a3513fbeb 


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


Testing
---


Thanks,

Alexander Rukletsov