Re: Review Request 71151: Performed periodic storage local provider reconciliations.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [71143, 71144, 71145, 71146, 71147, 71148, 71149, 71150, 71151]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' 
BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose 
--disable-libtool-wrappers --disable-parallel-test-execution' 
ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh 2>&1 | tee 
build_71151"]

Error:
..
hino-java.
Preparing to unpack .../librhino-java_1.7R4-2_all.deb ...
Unpacking librhino-java (1.7R4-2) ...
Selecting previously unselected package libsaxon-java.
Preparing to unpack .../libsaxon-java_1%3a6.5.5-10_all.deb ...
Unpacking libsaxon-java (1:6.5.5-10) ...
Selecting previously unselected package libtinfo-dev:amd64.
Preparing to unpack .../libtinfo-dev_5.9+20140118-1ubuntu1_amd64.deb ...
Unpacking libtinfo-dev:amd64 (5.9+20140118-1ubuntu1) ...
Selecting previously unselected package libtool.
Preparing to unpack .../libtool_2.4.2-1.7ubuntu1_amd64.deb ...
Unpacking libtool (2.4.2-1.7ubuntu1) ...
Selecting previously unselected package libwagon2-java.
Preparing to unpack .../libwagon2-java_2.5-1_all.deb ...
Unpacking libwagon2-java (2.5-1) ...
Selecting previously unselected package libxom-java.
Preparing to unpack .../libxom-java_1.2.10-1_all.deb ...
Unpacking libxom-java (1.2.10-1) ...
Selecting previously unselected package lksctp-tools.
Preparing to unpack .../lksctp-tools_1.0.15+dfsg-1_amd64.deb ...
Unpacking lksctp-tools (1.0.15+dfsg-1) ...
Selecting previously unselected package llvm-3.5-runtime.
Preparing to unpack .../llvm-3.5-runtime_1%3a3.5-4ubuntu2~trusty2_amd64.deb ...
Unpacking llvm-3.5-runtime (1:3.5-4ubuntu2~trusty2) ...
Selecting previously unselected package llvm-3.5.
Preparing to unpack .../llvm-3.5_1%3a3.5-4ubuntu2~trusty2_amd64.deb ...
Unpacking llvm-3.5 (1:3.5-4ubuntu2~trusty2) ...
Selecting previously unselected package libffi-dev:amd64.
Preparing to unpack .../libffi-dev_3.1~rc1+r3.0.13-12ubuntu0.2_amd64.deb ...
Unpacking libffi-dev:amd64 (3.1~rc1+r3.0.13-12ubuntu0.2) ...
Selecting previously unselected package llvm-3.5-dev.
Preparing to unpack .../llvm-3.5-dev_1%3a3.5-4ubuntu2~trusty2_amd64.deb ...
Unpacking llvm-3.5-dev (1:3.5-4ubuntu2~trusty2) ...
Selecting previously unselected package manpages-dev.
Preparing to unpack .../manpages-dev_3.54-1ubuntu1_all.deb ...
Unpacking manpages-dev (3.54-1ubuntu1) ...
Selecting previously unselected package maven.
Preparing to unpack .../archives/maven_3.0.5-1_all.deb ...
Unpacking maven (3.0.5-1) ...
Selecting previously unselected package python3-pycurl.
Preparing to unpack .../python3-pycurl_7.19.3-0ubuntu3_amd64.deb ...
Unpacking python3-pycurl (7.19.3-0ubuntu3) ...
Selecting previously unselected package unattended-upgrades.
Preparing to unpack .../unattended-upgrades_0.82.1ubuntu2.5_all.deb ...
Unpacking unattended-upgrades (0.82.1ubuntu2.5) ...
Selecting previously unselected package python3-software-properties.
Preparing to unpack .../python3-software-properties_0.92.37.8_all.deb ...
Unpacking python3-software-properties (0.92.37.8) ...
Selecting previously unselected package rhino.
Preparing to unpack .../archives/rhino_1.7R4-2_all.deb ...
Unpacking rhino (1.7R4-2) ...
Selecting previously unselected package software-properties-common.
Preparing to unpack .../software-properties-common_0.92.37.8_all.deb ...
Unpacking software-properties-common (0.92.37.8) ...
Selecting previously unselected package tcpd.
Preparing to unpack .../tcpd_7.6.q-25_amd64.deb ...
Unpacking tcpd (7.6.q-25) ...
Selecting previously unselected package testng.
Preparing to unpack .../testng_6.8.7-2_all.deb ...
Unpacking testng (6.8.7-2) ...
Processing triggers for ureadahead (0.100.0-16) ...
Processing triggers for mime-support (3.54ubuntu1.1) ...
Setting up libroken18-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up libasn1-8-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up libkrb5support0:amd64 (1.12+dfsg-2ubuntu5.4) ...
Setting up libk5crypto3:amd64 (1.12+dfsg-2ubuntu5.4) ...
Setting up libkeyutils1:amd64 (1.5.6-1) ...
Setting up libkrb5-3:amd64 (1.12+dfsg-2ubuntu5.4) ...
Setting up libgssapi-krb5-2:amd64 (1.12+dfsg-2ubuntu5.4) ...
Setting up libidn11:amd64 (1.28-1ubuntu2.2) ...
Setting up libhcrypto4-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up libheimbase1-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up libwind0-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up libhx509-5-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up libkrb5-26-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up libheimntlm0-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up libgssapi3-heimdal:amd64 (1.6~git20131207+dfsg-1ubuntu1.2) ...
Setting up 

Re: Review Request 71083: Added enchanced multi-role capability support to the python bindings.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 23, 2019, 4:27 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71083/
> ---
> 
> (Updated July 23, 2019, 4:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9864
> https://issues.apache.org/jira/browse/MESOS-9864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds suppressed roles list to the arguments of the scheduler
> driver constructor and `revivieOffers()`/`suppressOffers` methods, and
> also adds support of the `updateFramework()` method.
> 
> 
> Diffs
> -
> 
>   src/python/interface/src/mesos/interface/__init__.py 
> 1200ef652f6458b9bbcd29b4d93d34c28230c7d0 
>   src/python/scheduler/src/mesos/scheduler/mesos_scheduler_driver_impl.hpp 
> 8c98d46fb842595956d58a3f91f805e44be2bcf2 
>   src/python/scheduler/src/mesos/scheduler/mesos_scheduler_driver_impl.cpp 
> 6dec9daa295cfc67176fb043322b8e98efdd7d4b 
> 
> 
> Diff: https://reviews.apache.org/r/71083/diff/2/
> 
> 
> Testing
> ---
> 
> Unsuppressing via update: `./bin/mesos-tests.sh 
> --gtest_filter="ExamplesTest.PythonFramework*" --gtest_break_on_failure 
> --gtest_repeat=50`  with the changes from the dependent patch.
> revivie/suppress: tested manually.
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71082: Added a helper to create std::vector from iterable into python bindings.

[Benjamin Mahler]

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


Fix it, then Ship it!




Looks good, I only held off on committing this because I don't have a python 
build handy to test that removing the redundant declaration would compile 
successfully.


src/python/native_common/common.hpp
Lines 155-159 (patched)


Is this still needed? Looks like a declaration of the definition above?



src/python/native_common/common.hpp
Lines 201 (patched)


Period at the end


- Benjamin Mahler


On July 23, 2019, 4:13 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71082/
> ---
> 
> (Updated July 23, 2019, 4:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9864
> https://issues.apache.org/jira/browse/MESOS-9864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper to create std::vector from iterable into python bindings.
> 
> 
> Diffs
> -
> 
>   src/python/native_common/common.hpp 
> ed743aab014301981fe2526a7e81f4a35c02e19b 
> 
> 
> Diff: https://reviews.apache.org/r/71082/diff/2/
> 
> 
> Testing
> ---
> 
> With valid arguments - see next patches.
> Ran with invalid arguments manually.
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71111: Added offer rescind logic for guarantees enforcement.

[Benjamin Mahler]

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


Fix it, then Ship it!




Ditto here, could you add the caveat about the under-rescind to the commit 
description?


src/master/quota_handler.cpp
Lines 662-667 (patched)


Thanks!



src/tests/master_quota_tests.cpp
Lines 1733 (patched)


Do you mean Offered here?



src/tests/master_quota_tests.cpp
Lines 1735-1736 (patched)


This seems a bit weak as a test? But I suppose we don't want to write a 
test that's revealing the quirks of the current under/over rescind 
implementation?



src/tests/master_quota_tests.cpp
Lines 1736 (patched)


Outstanding


- Benjamin Mahler


On July 23, 2019, 8:39 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7/
> ---
> 
> (Updated July 23, 2019, 8:39 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9812
> https://issues.apache.org/jira/browse/MESOS-9812
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Outstanding offers need to be rescinded as needed to ensure
> roles' requested guarantees can be satisfied.
> 
> For simplicity, and also due to the race between the master and
> the allocator, we pessimistically assume that what seems like
> "available" resources in the allocator are all gone. We greedily
> rescind offers until we can satisfy the guarantees.
> 
> Also added a test.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
>   src/tests/master_quota_tests.cpp 8105765172f17f3ea50aa09bc66fede8d21365ab 
> 
> 
> Diff: https://reviews.apache.org/r/7/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71068: Added offer rescind logic for limits enforcement.

[Benjamin Mahler]

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


Fix it, then Ship it!




Can you also include the additional caveats of the current approach in the 
commit description for posterity? As was done for the race caveat


src/master/quota_handler.cpp
Line 583 (original), 583-594 (patched)


Thanks!



src/master/quota_handler.cpp
Lines 585-586 (patched)


hm.. I think readers will wonder what compromise is being made here with 
tracking incorrect "resource state", can you clarify it here or point them to 
where they can see what this means?

(I assume this is the addition overlap only right?)



src/master/quota_handler.cpp
Lines 590 (patched)


s/churning/churn/



src/master/quota_handler.cpp
Lines 599-600 (patched)


Maybe make this a "NOTE: " comment?


- Benjamin Mahler


On July 23, 2019, 8:38 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71068/
> ---
> 
> (Updated July 23, 2019, 8:38 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9812
> https://issues.apache.org/jira/browse/MESOS-9812
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Outstanding offers of a role (including offers allocated to its
> subroles) are rescinded until the sum of its consumed quota and
> outstanding offered resources are below the requested limits.
> 
> The rescind logic here, however, overlooks the race between master
> and the allocator. Namely, there might be pending offers in the master
> mailbox that have yet to be processed. Here, for simplicity, we ignore
> this race. To properly handle this, the plan is to move offer
> management logic into the allocator and rescind offers there.
> 
> Also added a test.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
>   src/tests/master_quota_tests.cpp 8105765172f17f3ea50aa09bc66fede8d21365ab 
> 
> 
> Diff: https://reviews.apache.org/r/71068/diff/5/
> 
> 
> Testing
> ---
> 
> Ran continously without failure.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71111: Added offer rescind logic for guarantees enforcement.

[Meng Zhu]

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

(Updated July 23, 2019, 1:39 p.m.)


Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Changes
---

Added more comments about the best-effort rescinding approach.


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


Repository: mesos


Description
---

Outstanding offers need to be rescinded as needed to ensure
roles' requested guarantees can be satisfied.

For simplicity, and also due to the race between the master and
the allocator, we pessimistically assume that what seems like
"available" resources in the allocator are all gone. We greedily
rescind offers until we can satisfy the guarantees.

Also added a test.


Diffs (updated)
-

  src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
  src/tests/master_quota_tests.cpp 8105765172f17f3ea50aa09bc66fede8d21365ab 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 71084: Added unsuppressing via `updateFramework()` to python example framework.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71082, 71083, 71084]

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

- Mesos Reviewbot


On July 16, 2019, 3:14 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71084/
> ---
> 
> (Updated July 16, 2019, 3:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9864
> https://issues.apache.org/jira/browse/MESOS-9864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch ensures that the list of suppressed roles is passed through
> the `updateFramework()` of python V0 bindings. Now the framework
> subscribes with all roles suppressed and after that issues
> UPDATE_FRAMEWORK call to unsuppress offers.
> 
> 
> Diffs
> -
> 
>   src/examples/python/test_framework.py 
> 27bc052dc4e57e0290ef58a39b2f1ac82fdb42fd 
> 
> 
> Diff: https://reviews.apache.org/r/71084/diff/1/
> 
> 
> Testing
> ---
> 
> `./bin/mesos-tests.sh --gtest_filter="ExamplesTest.PythonFramework*" 
> --gtest_break_on_failure --gtest_repeat=5`
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71068: Added offer rescind logic for limits enforcement.

[Meng Zhu]

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

(Updated July 23, 2019, 1:38 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Added more comments about the best-effort rescind approach.


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


Repository: mesos


Description
---

Outstanding offers of a role (including offers allocated to its
subroles) are rescinded until the sum of its consumed quota and
outstanding offered resources are below the requested limits.

The rescind logic here, however, overlooks the race between master
and the allocator. Namely, there might be pending offers in the master
mailbox that have yet to be processed. Here, for simplicity, we ignore
this race. To properly handle this, the plan is to move offer
management logic into the allocator and rescind offers there.

Also added a test.


Diffs (updated)
-

  src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
  src/tests/master_quota_tests.cpp 8105765172f17f3ea50aa09bc66fede8d21365ab 


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

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


Testing
---

Ran continously without failure.


Thanks,

Meng Zhu

Review Request 71147: Update config factory to set resource provider reconciliation interval.

[Benjamin Bannier]

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

Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Update config factory to set resource provider reconciliation interval.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
69861265d94ddf344da96b593797ce145394413e 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 71146: Clarified a comment in storage local resource provider tests.

[Benjamin Bannier]

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

Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Clarified a comment in storage local resource provider tests.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
69861265d94ddf344da96b593797ce145394413e 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 71149: Renamed a storage provider function.

[Benjamin Bannier]

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

Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Renamed a storage provider function.


Diffs
-

  src/resource_provider/storage/provider.cpp 
6d632606f411d3ca99d3573a57c9f68b02ba8072 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 71151: Performed periodic storage local provider reconciliations.

[Benjamin Bannier]

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

Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Performed periodic storage local provider reconciliations.


Diffs
-

  src/resource_provider/storage/provider.cpp 
6d632606f411d3ca99d3573a57c9f68b02ba8072 
  src/tests/storage_local_resource_provider_tests.cpp 
69861265d94ddf344da96b593797ce145394413e 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 71145: Fixed signature to pass parameter by const ref instead of value.

[Benjamin Bannier]

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

Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Fixed signature to pass parameter by const ref instead of value.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
69861265d94ddf344da96b593797ce145394413e 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 71150: Factored out storage provider method to update resources.

[Benjamin Bannier]

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

Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Factored out storage provider method to update resources.


Diffs
-

  src/resource_provider/storage/provider.cpp 
6d632606f411d3ca99d3573a57c9f68b02ba8072 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 71148: Explicitly disabled periodic reconciliation for some provider tests.

[Benjamin Bannier]

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

Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Explicitly disabled periodic reconciliation for some provider tests.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
69861265d94ddf344da96b593797ce145394413e 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 71144: Added `reconciliation_interval_seconds` for storage resource providers.

[Benjamin Bannier]

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


Fix it, then Ship it!





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


WDYT about dropping the part about the default value? That way we would 
have freedom to e.g., switch to a flag-driven value down the line without break 
any formal API contract.

Similar in v1.


- Benjamin Bannier


On July 23, 2019, 9:53 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71144/
> ---
> 
> (Updated July 23, 2019, 9:53 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new configuration option controls how frequent a storage resource
> provider reconciles existing volumes and storage pools against its CSI
> plugin to detect new or missing disk resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/resource_provider/constants.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71144/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 71143: Moved default constants for CSI RPC retry to a new header.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On July 23, 2019, 9:53 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71143/
> ---
> 
> (Updated July 23, 2019, 9:53 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the default constants for CSI RPC retry do not depend on CSI
> versions, these constants are pulled off from version-specific headers
> to a common header.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/csi/constants.hpp PRE-CREATION 
>   src/csi/v0_volume_manager.cpp e19dc7c2933e2bbee5e817f3089bd569b259786b 
>   src/csi/v0_volume_manager_process.hpp 
> 4cfb5b564af9e187a6e3d81f86964db288ae852e 
>   src/csi/v1_volume_manager.cpp e7e032988bce576ef8d6a0c3457f26f1aad9bb58 
>   src/csi/v1_volume_manager_process.hpp 
> 30788c3593d4b4bd6271705a1188f73836bc7a85 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 38233053452259571743317326a6efc74d97bd29 
> 
> 
> Diff: https://reviews.apache.org/r/71143/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 71144: Added `reconciliation_interval_seconds` for storage resource providers.

[Chun-Hung Hsiao]

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

Review request for mesos and Benjamin Bannier.


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


Repository: mesos


Description
---

This new configuration option controls how frequent a storage resource
provider reconciles existing volumes and storage pools against its CSI
plugin to detect new or missing disk resources.


Diffs
-

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/resource_provider/constants.hpp PRE-CREATION 


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


Testing
---


Thanks,

Chun-Hung Hsiao

Review Request 71143: Moved default constants for CSI RPC retry to a new header.

[Chun-Hung Hsiao]

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

Review request for mesos and Benjamin Bannier.


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


Repository: mesos


Description
---

Since the default constants for CSI RPC retry do not depend on CSI
versions, these constants are pulled off from version-specific headers
to a common header.


Diffs
-

  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/csi/constants.hpp PRE-CREATION 
  src/csi/v0_volume_manager.cpp e19dc7c2933e2bbee5e817f3089bd569b259786b 
  src/csi/v0_volume_manager_process.hpp 
4cfb5b564af9e187a6e3d81f86964db288ae852e 
  src/csi/v1_volume_manager.cpp e7e032988bce576ef8d6a0c3457f26f1aad9bb58 
  src/csi/v1_volume_manager_process.hpp 
30788c3593d4b4bd6271705a1188f73836bc7a85 
  src/tests/storage_local_resource_provider_tests.cpp 
38233053452259571743317326a6efc74d97bd29 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 71083: Added enchanced multi-role capability support to the python bindings.

[Andrei Sekretenko]

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

(Updated July 23, 2019, 4:27 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


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


Repository: mesos


Description
---

This patch adds suppressed roles list to the arguments of the scheduler
driver constructor and `revivieOffers()`/`suppressOffers` methods, and
also adds support of the `updateFramework()` method.


Diffs (updated)
-

  src/python/interface/src/mesos/interface/__init__.py 
1200ef652f6458b9bbcd29b4d93d34c28230c7d0 
  src/python/scheduler/src/mesos/scheduler/mesos_scheduler_driver_impl.hpp 
8c98d46fb842595956d58a3f91f805e44be2bcf2 
  src/python/scheduler/src/mesos/scheduler/mesos_scheduler_driver_impl.cpp 
6dec9daa295cfc67176fb043322b8e98efdd7d4b 


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

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


Testing (updated)
---

Unsuppressing via update: `./bin/mesos-tests.sh 
--gtest_filter="ExamplesTest.PythonFramework*" --gtest_break_on_failure 
--gtest_repeat=50`  with the changes from the dependent patch.
revivie/suppress: tested manually.


Thanks,

Andrei Sekretenko

Re: Review Request 71130: Added a test to ensure allocations are restricted by quota limits.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/tests/hierarchical_allocator_tests.cpp
Lines 3573-3575 (patched)


There needs to be a settle before this to make sure the addSlave call 
completed executing?

As an aside, it would be nice if we surfaced the Futures from these calls 
(e.g. addSlave) rather than hiding them with a void return type. Then we could 
just AWAIT_READY here instead of settling.



src/tests/hierarchical_allocator_tests.cpp
Lines 3588-3589 (patched)


"Now raise the limit from 0 to ... which should make the role get resources 
up to this limit" ?



src/tests/hierarchical_allocator_tests.cpp
Lines 3627-3628 (patched)


use `*` operator instead of .get() here and above?


- Benjamin Mahler


On July 19, 2019, 9:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71130/
> ---
> 
> (Updated July 19, 2019, 9:32 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure allocations are restricted by quota limits.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9ebeeb6501e909544f5461710059ded66fb70eed 
> 
> 
> Diff: https://reviews.apache.org/r/71130/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71082: Added a helper to create std::vector from iterable into python bindings.

[Andrei Sekretenko]

> On July 18, 2019, 5:59 p.m., Benjamin Mahler wrote:
> > Looks like I will wait until the egg issue is fixed

I haven't managed to find a **simple**, portable and non-racy way to 
simultaneously build two python eggs which need the same source. 
Gave up and moved that template specialization into the header (with `inline` 
specifier).

The right solution is to build the common code as a static library and use it 
from all eggs, however I'm not sure that this 10-line function is worth it. 
Left that as TODO.


- Andrei


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


On July 23, 2019, 4:13 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71082/
> ---
> 
> (Updated July 23, 2019, 4:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9864
> https://issues.apache.org/jira/browse/MESOS-9864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper to create std::vector from iterable into python bindings.
> 
> 
> Diffs
> -
> 
>   src/python/native_common/common.hpp 
> ed743aab014301981fe2526a7e81f4a35c02e19b 
> 
> 
> Diff: https://reviews.apache.org/r/71082/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71082: Added a helper to create std::vector from iterable into python bindings.

[Andrei Sekretenko]

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

(Updated July 23, 2019, 4:13 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Used inline template specialization instead of using the same .cpp in two 
python eggs.


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


Repository: mesos


Description
---

Added a helper to create std::vector from iterable into python bindings.


Diffs (updated)
-

  src/python/native_common/common.hpp ed743aab014301981fe2526a7e81f4a35c02e19b 


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

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


Testing
---


Thanks,

Andrei Sekretenko

Re: Review Request 71131: Refactored allocator test helper `createQuota`.

[Benjamin Mahler]

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


Ship it!





src/tests/allocator.cpp
Lines 31-35 (original), 30-34 (patched)


maybe just use the other one:

```
return createQuota(resources, resources);
```


- Benjamin Mahler


On July 19, 2019, 9:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71131/
> ---
> 
> (Updated July 19, 2019, 9:32 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the unnecessary role argument.
> And added an overload to specify guarantees and limits differently.
> 
> 
> Diffs
> -
> 
>   src/tests/allocator.hpp 346b8224d85722b22c9f0df2c60f187fd87173b7 
>   src/tests/allocator.cpp 2a04fed9cbd34ae7ce4e833741a27db08efc999c 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> f3a2c4a3ab45164492e9c260c783dcef52a45e1f 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9ebeeb6501e909544f5461710059ded66fb70eed 
> 
> 
> Diff: https://reviews.apache.org/r/71131/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71139: Removed `quota_info` in the `GET_QUOTA` authorization object.

[Benjamin Mahler]

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


Ship it!





src/master/quota_handler.cpp
Lines 420-423 (original), 393-397 (patched)


Using a single index integer might read a bit easier?


- Benjamin Mahler


On July 23, 2019, 1:46 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71139/
> ---
> 
> (Updated July 23, 2019, 1:46 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9668
> https://issues.apache.org/jira/browse/MESOS-9668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, the `GET_QUOTA` authorizable action set both  `value`
> and `quota_info` fields. The `value` field is set due to
> backward compatibility for the `GET_QUOTA_WITH_ROLE` action.
> 
> This patch makes the `GET_QUOTA` action only set the `value`
> field with the role name. Since the `quota.QuotaInfo` field
> is being deprecated, it is no longer set (the local authorizer
> only looks at the `value` field, it is also probably the case
> for any external authorizer modules).
> 
> Also refactored `QuotaHandler::status`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> d09c2341154a39dc74e304742920e01183f0cab1 
>   src/authorizer/local/authorizer.cpp 
> 85821c6b4877e319a2879b26492338b2cd903a6f 
>   src/master/master.hpp 5c229c53f96dda6b30731017b8adbf57c9fc9f95 
>   src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
> 
> 
> Diff: https://reviews.apache.org/r/71139/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71140: Fixed devolution of 'max_grace_period' field in DRAIN_AGENT call.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [71140]

Error:
2019-07-23 09:08:30 URL:https://reviews.apache.org/r/71140/diff/raw/ 
[1871/1871] -> "71140.patch" [1]
error: patch failed: src/internal/devolve.cpp:280
error: src/internal/devolve.cpp: patch does not apply
error: patch failed: src/tests/api_tests.cpp:5588
error: src/tests/api_tests.cpp: patch does not apply

- Mesos Reviewbot


On July 23, 2019, 1:51 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71140/
> ---
> 
> (Updated July 23, 2019, 1:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed devolution of 'max_grace_period' field in DRAIN_AGENT call.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.cpp 2809c25fcf972e8a22528e55cd439dd64bd04dbb 
>   src/tests/api_tests.cpp 3479ed31e9cbbe4b61a4fe092b684677f488c621 
> 
> 
> Diff: https://reviews.apache.org/r/71140/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>