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

[Mesos Reviewbot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot


On Nov. 7, 2017, 1:12 a.m., Julien Pepy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63598/
> ---
> 
> (Updated Nov. 7, 2017, 1:12 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> 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
> 
>

Re: Review Request 63585: Improved support/mesos-style.py structure.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63585 was successfully built and tested.

Reviews applied: `['63581', '63582', '62214', '63585']`

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

- Mesos Reviewbot Windows


On Nov. 6, 2017, 4:59 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63585/
> ---
> 
> (Updated Nov. 6, 2017, 4:59 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Python linter now also uses the function `run_command_in_virtualenv`. 
> The methods of `LinterBase` have been ordered alphabetically.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
> 
> 
> Diff: https://reviews.apache.org/r/63585/diff/1/
> 
> 
> Testing
> ---
> 
> Used the linters after applying this patch.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 63279: Increased check tests task resources for Windows.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63268, 63269, 63271, 63272, 63273, 63274, 63275, 63276, 
63277, 63278, 63279]

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

- Mesos Reviewbot


On Nov. 2, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63279/
> ---
> 
> (Updated Nov. 2, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Alexander Rukletsov, Jeff 
> Coffler, Gaston Kleiman, Jie Yu, John Kordich, Joseph Wu, Li Li, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6690
> https://issues.apache.org/jira/browse/MESOS-6690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unfortunately, due to PowerShell's resource usage as a .NET program, on
> Windows 32 MB is not enough memory to run these tests. One instance of
> PowerShell takes > 128 MB, and two instances take > 256 MB.
> Realistically, the safe minimum is 512 MB of memory.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d3ffc0b0204d021f1ed734ece189d0e3a3fd8844 
> 
> 
> Diff: https://reviews.apache.org/r/63279/diff/2/
> 
> 
> Testing
> ---
> 
> Built and ran `mesos-tests.exe` on Windows repeatedly, verified all tests 
> still pass consistently (likewise for `stout` and `libprocess` tests.
> 
> ~~Currently verifying no breaks on Linux.~~ Built and ran tests on Linux.
> 
> NOTE: There are more check tests that are currently disabled for Windows, 
> that I think should be enabled, but did immediately work for me, so I've left 
> them disabled to unblock myself. Similarly, I would ideally like to port the 
> balloon example framework, and use that to prove the effectiveness of the job 
> object memory hard-cap. Having not yet ported it though, I manually verified 
> the effectiveness of the new isolators by launching test CPU and memory test 
> tasks on a deployed cluster my these changes (and the fact that PowerShell 
> OOM'ed was a nice verification too).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63342: Fixed CMake binary dependencies.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63342 was successfully built and tested.

Reviews applied: `['63340', '63341', '63342']`

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

- Mesos Reviewbot Windows


On Nov. 3, 2017, 5:31 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63342/
> ---
> 
> (Updated Nov. 3, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-8035
> https://issues.apache.org/jira/browse/MESOS-8035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This resolves MESOS-8035 so that building just the `mesos-agent`, etc.
> target should correctly build its runtime dependencies (such as the
> containerizer, executor, etc.).
> 
> 
> Diffs
> -
> 
>   src/local/CMakeLists.txt 5e5bee3b70b2407bd86d9e7ffe0d169346054942 
>   src/slave/CMakeLists.txt 6f08f3dc95f3a1408cbef7b8f0b0cc2522558924 
>   src/tests/CMakeLists.txt 81c85d9bb1b50c6897526d5c207d09042883d771 
> 
> 
> Diff: https://reviews.apache.org/r/63342/diff/3/
> 
> 
> Testing
> ---
> 
> Cleaned build folder, built just the `mesos-agent` target on Windows and 
> Linux, verified it could launch and connect to a master (built clean with 
> just `--target mesos-master`. Launched a simple task via Marathon to confirm 
> end-to-end support.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 63606: Avoid an extra copy during ProtobufProcess::send.

[Benjamin Mahler]

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

Review request for mesos, Benjamin Hindman and Jiang Yan Xu.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/include/process/protobuf.hpp 
fe152f273332470ac50f9715291897bb04cf95b9 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 63597: Refactored XFS quota tests for more reuse for upcoming XFS related tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63597]

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

- Mesos Reviewbot


On Nov. 7, 2017, 12:10 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63597/
> ---
> 
> (Updated Nov. 7, 2017, 12:10 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored XFS quota tests for more reuse for upcoming XFS related tests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 1c97b1fd8151f87c4e9e6d62884b0ef7d582c312 
>   src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76 
>   src/tests/containerizer/xfs.hpp PRE-CREATION 
>   src/tests/containerizer/xfs.cpp PRE-CREATION 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 83fa7f2a7e0d31891834669d7b31f18d2fa08035 
> 
> 
> Diff: https://reviews.apache.org/r/63597/diff/1/
> 
> 
> Testing
> ---
> 
> Verified existing XSF tests, namely `xfs_quota_tests`, on GNU/Linux.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 63604: Removed an unnecessary serialization in protobuf process.

[Benjamin Mahler]

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

Review request for mesos, Benjamin Hindman and Jiang Yan Xu.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/include/process/protobuf.hpp 
fe152f273332470ac50f9715291897bb04cf95b9 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 63605: Support moving in data during ProcessBase::send.

[Benjamin Mahler]

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

Review request for mesos.


Repository: mesos


Description
---

This allows callers that can move in data to avoid an extra copy.


Diffs
-

  3rdparty/libprocess/include/process/event.hpp 
2e246205020c3c5b8c2eb5187a8eb3643d1e6d4d 
  3rdparty/libprocess/include/process/process.hpp 
dc3375ce62556322eb2bc60ade61f313ade123b8 
  3rdparty/libprocess/src/process.cpp 71ae7129ffbd0e22eda2863b17bbcf588298c37b 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
e6c77d565d5acf72b475a085e9504679253b4b97 
  3rdparty/libprocess/src/tests/process_tests.cpp 
952c92c033e2363cff0c2c68610d3820b97d177e 


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


Testing
---

Ran benchmarks before and after per benh's request, no difference:
https://docs.google.com/spreadsheets/d/1I6iUziBwouQSn2veoj-xVpq7B9n6zdO9zPimi1IlJZ4/edit?usp=sharing


Thanks,

Benjamin Mahler

Re: Review Request 62762: Added `LocalResourceProvider::principal()` for authentication.

[Joseph Wu]

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




configure.ac
Lines 2020-2021 (patched)


This definition probably belongs in a separate review.  Along with the 
other build changes related to this flag, in this review.


- Joseph Wu


On Nov. 3, 2017, 5:49 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62762/
> ---
> 
> (Updated Nov. 3, 2017, 5:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8100
> https://issues.apache.org/jira/browse/MESOS-8100
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `LocalResourceProvider::principal()` function takes a
> `ResourceProviderInfo` and generates a principal with the following
> claim:
> 
>   {"cid_prefix", name prefix for the resource provider}
> 
> where the name prefix is a unique prefix for all containers that will be
> launched by the resource provider. For example, for resource provider
> with type 'org.apache.rp.local.storage' and name 'foo-bar', the claim
> would be:
> 
>   {"cid_prefix", "mesos-slrp-foo%2Dbar-"}
> 
> In the future, we could add more claims for authorizing other
> operations, such as authorization for Resource Provider API.
> 
> 
> Diffs
> -
> 
>   configure.ac 067005e1be3a09e8320be120c8784f195e30ac5d 
>   src/Makefile.am 1c97b1fd8151f87c4e9e6d62884b0ef7d582c312 
>   src/resource_provider/local.hpp ebaa07d03ad77d516066ee2d4b60864be0611b5f 
>   src/resource_provider/local.cpp ad98f333c5668ca81de6e7ed3fc8f59323b151da 
>   src/resource_provider/storage/provider.hpp 
> 6de88c2329b358fcf48bc39ddda0132170991c3c 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
> 
> 
> Diff: https://reviews.apache.org/r/62762/diff/5/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 62635: Added an optional `unsafe` parameter to `http::encode`.

[Joseph Wu]

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




3rdparty/libprocess/include/process/http.hpp
Lines 888-896 (original), 888-902 (patched)


We sure are inconsistent about comment block usage of `/**/` vs `//` in 
this file :|



3rdparty/libprocess/include/process/http.hpp
Lines 892-896 (patched)


s/arleady/already/

s/@param safe/@param unsafe/

Also, consider calling the new parameter something more explicit, like 
`additional_chars`.


- Joseph Wu


On Oct. 16, 2017, 4:25 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62635/
> ---
> 
> (Updated Oct. 16, 2017, 4:25 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `unsafe` parameter specifies the extra characters to be
> percent-encoded, in addition to the ones specified in RFC 3986.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ba1b086a1dba140df491387077723d5c359acbcc 
>   3rdparty/libprocess/src/http.cpp a4d71fb6c345d3c7a7611004830f6c2c0fbf6046 
> 
> 
> Diff: https://reviews.apache.org/r/62635/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 62636: Generated authentication tokens for local resource providers.

[Joseph Wu]

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



You could probably combine this review with https://reviews.apache.org/r/62633/ 
, as that review is basically just plumbing the auth token from Daemon -> Local 
-> SLRP.  And this review adds plumbing of the token (via the secret generator) 
from Agent -> Daemon.


src/resource_provider/daemon.cpp
Lines 96-99 (original), 114-118 (patched)


I'd consider an error at this step to be a fatal error (basically a 
misconfiguration), so we should fail-fast here and exit the agent.



src/resource_provider/daemon.cpp
Lines 207 (patched)


s/insteaf/instead/



src/slave/slave.cpp
Line 1234 (original), 1234-1236 (patched)


Consider moving this comment above `start` in `daemon.hpp`.  (With the side 
benefit of not needing to duplicate the comment in two places)


- Joseph Wu


On Nov. 3, 2017, 5:51 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62636/
> ---
> 
> (Updated Nov. 3, 2017, 5:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8100
> https://issues.apache.org/jira/browse/MESOS-8100
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `LocalResourceProviderDaemon` now uses `Slave::secretGenerater` to
> generate an authentication token for each local resource provider. The
> authentication token can then be used to call the V1 agent API. In order
> to generate the tokens, `LocalResourceProviderDaemon::load()` is now an
> asynchronous function.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/daemon.hpp ef6c356cb6ddb2594d767d7dd6052e9fd8df8263 
>   src/resource_provider/daemon.cpp d584eb9d7aa75522aec97277674321061b90fbed 
>   src/slave/slave.cpp 337083dbe60bba2d3773b785bdceeaf0b8fcd070 
> 
> 
> Diff: https://reviews.apache.org/r/62636/diff/5/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63589: Fixed bug in tests leading to orphaned containers.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['63589']`

Failed command: 
`C:\DCOS\mesos\3rdparty\libprocess\src\tests\Debug\libprocess-tests.exe`

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

Relevant logs:

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

```
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp(446): error: 
(await_subprocess(client.get(), 0)).failure(): 
[++] Subprocess output.
[==] Running 1 test from 1 test case.

[--] Global test environment set-up.

[--] 1 test from SSLClientTest

[ RUN  ] SSLClientTest.client

[   OK ] SSLClientTest.client (18 ms)

[--] 1 test from SSLClientTest (18 ms total)



[--] Global test environment tear-down

[==] 1 test from 1 test case ran. (18 ms total)

[  PASSED  ] 1 test.

[++]

[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0, where GetParam() = 
"false" (723 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (1150 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (3572 ms total)

[--] Global test environment tear-down
[==] 201 tests from 33 test cases ran. (45754 ms total)
[  PASSED  ] 200 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0, where GetParam() = 
"false"

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I1107 02:27:07.658520  3448 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 02:27:07.658520  3448 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peWARNING: Logging before 
InitGoogleLogging() is written to STDERR
I1107 02:27:07.980528  2608 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 02:27:07.981528  2608 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 02:27:07.981528  2608 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 02:27:07.981528  2608 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\CYDW2S\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1107 02:27:09.207059  2504 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 02:27:09.212220  2504 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 02:27:09.213196  2504 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1107 02:27:09.213196  2504 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 02:27:09.213196  2504 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\WJnCf3\cert.pem
er certificate verification
I1107 02:27:07.687523  3448 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 02:27:07.688521  3448 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\CYDW2S\cert.pem
I1107 02:27:08.904052  3448 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 02:27:08.905052  3448 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 02:27:08.905052  3448 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1107 02:27:08.905052  3448 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 02:27:08.905052  3448 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\WJnCf3\cert.pem
I1107 02:27:09.432168  1944 process.cpp:1067] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Nov. 6, 2017, 10:41 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63589/
> 

Re: Review Request 63589: Fixed bug in tests leading to orphaned containers.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63589]

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

- Mesos Reviewbot


On Nov. 6, 2017, 10:41 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63589/
> ---
> 
> (Updated Nov. 6, 2017, 10:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benno Evers, and Gilbert Song.
> 
> 
> Bugs: MESOS-7506
> https://issues.apache.org/jira/browse/MESOS-7506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, some tests tried to advance the clock until task status
> update was sent, while task's container was destroying. Container
> destruction consists of multiple steps, where some steps have a timeout
> specified, e.g. `cgroups::DESTROY_TIMEOUT`. So, there was a race
> between container destruction process and the loop that advanced the
> clock, leading to the following outcomes:
> 
>   (1) Container destroyed, before clock advancing reaches timeout.
> 
>   (2) Triggered timeout due to clock advancing, before container
>   destruction completes. That results in leaving orphaned
>   containers that will be detected by Slave destructor in
>   `tests/cluster.cpp`, so the test will fail.
> 
> This change gets rid of the loop and resumes clock after a single
> advancing of the clock.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp db337ba4e213820e7ad0c3f1b480388a2e456556 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63589/diff/1/
> 
> 
> Testing
> ---
> 
> 1. make check
> 2. internal ci (5x)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

[Andrew Schwartzmeyer]

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




src/tests/default_executor_tests.cpp
Line 1648 (original), 1646-1658 (patched)


I think rather than by wrapping the last argument to a function in an 
`ifdef`, this should instead just be a `const string` aobve the call to 
`createTask`, followed by the PowerShell command in another guard.


- Andrew Schwartzmeyer


On Nov. 6, 2017, 10:10 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Nov. 6, 2017, 10:10 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/4/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

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

[Mesos Reviewbot Windows]

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



Bad review!

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot Windows


On Nov. 7, 2017, 1:12 a.m., Julien Pepy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63598/
> ---
> 
> (Updated Nov. 7, 2017, 1:12 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> 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
> 
>

Re: Review Request 63581: Created virtual environment for linters in /support.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63581 was successfully built and tested.

Reviews applied: `['63581']`

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

- Mesos Reviewbot Windows


On Nov. 6, 2017, 8:50 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63581/
> ---
> 
> (Updated Nov. 6, 2017, 8:50 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change affects the Python linter but not the C++ linter as we
> use a customized version of cpplint that we cannot get using pip.
> 
> 
> Diffs
> -
> 
>   src/python/pylint.config  
>   support/build-virtualenv PRE-CREATION 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
>   support/pip-requirements.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63581/diff/2/
> 
> 
> Testing
> ---
> 
> Used the linters after applying this patch.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

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

[Julien Pepy]

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

Review request for mesos.


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

Re: Review Request 61128: Improved log messages in master when adding/removing tasks/executors.

[Qian Zhang]

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


Fix it, then Ship it!





src/master/master.cpp
Lines 9674-9676 (original), 9675-9678 (patched)


To be consistent with this log message, I think we also need to change the 
log message in `Master::removeTask()` from:
```
LOG(INFO) << "Removing task " << task->task_id()
  << " with resources " << task->resources()
  << " of framework " << task->framework_id()
  << " on agent " << *slave;
```
to:
```
LOG(INFO) << "Removing task " << task->task_id()
  << " of framework " << task->framework_id()
  << " with resources " << task->resources()
  << " on agent " << *slave;
```
And there is also a warning message in `Master::removeTask()`, I think we 
need to change that one too.


- Qian Zhang


On July 26, 2017, 7:46 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61128/
> ---
> 
> (Updated July 26, 2017, 7:46 a.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the log messages more consistent and also added one for adding
> an executor.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp e12c997dad04f8a4ddb47a993a84b2b05c9e2f32 
> 
> 
> Diff: https://reviews.apache.org/r/61128/diff/1/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 63519: Allowed toggling of agent capabilities via command line flags.

[Jie Yu]

> On Nov. 7, 2017, 12:08 a.m., Jie Yu wrote:
> > src/slave/slave.cpp
> > Lines 8403 (patched)
> > 
> >
> > I'd avoid this static helper by just inline the code in the initializer:
> > ```
> > capabilities(_flags.agent_features.isNone()
> >   : AGENT_CAPABILITIES()
> >   ? _flags.agent_features->capabilities()),
> > ```

```
capabilities(_flags.agent_features.isNone()
  : protobuf::slave::Capabilities(AGENT_CAPABILITIES())
  ? protobuf::slave::Capabilities(_flags.agent_features->capabilities())),
```


- Jie


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


On Nov. 6, 2017, 12:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63519/
> ---
> 
> (Updated Nov. 6, 2017, 12:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces a new agent command line flags
> '--agent_features' which can be used to whitelist capabilities to
> enable. This flag is intended to enable introducing feature flags in
> the future so experimental features can be added while still reducing
> the risk of breaking existing functionality. Currently only the
> 'RESOURCE_PROVIDER' capability can be toggled.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 116e7c6b10d6eafdf6a163c821d2204db29f6d4c 
>   src/slave/flags.hpp d02edbfd68266c9f2d5c78fdbd5c2ba5f497adba 
>   src/slave/flags.cpp 789b45b8d84fc01733a885754204fe2fdd6d6807 
>   src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
> 
> 
> Diff: https://reviews.apache.org/r/63519/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`, tested as part of https://reviews.apache.org/r/63496/
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63538: Removed expectation for scheduler connected events in some tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63538]

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

- Mesos Reviewbot


On Nov. 3, 2017, 4:32 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63538/
> ---
> 
> (Updated Nov. 3, 2017, 4:32 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8093
> https://issues.apache.org/jira/browse/MESOS-8093
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> ee1e5e67b9eacb5909964b2951f659d1bd2233e5 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 0d68dd634771c016553313d67ddb33504d1a13c2 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 81c0d825c689b3d00cbc21c7567c8c0c8836bf75 
> 
> 
> Diff: https://reviews.apache.org/r/63538/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> sudo ./mesos-tests.sh 
> --gtest_filter="CgroupsIsolatorTest.ROOT_CGROUPS_LimitSwap" 
> --gtest_break_on_failure --gtest_repeat=100
> sudo ./mesos-tests.sh 
> --gtest_filter="DockerRuntimeIsolatorTest.ROOT_INTERNET_CURL_NestedSimpleCommand"
>  --gtest_break_on_failure --gtest_repeat=100
> ```
> And `make check`.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 63519: Allowed toggling of agent capabilities via command line flags.

[Jie Yu]

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




src/slave/main.cpp
Lines 377-394 (patched)


You can in fact do this (leverage the existing 
`protobuf::slave::Capabilities`) 

```
protobuf::slave::Capabilities capabilities(
flags.agent_features->capabilities());

if (!capabilities.multiRole ||
!capabilities.hierarchicalRole ||
!capabilities.reservationRefinement) {
  return Error("...");
}

return None();
```



src/slave/slave.cpp
Lines 8403 (patched)


I'd avoid this static helper by just inline the code in the initializer:
```
capabilities(_flags.agent_features.isNone()
  : AGENT_CAPABILITIES()
  ? _flags.agent_features->capabilities()),
```


- Jie Yu


On Nov. 6, 2017, 12:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63519/
> ---
> 
> (Updated Nov. 6, 2017, 12:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces a new agent command line flags
> '--agent_features' which can be used to whitelist capabilities to
> enable. This flag is intended to enable introducing feature flags in
> the future so experimental features can be added while still reducing
> the risk of breaking existing functionality. Currently only the
> 'RESOURCE_PROVIDER' capability can be toggled.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 116e7c6b10d6eafdf6a163c821d2204db29f6d4c 
>   src/slave/flags.hpp d02edbfd68266c9f2d5c78fdbd5c2ba5f497adba 
>   src/slave/flags.cpp 789b45b8d84fc01733a885754204fe2fdd6d6807 
>   src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
> 
> 
> Diff: https://reviews.apache.org/r/63519/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`, tested as part of https://reviews.apache.org/r/63496/
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63491: Added resource version to resource provider UpdateTotalResources call.

[Jie Yu]

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


Fix it, then Ship it!





src/resource_provider/message.hpp
Line 39 (original), 40 (patched)


please move `{` to the next line.


- Jie Yu


On Nov. 6, 2017, 10:12 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63491/
> ---
> 
> (Updated Nov. 6, 2017, 10:12 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch surfaces this information to resource provider manager
> users like the agent. In a later patch we will modify the agent to
> forward this information to the master.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63491/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63519: Allowed toggling of agent capabilities via command line flags.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/main.cpp
Lines 375-395 (patched)


Can you move this to `add` method in src/slave/flags.cpp?

`add` takes a validation lambda.



src/slave/slave.cpp
Lines 8409 (patched)


Do you need this. I thought `protobuf::slave::Capabilities` already has a 
constructor that takes a Iterable? Can you do:
```
if (flags.agent_features.isNone()) {
  return AGENT_CAPABILITIES();
}

return flags.agent_features->capabilities();
```


- Jie Yu


On Nov. 6, 2017, 12:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63519/
> ---
> 
> (Updated Nov. 6, 2017, 12:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces a new agent command line flags
> '--agent_features' which can be used to whitelist capabilities to
> enable. This flag is intended to enable introducing feature flags in
> the future so experimental features can be added while still reducing
> the risk of breaking existing functionality. Currently only the
> 'RESOURCE_PROVIDER' capability can be toggled.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 116e7c6b10d6eafdf6a163c821d2204db29f6d4c 
>   src/slave/flags.hpp d02edbfd68266c9f2d5c78fdbd5c2ba5f497adba 
>   src/slave/flags.cpp 789b45b8d84fc01733a885754204fe2fdd6d6807 
>   src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
> 
> 
> Diff: https://reviews.apache.org/r/63519/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`, tested as part of https://reviews.apache.org/r/63496/
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63278: Windows: Documented the `cpu` and `mem` isolators.

[Andrew Schwartzmeyer]

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

(Updated Nov. 6, 2017, 3:19 p.m.)


Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
Kordich, Joseph Wu, Li Li, and Michael Park.


Changes
---

Renamed `cpuset` to `cpu`.


Summary (updated)
-

Windows: Documented the `cpu` and `mem` isolators.


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


Repository: mesos


Description
---

This adds documentation on the usage of the job object isolators, which
enable task limits, as well as the statistics they report.


Diffs (updated)
-

  docs/isolators/windows.md PRE-CREATION 
  docs/mesos-containerizer.md a194eb05703650dbce6b9e2fb84c9ff4aa9d9522 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63277: Windows: Ported CPU and memory isolator tests.

[Andrew Schwartzmeyer]

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

(Updated Nov. 6, 2017, 3:19 p.m.)


Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
Kordich, Joseph Wu, Li Li, and Michael Park.


Changes
---

Renamed `cpuset` to `cpu`.


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


Repository: mesos


Description (updated)
---

These test the limitation and usage reporting of the new Windows `Cpu`
and `Mem` isolators.


Diffs (updated)
-

  src/tests/CMakeLists.txt 81c85d9bb1b50c6897526d5c207d09042883d771 
  src/tests/containerizer/cpu_isolator_tests.cpp 
846b2e255547a02f5eb0590747edca62bc560ac3 
  src/tests/containerizer/memory_isolator_tests.cpp 
b8ea5d35b3a0a4820d9ec4c6d7d916dc6101b570 
  src/tests/mesos.cpp 9185b5bf2175be5b0f6b6a03a04e9e5445bf22fd 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63276: Windows: Added `Cpu` and `Mem` isolators.

[Andrew Schwartzmeyer]

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

(Updated Nov. 6, 2017, 3:18 p.m.)


Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
Kordich, Joseph Wu, Li Li, and Michael Park.


Changes
---

Renamed `cpuset` to `cpu`.


Summary (updated)
-

Windows: Added `Cpu` and `Mem` isolators.


Bugs: MESOS-5462 and MESOS-6690
https://issues.apache.org/jira/browse/MESOS-5462
https://issues.apache.org/jira/browse/MESOS-6690


Repository: mesos


Description (updated)
---

Instead of deriving from the POSIX isolators, we now have two real
Windows isolators that can be used together or separately. The `Cpu`
isolator enables a hard-cap CPU limit, and the `Mem` isolator enables a
hard-cap memory limit on the job object for the container.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/isolators/windows.hpp 
b0621a5fc411b8812722f7fcf6580ed64dac5382 
  src/slave/flags.cpp 63aaac218fdc067742d39f1fc8497723784d9595 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63584: Added agent ID to offer operation protobuf helper.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: patch failed: src/common/protobuf_utils.hpp:163
error: src/common/protobuf_utils.hpp: patch does not apply
error: patch failed: src/common/protobuf_utils.cpp:443
error: src/common/protobuf_utils.cpp: patch does not apply
error: patch failed: src/master/master.cpp:5202
error: src/master/master.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 6, 2017, 2:42 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63584/
> ---
> 
> (Updated Nov. 6, 2017, 2:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7235
> https://issues.apache.org/jira/browse/MESOS-7235
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current code expects an agent ID to be set for every instance of
> 'OfferOperation'. The absence of an agent ID would indicate an operation
> meant for an external resource provider which isn't supported yet.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp da0a84eb828e2fb064ba466d9e525cd3a85fc43a 
>   src/common/protobuf_utils.cpp 4ce7021b01b9fb52ae1a8d95aa686c5b79164d67 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
> 
> 
> Diff: https://reviews.apache.org/r/63584/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 62938: Added ZooKeeper leader resolution to CLI.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62938 was successfully built and tested.

Reviews applied: `['62938']`

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

- Mesos Reviewbot Windows


On Oct. 26, 2017, 4:52 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62938/
> ---
> 
> (Updated Oct. 26, 2017, 4:52 p.m.)
> 
> 
> Review request for mesos, Eric Chung and Kevin Klues.
> 
> 
> Bugs: MESOS-8012
> https://issues.apache.org/jira/browse/MESOS-8012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change allows users to use the CLI with a Mesos running in high
> availability mode. The `zookeeper` field was already here before this
> commit, with an `addresses` array and a `path` field. This change
> only adds the backend to actually make it usable.
> 
> Interacting with ZooKeeper requires a new dependency, kazoo, that has
> been added to the list of requirements for the CLI.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md a4b270d9b54cdd83bd62530fb44ddfaffb8a014b 
>   src/python/cli_new/lib/cli/util.py dd109c09368e650b7d7dabd663c11bc2e3e5180a 
>   src/python/cli_new/pip-requirements.txt 
> 7aeac344c47ccd2588fded44d7314db7abd85653 
> 
> 
> Diff: https://reviews.apache.org/r/62938/diff/3/
> 
> 
> Testing
> ---
> 
> Tested with a Mesos cluster running with 3 masters and 1 agent, run some CLI 
> commands successfully.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 59687: Added tests for recovering ContainerConfig.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 27, 2017, 11:02 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59687/
> ---
> 
> (Updated Oct. 27, 2017, 11:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for recovering ContainerConfig.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> fbd2887800ccfd64c8628c5d6fd77a511c8f91d5 
> 
> 
> Diff: https://reviews.apache.org/r/59687/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63495: Added comparison operators for 'ResourceVerionUUID'.

[Benjamin Bannier]

> On Nov. 6, 2017, 11:51 p.m., Greg Mann wrote:
> > src/messages/messages.cpp
> > Lines 42-46 (patched)
> > 
> >
> > You could simply do
> > ```
> > return left.uuid() == right.uuid();
> > ```
> > but not a big deal; feel free to drop this issue if you desire.

I follow a general pattern here where we return `false` if any equality check 
fails, `true` otherwise. This particular pattern has the advantage of being 
trivially extensible should we add more fields (unlikely here though).


- Benjamin


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


On Nov. 2, 2017, 2:57 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63495/
> ---
> 
> (Updated Nov. 2, 2017, 2:57 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added comparison operators for 'ResourceVerionUUID'.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.hpp 59e670532c8eb1ab1ec6cb063ace03481cd6a33d 
>   src/messages/messages.cpp 7f1da63ae67b3918851c2c1dd225bfd2d0044387 
> 
> 
> Diff: https://reviews.apache.org/r/63495/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63495: Added comparison operators for 'ResourceVerionUUID'.

[Greg Mann]

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


Fix it, then Ship it!





src/messages/messages.cpp
Lines 42-46 (patched)


You could simply do
```
return left.uuid() == right.uuid();
```
but not a big deal; feel free to drop this issue if you desire.


- Greg Mann


On Nov. 2, 2017, 1:57 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63495/
> ---
> 
> (Updated Nov. 2, 2017, 1:57 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added comparison operators for 'ResourceVerionUUID'.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.hpp 59e670532c8eb1ab1ec6cb063ace03481cd6a33d 
>   src/messages/messages.cpp 7f1da63ae67b3918851c2c1dd225bfd2d0044387 
> 
> 
> Diff: https://reviews.apache.org/r/63495/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63588: Updated the offer operation bookeeping in the master.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:45 p.m.)


Review request for mesos, Benjamin Bannier and Jan Schlicht.


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


Repository: mesos


Description
---

(This is based on https://reviews.apache.org/r/63539)


Diffs
-

  include/mesos/mesos.proto 68a5538904b601a91a40db68cebde96f74a2f160 
  include/mesos/v1/mesos.proto c46cec78db63dc3714e357a954a4e7bf080485fd 
  src/master/master.hpp afcc2e46882e4c610e9047ab3db7b6f100d47e17 
  src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
  src/messages/messages.proto 2fbca22e1285aa25900f5420bb4370bda6a1bb12 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63592: Implemented updateOfferOperation in the master.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:45 p.m.)


Review request for mesos, Benjamin Bannier and Jan Schlicht.


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


Repository: mesos


Description
---

Implemented updateOfferOperation in the master.


Diffs
-

  src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63485: Added initial code for offer operation status update in master.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:44 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
Greg Mann, and Jan Schlicht.


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


Repository: mesos


Description
---

Added initial code for offer operation status update in master.


Diffs
-

  src/master/master.hpp 76f884c61f8d4605725e93e720dc693b861918e3 
  src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63511: Introduced ResourceConversion to represent conversion to Resources.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:45 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Currently, we couple offer operations with resource conversions. For
instance, we have interfaces like `Resources::apply` that takes an
offer operation. This becomes less ideal because an offer operation
might not represent a `conversion` anymore with new offer operation
like `CREATE_VOLUME` being introduced.

This patch decoupled resource conversions from offer operations.
`Resources::apply` will now take a `ResourceConversion` object. We
also provide some helpers to get a `ResourceConversion` from an offer
operation (if supported).


Diffs
-

  include/mesos/resources.hpp 53152b393ec38ca2fc2e2a66c18a29a4418fe59e 
  include/mesos/v1/resources.hpp 6c2191c044c52607618283891f82401c4861c441 
  src/common/resources.cpp 914d3e2b1e8fe55ecab9c71643954addd6a81244 
  src/common/resources_utils.hpp 18e3d9d4baad23669d00542594f5c15a989b7b9e 
  src/common/resources_utils.cpp e34cd8a3c9046a6f12c12a275a7b3a852b492f4c 
  src/v1/resources.cpp 58568b8d25cb1402a875d3975d2decb4270d2725 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63484: Changed slave_id field to be optional in OfferOperationStatusUpdate.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:44 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
Greg Mann, and Jan Schlicht.


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


Repository: mesos


Description
---

We plan to reuse this message for communication between resource
provider manager in the master to master as well.


Diffs
-

  src/messages/messages.proto 9dfe7f07c2e1171be17f1bd7b6a54a80410c44f6 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63483: Added some protobuf helper for offer operations.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:44 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
Greg Mann, and Jan Schlicht.


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


Repository: mesos


Description
---

Added some protobuf helper for offer operations.


Diffs
-

  src/common/protobuf_utils.hpp da0a84eb828e2fb064ba466d9e525cd3a85fc43a 
  src/common/protobuf_utils.cpp 4ce7021b01b9fb52ae1a8d95aa686c5b79164d67 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63481: Added latest_status field to OfferOperation protobuf.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:43 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
Greg Mann, and Jan Schlicht.


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


Repository: mesos


Description
---

Added latest_status field to OfferOperation protobuf.


Diffs
-

  include/mesos/mesos.proto 4966cd259d5c804b835f79325c197f0b68080291 
  include/mesos/v1/mesos.proto e18ac00d6161f1e763b4134cbf7c67501f352048 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63480: Added the initial implementation for applying offer operations.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:43 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
Greg Mann, and Jan Schlicht.


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


Repository: mesos


Description
---

The resource provider manager provides an `applyOfferOperation` method
for offer operation affecting resource providers. The resources on
which the operation should be applied contains a resource provider ID.
This will be extracted and an event will be sent to the respective
resource provider.

(This is based on https://reviews.apache.org/r/61810)


Diffs
-

  src/messages/messages.proto 9dfe7f07c2e1171be17f1bd7b6a54a80410c44f6 
  src/resource_provider/manager.hpp 3b70e75c6b6721864ae0ee9c4a593b5035d8388f 
  src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e 
  src/slave/slave.cpp 817dca4fc3e1028ccc13b8a1bb1ee783afb77166 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63432: Updated the comment about slaves.removed in master.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:43 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Updated the comment about slaves.removed in master.


Diffs
-

  src/master/master.hpp 76f884c61f8d4605725e93e720dc693b861918e3 


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


Testing
---

trivial


Thanks,

Jie Yu

Re: Review Request 63480: Added the initial implementation for applying offer operations.

[Jie Yu]

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

(Updated Nov. 6, 2017, 10:43 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
Greg Mann, and Jan Schlicht.


Bugs: https://reviews.apache.org/r/63413
https://issues.apache.org/jira/browse/https://reviews.apache.org/r/63413


Repository: mesos


Description
---

The resource provider manager provides an `applyOfferOperation` method
for offer operation affecting resource providers. The resources on
which the operation should be applied contains a resource provider ID.
This will be extracted and an event will be sent to the respective
resource provider.

(This is based on https://reviews.apache.org/r/61810)


Diffs
-

  src/messages/messages.proto 9dfe7f07c2e1171be17f1bd7b6a54a80410c44f6 
  src/resource_provider/manager.hpp 3b70e75c6b6721864ae0ee9c4a593b5035d8388f 
  src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e 
  src/slave/slave.cpp 817dca4fc3e1028ccc13b8a1bb1ee783afb77166 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63584: Added agent ID to offer operation protobuf helper.

[Jie Yu]

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




src/common/protobuf_utils.cpp
Lines 447 (patched)


I'd use slaveId here for consistency.


- Jie Yu


On Nov. 6, 2017, 10:42 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63584/
> ---
> 
> (Updated Nov. 6, 2017, 10:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7235
> https://issues.apache.org/jira/browse/MESOS-7235
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current code expects an agent ID to be set for every instance of
> 'OfferOperation'. The absence of an agent ID would indicate an operation
> meant for an external resource provider which isn't supported yet.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp da0a84eb828e2fb064ba466d9e525cd3a85fc43a 
>   src/common/protobuf_utils.cpp 4ce7021b01b9fb52ae1a8d95aa686c5b79164d67 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
> 
> 
> Diff: https://reviews.apache.org/r/63584/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63584: Added agent ID to offer operation protobuf helper.

[Jan Schlicht]

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

(Updated Nov. 6, 2017, 10:42 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

The current code expects an agent ID to be set for every instance of
'OfferOperation'. The absence of an agent ID would indicate an operation
meant for an external resource provider which isn't supported yet.


Diffs
-

  src/common/protobuf_utils.hpp da0a84eb828e2fb064ba466d9e525cd3a85fc43a 
  src/common/protobuf_utils.cpp 4ce7021b01b9fb52ae1a8d95aa686c5b79164d67 
  src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 63492: Synchronized agent clock with master via 'UpdateSlaveMessage'.

[Greg Mann]

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




src/master/master.cpp
Lines 6923-6924 (original), 6930-6954 (patched)


Need another newline both before and after this function.



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


Hmm this should be `UUID::fromBytes`, right?



src/slave/slave.cpp
Lines 1202 (patched)


Should this be `uuid.toBytes()`?


- Greg Mann


On Nov. 6, 2017, 12:38 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63492/
> ---
> 
> (Updated Nov. 6, 2017, 12:38 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit introduces agent clocks to the master and agents. Agents
> are responsible for maintaining their clock. The clocks are
> synchronized with the master via 'UpdateSlaveMessage'.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp afcc2e46882e4c610e9047ab3db7b6f100d47e17 
>   src/master/master.cpp 49bc50ead75adba82a7d8de23a87b06ccd269c48 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
> 
> 
> Diff: https://reviews.apache.org/r/63492/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63588: Updated the offer operation bookeeping in the master.

[Jie Yu]

> On Nov. 6, 2017, 6:47 p.m., Benjamin Bannier wrote:
> > src/master/master.hpp
> > Lines 925-926 (original), 935-936 (patched)
> > 
> >
> > We will need to update this comment at some point.

Done.


> On Nov. 6, 2017, 6:47 p.m., Benjamin Bannier wrote:
> > src/master/master.hpp
> > Lines 2768 (patched)
> > 
> >
> > I don't think we need to strictly require this.

Why? I think this is necessary because it's a `Framework` method. Just an 
internal invariant.


> On Nov. 6, 2017, 6:47 p.m., Benjamin Bannier wrote:
> > src/master/master.hpp
> > Lines 2889 (patched)
> > 
> >
> > This `CHECK` is redundant with above `if` condition in this universe.

This is `totalOfferedResources`, right?


- Jie


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


On Nov. 6, 2017, 6:14 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63588/
> ---
> 
> (Updated Nov. 6, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> (This is based on https://reviews.apache.org/r/63539)
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 68a5538904b601a91a40db68cebde96f74a2f160 
>   include/mesos/v1/mesos.proto c46cec78db63dc3714e357a954a4e7bf080485fd 
>   src/master/master.hpp afcc2e46882e4c610e9047ab3db7b6f100d47e17 
>   src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
>   src/messages/messages.proto 2fbca22e1285aa25900f5420bb4370bda6a1bb12 
> 
> 
> Diff: https://reviews.apache.org/r/63588/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63491: Added resource version to resource provider UpdateTotalResources call.

[Benjamin Bannier]

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

(Updated Nov. 6, 2017, 11:12 p.m.)


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


Changes
---

Implemented suggestions by Greg.


Repository: mesos


Description
---

This patch surfaces this information to resource provider manager
users like the agent. In a later patch we will modify the agent to
forward this information to the master.


Diffs (updated)
-

  src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e 
  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 


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

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


Testing
---

`make check`, additional testing as part of https://reviews.apache.org/r/63496/.


Thanks,

Benjamin Bannier

Re: Review Request 63491: Added resource version to resource provider UpdateTotalResources call.

[Benjamin Bannier]

> On Nov. 6, 2017, 10:51 p.m., Greg Mann wrote:
> > src/resource_provider/manager.cpp
> > Lines 368-371 (patched)
> > 
> >
> > I found a couple other spots where we indent four spaces in this 
> > situation, perhaps we should do the same here? I don't think our style 
> > guide explicitly mentions indentation of initialization lists, but the 
> > Google style guide does say:
> > 
> > "Format a braced initializer list exactly like you would format a 
> > function call in its place."

Yeah, this makes sense to me. This is an aggregate initialization and the 
Google style guide suggests just what you wrote above. Strangly enough our 
`clang-format` indents this correctly when written as a function call with 
`()`, but gave above format for `{}`.


- Benjamin


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


On Nov. 6, 2017, 11:12 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63491/
> ---
> 
> (Updated Nov. 6, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch surfaces this information to resource provider manager
> users like the agent. In a later patch we will modify the agent to
> forward this information to the master.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63491/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 63597: Refactored XFS quota tests for more reuse for upcoming XFS related tests.

[Meng Zhu]

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

Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

Refactored XFS quota tests for more reuse for upcoming XFS related tests.


Diffs
-

  src/Makefile.am 1c97b1fd8151f87c4e9e6d62884b0ef7d582c312 
  src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76 
  src/tests/containerizer/xfs.hpp PRE-CREATION 
  src/tests/containerizer/xfs.cpp PRE-CREATION 
  src/tests/containerizer/xfs_quota_tests.cpp 
83fa7f2a7e0d31891834669d7b31f18d2fa08035 


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


Testing
---

Verified existing XSF tests, namely `xfs_quota_tests`, on GNU/Linux.


Thanks,

Meng Zhu

Re: Review Request 63577: Fixed a task status update race in default executor tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63577]

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

- Mesos Reviewbot


On Nov. 6, 2017, 1:31 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63577/
> ---
> 
> (Updated Nov. 6, 2017, 1:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gaston Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously in the test `DefaultExecutorTest.KillMultipleTasks` and
> `DefaultExecutorTest.KillTaskGroupOnTaskFailure`, when launching a
> task group which has multiple tasks, we expected the scheduler will
> receive all the TASK_STARTING status updates before receiving any
> TASK_RUNNING status updates. However this is not guaranteed, e.g.,
> it is possible for the scheduler to receive TASK_RUNNING for the
> first task before receiving TASK_STARTING for the second task.
> 
> So in this patch, we used `Sequence` to guarantee the order of
> TASK_STARTING and TASK_RUNNING for each task but do not care about
> the order between tasks.
> 
> The following 3 tests have their own solutions to handle this issue,
> in this patch, I updated them to use the above solution.
>   `DefaultExecutorTest.KillTask`
>   `DefaultExecutorTest.CommitSuicideOnKillTask`
>   `DefaultExecutorTest.ROOT_ContainerStatusForTask`
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 65113985acc0a8fac00374b074ef568aaa22c7ac 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63577/diff/1/
> 
> 
> Testing
> ---
> 
> This patch touched 5 tests:
> 1. KillTask
> 2. KillMultipleTasks
> 3. KillTaskGroupOnTaskFailure
> 4. CommitSuicideOnKillTask
> 5. ROOT_ContainerStatusForTask
> 
> I ran each of them repeatedly (20 times), and all of them succeeded.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 63568: MesosTidy: removed redundant `get()` on smart pointers in mesos.

[Benjamin Bannier]

> On Nov. 6, 2017, 10:47 p.m., Benjamin Bannier wrote:
> > I confirmed that just running clang-tidy with 
> > `google-readability-redundant-smartptr-get` and `-fix` does produce this 
> > patch. I did however run into trouble building this patch on top of 
> > `de19f7381108d491fb41fe3d8ae13e9893213bbc`. Could you make sure it compiles 
> > yet still produces no diagnostics?

Something was wrong with my build dir, everything looking good.


- Benjamin


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


On Nov. 6, 2017, 7:59 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63568/
> ---
> 
> (Updated Nov. 6, 2017, 7:59 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ```
> 3rdparty/libprocess/src/../include/process/owned.hpp:117:7:
> warning: redundant get() call on smart pointer
> [google-readability-redundant-smartptr-get]
> ```
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp fa0f60cad1f699886df1163ecfd2e34f9154d77f 
>   src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
>   src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
>   src/tests/authorization_tests.cpp f65bf2f89b5c22fb6f366cf3998713f48b52dcd7 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 0d68dd634771c016553313d67ddb33504d1a13c2 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> c3410cdbb21b974455d443a18e4af09eddea59ca 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> e61a85df6ec5308ccd2832e66df803b0ad7b53ee 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> fbd2887800ccfd64c8628c5d6fd77a511c8f91d5 
>   src/tests/hook_tests.cpp 542878210d0d335f504220ac21284a3239f419ee 
>   src/tests/master_tests.cpp 7f4a8073d3ea5dd6ca5fa84171797565eb419e9b 
>   src/tests/partition_tests.cpp eead9ceb115c84d1bc8d0d2376fa7c89dd077081 
>   src/tests/reconciliation_tests.cpp 671417f62536bf630c1f927cc184d108cc277a08 
>   src/tests/slave_recovery_tests.cpp db337ba4e213820e7ad0c3f1b480388a2e456556 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
>   src/uri/fetchers/hadoop.cpp 2fa5b02ac5467a2c0c58cd024a2c8b48c4cea2f0 
> 
> 
> Diff: https://reviews.apache.org/r/63568/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 63568: MesosTidy: removed redundant `get()` on smart pointers in mesos.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On Nov. 6, 2017, 7:59 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63568/
> ---
> 
> (Updated Nov. 6, 2017, 7:59 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ```
> 3rdparty/libprocess/src/../include/process/owned.hpp:117:7:
> warning: redundant get() call on smart pointer
> [google-readability-redundant-smartptr-get]
> ```
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp fa0f60cad1f699886df1163ecfd2e34f9154d77f 
>   src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
>   src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
>   src/tests/authorization_tests.cpp f65bf2f89b5c22fb6f366cf3998713f48b52dcd7 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 0d68dd634771c016553313d67ddb33504d1a13c2 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> c3410cdbb21b974455d443a18e4af09eddea59ca 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> e61a85df6ec5308ccd2832e66df803b0ad7b53ee 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> fbd2887800ccfd64c8628c5d6fd77a511c8f91d5 
>   src/tests/hook_tests.cpp 542878210d0d335f504220ac21284a3239f419ee 
>   src/tests/master_tests.cpp 7f4a8073d3ea5dd6ca5fa84171797565eb419e9b 
>   src/tests/partition_tests.cpp eead9ceb115c84d1bc8d0d2376fa7c89dd077081 
>   src/tests/reconciliation_tests.cpp 671417f62536bf630c1f927cc184d108cc277a08 
>   src/tests/slave_recovery_tests.cpp db337ba4e213820e7ad0c3f1b480388a2e456556 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
>   src/uri/fetchers/hadoop.cpp 2fa5b02ac5467a2c0c58cd024a2c8b48c4cea2f0 
> 
> 
> Diff: https://reviews.apache.org/r/63568/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 63491: Added resource version to resource provider UpdateTotalResources call.

[Greg Mann]

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


Fix it, then Ship it!





src/resource_provider/manager.cpp
Lines 368-371 (patched)


I found a couple other spots where we indent four spaces in this situation, 
perhaps we should do the same here? I don't think our style guide explicitly 
mentions indentation of initialization lists, but the Google style guide does 
say:

"Format a braced initializer list exactly like you would format a function 
call in its place."



src/resource_provider/message.hpp
Lines 21 (patched)


This is no longer needed.


- Greg Mann


On Nov. 6, 2017, 12:38 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63491/
> ---
> 
> (Updated Nov. 6, 2017, 12:38 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch surfaces this information to resource provider manager
> users like the agent. In a later patch we will modify the agent to
> forward this information to the master.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63491/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 55335: Backfilled required fields in TaskInfo in MesosContainerizer* tests.

[Gilbert Song]

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


Fix it, then Ship it!





src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 1091-1092 (original), 1096-1105 (patched)


do you miss these?



src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 1180-1186 (patched)


ditto.


- Gilbert Song


On Oct. 27, 2017, 11:01 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55335/
> ---
> 
> (Updated Oct. 27, 2017, 11:01 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests were using a `TaskInfo` field which is missing required
> protobuf fields. After the previous patch begins to checkpoint
> `ContainerConfig`, the incomplete `TaskInfo` cannot be checkpointed,
> thus caused these tests to fail.
> 
> This patch backfills these fields to make these tests pass.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> e61a85df6ec5308ccd2832e66df803b0ad7b53ee 
> 
> 
> Diff: https://reviews.apache.org/r/55335/diff/7/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="MesosContainerizer*" make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63568: MesosTidy: removed redundant `get()` on smart pointers in mesos.

[Benjamin Bannier]

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



I confirmed that just running clang-tidy with 
`google-readability-redundant-smartptr-get` and `-fix` does produce this patch. 
I did however run into trouble building this patch on top of 
`de19f7381108d491fb41fe3d8ae13e9893213bbc`. Could you make sure it compiles yet 
still produces no diagnostics?

- Benjamin Bannier


On Nov. 6, 2017, 7:59 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63568/
> ---
> 
> (Updated Nov. 6, 2017, 7:59 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ```
> 3rdparty/libprocess/src/../include/process/owned.hpp:117:7:
> warning: redundant get() call on smart pointer
> [google-readability-redundant-smartptr-get]
> ```
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp fa0f60cad1f699886df1163ecfd2e34f9154d77f 
>   src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
>   src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
>   src/tests/authorization_tests.cpp f65bf2f89b5c22fb6f366cf3998713f48b52dcd7 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 0d68dd634771c016553313d67ddb33504d1a13c2 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> c3410cdbb21b974455d443a18e4af09eddea59ca 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> e61a85df6ec5308ccd2832e66df803b0ad7b53ee 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> fbd2887800ccfd64c8628c5d6fd77a511c8f91d5 
>   src/tests/hook_tests.cpp 542878210d0d335f504220ac21284a3239f419ee 
>   src/tests/master_tests.cpp 7f4a8073d3ea5dd6ca5fa84171797565eb419e9b 
>   src/tests/partition_tests.cpp eead9ceb115c84d1bc8d0d2376fa7c89dd077081 
>   src/tests/reconciliation_tests.cpp 671417f62536bf630c1f927cc184d108cc277a08 
>   src/tests/slave_recovery_tests.cpp db337ba4e213820e7ad0c3f1b480388a2e456556 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
>   src/uri/fetchers/hadoop.cpp 2fa5b02ac5467a2c0c58cd024a2c8b48c4cea2f0 
> 
> 
> Diff: https://reviews.apache.org/r/63568/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Gilbert Song]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp
Lines 777 (patched)


Let's use VLOG(1) instead since there might be a lot of containers.



src/slave/containerizer/mesos/containerizer.cpp
Line 807 (original), 839-842 (patched)


Seems like you forget to remove these.



src/slave/containerizer/mesos/containerizer.cpp
Lines 878 (patched)


ditto.



src/slave/containerizer/mesos/paths.hpp
Lines 48-63 (original), 48-63 (patched)


could you update this comment?


- Gilbert Song


On Oct. 27, 2017, 11 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Oct. 27, 2017, 11 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/13/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63592: Implemented updateOfferOperation in the master.

[Jie Yu]

> On Nov. 6, 2017, 8:07 p.m., Benjamin Bannier wrote:
> > src/master/master.cpp
> > Lines 9645 (patched)
> > 
> >
> > Should we `CHECK(operation->has_latest_status())` and somewhere above?

`latest_status` in `OfferOperation` is required.


> On Nov. 6, 2017, 8:07 p.m., Benjamin Bannier wrote:
> > src/master/master.cpp
> > Lines 9665 (patched)
> > 
> >
> > Should this be `UUID::fromBytes`?
> 
> Jie Yu wrote:
> good catch! Thanks!

I fix the other incorrect fromString as well.


- Jie


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


On Nov. 6, 2017, 6:55 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63592/
> ---
> 
> (Updated Nov. 6, 2017, 6:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented updateOfferOperation in the master.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
> 
> 
> Diff: https://reviews.apache.org/r/63592/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63594: Improved the fetcher exit status log message.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On Nov. 6, 2017, 12:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63594/
> ---
> 
> (Updated Nov. 6, 2017, 12:36 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8173
> https://issues.apache.org/jira/browse/MESOS-8173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the fetcher fails, we emit a message with its exit status,
> but the `status` is the return value from wait(2) so we should
> be using WSTRINGIFY.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> ba5b0979aa427c5b3dbacf39661d6027da13e0d6 
> 
> 
> Diff: https://reviews.apache.org/r/63594/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63592: Implemented updateOfferOperation in the master.

[Jie Yu]

> On Nov. 6, 2017, 8:07 p.m., Benjamin Bannier wrote:
> > src/master/master.cpp
> > Lines 9665 (patched)
> > 
> >
> > Should this be `UUID::fromBytes`?

good catch! Thanks!


- Jie


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


On Nov. 6, 2017, 6:55 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63592/
> ---
> 
> (Updated Nov. 6, 2017, 6:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented updateOfferOperation in the master.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
> 
> 
> Diff: https://reviews.apache.org/r/63592/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63523: Updated `Allocator::updateAllocation` to take `ResourceConversion`s.

[Jie Yu]

> On Nov. 6, 2017, 3:03 p.m., Benjamin Bannier wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 5001-5003 (original), 5015-5017 (patched)
> > 
> >
> > This is weird now since `launch` is only used to calculate the 
> > expectation below.
> > 
> > We should either try to inject it in the allocator just like `create`, 
> > or try to get rid of it completely.

I get rid of it completely! thanks for catching this!


- Jie


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


On Nov. 2, 2017, 7:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63523/
> ---
> 
> (Updated Nov. 2, 2017, 7:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of taking offer operations, it's more clear to make allocator
> not aware of offer operations. This patch changes the interface of the
> `updateAllocation` call to take `ResourceConversion`s.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 537658bf4e68ac7f271248af0e1e35df2b19aef8 
>   src/master/allocator/mesos/allocator.hpp 
> 903edf68e812e022cddedd40a2cb51726b2a181b 
>   src/master/allocator/mesos/hierarchical.hpp 
> c2346054b2c98516f15ab8ce2dc798224ff4def4 
>   src/master/allocator/mesos/hierarchical.cpp 
> 848e9da1b61f411449c0cd2409e98ec733ac10ee 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
>   src/tests/allocator.hpp f1c0d148886b7b2936918b7d7c77c03ee766ee4a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 48b48ad70f44cc2232c2a29699267027f9937b8a 
> 
> 
> Diff: https://reviews.apache.org/r/63523/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63400: Fix bad links.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: patch failed: docs/working-groups.md:30
error: docs/working-groups.md: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 6, 2017, 2:23 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63400/
> ---
> 
> (Updated Nov. 6, 2017, 2:23 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Joerg Schad, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix bad links.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> 6e73e4e1909c315d566fa29b56a54ac47c711e79 
>   docs/cmake.md 1e1a27ef67d256eb892a5eb455928c2424bb3fed 
>   docs/cni.md 09159cd26162fe06f7be0519909720b16166ad1e 
>   docs/isolators/network-port-mapping.md 
> 522f2edb8ba2c1db28617d22237e6c20a2bf2aa8 
>   docs/upgrades.md 6370c06a497161317884f61f376308423b744fe1 
>   docs/working-groups.md f6af1e39375cab0d0ed61d8c1fa18e4e4c0c1d42 
>   site/source/blog/2014-03-28-mesos-community-update-1.md 
> 3b8114d6c2eb33200800999c410d1d6030fa5148 
>   site/source/layouts/community_section.erb 
> 07fd28853e119c1d9298a662cc6d73054b6e415f 
> 
> 
> Diff: https://reviews.apache.org/r/63400/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 63538: Removed expectation for scheduler connected events in some tests.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63538 was successfully built and tested.

Reviews applied: `['63538']`

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

- Mesos Reviewbot Windows


On Nov. 3, 2017, 7:32 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63538/
> ---
> 
> (Updated Nov. 3, 2017, 7:32 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8093
> https://issues.apache.org/jira/browse/MESOS-8093
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> ee1e5e67b9eacb5909964b2951f659d1bd2233e5 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 0d68dd634771c016553313d67ddb33504d1a13c2 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 81c0d825c689b3d00cbc21c7567c8c0c8836bf75 
> 
> 
> Diff: https://reviews.apache.org/r/63538/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> sudo ./mesos-tests.sh 
> --gtest_filter="CgroupsIsolatorTest.ROOT_CGROUPS_LimitSwap" 
> --gtest_break_on_failure --gtest_repeat=100
> sudo ./mesos-tests.sh 
> --gtest_filter="DockerRuntimeIsolatorTest.ROOT_INTERNET_CURL_NestedSimpleCommand"
>  --gtest_break_on_failure --gtest_repeat=100
> ```
> And `make check`.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 63594: Improve fetcher exit status log message.

[James Peach]

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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

When the fetcher fails, we emit a message with its exit status,
but the `status` is the return value from wait(2) so we should
be using WSTRINGIFY.


Diffs
-

  src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6 


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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 63592: Implemented updateOfferOperation in the master.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/master/master.cpp
Line 9629 (original), 9629 (patched)


Can we take this by `const` ref since we don't take ownership?



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


Let's either initialize this variable with a good default or use an 
`Option` with a `CHECK`.



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


Should we `CHECK(operation->has_latest_status())` and somewhere above?



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


Should this be `UUID::fromBytes`?



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


Can be a `const` ref.


- Benjamin Bannier


On Nov. 6, 2017, 7:55 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63592/
> ---
> 
> (Updated Nov. 6, 2017, 7:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented updateOfferOperation in the master.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
> 
> 
> Diff: https://reviews.apache.org/r/63592/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63585: Improved support/mesos-style.py structure.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63581, 63582, 62214, 63585]

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

- Mesos Reviewbot


On Nov. 6, 2017, 8:59 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63585/
> ---
> 
> (Updated Nov. 6, 2017, 8:59 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Python linter now also uses the function `run_command_in_virtualenv`. 
> The methods of `LinterBase` have been ordered alphabetically.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
> 
> 
> Diff: https://reviews.apache.org/r/63585/diff/1/
> 
> 
> Testing
> ---
> 
> Used the linters after applying this patch.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 63523: Updated `Allocator::updateAllocation` to take `ResourceConversion`s.

[Jie Yu]

> On Nov. 6, 2017, 3:03 p.m., Benjamin Bannier wrote:
> > src/master/master.cpp
> > Lines 4888-4889 (original)
> > 
> >
> > This is weird. Any idea why we did inject this information into 
> > allocators? Is there any expectation that a custom allocator should be able 
> > to access this information?

It's for shared resources. `updateAllocation` expect task launch operation so 
that it can calculate how much new "shared" resources should be allocated. See 
my response above.


- Jie


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


On Nov. 2, 2017, 7:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63523/
> ---
> 
> (Updated Nov. 2, 2017, 7:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of taking offer operations, it's more clear to make allocator
> not aware of offer operations. This patch changes the interface of the
> `updateAllocation` call to take `ResourceConversion`s.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 537658bf4e68ac7f271248af0e1e35df2b19aef8 
>   src/master/allocator/mesos/allocator.hpp 
> 903edf68e812e022cddedd40a2cb51726b2a181b 
>   src/master/allocator/mesos/hierarchical.hpp 
> c2346054b2c98516f15ab8ce2dc798224ff4def4 
>   src/master/allocator/mesos/hierarchical.cpp 
> 848e9da1b61f411449c0cd2409e98ec733ac10ee 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
>   src/tests/allocator.hpp f1c0d148886b7b2936918b7d7c77c03ee766ee4a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 48b48ad70f44cc2232c2a29699267027f9937b8a 
> 
> 
> Diff: https://reviews.apache.org/r/63523/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63523: Updated `Allocator::updateAllocation` to take `ResourceConversion`s.

[Jie Yu]

> On Nov. 6, 2017, 3:03 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 888-896 (original), 809-817 (patched)
> > 
> >
> > This could be more general, and I am not sure I agree with the general 
> > direction the `TODO` outlines. In the end we only need to handle 
> > conversions which actually consume resources here, and can drop all which 
> > don't. This can in principle be independent from e.g., launches on shared 
> > resources.

Currently, for shared resources, we actually "increase" the allocation during 
task launch, thus the reason for calling `updateAllocation` here. However, for 
those shared resources, the newly allocated "shared" resources are not in the 
"total" resources (which I think is a bug).

I chatted with YanX on this, I think the best way to remove this TODO is to 
update agent's total resources during allocation for shared resources so that 
any additional allocation due to task launch can be dealt with the same way as 
other resources.


- Jie


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


On Nov. 2, 2017, 7:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63523/
> ---
> 
> (Updated Nov. 2, 2017, 7:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of taking offer operations, it's more clear to make allocator
> not aware of offer operations. This patch changes the interface of the
> `updateAllocation` call to take `ResourceConversion`s.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 537658bf4e68ac7f271248af0e1e35df2b19aef8 
>   src/master/allocator/mesos/allocator.hpp 
> 903edf68e812e022cddedd40a2cb51726b2a181b 
>   src/master/allocator/mesos/hierarchical.hpp 
> c2346054b2c98516f15ab8ce2dc798224ff4def4 
>   src/master/allocator/mesos/hierarchical.cpp 
> 848e9da1b61f411449c0cd2409e98ec733ac10ee 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
>   src/tests/allocator.hpp f1c0d148886b7b2936918b7d7c77c03ee766ee4a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 48b48ad70f44cc2232c2a29699267027f9937b8a 
> 
> 
> Diff: https://reviews.apache.org/r/63523/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63278: Windows: Documented the `cpuset` and `mem` isolators.

[Andrew Schwartzmeyer]

> On Nov. 2, 2017, 4:48 p.m., Benjamin Mahler wrote:
> > docs/isolators/windows.md
> > Lines 8 (patched)
> > 
> >
> > Hm.. why is it called cpuset? That word comes from the cgroup subsystem 
> > that lets you restrict which cores can be used by a cgroup, but the windows 
> > mechanisms don't sound the same at all?
> > 
> > Would just `windows/cpu` be more appropriate?
> 
> Andrew Schwartzmeyer wrote:
> Jie and Joe argued for `cpuset` because it's a hard cap, and so more like 
> cgroup's `cpuset` than just the `cpu` isolator.
> 
> But if in fact `cpuset` means:
> > that lets you restrict which cores can be used by a cgroup
> 
> Then yeah, this should be `cpu`.
> 
> Joseph Wu wrote:
> Since `cpuset` is not the correct term here (even though the semantics 
> are closer), let's go with `cpu`.  But can you add a note in the docs that 
> explicitly calls out the difference between this hard cap and the cgroups 
> soft cap?  (More explicit than the existing mention of a hard cap.)

Can do!


- Andrew


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


On Nov. 2, 2017, 1:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63278/
> ---
> 
> (Updated Nov. 2, 2017, 1:40 p.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6690
> https://issues.apache.org/jira/browse/MESOS-6690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds documentation on the usage of the job object isolators, which
> enable task limits, as well as the statistics they report.
> 
> 
> Diffs
> -
> 
>   docs/isolators/windows.md PRE-CREATION 
>   docs/mesos-containerizer.md 9cb071273a5ce42784457ea01b363911fdb7773d 
> 
> 
> Diff: https://reviews.apache.org/r/63278/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63278: Windows: Documented the `cpuset` and `mem` isolators.

[Joseph Wu]

> On Nov. 2, 2017, 4:48 p.m., Benjamin Mahler wrote:
> > docs/isolators/windows.md
> > Lines 8 (patched)
> > 
> >
> > Hm.. why is it called cpuset? That word comes from the cgroup subsystem 
> > that lets you restrict which cores can be used by a cgroup, but the windows 
> > mechanisms don't sound the same at all?
> > 
> > Would just `windows/cpu` be more appropriate?
> 
> Andrew Schwartzmeyer wrote:
> Jie and Joe argued for `cpuset` because it's a hard cap, and so more like 
> cgroup's `cpuset` than just the `cpu` isolator.
> 
> But if in fact `cpuset` means:
> > that lets you restrict which cores can be used by a cgroup
> 
> Then yeah, this should be `cpu`.

Since `cpuset` is not the correct term here (even though the semantics are 
closer), let's go with `cpu`.  But can you add a note in the docs that 
explicitly calls out the difference between this hard cap and the cgroups soft 
cap?  (More explicit than the existing mention of a hard cap.)


- Joseph


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


On Nov. 2, 2017, 1:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63278/
> ---
> 
> (Updated Nov. 2, 2017, 1:40 p.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6690
> https://issues.apache.org/jira/browse/MESOS-6690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds documentation on the usage of the job object isolators, which
> enable task limits, as well as the statistics they report.
> 
> 
> Diffs
> -
> 
>   docs/isolators/windows.md PRE-CREATION 
>   docs/mesos-containerizer.md 9cb071273a5ce42784457ea01b363911fdb7773d 
> 
> 
> Diff: https://reviews.apache.org/r/63278/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63590: Enabled CredentialsTests on Windows.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Nov. 6, 2017, 10:49 a.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63590/
> ---
> 
> (Updated Nov. 6, 2017, 10:49 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-6703
> https://issues.apache.org/jira/browse/MESOS-6703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled CredentialsTests on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/credentials_tests.cpp 7eb5abd21a1be35d4739c4928cb922f028cfc9a7 
> 
> 
> Diff: https://reviews.apache.org/r/63590/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Windows, all tests pass. This is a set of tests that was not 
> enabled after the authentication changes. Not tested on non-Windows systems, 
> as this change should have zero effect on non-Windows systems.
> 
> On Windows:
> [--] 3 tests from CredentialsTest
> [ RUN  ] CredentialsTest.AuthenticatedSlave
> [   OK ] CredentialsTest.AuthenticatedSlave (123 ms)
> [ RUN  ] CredentialsTest.AuthenticatedSlaveText
> [   OK ] CredentialsTest.AuthenticatedSlaveText (119 ms)
> [ RUN  ] CredentialsTest.AuthenticatedSlaveJSON
> [   OK ] CredentialsTest.AuthenticatedSlaveJSON (114 ms)
> [--] 3 tests from CredentialsTest (370 ms total)
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 63590: Enabled CredentialsTests on Windows.

[Andrew Schwartzmeyer]

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


Ship it!




Nice and simple!

- Andrew Schwartzmeyer


On Nov. 6, 2017, 10:49 a.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63590/
> ---
> 
> (Updated Nov. 6, 2017, 10:49 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-6703
> https://issues.apache.org/jira/browse/MESOS-6703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled CredentialsTests on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/credentials_tests.cpp 7eb5abd21a1be35d4739c4928cb922f028cfc9a7 
> 
> 
> Diff: https://reviews.apache.org/r/63590/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on Windows, all tests pass. This is a set of tests that was not 
> enabled after the authentication changes. Not tested on non-Windows systems, 
> as this change should have zero effect on non-Windows systems.
> 
> On Windows:
> [--] 3 tests from CredentialsTest
> [ RUN  ] CredentialsTest.AuthenticatedSlave
> [   OK ] CredentialsTest.AuthenticatedSlave (123 ms)
> [ RUN  ] CredentialsTest.AuthenticatedSlaveText
> [   OK ] CredentialsTest.AuthenticatedSlaveText (119 ms)
> [ RUN  ] CredentialsTest.AuthenticatedSlaveJSON
> [   OK ] CredentialsTest.AuthenticatedSlaveJSON (114 ms)
> [--] 3 tests from CredentialsTest (370 ms total)
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 63568: MesosTidy: removed redundant `get()` on smart pointers in mesos.

[Michael Park]

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

(Updated Nov. 6, 2017, 10:59 a.m.)


Review request for mesos and Benjamin Bannier.


Changes
---

Addressed Benjamin's comment.


Repository: mesos


Description (updated)
---

```
3rdparty/libprocess/src/../include/process/owned.hpp:117:7:
warning: redundant get() call on smart pointer
[google-readability-redundant-smartptr-get]
```


Diffs (updated)
-

  src/master/http.cpp fa0f60cad1f699886df1163ecfd2e34f9154d77f 
  src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
  src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b 
  src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
  src/tests/authorization_tests.cpp f65bf2f89b5c22fb6f366cf3998713f48b52dcd7 
  src/tests/containerizer/cni_isolator_tests.cpp 
0d68dd634771c016553313d67ddb33504d1a13c2 
  src/tests/containerizer/io_switchboard_tests.cpp 
c3410cdbb21b974455d443a18e4af09eddea59ca 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
e61a85df6ec5308ccd2832e66df803b0ad7b53ee 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
fbd2887800ccfd64c8628c5d6fd77a511c8f91d5 
  src/tests/hook_tests.cpp 542878210d0d335f504220ac21284a3239f419ee 
  src/tests/master_tests.cpp 7f4a8073d3ea5dd6ca5fa84171797565eb419e9b 
  src/tests/partition_tests.cpp eead9ceb115c84d1bc8d0d2376fa7c89dd077081 
  src/tests/reconciliation_tests.cpp 671417f62536bf630c1f927cc184d108cc277a08 
  src/tests/slave_recovery_tests.cpp db337ba4e213820e7ad0c3f1b480388a2e456556 
  src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
  src/uri/fetchers/hadoop.cpp 2fa5b02ac5467a2c0c58cd024a2c8b48c4cea2f0 


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

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 63568: MesosTidy: removed redundant `get()` on smart pointers in mesos.

[Michael Park]

> On Nov. 6, 2017, 3:35 a.m., Benjamin Bannier wrote:
> > include/mesos/slave/containerizer.hpp
> > Line 122 (original), 122 (patched)
> > 
> >
> > Was this generated by a clang-tidy fixit hint? I suspect not since it 
> > changes semantics.
> > 
> > Could you split the cleanups and semantics changes into two separate 
> > patches?

Pulled out to https://reviews.apache.org/r/63591/


- Michael


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


On Nov. 5, 2017, 4:11 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63568/
> ---
> 
> (Updated Nov. 5, 2017, 4:11 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ```
> 3rdparty/libprocess/src/../include/process/owned.hpp:117:7:
> warning: redundant get() call on smart pointer
> [google-readability-redundant-smartptr-get]
> ```
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.hpp 
> 838a8c394746afb415d00a7764862fb44564f79a 
>   src/master/http.cpp fa0f60cad1f699886df1163ecfd2e34f9154d77f 
>   src/master/master.cpp 49bc50ead75adba82a7d8de23a87b06ccd269c48 
>   src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
>   src/tests/authorization_tests.cpp f65bf2f89b5c22fb6f366cf3998713f48b52dcd7 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 0d68dd634771c016553313d67ddb33504d1a13c2 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> c3410cdbb21b974455d443a18e4af09eddea59ca 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> e61a85df6ec5308ccd2832e66df803b0ad7b53ee 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> fbd2887800ccfd64c8628c5d6fd77a511c8f91d5 
>   src/tests/hook_tests.cpp 542878210d0d335f504220ac21284a3239f419ee 
>   src/tests/master_tests.cpp 7f4a8073d3ea5dd6ca5fa84171797565eb419e9b 
>   src/tests/partition_tests.cpp 7b11264718b1ab7edbe594335a1f8130bef0236d 
>   src/tests/reconciliation_tests.cpp 8ae2860591afc8eec04b7c23322cd7dd964bbcb3 
>   src/tests/slave_recovery_tests.cpp db337ba4e213820e7ad0c3f1b480388a2e456556 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
>   src/uri/fetchers/hadoop.cpp 2fa5b02ac5467a2c0c58cd024a2c8b48c4cea2f0 
> 
> 
> Diff: https://reviews.apache.org/r/63568/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 63591: Flattened unnecessary `Optional<Shared>` to simply `Shared`.

[Benjamin Bannier]

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


Ship it!




Thanks for the cleanup mpark!

- Benjamin Bannier


On Nov. 6, 2017, 7:54 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63591/
> ---
> 
> (Updated Nov. 6, 2017, 7:54 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since `Shared` is already nullable, avoiding the null state of
> a `Shared` and wrapping it in an `Optional` doesn't add anything.
> We should just use `Shared` as it is, and make use of the null state.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.hpp 
> 838a8c394746afb415d00a7764862fb44564f79a 
> 
> 
> Diff: https://reviews.apache.org/r/63591/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 63567: MesosTidy: Removed redundant `get()` on smart pointers in libprocess.

[Michael Park]

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

(Updated Nov. 6, 2017, 10:55 a.m.)


Review request for mesos and Benjamin Bannier.


Changes
---

Simply removed the redundant `get()` checks.


Repository: mesos


Description (updated)
---

```
3rdparty/libprocess/src/../include/process/owned.hpp:117:7:
warning: redundant get() call on smart pointer
[google-readability-redundant-smartptr-get]
```


Diffs (updated)
-

  3rdparty/libprocess/include/process/owned.hpp 
e1ae6dcc771cf2244bc16bbab19e9f05fa1704cc 
  3rdparty/libprocess/include/process/shared.hpp 
d58c885349bca279e057def0401a3bddf4d994d8 
  3rdparty/libprocess/src/tests/owned_tests.cpp 
f9f4ccef9da23d46f8764bed27c07d6a65ae061a 
  3rdparty/libprocess/src/tests/shared_tests.cpp 
2a2ffe76b7b7ce016b559de7b5d3a28a06f422ef 


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

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 63592: Implemented updateOfferOperation in the master.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier and Jan Schlicht.


Repository: mesos


Description
---

Implemented updateOfferOperation in the master.


Diffs
-

  src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 


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


Testing
---

make check


Thanks,

Jie Yu

Review Request 63591: Flattened unnecessary `Optional<Shared>` to simply `Shared`.

[Michael Park]

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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

Since `Shared` is already nullable, avoiding the null state of
a `Shared` and wrapping it in an `Optional` doesn't add anything.
We should just use `Shared` as it is, and make use of the null state.


Diffs
-

  include/mesos/slave/containerizer.hpp 
838a8c394746afb415d00a7764862fb44564f79a 


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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 63590: Enabled CredentialsTests on Windows.

[John Kordich via Review Board]

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

Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Enabled CredentialsTests on Windows.


Diffs
-

  src/tests/credentials_tests.cpp 7eb5abd21a1be35d4739c4928cb922f028cfc9a7 


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


Testing
---

Tested on Windows, all tests pass. This is a set of tests that was not enabled 
after the authentication changes. Not tested on non-Windows systems, as this 
change should have zero effect on non-Windows systems.

On Windows:
[--] 3 tests from CredentialsTest
[ RUN  ] CredentialsTest.AuthenticatedSlave
[   OK ] CredentialsTest.AuthenticatedSlave (123 ms)
[ RUN  ] CredentialsTest.AuthenticatedSlaveText
[   OK ] CredentialsTest.AuthenticatedSlaveText (119 ms)
[ RUN  ] CredentialsTest.AuthenticatedSlaveJSON
[   OK ] CredentialsTest.AuthenticatedSlaveJSON (114 ms)
[--] 3 tests from CredentialsTest (370 ms total)


Thanks,

John Kordich

Re: Review Request 63588: Updated the offer operation bookeeping in the master.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/master/master.hpp
Lines 887-888 (patched)


There is no `framework` here. If you referring to the `optional 
FrameworkID` inside `OfferOperation` the comment needs an update.



src/master/master.hpp
Lines 925-926 (original), 935-936 (patched)


We will need to update this comment at some point.



src/master/master.hpp
Lines 2768 (patched)


I don't think we need to strictly require this.



src/master/master.hpp
Lines 2795 (patched)


Let's update this comment so it documents that these operations are 
speculatively applied and not tracked as `used` until feedback arrives.



src/master/master.hpp
Lines 2849 (patched)


Let's update this comment so it documents that these operations are 
speculatively applied and not tracked as `used` until feedback arrives.



src/master/master.hpp
Lines 2868-2869 (patched)


Should we add a similar `CHECK` for `totalUsedResources`?



src/master/master.hpp
Lines 2869 (patched)


From this message it isn't very clear what happened, what about e.g.,

Tried to recover resources %{consumed} of agent %{slaveId} which do not 
seem used



src/master/master.hpp
Lines 2889 (patched)


This `CHECK` is redundant with above `if` condition in this universe.



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


Let's update this comment so it documents that these operations are 
speculatively applied and not tracked as `used` until feedback arrives.



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


Let's update this comment so it documents that these operations are 
speculatively applied and not tracked as `used` until feedback arrives.


- Benjamin Bannier


On Nov. 6, 2017, 7:14 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63588/
> ---
> 
> (Updated Nov. 6, 2017, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> (This is based on https://reviews.apache.org/r/63539)
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 68a5538904b601a91a40db68cebde96f74a2f160 
>   include/mesos/v1/mesos.proto c46cec78db63dc3714e357a954a4e7bf080485fd 
>   src/master/master.hpp afcc2e46882e4c610e9047ab3db7b6f100d47e17 
>   src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
>   src/messages/messages.proto 2fbca22e1285aa25900f5420bb4370bda6a1bb12 
> 
> 
> Diff: https://reviews.apache.org/r/63588/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 63589: Fixed bug in tests leading to orphaned containers.

[Andrei Budnik]

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

Review request for mesos, Alexander Rukletsov, Benno Evers, and Gilbert Song.


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


Repository: mesos


Description
---

Previously, some tests tried to advance the clock until task status
update was sent, while task's container was destroying. Container
destruction consists of multiple steps, where some steps have a timeout
specified, e.g. `cgroups::DESTROY_TIMEOUT`. So, there was a race
between container destruction process and the loop that advanced the
clock, leading to the following outcomes:

  (1) Container destroyed, before clock advancing reaches timeout.

  (2) Triggered timeout due to clock advancing, before container
  destruction completes. That results in leaving orphaned
  containers that will be detected by Slave destructor in
  `tests/cluster.cpp`, so the test will fail.

This change gets rid of the loop and resumes clock after a single
advancing of the clock.


Diffs
-

  src/tests/slave_recovery_tests.cpp db337ba4e213820e7ad0c3f1b480388a2e456556 
  src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 


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


Testing
---

1. make check
2. internal ci (5x)


Thanks,

Andrei Budnik

Re: Review Request 63511: Introduced ResourceConversion to represent conversion to Resources.

[Jie Yu]

> On Nov. 6, 2017, 2:43 p.m., Benjamin Bannier wrote:
> > src/common/resources_utils.cpp
> > Lines 103 (patched)
> > 
> >
> > In what scenario would this actually return a list of conversions? 
> > Right now it returns at most one conversion.
> > 
> > I'd either make this return a `Try`, or 
> > alternatively take an iterable of `TOfferOperation` (it seems that a 
> > non-list, single element version should be good enough for now, and also be 
> > a good building block should we want to work on lists of operations at some 
> > later point).

For `RESERVE/UNRESERVE/CREATE/DESTROY`, it returns a list of conversions (the 
foreach loop). I made it this way so that the semantics is exactly the same as 
it is right now. We might be able to consolidate all conversions from an 
operation into a single conversion, but this is slightly tricky. I'll punt that 
for now.


- Jie


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


On Nov. 2, 2017, 7:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63511/
> ---
> 
> (Updated Nov. 2, 2017, 7:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, we couple offer operations with resource conversions. For
> instance, we have interfaces like `Resources::apply` that takes an
> offer operation. This becomes less ideal because an offer operation
> might not represent a `conversion` anymore with new offer operation
> like `CREATE_VOLUME` being introduced.
> 
> This patch decoupled resource conversions from offer operations.
> `Resources::apply` will now take a `ResourceConversion` object. We
> also provide some helpers to get a `ResourceConversion` from an offer
> operation (if supported).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 53152b393ec38ca2fc2e2a66c18a29a4418fe59e 
>   include/mesos/v1/resources.hpp 6c2191c044c52607618283891f82401c4861c441 
>   src/common/resources.cpp 914d3e2b1e8fe55ecab9c71643954addd6a81244 
>   src/common/resources_utils.hpp 18e3d9d4baad23669d00542594f5c15a989b7b9e 
>   src/common/resources_utils.cpp e34cd8a3c9046a6f12c12a275a7b3a852b492f4c 
>   src/v1/resources.cpp 58568b8d25cb1402a875d3975d2decb4270d2725 
> 
> 
> Diff: https://reviews.apache.org/r/63511/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63341: Set `BUILD_FLAGS` flag in CMake.

[Andrew Schwartzmeyer]

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

(Updated Nov. 6, 2017, 10:34 a.m.)


Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Michael 
Park.


Changes
---

Added guard to avoid having to always define `BUILD_JAVA_JVM_LIBRARY`.


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


Repository: mesos


Description
---

This resolves MESOS-5455, and consolidates the `BUILD` variables into
one location.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake deb574200bceb605a60b536a396c98d9247c6018 
  src/CMakeLists.txt b0e223121f24ac0dc604dc2c639158b817eec535 
  src/common/build.cpp 4192b89cbee1c9d7a75213f55b189565cd8a10f1 
  src/common/build_config.hpp.in ac7059ab2aa54011197b9e1a46f9d7fd39ac39c6 


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

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


Testing
---

Checked the emitted `build_config.hpp` under various conditions; flags are set 
correctly (and can still build of course).


Thanks,

Andrew Schwartzmeyer

Re: Review Request 62734: Added Markdown, bug, and reviewers support to `post-reviews.py`.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Nov. 6, 2017, 10:32 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62734/
> ---
> 
> (Updated Nov. 6, 2017, 10:32 a.m.)
> 
> 
> Review request for mesos, Joseph Wu, Michael Park, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These options are appropriately forwarded to `rbt`.
> 
> We enable Markdown parsing by default, with `--no-markdown` to disable
> it; this is safe as most commit formatting is intended to be Markdown,
> and unformatted text is valid markdown. This prevents `rbt` from
> escaping e.g. ticks used in commit messages that are then manually
> removed from reviews on Review Board.
> 
> The `--bugs-closed` flag lets users attach a bug automatically to every
> review in the chain they're posting. Similarly `--target-people` will
> attach the specified reviewers to every review. This reduces the amount
> of work it takes to prepare chains of reviews.
> 
> We added help to the argument parser of the script, mostly stolen from
> the descriptions in `rbt post --help`.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py c9378719f5ab38052bd87fab73ab9b5bd37e0154 
> 
> 
> Diff: https://reviews.apache.org/r/62734/diff/4/
> 
> 
> Testing
> ---
> 
> Used it to post this patch.
> 
> `./support/post-reviews.py --help` now looks like:
> 
> ```
> usage: post-reviews.py [-h] [--server SERVER] [--no-markdown]
>[--bugs-closed BUGS_CLOSED]
>[--target-people TARGET_PEOPLE]
>[--tracking-branch TRACKING_BRANCH]
> 
> optional arguments:
>   -h, --helpshow this help message and exit
>   --server SERVER   Specifies the Review Board server to use.
>   --no-markdown Specifies if the commit text should not be treated as
> Markdown.
>   --bugs-closed BUGS_CLOSED
> The comma-separated list of bug IDs closed.
>   --target-people TARGET_PEOPLE
> The usernames of the people who should perform the
> review.
>   --tracking-branch TRACKING_BRANCH
> The remote tracking branch from which your local
> branch is derived.
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 62734: Added Markdown, bug, and reviewers support to `post-reviews.py`.

[Andrew Schwartzmeyer]

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

(Updated Nov. 6, 2017, 10:32 a.m.)


Review request for mesos, Joseph Wu, Michael Park, and Till Toenshoff.


Changes
---

Added help string for `--server`.


Repository: mesos


Description
---

These options are appropriately forwarded to `rbt`.

We enable Markdown parsing by default, with `--no-markdown` to disable
it; this is safe as most commit formatting is intended to be Markdown,
and unformatted text is valid markdown. This prevents `rbt` from
escaping e.g. ticks used in commit messages that are then manually
removed from reviews on Review Board.

The `--bugs-closed` flag lets users attach a bug automatically to every
review in the chain they're posting. Similarly `--target-people` will
attach the specified reviewers to every review. This reduces the amount
of work it takes to prepare chains of reviews.

We added help to the argument parser of the script, mostly stolen from
the descriptions in `rbt post --help`.


Diffs (updated)
-

  support/post-reviews.py c9378719f5ab38052bd87fab73ab9b5bd37e0154 


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

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


Testing (updated)
---

Used it to post this patch.

`./support/post-reviews.py --help` now looks like:

```
usage: post-reviews.py [-h] [--server SERVER] [--no-markdown]
   [--bugs-closed BUGS_CLOSED]
   [--target-people TARGET_PEOPLE]
   [--tracking-branch TRACKING_BRANCH]

optional arguments:
  -h, --helpshow this help message and exit
  --server SERVER   Specifies the Review Board server to use.
  --no-markdown Specifies if the commit text should not be treated as
Markdown.
  --bugs-closed BUGS_CLOSED
The comma-separated list of bug IDs closed.
  --target-people TARGET_PEOPLE
The usernames of the people who should perform the
review.
  --tracking-branch TRACKING_BRANCH
The remote tracking branch from which your local
branch is derived.
```


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63485: Added initial code for offer operation status update in master.

[Jie Yu]

> On Nov. 6, 2017, 3:29 p.m., Benjamin Bannier wrote:
> > src/master/master.cpp
> > Lines 7257 (patched)
> > 
> >
> > I feel it might make more sense to log the ID of the agent injecting 
> > faulty operation IDs than the framework which triggered the operation.

Good point. I'll address that in https://reviews.apache.org/r/63588 to avoid 
conflicts. Framework might be optional.


- Jie


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


On Nov. 1, 2017, 11:18 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63485/
> ---
> 
> (Updated Nov. 1, 2017, 11:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> Greg Mann, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added initial code for offer operation status update in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 76f884c61f8d4605725e93e720dc693b861918e3 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
> 
> 
> Diff: https://reviews.apache.org/r/63485/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63577: Fixed a task status update race in default executor tests.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63577 was successfully built and tested.

Reviews applied: `['63577']`

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

- Mesos Reviewbot Windows


On Nov. 6, 2017, 2:31 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63577/
> ---
> 
> (Updated Nov. 6, 2017, 2:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gaston Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously in the test `DefaultExecutorTest.KillMultipleTasks` and
> `DefaultExecutorTest.KillTaskGroupOnTaskFailure`, when launching a
> task group which has multiple tasks, we expected the scheduler will
> receive all the TASK_STARTING status updates before receiving any
> TASK_RUNNING status updates. However this is not guaranteed, e.g.,
> it is possible for the scheduler to receive TASK_RUNNING for the
> first task before receiving TASK_STARTING for the second task.
> 
> So in this patch, we used `Sequence` to guarantee the order of
> TASK_STARTING and TASK_RUNNING for each task but do not care about
> the order between tasks.
> 
> The following 3 tests have their own solutions to handle this issue,
> in this patch, I updated them to use the above solution.
>   `DefaultExecutorTest.KillTask`
>   `DefaultExecutorTest.CommitSuicideOnKillTask`
>   `DefaultExecutorTest.ROOT_ContainerStatusForTask`
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 65113985acc0a8fac00374b074ef568aaa22c7ac 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63577/diff/1/
> 
> 
> Testing
> ---
> 
> This patch touched 5 tests:
> 1. KillTask
> 2. KillMultipleTasks
> 3. KillTaskGroupOnTaskFailure
> 4. CommitSuicideOnKillTask
> 5. ROOT_ContainerStatusForTask
> 
> I ran each of them repeatedly (20 times), and all of them succeeded.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 63565: MesosTidy: Added some `google-*` checks to the default set of checks.

[Michael Park]

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

(Updated Nov. 6, 2017, 10:17 a.m.)


Review request for mesos and Benjamin Bannier.


Changes
---

Addressed Benjamin's comment.


Repository: mesos


Description
---

MesosTidy: Added some `google-*` checks to the default set of checks.


Diffs (updated)
-

  support/mesos-tidy.sh 875108ae18c583e24411264983433b902b43412c 


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

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


Testing
---


Thanks,

Michael Park

Review Request 63588: Updated the offer operation bookeeping in the master.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier and Jan Schlicht.


Repository: mesos


Description
---

(This is based on https://reviews.apache.org/r/63539)


Diffs
-

  include/mesos/mesos.proto 68a5538904b601a91a40db68cebde96f74a2f160 
  include/mesos/v1/mesos.proto c46cec78db63dc3714e357a954a4e7bf080485fd 
  src/master/master.hpp afcc2e46882e4c610e9047ab3db7b6f100d47e17 
  src/master/master.cpp f349fec8b61fa424c63d77253b6ab3ded08c8cdb 
  src/messages/messages.proto 2fbca22e1285aa25900f5420bb4370bda6a1bb12 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 63485: Added initial code for offer operation status update in master.

[Benjamin Bannier]

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


Ship it!




Talked with Jie offline about the use of raw ptrs here. His idea was to use a 
pattern conforming to the current handling of `Task*`. We agreed that this 
pattern has complicated ownership semantics (e.g., who calls `delete`), and 
that Jie would clean it up in a subsequent patch.

- Benjamin Bannier


On Nov. 2, 2017, 12:18 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63485/
> ---
> 
> (Updated Nov. 2, 2017, 12:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> Greg Mann, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added initial code for offer operation status update in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 76f884c61f8d4605725e93e720dc693b861918e3 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
> 
> 
> Diff: https://reviews.apache.org/r/63485/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63485: Added initial code for offer operation status update in master.

[Jie Yu]

> On Nov. 6, 2017, 3:29 p.m., Benjamin Bannier wrote:
> > src/master/master.cpp
> > Lines 5161-5162 (original), 5164-5165 (patched)
> > 
> >
> > I am not sure why we went from a managed ptr to a raw pointer here and 
> > below. It e.g., seems we now leak `offerOperation`s (this patch just adds 
> > `new`, but no matching `delete`).
> > 
> > Could we just keep using `Owned` here?
> 
> Jie Yu wrote:
> The 'deletion' will be addressed in the following patches (in 
> `removeOfferOperation`). The reason for a raw pointer here is because 
> Framework needs to maintain a hashmap to `OfferOperation` pointer too. I made 
> it consistent with Task.

I'll probably followup with a patch to change raw pointer to `shared_ptr`


- Jie


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


On Nov. 1, 2017, 11:18 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63485/
> ---
> 
> (Updated Nov. 1, 2017, 11:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> Greg Mann, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added initial code for offer operation status update in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 76f884c61f8d4605725e93e720dc693b861918e3 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
> 
> 
> Diff: https://reviews.apache.org/r/63485/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63485: Added initial code for offer operation status update in master.

[Jie Yu]

> On Nov. 6, 2017, 3:29 p.m., Benjamin Bannier wrote:
> > src/master/master.cpp
> > Lines 5161-5162 (original), 5164-5165 (patched)
> > 
> >
> > I am not sure why we went from a managed ptr to a raw pointer here and 
> > below. It e.g., seems we now leak `offerOperation`s (this patch just adds 
> > `new`, but no matching `delete`).
> > 
> > Could we just keep using `Owned` here?

The 'deletion' will be addressed in the following patches (in 
`removeOfferOperation`). The reason for a raw pointer here is because Framework 
needs to maintain a hashmap to `OfferOperation` pointer too. I made it 
consistent with Task.


- Jie


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


On Nov. 1, 2017, 11:18 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63485/
> ---
> 
> (Updated Nov. 1, 2017, 11:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> Greg Mann, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added initial code for offer operation status update in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 76f884c61f8d4605725e93e720dc693b861918e3 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
> 
> 
> Diff: https://reviews.apache.org/r/63485/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62734: Added Markdown, bug, and reviewers support to `post-reviews.py`.

[Andrew Schwartzmeyer]

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

(Updated Nov. 6, 2017, 10:07 a.m.)


Review request for mesos, Joseph Wu, Michael Park, and Till Toenshoff.


Changes
---

Fixed boolean bug.


Repository: mesos


Description (updated)
---

These options are appropriately forwarded to `rbt`.

We enable Markdown parsing by default, with `--no-markdown` to disable
it; this is safe as most commit formatting is intended to be Markdown,
and unformatted text is valid markdown. This prevents `rbt` from
escaping e.g. ticks used in commit messages that are then manually
removed from reviews on Review Board.

The `--bugs-closed` flag lets users attach a bug automatically to every
review in the chain they're posting. Similarly `--target-people` will
attach the specified reviewers to every review. This reduces the amount
of work it takes to prepare chains of reviews.

We added help to the argument parser of the script, mostly stolen from
the descriptions in `rbt post --help`.


Diffs (updated)
-

  support/post-reviews.py c9378719f5ab38052bd87fab73ab9b5bd37e0154 


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

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


Testing
---

Used it to post this patch.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 62734: Added Markdown, bug, and reviewers support to `post-reviews.py`.

[Andrew Schwartzmeyer]

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

(Updated Nov. 6, 2017, 10:06 a.m.)


Review request for mesos, Joseph Wu, Michael Park, and Till Toenshoff.


Summary (updated)
-

Added Markdown, bug, and reviewers support to `post-reviews.py`.


Repository: mesos


Description (updated)
---

These options are appropriately forwarded to `rbt`.

We enable Markdown parsing by default, with `--no-markdown` to disable
it; this is safe as most commit formatting is intended to be Markdown,
and unformatted text is valid markdown. This prevents `rbt` from
escaping e.g. ticks used in commit messages that are then manually
removed from reviews on Review Board.

The `--bugs-closed` flag lets users attach a bug automatically to every
review in the chain they're posting. Similarly `--target-people` will
attach the specified reviewers to every review. This reduces the amount
of work it takes to prepare chains of reviews.

We added help to the argument parser of the script, mostly stolen from
the descriptions in `rbt post --help`.


Diffs (updated)
-

  support/post-reviews.py c9378719f5ab38052bd87fab73ab9b5bd37e0154 


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

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


Testing
---

Used it to post this patch.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Greg Mann]

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




src/slave/slave.cpp
Lines 1280-1287 (patched)


It would be nice if we only did this when necessary - i.e., if there have, 
in fact, been additional RP registrations. The agent could keep an optional 
list of RPs which register/update during that period; I think it would be 
pretty easy to insert them in the  handleResourceProviderMessage handler when 
the agent isn't registered. Here, and in reregistered().

Do you think it's worth it?



src/tests/slave_tests.cpp
Lines 8529-8531 (patched)


"of for the interaction with the usual oversubscription protocol" - I'm not 
sure what you're saying here, could you re-word?



src/tests/slave_tests.cpp
Lines 8536 (patched)


s/an a/and a/



src/tests/slave_tests.cpp
Lines 8543-8546 (patched)


Could you comment on the reason for setting `authenticate_http_readwrite = 
false` as well?


- Greg Mann


On Nov. 6, 2017, 12:38 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61183/
> ---
> 
> (Updated Nov. 6, 2017, 12:38 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent's resource provider manager sends a
> 'ResourceProviderMessage' when its managed resources change. This
> commit adds handling in the agent so that an 'UpdateSlaveMessage' is
> sent to the master to update the total resource available on the
> agent.
> 
> In order to provide push-like handling of the resource provider
> manager's message queue, we chain recursive calls to the handler for
> continuous processing. Initially, processing is kicked off from
> 'Slave::initialize'. In this simple implementation we e.g., provide no
> direct way to stop processing of messages, yet, but it can be achieved
> by e.g., replacing the manager with a new instance (this would also
> require updating routes).
> 
> Since the agent can only send an 'UpdateSlaveMessage' when it is
> registered with a master, a simple back-off of 5 s is implemented which
> will defer processing of a ready message should the agent not yet have
> registered.
> 
> To facilitate logging we add a stringification function for
> 'ResourceProviderMessage's.
> 
> This patch also adjusts a number of tests to no expect two
> 'UpdateSlaveMessage's.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/61183/diff/21/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63400: Fix bad links.

[Vinod Kone]

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



Looks like this needs a rebase.

- Vinod Kone


On Nov. 6, 2017, 2:23 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63400/
> ---
> 
> (Updated Nov. 6, 2017, 2:23 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Joerg Schad, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix bad links.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> 6e73e4e1909c315d566fa29b56a54ac47c711e79 
>   docs/cmake.md 1e1a27ef67d256eb892a5eb455928c2424bb3fed 
>   docs/cni.md 09159cd26162fe06f7be0519909720b16166ad1e 
>   docs/isolators/network-port-mapping.md 
> 522f2edb8ba2c1db28617d22237e6c20a2bf2aa8 
>   docs/upgrades.md 6370c06a497161317884f61f376308423b744fe1 
>   docs/working-groups.md f6af1e39375cab0d0ed61d8c1fa18e4e4c0c1d42 
>   site/source/blog/2014-03-28-mesos-community-update-1.md 
> 3b8114d6c2eb33200800999c410d1d6030fa5148 
>   site/source/layouts/community_section.erb 
> 07fd28853e119c1d9298a662cc6d73054b6e415f 
> 
> 
> Diff: https://reviews.apache.org/r/63400/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Review Request 63585: Improved support/mesos-style.py structure.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

The Python linter now also use the function `run_command_in_virtualenv`
to run. The methods of LinterBase have been ordered alphabetically.


Diffs
-

  support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 


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


Testing
---

Used the linters after applying this patch.


Thanks,

Armand Grillet

Re: Review Request 63584: Added agent ID to offer operation protobuf helper.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 6, 2017, 2:50 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63584/
> ---
> 
> (Updated Nov. 6, 2017, 2:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current code expects an agent ID to be set for every instance of
> 'OfferOperation'. The absence of an agent ID would indicate an operation
> meant for an external resource provider which isn't supported yet.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp da0a84eb828e2fb064ba466d9e525cd3a85fc43a 
>   src/common/protobuf_utils.cpp 4ce7021b01b9fb52ae1a8d95aa686c5b79164d67 
>   src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 
> 
> 
> Diff: https://reviews.apache.org/r/63584/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 63582: Removed pylint from the CLI requirements.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

Due to the new virtual environment located in /support, we do
not need to have pylint in the CLI virtual environment anymore.


Diffs
-

  src/python/cli_new/pip-requirements.txt 
7aeac344c47ccd2588fded44d7314db7abd85653 


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


Testing
---

Used the linters after applying this patch.


Thanks,

Armand Grillet

Re: Review Request 63581: Created virtual environment for linters in /support.

[Armand Grillet]

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

(Updated Nov. 6, 2017, 4:50 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

This change affects the Python linter but not the C++ linter as we
use a customized version of cpplint that we cannot get using pip.


Diffs (updated)
-

  src/python/pylint.config  
  support/build-virtualenv PRE-CREATION 
  support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
  support/pip-requirements.txt PRE-CREATION 


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

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


Testing
---

Used the linters after applying this patch.


Thanks,

Armand Grillet

Re: Review Request 63480: Added the initial implementation for applying offer operations.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/resource_provider/manager.cpp
Lines 165 (patched)


There is no need to use an `Owned` here; a `ResourceProvider` would work 
just fine.

The mutating `update*` functions should just take a `ResourceProvider*` in 
that case. They should do this even if we'd store an `Owned` here since they do 
not do anything with the passed provider's `Owned`'ness.



src/resource_provider/manager.cpp
Lines 348-349 (patched)


Does it make sense to just drop this message with some logging instead of 
failing over the agent?



src/resource_provider/manager.cpp
Line 337 (original), 413 (patched)


We should just take a `ResourceProvider*` here; these functions do not care 
about `resourceProvider`'s `Owned`'ness.



src/resource_provider/manager.cpp
Line 345 (original), 421 (patched)


We should just take a `ResourceProvider*` here; these functions do not care 
about `resourceProvider`'s `Owned`'ness.



src/resource_provider/manager.cpp
Lines 482 (patched)


Not sure if this function should be `const` as it maps a non-`const` 
process function.


- Benjamin Bannier


On Nov. 2, 2017, 12:14 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63480/
> ---
> 
> (Updated Nov. 2, 2017, 12:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> Greg Mann, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The resource provider manager provides an `applyOfferOperation` method
> for offer operation affecting resource providers. The resources on
> which the operation should be applied contains a resource provider ID.
> This will be extracted and an event will be sent to the respective
> resource provider.
> 
> (This is based on https://reviews.apache.org/r/61810)
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto 9dfe7f07c2e1171be17f1bd7b6a54a80410c44f6 
>   src/resource_provider/manager.hpp 3b70e75c6b6721864ae0ee9c4a593b5035d8388f 
>   src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e 
>   src/slave/slave.cpp 817dca4fc3e1028ccc13b8a1bb1ee783afb77166 
> 
> 
> Diff: https://reviews.apache.org/r/63480/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 63550: Included commit message in `push-commits.py`.

[Vinod Kone]

> On Nov. 4, 2017, 1:44 a.m., Michael Park wrote:
> > support/push-commits.py
> > Lines 48-55 (original), 51-62 (patched)
> > 
> >
> > Since we're shelling out to `git` multiple times anyway, I think it'd 
> > be simpler to just do:
> > ```
> > reviews = check_output(['git', 'rev-list', '--reverse', merge_base + 
> > ".." + current_branch_ref]).split('\n')
> > ```
> > and then do:
> > ```
> > for review in reviews:
> >   commit_msg = check_output(['git', '--no-pager', 'log', '--no-color', 
> > '-n 1', review])
> >   # ...
> > ```
> > Relying on `--skip` going too far which in turn makes `commit_log` 
> > become a empty string and thus checking for `if not commit_log` is more 
> > complex than we need it to be I think.

I used `git show --no-patch` instead of `git log -n 1` as discussed offline.


- Vinod


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


On Nov. 6, 2017, 4:09 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63550/
> ---
> 
> (Updated Nov. 6, 2017, 4:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script previously closed the reviews without any description.
> Now it includes the commit log for easy reference.
> 
> 
> Diffs
> -
> 
>   support/push-commits.py 3e9d05cb03443e5a46422da67e91d43a119a66c6 
> 
> 
> Diff: https://reviews.apache.org/r/63550/diff/3/
> 
> 
> Testing
> ---
> 
> Tested manually by running in dry run mode.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>