Re: Review Request 67960: Added support for instrumenting HTTP endpoints.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['67955', '67956', '67957', '67958', '67959', '67960']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1951/mesos-review-67960

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1951/mesos-review-67960/logs/libprocess-tests-stdout.log):

```
[ RUN  ] Scheme/HTTPTest.Headers/0
[   OK ] Scheme/HTTPTest.Headers/0 (3 ms)
[ RUN  ] Scheme/HTTPTest.CaseInsensitiveHeaders/0
[   OK ] Scheme/HTTPTest.CaseInsensitiveHeaders/0 (2 ms)
[ RUN  ] Scheme/HTTPTest.WWWAuthenticateHeader/0
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/0 (4 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/0
[   OK ] Scheme/HTTPTest.Accepts/0 (10 ms)
[--] 20 tests from Scheme/HTTPTest (249 ms total)

[--] 1 test from Scheme/InstrumentHttpEndpointsTest
[ RUN  ] Scheme/InstrumentHttpEndpointsTest.InstrumentHttpEndpoints/0
d:\dcos\mesos\mesos\3rdparty\libprocess\src\tests\http_tests.cpp(2670): error:  
 Expected: 1u
  Which is: 1
To be equal to: json->values.count(metric)
  Which is: 0
[  FAILED  ] Scheme/InstrumentHttpEndpointsTest.InstrumentHttpEndpoints/0, 
where GetParam() = "http" (54 ms)
[--] 1 test from Scheme/InstrumentHttpEndpointsTest (56 ms total)

[--] Global test environment tear-down
[==] 214 tests from 37 test cases ran. (3042 ms total)
[  PASSED  ] 212 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] InstrumentAuthenticatedHttpEndpointsTest.InstrumentHttpEndpoints
[  FAILED  ] Scheme/InstrumentHttpEndpointsTest.InstrumentHttpEndpoints/0, 
where GetParam() = "http"

 2 FAILED TESTS
  YOU HAVE 14 DISABLED TESTS

```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1951/mesos-review-67960/logs/libprocess-tests-stderr.log):

```
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0718 02:50:54.581737 16360 process.cpp:1005] Stopped the socket accept loop
I0718 02:50:55.077747 16132 process.cpp:1005] Stopped the socket accept loop
I0718 02:50:55.115748  5976 process.cpp:1005] Stopped the socket accept loop
I0718 02:50:57.093072  9276 process.cpp:1005] Stopped the socket accept loop
I0718 02:50:57.130070 16892 process.cpp:1005] Stopped the socket accept loop
I0718 02:50:57.150072  2900 process.cpp:1005] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On July 18, 2018, 1:28 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67960/
> ---
> 
> (Updated July 18, 2018, 1:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for instrumenting HTTP endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/process.hpp 
> 7c255acc21c695eba37062a3dcf72ce33f650cd0 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 23d7aee963b6fb489403a94500d39e3413c7fcdd 
>   3rdparty/libprocess/src/process.cpp 
> 7c0a0bc0c1e50354b6da219032ac830cbeec0a0d 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> acbd6973829411652fc5d57ef473c0d8ba9cd5b4 
> 
> 
> Diff: https://reviews.apache.org/r/67960/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 67952: Windows: Ported remaining tests in the `HTTPTest` suite.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['67951', '67952']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1950/mesos-review-67952

Relevant logs:

- 
[mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1950/mesos-review-67952/logs/mesos-tests-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500):
 warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define 
_WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170):
 warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 

Re: Review Request 67823: Added a master benchmark test for metrics.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 67809.

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

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1949/mesos-review-67823

Relevant logs:

- 
[apply-review-67809-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1949/mesos-review-67823/logs/apply-review-67809-stdout.log):

```
error: patch failed: src/master/master.hpp:380
error: src/master/master.hpp: patch does not apply
```

- Mesos Reviewbot Windows


On July 18, 2018, 1:47 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67823/
> ---
> 
> (Updated July 18, 2018, 1:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.
> 
> 
> Bugs: MESOS-8911
> https://issues.apache.org/jira/browse/MESOS-8911
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a master benchmark test for metrics.
> 
> 
> Diffs
> -
> 
>   src/tests/master_benchmarks.cpp fbfffb69930c30b038f74e0b831fc0ae41c820f0 
> 
> 
> Diff: https://reviews.apache.org/r/67823/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 67953: Disabled the clang `-Winconsistent-missing-override` warning.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67953 was successfully built and tested.

Reviews applied: `['67953']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1948/mesos-review-67953

- Mesos Reviewbot Windows


On July 18, 2018, 12:05 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67953/
> ---
> 
> (Updated July 18, 2018, 12:05 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and 
> Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clang can emit a `-Winconsistent-missing-override` warning for most
> uses of the Google Mock `MOCK_METHOD` family of macros. See also
> https://github.com/google/googletest/issues/533.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 49b2dc0deec939e4c61b39c91bc5be4765fee194 
>   configure.ac 5f64168ce8867b23f0aed20f30682ebd35711ceb 
> 
> 
> Diff: https://reviews.apache.org/r/67953/diff/1/
> 
> 
> Testing
> ---
> 
> make (Fedora 28)
> cmake (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67235: Added per-framework metrics for types of resources contained in offers.

[Greg Mann]

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

(Updated July 18, 2018, 1:46 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


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


Repository: mesos


Description
---

Added per-framework metrics for types of resources contained in offers.


Diffs
-

  src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
  src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67823: Added a master benchmark test for metrics.

[Greg Mann]

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

(Updated July 18, 2018, 1:47 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.


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


Repository: mesos


Description
---

Added a master benchmark test for metrics.


Diffs (updated)
-

  src/tests/master_benchmarks.cpp fbfffb69930c30b038f74e0b831fc0ae41c820f0 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67187: Tested per-framework task state metrics.

[Greg Mann]

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

(Updated July 18, 2018, 1:46 a.m.)


Review request for mesos, Gastón Kleiman and Gilbert Song.


Changes
---

Updated metric keys.


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


Repository: mesos


Description
---

This patch adds `MasterTest.TaskStateMetrics`, which verifies that
per-framework task state metrics for both terminal and active task
states report correct values, even after agent reregistration.


Diffs (updated)
-

  src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb 


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

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


Testing
---

The new test was run ~10,000 times with no failures.


Thanks,

Greg Mann

Re: Review Request 67878: Added/updated tests to check per-framework metrics.

[Greg Mann]

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

(Updated July 18, 2018, 1:46 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Gilbert Song.


Changes
---

Removed the filter metric test.


Repository: mesos


Description
---

Added/updated tests to check per-framework metrics.


Diffs (updated)
-

  src/tests/master_tests.cpp 8e04023ed04e79881e0d323c2e2283bebaf262eb 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66870: Added per-framework metrics for suppressed roles.

[Greg Mann]

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

(Updated July 18, 2018, 1:45 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

Added per-framework metrics for suppressed roles.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
  src/master/allocator/mesos/metrics.hpp 
6d386225c301d5ab44f3cc0ecdd1478fb5162e5b 
  src/master/allocator/mesos/metrics.cpp 
82990b2dc0b827a43a392d898667eaf58c77ea36 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 66856: Tracked completed framework metrics in the allocator.

[Greg Mann]

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

(Updated July 18, 2018, 1:45 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

This ensures that per-framework metrics which are tracked in the
allocator will be retained as long as the per-framework metrics
which are tracked in the master.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc 
  src/master/allocator/mesos/allocator.hpp 
900c8ee405da6e44532dee598edaa42373ebd4e5 
  src/master/allocator/mesos/hierarchical.hpp 
0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp 
5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
  src/master/master.cpp 487ee349ef0cd78903ff6bcdea3f24688810cdc5 
  src/tests/allocator.hpp 73fc06043746a0498d0dd1846fae9433db136d49 
  src/tests/api_tests.cpp 728fb4c38c17c98b6c2a85447d3f32519c035e56 
  src/tests/master_allocator_tests.cpp 824a7554858fb8356751f34699607505bd98 
  src/tests/master_quota_tests.cpp d836482f5593b462ad235620741148678ac2651d 
  src/tests/reservation_tests.cpp 058a66d7914c7a84f0ba86dfd3ff2e3c0bbcb5c6 
  src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8 
  src/tests/slave_recovery_tests.cpp e8333402d8524a4bf302872d246fe5f5c006bdc5 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 66861: Added per-framework DRF position metrics to the allocator.

[Greg Mann]

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

(Updated July 18, 2018, 1:44 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

During each allocation cycle, the allocator re-sorts roles and
frameworks for each agent in the cluster. This means that for each
agent there exists a total order of (role, framework) tuples.

This patch adds per-framework, per-role metrics which track the
minimum and maximum positions attained by the framework in this
sorting process, from the most recent allocation cycle.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
  src/master/allocator/mesos/metrics.hpp 
6d386225c301d5ab44f3cc0ecdd1478fb5162e5b 
  src/master/allocator/mesos/metrics.cpp 
82990b2dc0b827a43a392d898667eaf58c77ea36 


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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 66855: Changed the 'capacity_' member of 'BoundedHashMap' to non-const.

[Greg Mann]

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

(Updated July 18, 2018, 1:44 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

This prevents the assignment operator from being implicitly deleted.


Diffs (updated)
-

  3rdparty/stout/include/stout/boundedhashmap.hpp 
09a9b96f0b4a58c718777aa81a11c6ca4fdb6f8c 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 66843: Enabled per-framework metrics in the allocator.

[Greg Mann]

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

(Updated July 18, 2018, 1:43 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


Summary (updated)
-

Enabled per-framework metrics in the allocator.


Repository: mesos


Description (updated)
---

The `FrameworkMetrics` struct will hold per-framework metrics tracked
by the allocator.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp 
5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
  src/master/allocator/mesos/metrics.hpp 
6d386225c301d5ab44f3cc0ecdd1478fb5162e5b 
  src/master/allocator/mesos/metrics.cpp 
82990b2dc0b827a43a392d898667eaf58c77ea36 


Diff: https://reviews.apache.org/r/66843/diff/10/

Changes: https://reviews.apache.org/r/66843/diff/9-10/


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 67776: Added per-framework metrics for scheduler calls.

[Greg Mann]

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

(Updated July 18, 2018, 1:42 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Gilbert Song.


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


Repository: mesos


Description
---

Added per-framework metrics for scheduler calls.


Diffs (updated)
-

  src/master/http.cpp c855de1b7bb71cb455d111fcd71c4486f421f2c2 
  src/master/master.cpp 487ee349ef0cd78903ff6bcdea3f24688810cdc5 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 67147: Included a missing header in the master metrics.

[Greg Mann]

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

(Updated July 18, 2018, 1:41 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Gilbert Song.


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


Repository: mesos


Description
---

Included a missing header in the master metrics.


Diffs
-

  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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


Testing
---

The subsequent patch in this chain exposes this missing header, since it 
includes 'src/master/metrics.hpp' in the allocator metrics code, in a 
compilation unit in which the scheduler header is not included.


Thanks,

Greg Mann

Re: Review Request 66841: Added a hash function for 'Duration'.

[Greg Mann]

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

(Updated July 18, 2018, 1:36 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

Added a hash function for 'Duration'.


Diffs
-

  3rdparty/stout/include/stout/duration.hpp 
42c43cda21c75fc3bef962af67c4a09df68a95af 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67813: Added per-framework metrics for task states.

[Greg Mann]

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

(Updated July 18, 2018, 1:35 a.m.)


Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gastón Kleiman, 
Gilbert Song, and Vinod Kone.


Changes
---

Eliminated terminal task reason metrics, updated metric keys.


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


Repository: mesos


Description
---

Added per-framework metrics for task states.


Diffs (updated)
-

  src/master/master.hpp 2ce71dca52245b41533728a7564c65daa135b224 
  src/master/master.cpp 487ee349ef0cd78903ff6bcdea3f24688810cdc5 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 67812: Added per-framework offer metrics.

[Greg Mann]

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

(Updated July 18, 2018, 1:34 a.m.)


Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gastón Kleiman, 
Gilbert Song, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added per-framework offer metrics.


Diffs (updated)
-

  src/master/master.cpp 487ee349ef0cd78903ff6bcdea3f24688810cdc5 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 67809: Added per-framework metrics for scheduler events.

[Greg Mann]

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

(Updated July 18, 2018, 1:34 a.m.)


Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gastón Kleiman, 
Gilbert Song, and Vinod Kone.


Changes
---

Eliminated the per-task-state update event metrics.


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


Repository: mesos


Description
---

Added per-framework metrics for scheduler events.


Diffs (updated)
-

  src/master/master.hpp 2ce71dca52245b41533728a7564c65daa135b224 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66820: Added per-framework 'subscribed' metric and helpers.

[Gilbert Song]

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

(Updated July 18, 2018, 1:33 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Summary (updated)
-

Added per-framework 'subscribed' metric and helpers.


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


Repository: mesos


Description (updated)
---

Added per-framework 'subscribed' metric and helpers.


Diffs (updated)
-

  src/master/master.hpp 2ce71dca52245b41533728a7564c65daa135b224 
  src/master/master.cpp 487ee349ef0cd78903ff6bcdea3f24688810cdc5 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66820/diff/8/

Changes: https://reviews.apache.org/r/66820/diff/7-8/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 67776: Added per-framework metrics for scheduler calls.

[Greg Mann]

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

(Updated July 18, 2018, 1:34 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Gilbert Song.


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


Repository: mesos


Description
---

Added per-framework metrics for scheduler calls.


Diffs (updated)
-

  src/master/http.cpp c855de1b7bb71cb455d111fcd71c4486f421f2c2 
  src/master/master.cpp 487ee349ef0cd78903ff6bcdea3f24688810cdc5 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Review Request 67962: Enabled per-framework metrics in the master.

[Greg Mann]

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

Review request for mesos and Gastón Kleiman.


Repository: mesos


Description
---

The `FrameworkMetrics` struct will be used to track per-framework
metrics in the master.


Diffs
-

  src/master/master.hpp 2ce71dca52245b41533728a7564c65daa135b224 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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


Testing
---


Thanks,

Greg Mann

Review Request 67959: Added support for instrumenting processes.

[Benjamin Hindman]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Added support for instrumenting processes.


Diffs
-

  3rdparty/libprocess/include/process/process.hpp 
7c255acc21c695eba37062a3dcf72ce33f650cd0 
  3rdparty/libprocess/src/process.cpp 7c0a0bc0c1e50354b6da219032ac830cbeec0a0d 
  3rdparty/libprocess/src/tests/process_tests.cpp 
8baf60d8bbb675e26fea5e76c825ef73fedbc629 


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


Testing
---

make check


Thanks,

Benjamin Hindman

Review Request 67958: Added LibprocessTest for easily configuring the library for a test.

[Benjamin Hindman]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

There was not a generic way for configuring and then reinitializing
the libprocess library in a test and the LibprocessTest fixture
provides this functionality.


Diffs
-

  3rdparty/libprocess/include/process/gtest.hpp 
fd4de8ab7c79940519b2e890a9b0b615ca9ca292 


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


Testing
---

make check


Thanks,

Benjamin Hindman

Review Request 67960: Added support for instrumenting HTTP endpoints.

[Benjamin Hindman]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Added support for instrumenting HTTP endpoints.


Diffs
-

  3rdparty/libprocess/include/process/process.hpp 
7c255acc21c695eba37062a3dcf72ce33f650cd0 
  3rdparty/libprocess/include/process/ssl/gtest.hpp 
23d7aee963b6fb489403a94500d39e3413c7fcdd 
  3rdparty/libprocess/src/process.cpp 7c0a0bc0c1e50354b6da219032ac830cbeec0a0d 
  3rdparty/libprocess/src/tests/http_tests.cpp 
acbd6973829411652fc5d57ef473c0d8ba9cd5b4 


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


Testing
---

make check


Thanks,

Benjamin Hindman

Review Request 67957: Refactored TemporaryDirectoryTest to be a mixin.

[Benjamin Hindman]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Currently you can't easily "mixin" the functionality of the
TemporaryDirectoryTest which makes for tests that might have a weird
inheritance structure. This refactor makes it easy for you to get the
functionality of the TemporaryDirectoryTest while easily composing it
with other test classes.


Diffs
-

  3rdparty/stout/include/stout/tests/utils.hpp 
eb13cceec29c5db8a75067aac5a00b8f8e9b5046 


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


Testing
---

make check


Thanks,

Benjamin Hindman

Review Request 67956: Removed some generic flag parsers that are now in stout.

[Benjamin Hindman]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Removed some generic flag parsers that are now in stout.


Diffs
-

  src/common/parse.hpp 03814e3112a043a1001764b316b9d49501d33665 


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


Testing
---

make check


Thanks,

Benjamin Hindman

Review Request 67955: Added some new generic flag parsers.

[Benjamin Hindman]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Added some new generic flag parsers.


Diffs
-

  3rdparty/stout/include/stout/flags/parse.hpp 
4566b798b8b66d47779a28cabdea06f588012686 


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


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 67875: Added `normalizeMetricKey()` and `getFrameworkMetricPrefix()`.

[Greg Mann]

> On July 16, 2018, 8:28 p.m., Gastón Kleiman wrote:
> > src/master/metrics.cpp
> > Lines 551-554 (patched)
> > 
> >
> > Per offline discussion we can inline this method in 
> > `getFrameworkMetricPrefix()`.

I've rolled this patch into the subsequent one.


- Greg


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


On July 10, 2018, 3:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67875/
> ---
> 
> (Updated July 10, 2018, 3:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `normalizeMetricKey()` and `getFrameworkMetricPrefix()`.
> 
> 
> Diffs
> -
> 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/67875/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 67952: Windows: Ported remaining tests in the `HTTPTest` suite.

[Andrew Schwartzmeyer]

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

(Updated July 17, 2018, 5:52 p.m.)


Review request for mesos, Eric Mumau, John Kordich, Joseph Wu, Liangyu Zhao, 
and Radhika Jandhyala.


Changes
---

Manually add dependency because apparently the script didn't.


Bugs: MESOS-5904 and MESOS-8564
https://issues.apache.org/jira/browse/MESOS-5904
https://issues.apache.org/jira/browse/MESOS-8564


Repository: mesos


Description
---

These were enabled by fixing the use of `Path()` in `process.cpp` to
explicitly use '/' as the separator, as it's being used to manipulate
URLs, not filesystem paths, and previously this failed due the Windows
path separator being '\'.


Diffs
-

  3rdparty/libprocess/src/process.cpp 7c0a0bc0c1e50354b6da219032ac830cbeec0a0d 
  3rdparty/libprocess/src/tests/http_tests.cpp 
acbd6973829411652fc5d57ef473c0d8ba9cd5b4 


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


Testing
---

`stout-tests` and `libprocess-tests` passed on Windows and Ubuntu


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67952: Windows: Ported remaining tests in the `HTTPTest` suite.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['67952']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1947/mesos-review-67952

Relevant logs:

- 
[libprocess-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1947/mesos-review-67952/logs/libprocess-tests-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2292):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2301):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2401):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 cl : Command line warning D9002: ignoring unknown option '-fPIC' 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1415):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1520):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1689):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2273):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2280):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2292):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2301):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2401):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]


   "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
(default target) (1) ->
   "D:\DCOS\mesos\3rdparty\libprocess\examples\example.vcxproj" (default 
target) (9) ->
   "D:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj" (default target) 
(16) ->
   (ClCompile target) -> 
 d:\dcos\mesos\mesos\3rdparty\libprocess\src\process.cpp(3606): error 
C2440: '': cannot convert from 'initializer list' to 
'Path' 

Re: Review Request 67951: Added optional `path_separator` parameter to `Path` constructor.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['67951']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1946/mesos-review-67951

Relevant logs:

- 
[mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1946/mesos-review-67951/logs/mesos-tests-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500):
 warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define 
_WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170):
 warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 

Re: Review Request 67953: Disabled the clang `-Winconsistent-missing-override` warning.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 17, 2018, 5:05 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67953/
> ---
> 
> (Updated July 17, 2018, 5:05 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and 
> Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clang can emit a `-Winconsistent-missing-override` warning for most
> uses of the Google Mock `MOCK_METHOD` family of macros. See also
> https://github.com/google/googletest/issues/533.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 49b2dc0deec939e4c61b39c91bc5be4765fee194 
>   configure.ac 5f64168ce8867b23f0aed20f30682ebd35711ceb 
> 
> 
> Diff: https://reviews.apache.org/r/67953/diff/1/
> 
> 
> Testing
> ---
> 
> make (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 67953: Disabled the clang `-Winconsistent-missing-override` warning.

[James Peach]

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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Benjamin 
Mahler.


Repository: mesos


Description
---

Clang can emit a `-Winconsistent-missing-override` warning for most
uses of the Google Mock `MOCK_METHOD` family of macros. See also
https://github.com/google/googletest/issues/533.


Diffs
-

  cmake/CompilationConfigure.cmake 49b2dc0deec939e4c61b39c91bc5be4765fee194 
  configure.ac 5f64168ce8867b23f0aed20f30682ebd35711ceb 


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


Testing
---

make (Fedora 28)


Thanks,

James Peach

Re: Review Request 67950: Disabled override warnings for mocked methods.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67950 was successfully built and tested.

Reviews applied: `['67950']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1945/mesos-review-67950

- Mesos Reviewbot Windows


On July 17, 2018, 2:54 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67950/
> ---
> 
> (Updated July 17, 2018, 2:54 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clang can emit a `-Winconsistent-missing-override` warning for most
> uses of the Google Mock `MOCK_METHOD` family of macros. See also
> https://github.com/google/googletest/issues/533.
> 
> 
> Diffs
> -
> 
>   src/tests/allocator.hpp 6139492bf4d18f31e558bd0216bcccd2705af4d7 
>   src/tests/containerizer.hpp e05ce0323e032911d46d8682661ffe4463fdc276 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> c15a6fad642474765e4ad1952af6cd9ee937379e 
>   src/tests/containerizer/isolator.hpp 
> 5861b8b8abbba65c499aa57fbf489d0c74414a5b 
>   src/tests/containerizer/launcher.hpp 
> 6567e9663b3b8d143f4bc38421deee7876c19e37 
>   src/tests/containerizer/mock_containerizer.hpp 
> 8700257a9347199ed6e32b8b6b2a58787d68af03 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bf56d603fcc1d99f73beca1871be45787fa1640d 
>   src/tests/disk_profile_server.hpp 1a8d2913eaf91b3b839471fe2a600e4bc59ecaed 
>   src/tests/fetcher_tests.cpp f3ea7092635c88b9dddcc2998a4c7350fb56110c 
>   src/tests/http_authentication_tests.cpp 
> 0c2537408aff8b018d9af39f92bbab3ebad938be 
>   src/tests/log_tests.cpp a8980e3676f51d14087f56338ff45de2927ea992 
>   src/tests/mesos.hpp 8142f3f3ab8e4e9230efdcd2e6846d0e0bd293b2 
>   src/tests/mock_csi_plugin.hpp 4642326b65ba7145bc57ac4a56d492697a7501b9 
>   src/tests/mock_docker.hpp 4a2266fb1239cbc96c7df74b997212e3b3b01c75 
>   src/tests/mock_fetcher.hpp fedb737ce42a1c68aa117e3db624c860d8afa7e4 
>   src/tests/mock_registrar.hpp 2a1e6dce433a4c53ec6639382aacaf0d9dc3 
>   src/tests/mock_slave.hpp 9a74bf35d2cab0a72ba6376392239d8080a49304 
>   src/tests/operation_status_update_manager_tests.cpp 
> e6175700caefb47a95b1edc4ae41755f06781fb4 
>   src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8 
>   src/tests/uri_fetcher_tests.cpp 260ae9c96f78ee5528cbd61983e1d61bda48c23b 
> 
> 
> Diff: https://reviews.apache.org/r/67950/diff/1/
> 
> 
> Testing
> ---
> 
> make check CXX=clang++ (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67932: Use `wclayer` from `hcsshim` to extract file layers.

[Mesos Reviewbot Windows]

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



Bad review!

Error:
Circular dependency detected for review 67931. Please fix the 'depends_on' 
field.

- Mesos Reviewbot Windows


On July 17, 2018, 4:15 p.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67932/
> ---
> 
> (Updated July 17, 2018, 4:15 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `tar` command cannot work successfully on Windows, so use `wclayer`
> instead. Note that the folder generated from extraction also cannot be
> deleted by `rmdir`, so the GC is also changed to use `wclayer remove`.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.hpp 525f9c1c37b74f7e4cc71fdc8d52944226998ddc 
>   src/common/command_utils.cpp 7dfcc9ff74bcf044d47b803ebc42cf63fba89d17 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 85aad25ac8ecfda125be85fb46d882c3982f3930 
> 
> 
> Diff: https://reviews.apache.org/r/67932/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 67951: Added optional `path_separator` parameter to `Path` constructor.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On July 17, 2018, 10:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67951/
> ---
> 
> (Updated July 17, 2018, 10:27 p.m.)
> 
> 
> Review request for mesos, Eric Mumau, John Kordich, Joseph Wu, Liangyu Zhao, 
> and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5904 and MESOS-8564
> https://issues.apache.org/jira/browse/MESOS-5904
> https://issues.apache.org/jira/browse/MESOS-8564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This defaults to `os::PATH_SEPARATOR` and so by default retains the
> previous behavior. However, now `Path` can be arbitrarily used with,
> e.g., URLs on Windows by providing `/` as the separator.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/path.hpp 
> 27438d31617b3b78bf3d4deffd25c93340610e8d 
> 
> 
> Diff: https://reviews.apache.org/r/67951/diff/1/
> 
> 
> Testing
> ---
> 
> `stout-tests` and `libprocess-tests` passed on Windows and Ubuntu
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67932: Use `wclayer` from `hcsshim` to extract file layers.

[Liangyu Zhao via Review Board]

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

(Updated July 17, 2018, 11:15 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.


Repository: mesos


Description
---

The `tar` command cannot work successfully on Windows, so use `wclayer`
instead. Note that the folder generated from extraction also cannot be
deleted by `rmdir`, so the GC is also changed to use `wclayer remove`.


Diffs (updated)
-

  src/common/command_utils.hpp 525f9c1c37b74f7e4cc71fdc8d52944226998ddc 
  src/common/command_utils.cpp 7dfcc9ff74bcf044d47b803ebc42cf63fba89d17 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
85aad25ac8ecfda125be85fb46d882c3982f3930 


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

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


Testing
---


Thanks,

Liangyu Zhao

Re: Review Request 67930: Get tests ready for Windows UCR development.

[Liangyu Zhao via Review Board]

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

(Updated July 17, 2018, 11:15 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.


Repository: mesos


Description
---

Enabled Internet test environment on Windows. Disabled Internet
HealthCheckTests on Windows, since they require complete development.
Modified DockerFetcherPluginTest to fetch `microsoft/nanoserver` for
more extensive test for fetcher on Windows.


Diffs (updated)
-

  src/tests/environment.cpp 3b84c0a413193badbbdb6d3ee71d74f3ab85c90b 
  src/tests/health_check_tests.cpp 34102e2e3ff90c194bea83ae8a702181121d6522 
  src/tests/uri_fetcher_tests.cpp 816d8ec2503ea79d069c062dfa2f01224b263bf6 


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

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


Testing
---


Thanks,

Liangyu Zhao

Re: Review Request 67931: Support Image Manifest Version 2 Schema 2.

[Liangyu Zhao via Review Board]

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

(Updated July 17, 2018, 11:14 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.


Repository: mesos


Description (updated)
---

Support parsing schema 2 and fetching blob with multiple URLs as
specified in schema 2. Update `curl` version to 7.61.0 because of bug
encountered in version 7.57.0.


Diffs (updated)
-

  3rdparty/CMakeLists.txt b58996d2ed7521e42d6298d174cc8c494b84eb8f 
  3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
  include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
  include/mesos/docker/v2_2.hpp PRE-CREATION 
  include/mesos/docker/v2_2.proto PRE-CREATION 
  include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
  src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
  src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
  src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
  src/slave/containerizer/mesos/containerizer.cpp 
98129d006cda9b65804b518619b6addc8990410a 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
  src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
  src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
  src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
  src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 


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

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


Testing
---


Thanks,

Liangyu Zhao

Re: Review Request 67952: Windows: Ported remaining tests in the `HTTPTest` suite.

[John Kordich via Review Board]

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


Ship it!




Ship It!

- John Kordich


On July 17, 2018, 10:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67952/
> ---
> 
> (Updated July 17, 2018, 10:27 p.m.)
> 
> 
> Review request for mesos, Eric Mumau, John Kordich, Joseph Wu, Liangyu Zhao, 
> and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5904 and MESOS-8564
> https://issues.apache.org/jira/browse/MESOS-5904
> https://issues.apache.org/jira/browse/MESOS-8564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These were enabled by fixing the use of `Path()` in `process.cpp` to
> explicitly use '/' as the separator, as it's being used to manipulate
> URLs, not filesystem paths, and previously this failed due the Windows
> path separator being '\'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 7c0a0bc0c1e50354b6da219032ac830cbeec0a0d 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> acbd6973829411652fc5d57ef473c0d8ba9cd5b4 
> 
> 
> Diff: https://reviews.apache.org/r/67952/diff/1/
> 
> 
> Testing
> ---
> 
> `stout-tests` and `libprocess-tests` passed on Windows and Ubuntu
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67951: Added optional `path_separator` parameter to `Path` constructor.

[John Kordich via Review Board]

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


Ship it!




Ship It!

- John Kordich


On July 17, 2018, 10:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67951/
> ---
> 
> (Updated July 17, 2018, 10:27 p.m.)
> 
> 
> Review request for mesos, Eric Mumau, John Kordich, Joseph Wu, Liangyu Zhao, 
> and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5904 and MESOS-8564
> https://issues.apache.org/jira/browse/MESOS-5904
> https://issues.apache.org/jira/browse/MESOS-8564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This defaults to `os::PATH_SEPARATOR` and so by default retains the
> previous behavior. However, now `Path` can be arbitrarily used with,
> e.g., URLs on Windows by providing `/` as the separator.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/path.hpp 
> 27438d31617b3b78bf3d4deffd25c93340610e8d 
> 
> 
> Diff: https://reviews.apache.org/r/67951/diff/1/
> 
> 
> Testing
> ---
> 
> `stout-tests` and `libprocess-tests` passed on Windows and Ubuntu
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 67951: Added optional `path_separator` parameter to `Path` constructor.

[Andrew Schwartzmeyer]

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

Review request for mesos, Eric Mumau, John Kordich, Joseph Wu, Liangyu Zhao, 
and Radhika Jandhyala.


Bugs: MESOS-5904 and MESOS-8564
https://issues.apache.org/jira/browse/MESOS-5904
https://issues.apache.org/jira/browse/MESOS-8564


Repository: mesos


Description
---

This defaults to `os::PATH_SEPARATOR` and so by default retains the
previous behavior. However, now `Path` can be arbitrarily used with,
e.g., URLs on Windows by providing `/` as the separator.


Diffs
-

  3rdparty/stout/include/stout/path.hpp 
27438d31617b3b78bf3d4deffd25c93340610e8d 


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


Testing
---

`stout-tests` and `libprocess-tests` passed on Windows and Ubuntu


Thanks,

Andrew Schwartzmeyer

Review Request 67952: Windows: Ported remaining tests in the `HTTPTest` suite.

[Andrew Schwartzmeyer]

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

Review request for mesos, Eric Mumau, John Kordich, Joseph Wu, Liangyu Zhao, 
and Radhika Jandhyala.


Bugs: MESOS-5904 and MESOS-8564
https://issues.apache.org/jira/browse/MESOS-5904
https://issues.apache.org/jira/browse/MESOS-8564


Repository: mesos


Description
---

These were enabled by fixing the use of `Path()` in `process.cpp` to
explicitly use '/' as the separator, as it's being used to manipulate
URLs, not filesystem paths, and previously this failed due the Windows
path separator being '\'.


Diffs
-

  3rdparty/libprocess/src/process.cpp 7c0a0bc0c1e50354b6da219032ac830cbeec0a0d 
  3rdparty/libprocess/src/tests/http_tests.cpp 
acbd6973829411652fc5d57ef473c0d8ba9cd5b4 


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


Testing
---

`stout-tests` and `libprocess-tests` passed on Windows and Ubuntu


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1944/mesos-review-67827

Relevant logs:

- 
[apply-review-67827-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1944/mesos-review-67827/logs/apply-review-67827-stdout.log):

```
error: patch failed: src/master/allocator/mesos/hierarchical.hpp:675
error: src/master/allocator/mesos/hierarchical.hpp: patch does not apply
```

- Mesos Reviewbot Windows


On July 17, 2018, 9:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> ---
> 
> (Updated July 17, 2018, 9:32 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
> https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67936: Fixed ephemeral ports deallocation in network/port_mapping isolator.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67936 was successfully built and tested.

Reviews applied: `['67936']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1942/mesos-review-67936

- Mesos Reviewbot Windows


On July 17, 2018, 8:12 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> ---
> 
> (Updated July 17, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> a29282ed0488d83966f222084031ed1d2b6ab1f5 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 553f53043a52cfe235fc2b2e88fb54d0f2725d3e 
> 
> 
> Diff: https://reviews.apache.org/r/67936/diff/2/
> 
> 
> Testing
> ---
> 
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that 
> ephemeral ports are properly deallocated when the container was not isolated 
> and manually checked that if fails without the fix. Ran `sudo make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 67947: Passed the default options when making gRPC calls.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67947 was successfully built and tested.

Reviews applied: `['67938', '67947']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1943/mesos-review-67947

- Mesos Reviewbot Windows


On July 17, 2018, 8:21 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67947/
> ---
> 
> (Updated July 17, 2018, 8:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9055
> https://issues.apache.org/jira/browse/MESOS-9055
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passed the default options when making gRPC calls.
> 
> 
> Diffs
> -
> 
>   src/csi/client.cpp a36e6228f39405f729e347db51ccd95c5fb76cdb 
> 
> 
> Diff: https://reviews.apache.org/r/67947/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 67950: Disabled override warnings for mocked methods.

[James Peach]

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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

Clang can emit a `-Winconsistent-missing-override` warning for most
uses of the Google Mock `MOCK_METHOD` family of macros. See also
https://github.com/google/googletest/issues/533.


Diffs
-

  src/tests/allocator.hpp 6139492bf4d18f31e558bd0216bcccd2705af4d7 
  src/tests/containerizer.hpp e05ce0323e032911d46d8682661ffe4463fdc276 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
c15a6fad642474765e4ad1952af6cd9ee937379e 
  src/tests/containerizer/isolator.hpp 5861b8b8abbba65c499aa57fbf489d0c74414a5b 
  src/tests/containerizer/launcher.hpp 6567e9663b3b8d143f4bc38421deee7876c19e37 
  src/tests/containerizer/mock_containerizer.hpp 
8700257a9347199ed6e32b8b6b2a58787d68af03 
  src/tests/containerizer/provisioner_docker_tests.cpp 
bf56d603fcc1d99f73beca1871be45787fa1640d 
  src/tests/disk_profile_server.hpp 1a8d2913eaf91b3b839471fe2a600e4bc59ecaed 
  src/tests/fetcher_tests.cpp f3ea7092635c88b9dddcc2998a4c7350fb56110c 
  src/tests/http_authentication_tests.cpp 
0c2537408aff8b018d9af39f92bbab3ebad938be 
  src/tests/log_tests.cpp a8980e3676f51d14087f56338ff45de2927ea992 
  src/tests/mesos.hpp 8142f3f3ab8e4e9230efdcd2e6846d0e0bd293b2 
  src/tests/mock_csi_plugin.hpp 4642326b65ba7145bc57ac4a56d492697a7501b9 
  src/tests/mock_docker.hpp 4a2266fb1239cbc96c7df74b997212e3b3b01c75 
  src/tests/mock_fetcher.hpp fedb737ce42a1c68aa117e3db624c860d8afa7e4 
  src/tests/mock_registrar.hpp 2a1e6dce433a4c53ec6639382aacaf0d9dc3 
  src/tests/mock_slave.hpp 9a74bf35d2cab0a72ba6376392239d8080a49304 
  src/tests/operation_status_update_manager_tests.cpp 
e6175700caefb47a95b1edc4ae41755f06781fb4 
  src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8 
  src/tests/uri_fetcher_tests.cpp 260ae9c96f78ee5528cbd61983e1d61bda48c23b 


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


Testing
---

make check CXX=clang++ (Fedora 28)


Thanks,

James Peach

Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

[Meng Zhu]

> On July 5, 2018, 3:16 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 687 (patched)
> > 
> >
> > s/ based on the ..././
> > 
> > Or this might be clearer?
> > 
> > // Returns the subset of resources that the framework
> > // is capable of being offered.

I want to emphasize this function only considers framework capability, things 
like offer filters and etc. are not considered. This is why I think without the 
"based on the given framework capability" there could be doubt.


> On July 5, 2018, 3:16 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2660-2685 (patched)
> > 
> >
> > Should we re-write this now to just consistently use filter()?
> > 
> > ```
> > Resources HierarchicalAllocatorProcess::stripIncapableResources(
> > const Resources& resources, // Can take a const ref now?
> > const protobuf::framework::Capabilities& frameworkCapabilities) 
> > const
> > {
> >   return resources.filter([&](const Resource& resource) {
> > if (!frameworkCapabilities.sharedResources &&
> > Resources::isShared(resource)) {
> >   return false;
> > }
> > 
> > if (!frameworkCapabilities.revocableResources &&
> > Resources::isRevocable(resource)) {
> >   return false;
> > }
> > 
> > // When reservation refinements are present, old frameworks without 
> > the
> > // RESERVATION_REFINEMENT capability won't be able to understand the
> > // new format. While it's possible to translate the refined 
> > reservations
> > // into the old format by "hiding" the intermediate reservations in 
> > the
> > // "stack", this leads to ambiguity when processing RESERVE / 
> > UNRESERVE
> > // operations. This is due to the loss of information when we drop 
> > the
> > // intermediate reservations. Therefore, for now we simply filter 
> > out
> > // resources with refined reservations if the framework does not 
> > have
> > // the capability.
> > if (!frameworkCapabilities.reservationRefinement &&
> > Resources::hasRefinedReservations(resource)) {
> >   return false;
> > }
> > 
> > return true;
> >   });
> > }
> > ```
> > 
> > I think this approach avoids adding extra copyies as we add more cases?

Good point.


- Meng


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


On July 17, 2018, 2:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> ---
> 
> (Updated July 17, 2018, 2:32 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
> https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

[Meng Zhu]

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

(Updated July 17, 2018, 2:32 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

This helper removes any resources that the framework is not
capable of receiving based on the given framework capability.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp 
1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67945: Made getters in the Slave class in the allocator return references.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67945 was successfully built and tested.

Reviews applied: `['67945']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1941/mesos-review-67945

- Mesos Reviewbot Windows


On July 17, 2018, 7:18 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67945/
> ---
> 
> (Updated July 17, 2018, 7:18 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This eliminates some unnecessary copies.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
> 
> 
> Diff: https://reviews.apache.org/r/67945/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67444: Persisted role consumed quota info in the allocator.

[Meng Zhu]

> On July 5, 2018, 2:22 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2586-2588 (patched)
> > 
> >
> > It's pretty hard to reason from here about whether this is correct. For 
> > example, how do we know that these quantities were not already tracked 
> > because they were allocated prior to becoming reserved? If that invariant 
> > doesn't hold we'll double count?

Thanks for catching this!

The invariant should be:
(1) when tracking reservation, only track unallocated reservations
(2) when track allocation, only track unreserved allocations

(2) is already being done. (1) is hard to do given the current abstraction, we 
will need more than raw "reservations". Will need more refactoring on the Slave 
struct in the allocator to make this work and readable. Will come back to this 
later.

In addition, this reminds me that we do not have tests for the above scenario 
yet. Will add some.


- Meng


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


On June 28, 2018, 1:55 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67444/
> ---
> 
> (Updated June 28, 2018, 1:55 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8802
> https://issues.apache.org/jira/browse/MESOS-8802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The allocator needs to keep track of role consumed quota
> to maintain quota headroom. Currently this info is
> constructed on the fly prior to each allocation cycle.
> 
> This patch lets the allocator buildup and persist this info
> across allocation cycles to improve performance and reduce
> code complexity.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 
> cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 
> 
> 
> Diff: https://reviews.apache.org/r/67444/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67871: Optimized the generation of metrics snapshots.

[Greg Mann]

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

(Updated July 17, 2018, 8:36 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.


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


Repository: mesos


Description
---

Profiling of metrics generation revealed a large amount of time spent
in map operations. This patch does three things to mitigate this:

 * Stores the metrics as an ordered map so that we only pay the price
   of sorting when the metric is first added.
 * Makes use of vectors instead of maps for intermediate objects,
   which eliminates the need for another intermediate object.
 * Hints when inserting into the returned map, reducing the cost of
   insertion into that ordered container.


Diffs
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
f9b72029b2c85826c91b1d7656b0af94dc87010c 
  3rdparty/libprocess/src/metrics/metrics.cpp 
4883c9acaa0cc568e27944661a8208f7b2a356a1 


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


Testing
---

WITH per-framework metrics, BEFORE optimizations:
```
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
Test setup: 1 agents with a total of 105 frameworks
unversioned /metrics/snapshot' response took 157.1449ms
v1 'master::call::GetMetrics' application/x-protobuf response took 152.599692ms
v1 'master::call::GetMetrics' application/json response took 198.918334ms
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
 (835 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
Test setup: 1 agents with a total of 1005 frameworks
unversioned /metrics/snapshot' response took 1.319444199secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
1.257644596secs
v1 'master::call::GetMetrics' application/json response took 1.527225235secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
 (6553 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
Test setup: 1 agents with a total of 10005 frameworks
unversioned /metrics/snapshot' response took 15.479365874secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
14.542866983secs
v1 'master::call::GetMetrics' application/json response took 18.05492789secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
 (75455 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
Test setup: 1 agents with a total of 20005 frameworks
unversioned /metrics/snapshot' response took 31.908301664secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
32.128689785secs
v1 'master::call::GetMetrics' application/json response took 33.669376185secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
 (150440 ms)
```

WITH per-framework metrics, AFTER optimizations:
```
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
Test setup: 1 agents with a total of 105 frameworks
unversioned /metrics/snapshot' response took 104.577895ms
v1 'master::call::GetMetrics' application/x-protobuf response took 74.262533ms
v1 'master::call::GetMetrics' application/json response took 100.218618ms
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
 (562 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
Test setup: 1 agents with a total of 1005 frameworks
unversioned /metrics/snapshot' response took 921.175877ms
v1 'master::call::GetMetrics' application/x-protobuf response took 780.277639ms
v1 'master::call::GetMetrics' application/json response took 1.16865secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
 (5424 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
Test setup: 1 agents with a total of 10005 frameworks
unversioned /metrics/snapshot' response took 10.2413387secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
9.407992945secs
v1 'master::call::GetMetrics' application/json response took 10.582584848secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
 (57206 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
Test setup: 1 agents with a total of 20005 frameworks
unversioned /metrics/snapshot' response took 19.930542079secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
20.318739763secs
v1 'master::call::GetMetrics' application/json 

Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

[Meng Zhu]

> On July 5, 2018, 2:33 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 666 (patched)
> > 
> >
> > Can you just take the Framework struct here? Feels a little odd that we 
> > take framework capabilities but not agent capabilities (I realize we can't 
> > take agent capabilities only, but seems like the caller doesn't need to 
> > know which bits of framework and slave structs we need?)

I want to make it clear that this filter function only considers the framework 
capability, not other things like suppressed roles, offer filters and etc. What 
do you think?


- Meng


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


On July 17, 2018, 1:35 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6/
> ---
> 
> (Updated July 17, 2018, 1:35 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
> https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `isCapableOfReceivingAgent*(` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 
> 
> 
> Diff: https://reviews.apache.org/r/6/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67947: Passed the default options when making gRPC calls.

[Benjamin Bannier]

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


Ship it!




Nice!

Note that if we always wanted compilable commits, we could commit r/67938 with 
a default argument, then add explicit arguments for all users in this patch, 
and finally remove the default argument in a cleanup. That sounds like the 
overkill it is :D

- Benjamin Bannier


On July 17, 2018, 10:21 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67947/
> ---
> 
> (Updated July 17, 2018, 10:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9055
> https://issues.apache.org/jira/browse/MESOS-9055
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passed the default options when making gRPC calls.
> 
> 
> Diffs
> -
> 
>   src/csi/client.cpp a36e6228f39405f729e347db51ccd95c5fb76cdb 
> 
> 
> Diff: https://reviews.apache.org/r/67947/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

[Meng Zhu]

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

(Updated July 17, 2018, 1:35 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

`isCapableOfReceivingAgent*(` checks if a framework
is capable of receiving resources on the agent based on
the framework capability.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp 
1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67871: Optimized the generation of metrics snapshots.

[Greg Mann]

> On July 16, 2018, 7:46 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp
> > Lines 174 (patched)
> > 
> >
> > How about s/result/waited/?

SGTM


- Greg


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


On July 17, 2018, 8:33 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67871/
> ---
> 
> (Updated July 17, 2018, 8:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Profiling of metrics generation revealed a large amount of time spent
> in map operations. This patch does three things to mitigate this:
> 
>  * Stores the metrics as an ordered map so that we only pay the price
>of sorting when the metric is first added.
>  * Makes use of vectors instead of maps for intermediate objects,
>which eliminates the need for another intermediate object.
>  * Hints when inserting into the returned map, reducing the cost of
>insertion into that ordered container.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> f9b72029b2c85826c91b1d7656b0af94dc87010c 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 4883c9acaa0cc568e27944661a8208f7b2a356a1 
> 
> 
> Diff: https://reviews.apache.org/r/67871/diff/5/
> 
> 
> Testing
> ---
> 
> WITH per-framework metrics, BEFORE optimizations:
> ```
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
> Test setup: 1 agents with a total of 105 frameworks
> unversioned /metrics/snapshot' response took 157.1449ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 152.599692ms
> v1 'master::call::GetMetrics' application/json response took 198.918334ms
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
>  (835 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
> Test setup: 1 agents with a total of 1005 frameworks
> unversioned /metrics/snapshot' response took 1.319444199secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 1.257644596secs
> v1 'master::call::GetMetrics' application/json response took 1.527225235secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
>  (6553 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
> Test setup: 1 agents with a total of 10005 frameworks
> unversioned /metrics/snapshot' response took 15.479365874secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 14.542866983secs
> v1 'master::call::GetMetrics' application/json response took 18.05492789secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
>  (75455 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
> Test setup: 1 agents with a total of 20005 frameworks
> unversioned /metrics/snapshot' response took 31.908301664secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 32.128689785secs
> v1 'master::call::GetMetrics' application/json response took 33.669376185secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
>  (150440 ms)
> ```
> 
> WITH per-framework metrics, AFTER optimizations:
> ```
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
> Test setup: 1 agents with a total of 105 frameworks
> unversioned /metrics/snapshot' response took 104.577895ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 74.262533ms
> v1 'master::call::GetMetrics' application/json response took 100.218618ms
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
>  (562 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
> Test setup: 1 agents with a total of 1005 frameworks
> unversioned /metrics/snapshot' response took 921.175877ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 780.277639ms
> v1 'master::call::GetMetrics' application/json response took 1.16865secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
>  (5424 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
> Test setup: 1 agents with a total 

Re: Review Request 67871: Optimized the generation of metrics snapshots.

[Greg Mann]

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

(Updated July 17, 2018, 8:33 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.


Repository: mesos


Description
---

Profiling of metrics generation revealed a large amount of time spent
in map operations. This patch does three things to mitigate this:

 * Stores the metrics as an ordered map so that we only pay the price
   of sorting when the metric is first added.
 * Makes use of vectors instead of maps for intermediate objects,
   which eliminates the need for another intermediate object.
 * Hints when inserting into the returned map, reducing the cost of
   insertion into that ordered container.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
f9b72029b2c85826c91b1d7656b0af94dc87010c 
  3rdparty/libprocess/src/metrics/metrics.cpp 
4883c9acaa0cc568e27944661a8208f7b2a356a1 


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

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


Testing
---

WITH per-framework metrics, BEFORE optimizations:
```
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
Test setup: 1 agents with a total of 105 frameworks
unversioned /metrics/snapshot' response took 157.1449ms
v1 'master::call::GetMetrics' application/x-protobuf response took 152.599692ms
v1 'master::call::GetMetrics' application/json response took 198.918334ms
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
 (835 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
Test setup: 1 agents with a total of 1005 frameworks
unversioned /metrics/snapshot' response took 1.319444199secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
1.257644596secs
v1 'master::call::GetMetrics' application/json response took 1.527225235secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
 (6553 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
Test setup: 1 agents with a total of 10005 frameworks
unversioned /metrics/snapshot' response took 15.479365874secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
14.542866983secs
v1 'master::call::GetMetrics' application/json response took 18.05492789secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
 (75455 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
Test setup: 1 agents with a total of 20005 frameworks
unversioned /metrics/snapshot' response took 31.908301664secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
32.128689785secs
v1 'master::call::GetMetrics' application/json response took 33.669376185secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
 (150440 ms)
```

WITH per-framework metrics, AFTER optimizations:
```
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
Test setup: 1 agents with a total of 105 frameworks
unversioned /metrics/snapshot' response took 104.577895ms
v1 'master::call::GetMetrics' application/x-protobuf response took 74.262533ms
v1 'master::call::GetMetrics' application/json response took 100.218618ms
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
 (562 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
Test setup: 1 agents with a total of 1005 frameworks
unversioned /metrics/snapshot' response took 921.175877ms
v1 'master::call::GetMetrics' application/x-protobuf response took 780.277639ms
v1 'master::call::GetMetrics' application/json response took 1.16865secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
 (5424 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
Test setup: 1 agents with a total of 10005 frameworks
unversioned /metrics/snapshot' response took 10.2413387secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
9.407992945secs
v1 'master::call::GetMetrics' application/json response took 10.582584848secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
 (57206 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
Test setup: 1 agents with a total of 20005 frameworks
unversioned /metrics/snapshot' response took 19.930542079secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
20.318739763secs
v1 'master::call::GetMetrics' application/json response took 

Review Request 67947: Passed the default options when making gRPC calls.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Passed the default options when making gRPC calls.


Diffs
-

  src/csi/client.cpp a36e6228f39405f729e347db51ccd95c5fb76cdb 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 67938: Made gRPC call options configurable.

[Chun-Hung Hsiao]

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

(Updated July 17, 2018, 8:20 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed Benjamin's comments.


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


Repository: mesos


Description
---

Made gRPC call options configurable.


Diffs (updated)
-

  3rdparty/libprocess/include/process/grpc.hpp 
0ff8184791277ed02b30481b352be60f10743428 
  3rdparty/libprocess/src/tests/grpc_tests.cpp 
eb9621b674ebbeda0b264001e5100a49a217629b 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 67936: Fixed ephemeral ports deallocation in network/port_mapping isolator.

[Ilya Pronin]

> On July 16, 2018, 4:59 p.m., Jie Yu wrote:
> > src/tests/containerizer/port_mapping_tests.cpp
> > Lines 1826 (patched)
> > 
> >
> > instead of hard code 512 which is fragile, i'd get `ephemeral_ports` 
> > resources, and use more than half of that.

Fixed.


- Ilya


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


On July 17, 2018, 1:12 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> ---
> 
> (Updated July 17, 2018, 1:12 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> a29282ed0488d83966f222084031ed1d2b6ab1f5 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 553f53043a52cfe235fc2b2e88fb54d0f2725d3e 
> 
> 
> Diff: https://reviews.apache.org/r/67936/diff/2/
> 
> 
> Testing
> ---
> 
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that 
> ephemeral ports are properly deallocated when the container was not isolated 
> and manually checked that if fails without the fix. Ran `sudo make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 67936: Fixed ephemeral ports deallocation in network/port_mapping isolator.

[Ilya Pronin]

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

(Updated July 17, 2018, 1:12 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Ephemeral ports are allocated for a container during the preparation
phase and have to be deallocated during container cleanup regardless of
whether the container process was isolated or not. Otherwise those ports
can be leaked if the container is destroyed during preparation.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
a29282ed0488d83966f222084031ed1d2b6ab1f5 
  src/tests/containerizer/port_mapping_tests.cpp 
553f53043a52cfe235fc2b2e88fb54d0f2725d3e 


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

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


Testing
---

Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that 
ephemeral ports are properly deallocated when the container was not isolated 
and manually checked that if fails without the fix. Ran `sudo make check`.


Thanks,

Ilya Pronin

Re: Review Request 67826: Made `Slave::getAvailable()` return all shared resources.

[Meng Zhu]

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

(Updated July 17, 2018, 12:47 p.m.)


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


Changes
---

+yanxu

Re BenM:

- Updated the description regarding motivation.
- Documented the shared resource allocation strategy.
- This patch is self-contained -- it preserves the code correctness.
- No, it does not affect the current metrics in any way.


Repository: mesos


Description (updated)
---

Currently, depending on already allocated resources,
`HierarchicalAllocatorProcess::Slave::getAvailable()`
may not contain all the shared resources.
Thus it does not accurately reflect what is available.

Since shared resources are always allocatable, we should
include all shared resources in the agent available resources.
This would help remove the one off logic for removing and
adding back shared resources in the allocation loop.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 67945: Made getters in the Slave class in the allocator return references.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

This eliminates some unnecessary copies.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67931: Support Image Manifest Version 2 Schema 2.

[Liangyu Zhao via Review Board]

> On July 17, 2018, 6:16 p.m., Andrew Schwartzmeyer wrote:
> > include/mesos/docker/spec.hpp
> > Lines 141 (patched)
> > 
> >
> > Dumb question, but is there an S1/S2 for V1 as well?

I don't think so.


> On July 17, 2018, 6:16 p.m., Andrew Schwartzmeyer wrote:
> > include/mesos/docker/spec.hpp
> > Lines 141-164 (patched)
> > 
> >
> > I'm not sure, but would this be better as a variant?
> > 
> > ```
> > std::unique_ptr > v2_2::ImageManifest>> manifest;
> > ```
> > 
> > (Declared at point of use.)
> > 
> > P.S. Why do we need pointers?

I haven't used the `std::Variant` before. I think it is a better choice.

The reason why I used pointers is trying to avoid copying the object. This is 
just a minor optimization of efficiency.


> On July 17, 2018, 6:16 p.m., Andrew Schwartzmeyer wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
> > Lines 467 (patched)
> > 
> >
> > Why do we need a `shared_ptr` here?

Avoiding copying too. Just a minor optimization.


> On July 17, 2018, 6:16 p.m., Andrew Schwartzmeyer wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
> > Lines 473 (patched)
> > 
> >
> > Heyyy what is this...

I think using labels for flow control makes this part of the code more clean 
and readable.


> On July 17, 2018, 6:16 p.m., Andrew Schwartzmeyer wrote:
> > src/uri/fetcher.cpp
> > Lines 88 (patched)
> > 
> >
> > Why does this function throw away `urls`?

This is just a default parent class function. Currently, only 
`DockerFetcherPulgin` supports multiple URLs fetch, so it overrides this 
function. As for other plugins, this is the default way of handling extra URLs. 
Commented at `include\mesos\uri\fetcher.hpp` line 89.


- Liangyu


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


On July 16, 2018, 6:35 p.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> ---
> 
> (Updated July 16, 2018, 6:35 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing schema 2 and fetching blob with multiple URLs as
> specified in schema 2. Update `curl` version to 7.60.0 because of bug
> encountered in version 7.57.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
>   include/mesos/docker/v2_2.hpp PRE-CREATION 
>   include/mesos/docker/v2_2.proto PRE-CREATION 
>   include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
>   src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 98129d006cda9b65804b518619b6addc8990410a 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
>   src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
>   src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
>   src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 
> 
> 
> Diff: https://reviews.apache.org/r/67931/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 67932: Use `wclayer` from `hcsshim` to extract file layers.

[Andrew Schwartzmeyer]

> On July 16, 2018, 12:48 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> > 
> > Reviews applied: `['67930', '67931', '67932']`
> > 
> > Failed command: `Start-MesosCITesting`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1935/mesos-review-67932
> > 
> > Relevant logs:
> > 
> > - 
> > [stout-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1935/mesos-review-67932/logs/stout-tests-cmake-stdout.log):
> > 
> > ```
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1056): 
> > warning C4267: '=': conversion from 'size_t' to 'Int32', possible loss of 
> > data 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1191): 
> > warning C4267: '=': conversion from 'size_t' to 'Int32', possible loss of 
> > data 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1417): 
> > warning C4996: 'strcat': This function or variable may be unsafe. Consider 
> > using strcat_s instead. To disable deprecation, use 
> > _CRT_SECURE_NO_WARNINGS. See online help for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1418): 
> > warning C4996: 'strcat': This function or variable may be unsafe. Consider 
> > using strcat_s instead. To disable deprecation, use 
> > _CRT_SECURE_NO_WARNINGS. See online help for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1423): 
> > warning C4996: 'setmode': The POSIX name for this item is deprecated. 
> > Instead, use the ISO C and C++ conformant name: _setmode. See online help 
> > for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1425): 
> > warning C4996: 'fopen': This function or variable may be unsafe. Consider 
> > using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
> > See online help for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1431): 
> > warning C4996: 'fdopen': The POSIX name for this item is deprecated. 
> > Instead, use the ISO C and C++ conformant name: _fdopen. See online help 
> > for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj" (default target) (8) ->
> >(CustomBuild target) -> 
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 

Re: Review Request 67931: Support Image Manifest Version 2 Schema 2.

[Andrew Schwartzmeyer]

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



There is still more to review... I'll get to it.


include/mesos/docker/spec.hpp
Lines 124-126 (patched)


I think this and the commit before need to be re-ordered, as this commit is 
introducing logic required and used by the previous commit.



include/mesos/docker/spec.hpp
Lines 141 (patched)


Dumb question, but is there an S1/S2 for V1 as well?



include/mesos/docker/spec.hpp
Lines 141-164 (patched)


I'm not sure, but would this be better as a variant?

```
std::unique_ptr> manifest;
```

(Declared at point of use.)

P.S. Why do we need pointers?



include/mesos/uri/fetcher.hpp
Lines 84-96 (patched)


Maybe add to this comment (and the one below) that this is used by the 
V2_S2 schema.



src/docker/spec.cpp
Lines 390-426 (patched)


I think you'd get all of this for "free" out of a variant.



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Lines 467 (patched)


Why do we need a `shared_ptr` here?



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Lines 473 (patched)


Heyyy what is this...



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Lines 490 (patched)


Why are we making this on the heap and using a pointer?



src/uri/fetcher.cpp
Line 56 (original), 56 (patched)


We can delete this now! Also, add `MESOS-8570` to the "bug" field of this 
review, and let's get you a JIRA account so I can assign it to you.



src/uri/fetcher.cpp
Lines 88 (patched)


Why does this function throw away `urls`?


- Andrew Schwartzmeyer


On July 16, 2018, 11:35 a.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> ---
> 
> (Updated July 16, 2018, 11:35 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing schema 2 and fetching blob with multiple URLs as
> specified in schema 2. Update `curl` version to 7.60.0 because of bug
> encountered in version 7.57.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
>   include/mesos/docker/v2_2.hpp PRE-CREATION 
>   include/mesos/docker/v2_2.proto PRE-CREATION 
>   include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
>   src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 98129d006cda9b65804b518619b6addc8990410a 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
>   src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
>   src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
>   src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 
> 
> 
> Diff: https://reviews.apache.org/r/67931/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 67928: Added balloon framework metric for tasks which were running.

[Meng Zhu]

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


Ship it!




Ship It!

- Meng Zhu


On July 16, 2018, 3:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67928/
> ---
> 
> (Updated July 16, 2018, 3:01 p.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The framework currently exposes metric counters for various expected
> and unexpected task termination reasons. Interpreting these counters
> can be non-trivial since tasks might fail due to benign, but unknown
> external reasons.
> 
> This patch adds a counter for the tasks which actually made it to the
> running stage which can be correlated with the different terminal task
> counts.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 
> 
> 
> Diff: https://reviews.apache.org/r/67928/diff/2/
> 
> 
> Testing
> ---
> 
> Started the framework with a `local` master and examine the metric.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67488: Updated CLI to Python 3.

[Andrew Schwartzmeyer]

> On July 16, 2018, 1:32 p.m., Andrew Schwartzmeyer wrote:
> > Did you have an automated way of finding where we should apply `override` 
> > in existing code? I'd like to see if there's any Windows-only code/tests we 
> > should fix too.
> 
> Armand Grillet wrote:
> What do you mean by `override`? I have applied `2to3` on the CLI codebase 
> to find what needed to be updated. I haven't run `mesos-cli-tests` on a 
> Windows machine, that should tell us if we need to fix something else for 
> that operating system.

Wrong review... I think I was copy-pasting and had a couple tabs open. Sorry!


- Andrew


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


On July 6, 2018, 6:56 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67488/
> ---
> 
> (Updated July 6, 2018, 6:56 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The build tools are also up to date thus the CLI can still be built
> using Autotools and CMake. No features have been added to the CLI.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 
>   configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b 
>   src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada 
>   src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca 
>   src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 
>   src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c 
>   src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f 
>   src/python/cli_new/lib/cli/config.py 
> 6f92622725d8a042a2a728fd38c977ac690ef6be 
>   src/python/cli_new/lib/cli/docopt.py 
> 86a4e9c74326fb80cc59487113f07358dd96960d 
>   src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b 
>   src/python/cli_new/lib/cli/plugins/agent/main.py 
> 59280ece8ebd00bb96df3675b6356a26cc48a2c0 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> cc6cff56c71262729a8870017bef2e97636abe5a 
>   src/python/cli_new/lib/cli/tests/base.py 
> 89360e6cac5ca910044a5a82fab7237510edee7f 
>   src/python/cli_new/lib/cli/tests/tests.py 
> 79e1036f6d11c63884091fe43672607b03955c1a 
>   src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 
>   src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 
>   src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 
>   src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
>   support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 
> 
> 
> Diff: https://reviews.apache.org/r/67488/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25 with `python` being Python 2.7, `python3` being 
> Python 3.5 and `python36` being Python 3.6.
> 
> 
> For Autotools:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java
> $ make check
> ```
> 
> For CMake:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> build/src/cli was as expected, and that build/src/mesos was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67488: Updated CLI to Python 3.

[Armand Grillet]

> On July 16, 2018, 8:32 p.m., Andrew Schwartzmeyer wrote:
> > Did you have an automated way of finding where we should apply `override` 
> > in existing code? I'd like to see if there's any Windows-only code/tests we 
> > should fix too.

What do you mean by `override`? I have applied `2to3` on the CLI codebase to 
find what needed to be updated. I haven't run `mesos-cli-tests` on a Windows 
machine, that should tell us if we need to fix something else for that 
operating system.


- Armand


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


On July 6, 2018, 1:56 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67488/
> ---
> 
> (Updated July 6, 2018, 1:56 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The build tools are also up to date thus the CLI can still be built
> using Autotools and CMake. No features have been added to the CLI.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 
>   configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b 
>   src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada 
>   src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca 
>   src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 
>   src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c 
>   src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f 
>   src/python/cli_new/lib/cli/config.py 
> 6f92622725d8a042a2a728fd38c977ac690ef6be 
>   src/python/cli_new/lib/cli/docopt.py 
> 86a4e9c74326fb80cc59487113f07358dd96960d 
>   src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b 
>   src/python/cli_new/lib/cli/plugins/agent/main.py 
> 59280ece8ebd00bb96df3675b6356a26cc48a2c0 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> cc6cff56c71262729a8870017bef2e97636abe5a 
>   src/python/cli_new/lib/cli/tests/base.py 
> 89360e6cac5ca910044a5a82fab7237510edee7f 
>   src/python/cli_new/lib/cli/tests/tests.py 
> 79e1036f6d11c63884091fe43672607b03955c1a 
>   src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 
>   src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 
>   src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 
>   src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
>   support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 
> 
> 
> Diff: https://reviews.apache.org/r/67488/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25 with `python` being Python 2.7, `python3` being 
> Python 3.5 and `python36` being Python 3.6.
> 
> 
> For Autotools:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java
> $ make check
> ```
> 
> For CMake:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> build/src/cli was as expected, and that build/src/mesos was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67938: Made gRPC call options configurable.

[Benjamin Bannier]

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


Fix it, then Ship it!





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


Since this is a pure data container class `s/class/struct/`.

Then we can also drop the `public` below.



3rdparty/libprocess/include/process/grpc.hpp
Lines 122-123 (patched)


Could we add some documentation on the approximate semantics of these 
options, and possible how we got to these defaults?



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


Let's either pass a `const Option& = None()` here, or (since 
this is only used in `src/csi/client.cpp` and tests) probably even better get 
rid of the default arg and require users to always pass in parameters.



3rdparty/libprocess/src/tests/grpc_tests.cpp
Lines 115 (patched)


This could be a `static` method.


- Benjamin Bannier


On July 17, 2018, 3:27 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67938/
> ---
> 
> (Updated July 17, 2018, 3:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9055
> https://issues.apache.org/jira/browse/MESOS-9055
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made gRPC call options configurable.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 0ff8184791277ed02b30481b352be60f10743428 
>   3rdparty/libprocess/src/tests/grpc_tests.cpp 
> eb9621b674ebbeda0b264001e5100a49a217629b 
> 
> 
> Diff: https://reviews.apache.org/r/67938/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>