Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66615]

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

- Mesos Reviewbot


On April 13, 2018, 11:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66615/
> ---
> 
> (Updated April 13, 2018, 11:57 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and 
> Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Docker containerizer tests were manually creating the `TaskInfo`
> for the tasks they wanted to launch. We can remove some of that
> boilerplate by adopting the `createTask` helper.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 847258daadf3c37d9071151616b18fc79d850ce8 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
> 
> 
> Diff: https://reviews.apache.org/r/66615/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 27).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Chun-Hung Hsiao]

> On April 14, 2018, 1:52 a.m., Chun-Hung Hsiao wrote:
> > src/master/master.cpp
> > Lines 4976 (patched)
> > 
> >
> > As we discussed, we should validate that there is only one operation. 
> > Shouldn't we add the check in this function? Ditto for `SHRINK_VOLUME`.

Oh never mind. It's in the next patch.


- Chun-Hung


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


On April 10, 2018, 4:18 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated April 10, 2018, 4:18 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as speculative operations, but
> we will use validation to make them non-speculative on API level so that
> we can transition later without a breaking change.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp f7da675e8fe96159e5335c9e83b65b67ed90eda8 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Chun-Hung Hsiao]

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




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


" with additional resource "?



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


As we discussed, we should validate that there is only one operation. 
Shouldn't we add the check in this function? Ditto for `SHRINK_VOLUME`.



src/master/validation.cpp
Lines 2336 (patched)


For consistency, I prefer just `"Invalid resource: "`.

Or if you want to make it explicit, maybe:
`"Invalid resource in the 'volume' field: "`, or
`Invalid resource in the 'GrowVolume.volume' field: "`.

Ditto below for 'addition':
`"Invalid resource: "`, or
`"Invalid additional resource: "`, or
`"Invalid resource in the 'addition' field: "`, or
`"Invalid resource in the 'GrowVolume.addition' field: "`.

Note that I use `GrowVolume` instead of `grow_volume` since there is no 
guarantee that the field being validated by this funciton is called 
`grow_volume`.



src/master/validation.cpp
Lines 2376 (patched)


s/field/fields/



src/master/validation.cpp
Lines 2387 (patched)


Similar to the reason I mentioned above, let's do either of the followings:

`"Invalid resource: "`
`"Invalid resource in the 'volume' field: "`
`Invalid resource in the 'ShrinkVolume.volume' field: "`



src/master/validation.cpp
Lines 2392 (patched)


Also: `"The 'subtract' field must..."` or `"The 'ShrinkVolume.subtract' 
field must..."`.


- Chun-Hung Hsiao


On April 10, 2018, 4:18 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated April 10, 2018, 4:18 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as speculative operations, but
> we will use validation to make them non-speculative on API level so that
> we can transition later without a breaking change.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp f7da675e8fe96159e5335c9e83b65b67ed90eda8 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66616: Marked volume/block creation and destroy operations as experimental.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66616']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
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]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 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(512):
 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(543):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_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(548):
 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(569):
 warning C4996: 

Review Request 66616: Marked volume/block creation and destroy operations as experimental.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


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


Repository: mesos


Description
---

Marked volume/block creation and destroy operations as experimental.


Diffs
-

  include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
  include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 


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


Testing
---

No need for testing since only comments are changed.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66464: Implemented operation status reconciliation.

[Gaston Kleiman]

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

(Updated April 13, 2018, 5:51 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Addressed Greg's feedback.


Repository: mesos


Description
---

Implemented operation status reconciliation.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/master.cpp f7da675e8fe96159e5335c9e83b65b67ed90eda8 


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

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


Testing
---

`sudo bin/mesos-tests` on GNU/Linux

https://reviews.apache.org/r/66468/ adds new tests.


Thanks,

Gaston Kleiman

Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66615']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
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]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 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(512):
 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(543):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_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(548):
 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(569):
 warning C4996: 

Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

[Gaston Kleiman]

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



Thanks for the cleanup!

I think we could get rid of quite some boilerplate with a helper that creates a 
`ContainerInfo::DockerInfo`.


src/tests/containerizer/docker_containerizer_tests.cpp
Line 265 (original), 260 (patched)


Nit: I'd add an empty line above this one.

Here and in the other tests.


- Gaston Kleiman


On April 13, 2018, 4:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66615/
> ---
> 
> (Updated April 13, 2018, 4:57 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and 
> Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Docker containerizer tests were manually creating the `TaskInfo`
> for the tasks they wanted to launch. We can remove some of that
> boilerplate by adopting the `createTask` helper.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 847258daadf3c37d9071151616b18fc79d850ce8 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
> 
> 
> Diff: https://reviews.apache.org/r/66615/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 27).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

[James Peach]

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

Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and Zhitao 
Li.


Repository: mesos


Description
---

The Docker containerizer tests were manually creating the `TaskInfo`
for the tasks they wanted to launch. We can remove some of that
boilerplate by adopting the `createTask` helper.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
847258daadf3c37d9071151616b18fc79d850ce8 
  src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 


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


Testing
---

sudo make check (Fedora 27).


Thanks,

James Peach

Re: Review Request 66284: Tested `max_completion_time` support in docker executor.

[James Peach]

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




src/tests/containerizer/docker_containerizer_tests.cpp
Lines 646 (patched)


Use `createTask()` here?

```
TaskInfo task = createTask(
offer.slave_id(),
offer.resources(),
SLEEP_COMMAND(1000));
```



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 688 (patched)


Consolidate this into the `EXPECT_CALL` above?


- James Peach


On April 8, 2018, 12:53 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66284/
> ---
> 
> (Updated April 8, 2018, 12:53 a.m.)
> 
> 
> Review request for mesos, Jason Lai and James Peach.
> 
> 
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tested `max_completion_time` support in docker executor.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 847258daadf3c37d9071151616b18fc79d850ce8 
> 
> 
> Diff: https://reviews.apache.org/r/66284/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 12, 2018, 3:35 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65594/
> ---
> 
> (Updated April 12, 2018, 3:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SLRP now checkpoints profiles associated with storage pools, and does
> not depend on the `DiskProfileAdaptor` module to return the set of
> previously-known profiles during recovery.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/65594/diff/8/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64923: Added replicated log reader catch-up section to the upgrades doc.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Jan. 3, 2018, 7:19 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64923/
> ---
> 
> (Updated Jan. 3, 2018, 7:19 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added replicated log reader catch-up section to the upgrades doc.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md f2eb5a94d67b44a02d355981a673bd4ef5129653 
> 
> 
> Diff: https://reviews.apache.org/r/64923/diff/1/
> 
> 
> Testing
> ---
> 
> Verified on GitHub: 
> https://github.com/ipronin/mesos/blob/log-replica-catchup-docs/docs/upgrades.md
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 64922: Added VOTING replica catch-up section to replicated log docs.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Jan. 3, 2018, 7:18 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64922/
> ---
> 
> (Updated Jan. 3, 2018, 7:18 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added VOTING replica catch-up section to replicated log docs.
> 
> 
> Diffs
> -
> 
>   docs/replicated-log-internals.md 66497ccd7298103819501fbc3a800db437ca37d5 
> 
> 
> Diff: https://reviews.apache.org/r/64922/diff/1/
> 
> 
> Testing
> ---
> 
> Verified on GitHub: 
> https://github.com/ipronin/mesos/blob/log-replica-catchup-docs/docs/replicated-log-internals.md
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 64921: Fixed a typo and formatting in docs.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Jan. 3, 2018, 7:18 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64921/
> ---
> 
> (Updated Jan. 3, 2018, 7:18 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo and formatting in docs.
> 
> 
> Diffs
> -
> 
>   docs/replicated-log-internals.md 66497ccd7298103819501fbc3a800db437ca37d5 
> 
> 
> Diff: https://reviews.apache.org/r/64921/diff/1/
> 
> 
> Testing
> ---
> 
> Verified on GitHub: 
> https://github.com/ipronin/mesos/blob/log-replica-catchup-docs/docs/replicated-log-internals.md
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 66283: Added support of `max_completion_time` in docker executor.

[James Peach]

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




src/docker/executor.cpp
Lines 405 (patched)


Suggest:
```
Killing task $TASKID which exceeded its maximum completion time of $DURATION
```



src/docker/executor.cpp
Lines 412 (patched)


`shutdown()` runs `killTask()` with a grace period, so I think that you 
need to do more here.

Something like this would work:
```
killed = true;
killTask(driver, taskId, 0 /* grace */);
```



src/docker/executor.cpp
Lines 500 (patched)


Move this up into the `if`.



src/docker/executor.cpp
Lines 571 (patched)


You can omit this block since it can't happen because you only get here 
when `failReason` is `None`.



src/docker/executor.cpp
Lines 830 (patched)


Like the commant executor, I think you might as well be more explicit here 
with a `killedByCompletionTimeout` flag.


- James Peach


On April 8, 2018, 12:52 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66283/
> ---
> 
> (Updated April 8, 2018, 12:52 a.m.)
> 
> 
> Review request for mesos, Jason Lai and James Peach.
> 
> 
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `TaskInfo.max_completion_time` is set, docker executor will kill
> the task immediately. We reuse the `shutdown` method to achieve a
> forced kill ignoring any `KillPolicy`.
> 
> Framework should only received a `TASK_FAILED` state with
> `REASON_MAX_COMPLETION_TIME_REACHED` reason.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 1d672112bae889cc9d19343a59e0ff66569785c4 
> 
> 
> Diff: https://reviews.apache.org/r/66283/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63372: Added documentation for memory profiling.

[Benno Evers]

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

(Updated April 13, 2018, 6:49 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Added jeprof section.


Repository: mesos


Description
---

Added documentation for memory profiling.


Diffs (updated)
-

  docs/memory-profiling.md PRE-CREATION 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 63370: Added new --memory_profiling flag to agent and master binaries.

[Benno Evers]

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

(Updated April 13, 2018, 6:48 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Changed default to "false".


Repository: mesos


Description
---

This flag allows explicit disabling of the memory profiler
endpoint in the master and agent binaries.


Diffs (updated)
-

  src/master/flags.hpp 505786e5631c891a52d9d7db403eff312f461d3d 
  src/master/flags.cpp dbb35befd612f4be1019293c1889d19296118d07 
  src/master/main.cpp 3def0f29e4d580306d2ecef033a37aad5333c8ea 
  src/slave/flags.hpp beae47f0f8f2178b93a3484d168ce4d71c961841 
  src/slave/flags.cpp 4ea2286a6a874e6e7f473f97fb0431e17b19c1f8 
  src/slave/main.cpp dcc944840d4e4e611e8ed1b0110b8f1bb189bde9 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 66260: Tested `max_completion_time` support in command executor.

[James Peach]

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


Fix it, then Ship it!





src/tests/command_executor_tests.cpp
Lines 287 (patched)


Can you do it like this:
```
EXPECT_CALL(sched, statusUpdate(_, _))
.WillOnce(FutureArg<1>())
.WillOnce(FutureArg<1>())
.WillOnce(FutureArg<1>());
```


- James Peach


On April 8, 2018, 12:52 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66260/
> ---
> 
> (Updated April 8, 2018, 12:52 a.m.)
> 
> 
> Review request for mesos, Jason Lai and James Peach.
> 
> 
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tested `max_completion_time` support in command executor.
> 
> 
> Diffs
> -
> 
>   src/tests/command_executor_tests.cpp 
> 3c5687f918deb8cdda8a97abb0fbd8d9b089926a 
> 
> 
> Diff: https://reviews.apache.org/r/66260/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66258: Added `max_completion_time` to `TaskInfo` and new reason.

[James Peach]

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


Fix it, then Ship it!





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


I think that we can phrase this a little more clearly. How about:

```
Maximum duration for task completion. If the task is non-terminal at the end
of this duration, it will fail with the reason 
`REASON_MAX_COMPLETION_TIME_REACHED`.
Mesos supports this field for executor-less tasks, and tasks that use the 
Docker or
default executors. It is the executor's responsibility to implement this, 
so it might
not be supported by all custom executors.
```


- James Peach


On April 8, 2018, 12:51 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66258/
> ---
> 
> (Updated April 8, 2018, 12:51 a.m.)
> 
> 
> Review request for mesos, Jason Lai and James Peach.
> 
> 
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new field can be used support tasks which have a maximum duration
> scheduling requirement. The new reason is added to distinguish from
> normal task kills.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
>   include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 
> 
> 
> Diff: https://reviews.apache.org/r/66258/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66591: Added a validation that `max_completion_time` must be non-negative.

[James Peach]

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


Fix it, then Ship it!





src/master/validation.cpp
Lines 1320 (patched)


Too many newlines :)


- James Peach


On April 12, 2018, 9:20 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66591/
> ---
> 
> (Updated April 12, 2018, 9:20 p.m.)
> 
> 
> Review request for mesos, Jason Lai and James Peach.
> 
> 
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a validation that `max_completion_time` must be non-negative.
> 
> 
> Diffs
> -
> 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
> 
> 
> Diff: https://reviews.apache.org/r/66591/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66259: Added `max_completion_time` support to command executor.

[James Peach]

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




src/launcher/executor.cpp
Lines 590 (patched)


I think that it makes more sense to move this block to just before we call 
`launchTaskSubprocess`.

I wonder whether we should also make the log message show `Starting task 
$TASKID with maximum completion time of $DURATION`?



src/launcher/executor.cpp
Lines 750 (patched)


Move this into the `if` above



src/launcher/executor.cpp
Lines 936 (patched)


Should set maxCompletionTimer to `None()` as well, for consistency.



src/launcher/executor.cpp
Lines 1008 (patched)


Can `pid` ever be validly `None()` due to your changes? If it can, I'm not 
seeing the reason ...



src/launcher/executor.cpp
Lines 1036 (patched)


How about calling this `taskCompletionTimeout`?



src/launcher/executor.cpp
Lines 1037 (patched)


Consider adding a `CHECK(!killed)` here, since we should never do 
completion time handling after receiving a scheduler kill.



src/launcher/executor.cpp
Lines 1038 (patched)


I think this can be a `CHECK(!terminated)` since you cancel the completion 
timeout in `reaped()`.



src/launcher/executor.cpp
Lines 1043 (patched)


Suggest that we phrase this more naturally:
```
Killing task $TASKID which exceeded its maximum completion time
```

I'm assuming that you wanted to depend on the log message in `escalated` to 
capture the completion time?



src/launcher/executor.cpp
Lines 1049 (patched)


What happens if the scheduler sends a `kill` after the completion timeout 
fires? In this code, it seems like the `kill` will still be processed; there is 
also some inconsistency because we are killing the task but don't set the 
`killed` flag.

Maybe a better approach would be to call `kill()` from here with a 0 grace 
period? Then in `kill()` I'd argue that a 0 grace period should skip the 
`SIGTERM` phase.


- James Peach


On April 8, 2018, 12:51 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66259/
> ---
> 
> (Updated April 8, 2018, 12:51 a.m.)
> 
> 
> Review request for mesos, Jason Lai and James Peach.
> 
> 
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `TaskInfo.max_completion_time` is set, command executor will kill
> the task with `SIGKILL` immediately. Note that no KillPolicy will be
> observed. Framework should only received a `TASK_FAILED` state with
> `REASON_MAX_COMPLETION_TIME_REACHED` reason.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 8d0869cfcdfc693301d0e185a036e97204bad17f 
> 
> 
> Diff: https://reviews.apache.org/r/66259/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

[Zhitao Li]

> On April 12, 2018, 5:27 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 1975 (patched)
> > 
> >
> > Hmm I wonder if we should just use a `double` here? Does the 
> > `Value.Scalar` type provide some benefit?
> 
> Chun-Hung Hsiao wrote:
> From the API perspective, `double` is easier to use, but `Value.Scalar` 
> is more consistent with how we represesnt a disk resource.
> 
> From the implementation perspective, I prefer `Value.Scalar` because it's 
> arithmetic operators will do fixed-point conversions for me so I won't forget 
> that.
> 
> Greg Mann wrote:
> My recommendation would be to prioritize easy-of-use in the API over 
> ease-of-implementation when possible :)
> 
> In the implementation, as long as we toss the provided double into a 
> `Scalar::Value` before we do anything with it, I think we could get the same 
> benefit?

I feel that `Scalar` is actually more consistent than `double` here. I do not 
see either API is much easier to use.


- Zhitao


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


On April 13, 2018, 10:20 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66049/
> ---
> 
> (Updated April 13, 2018, 10:20 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added offer operation to grow and shrink persistent volumes.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
>   include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66049/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

[Zhitao Li]

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

(Updated April 13, 2018, 10:20 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Remove unnecessary whiteline.


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


Repository: mesos


Description
---

Added offer operation to grow and shrink persistent volumes.


Diffs (updated)
-

  include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
  include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 
  src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
  src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
  src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 
  src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

[Zhitao Li]

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

(Updated April 13, 2018, 10:15 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Add `UNIMPLEMENTED` annotations so this compiles.


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


Repository: mesos


Description
---

Added offer operation to grow and shrink persistent volumes.


Diffs (updated)
-

  include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
  include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 
  src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
  src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
  src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
  src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 
  src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 


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

Changes: https://reviews.apache.org/r/66049/diff/6-7/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66220: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

[Greg Mann]

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




src/tests/persistent_volume_tests.cpp
Lines 455-459 (patched)


Is this enforced somewhere in validation code? Can we check for expected 
behavior when a GROW/SHRINK operation is submitted for a MOUNT volume, rather 
than simply returning?



src/tests/persistent_volume_tests.cpp
Lines 471-479 (patched)


Nit: could you group all of the agent dependencies together?
i.e.
```
  Future updateSlaveMessage =
  FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);

  slave::Flags slaveFlags = CreateSlaveFlags();
  slaveFlags.resources = getSlaveResources();

  Owned detector = master.get()->createDetector();

  Try slave = StartSlave(detector.get(), slaveFlags);
  ASSERT_SOME(slave);
```



src/tests/persistent_volume_tests.cpp
Lines 492 (patched)


Is this necessary? Since the clock is paused, is it really possible that 
we'll get more offers than expected?



src/tests/persistent_volume_tests.cpp
Lines 496 (patched)


Could you move this to the top of the test to make it clearer that we will 
have the clock paused for the whole test?



src/tests/persistent_volume_tests.cpp
Lines 541-542 (patched)


Is this `Future` necessary? Since the task consumes the volume, it may be 
sufficient to await on the task status updates?



src/tests/persistent_volume_tests.cpp
Lines 542 (patched)


Indented too far.



src/tests/persistent_volume_tests.cpp
Lines 549 (patched)


Is this necessary?



src/tests/persistent_volume_tests.cpp
Lines 553-554 (patched)


Is this necessary? Awaiting on the task status updates may be sufficient?



src/tests/persistent_volume_tests.cpp
Lines 557 (patched)


Suggestion: you could use the `TaskStatusTaskIdEq` matcher in the 
`EXPECT_CALL(sched, statusUpdate(, _))` call to avoid these expectations 
for the task ID.



src/tests/persistent_volume_tests.cpp
Lines 597-603 (patched)


I'm not sure if these expectations are necessary - perhaps it's sufficient 
to confirm that the subsequent offer contains a volume of the expected size?

Ditto for the `shrinkVolumeMessage` below.



src/tests/persistent_volume_tests.cpp
Lines 613 (patched)


Is this comment correct? Won't this offer contain the grown volume?



src/tests/persistent_volume_tests.cpp
Lines 634-636 (patched)


I think these should be indented two more spaces.



src/tests/persistent_volume_tests.cpp
Lines 657-658 (patched)


Is this necessary?



src/tests/persistent_volume_tests.cpp
Lines 668 (patched)


Is this necessary?



src/tests/persistent_volume_tests.cpp
Lines 680 (patched)


Is this necessary?


- Greg Mann


On April 11, 2018, 9:19 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66220/
> ---
> 
> (Updated April 11, 2018, 9:19 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66220/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66608]

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

- Mesos Reviewbot


On April 13, 2018, 1:09 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66608/
> ---
> 
> (Updated April 13, 2018, 1:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While it was already possible to create a `hashmap` over move-only
> values, we still performed a copy in `put`, making it hard to
> dynamically add elements with the expected stout semantics.
> 
> This patch releases the requirements on the value argument to `put` so
> that instead of copyable we now only require move-constructible.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> 91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 
> 
> 
> Diff: https://reviews.apache.org/r/66608/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66568: Dropped GROW and SHRINK volume if combined with other operations.

[Greg Mann]

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


Fix it, then Ship it!





src/master/master.cpp
Lines 4965-4966 (patched)


Could you create a JIRA for the speculative -> non-speculative change and 
mention the ticket here, and below?



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


Suggestion: this is a very cheap piece of validation to perform, so I would 
probably put it near the top of this operation's `switch` case. Here and below 
for SHRINK_VOLUME as well.


- Greg Mann


On April 11, 2018, 9:18 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66568/
> ---
> 
> (Updated April 11, 2018, 9:18 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These two operations are intended to be non-speculative eventually but
> are implemented as speculative right now. To avoid frameworks opt-in to
> dangerous behavior, we require that accept can only container one
> `GROW_VOLUME` or `SHRINK_VOLUME` and no other operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp f7da675e8fe96159e5335c9e83b65b67ed90eda8 
> 
> 
> Diff: https://reviews.apache.org/r/66568/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

[Greg Mann]

> On April 13, 2018, 12:27 a.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 1975 (patched)
> > 
> >
> > Hmm I wonder if we should just use a `double` here? Does the 
> > `Value.Scalar` type provide some benefit?
> 
> Chun-Hung Hsiao wrote:
> From the API perspective, `double` is easier to use, but `Value.Scalar` 
> is more consistent with how we represesnt a disk resource.
> 
> From the implementation perspective, I prefer `Value.Scalar` because it's 
> arithmetic operators will do fixed-point conversions for me so I won't forget 
> that.

My recommendation would be to prioritize easy-of-use in the API over 
ease-of-implementation when possible :)

In the implementation, as long as we toss the provided double into a 
`Scalar::Value` before we do anything with it, I think we could get the same 
benefit?


- Greg


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


On March 28, 2018, 6:24 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66049/
> ---
> 
> (Updated March 28, 2018, 6:24 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added offer operation to grow and shrink persistent volumes.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
>   include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 
> 
> 
> Diff: https://reviews.apache.org/r/66049/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

[Alexander Rukletsov]

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




3rdparty/libprocess/src/memory_profiler.cpp
Lines 691-692 (original), 714-716 (patched)


`return http::Conflict("...");`


- Alexander Rukletsov


On April 10, 2018, 2:39 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> ---
> 
> (Updated April 10, 2018, 2:39 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class exposes profiling functionality of jemalloc memory allocator
> when it is detected to be the memory allocator of the current process.
> 
> In particular, it gives developers an easy method to collect and
> access heap profiles which report which pieces of code were
> responsible for allocating memory.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c895d3ac7b9cc5ffd6c8b57ff24def866eb0213d 
>   3rdparty/libprocess/include/Makefile.am 
> cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 
> c36df991b6a2c120ab0562e8ff907f9fbf8630d1 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 
> 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 66608: Improved support for move-only types in `hashmap`.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66608']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
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]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 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(512):
 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(543):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_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(548):
 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(569):
 warning C4996: 

Review Request 66608: Improved support for move-only types in `hashmap`.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov and Michael Park.


Repository: mesos


Description
---

While it was already possible to create a `hashmap` over move-only
values, we still performed a copy in `put`, making it hard to
dynamically add elements with the expected stout semantics.

This patch releases the requirements on the value argument to `put` so
that instead of copyable we now only require move-constructible.


Diffs
-

  3rdparty/stout/include/stout/hashmap.hpp 
91085b8d8ad5d35c39c8cc95e3d4765d82d9a8db 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63366: Added jemalloc release tarball and build rules.

[Benno Evers]

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

(Updated April 13, 2018, 12:03 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Summary (updated)
-

Added jemalloc release tarball and build rules.


Repository: mesos


Description (updated)
---

Added jemalloc release tarball and build rules.


Diffs
-

  3rdparty/CMakeLists.txt 488e906486a583e74faceb44d906cee5036a8b99 
  3rdparty/Makefile.am 10b29c9e2d10073a817273c7038226d0ed4fd6ad 
  3rdparty/cmake/Versions.cmake 93f0322c1ac926bcfdcd4c1cfd9ba9f22bcf7099 
  3rdparty/jemalloc-5.0.1.tar.bz2 PRE-CREATION 
  3rdparty/versions.am 1bc87bb1ec2e74ebb2072f63163e3c1e8b4aad00 
  cmake/CompilationConfigure.cmake 3cb072ddcd286c0e40d44eaeba210ddf1796975c 
  configure.ac f0f901f2e565352c2804cae7b2ac255da81ce45d 
  src/Makefile.am 9f4b6d369a23af337e2384e52e3e41f4017df38a 
  src/master/CMakeLists.txt b8953bd576586ac8548a1b51cf0a87bc8e9da4fc 
  src/slave/CMakeLists.txt 943e8f523f867e3dd4030f78eca7830500578faf 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

[Alexander Rukletsov]

> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/include/process/memory_profiler.hpp
> > Lines 104-113 (patched)
> > 
> >
> > Does this class have to be nested? How about making it 
> > `jemalloc::State` in the same file?
> 
> Benno Evers wrote:
> None of them technically *need* to be nested, but it feels a bit cleaner 
> to not litter the `process`-namespace unnecessarily. It also seems more 
> consistent to me to treat all three classes as similar as possible.

Hence the suggestion to put it into the `jemalloc` namespace.


> On April 6, 2018, 2:02 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/CMakeLists.txt
> > Lines 47 (patched)
> > 
> >
> > Please add *.hpp as well.
> 
> Benno Evers wrote:
> I think if we do this, it should be part of a separate patch series - 
> right now, none of the public headers are included in the `CMakeLists.txt`.

I'm not sure it has to be public, but okay.


- Alexander


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


On April 10, 2018, 2:39 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> ---
> 
> (Updated April 10, 2018, 2:39 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class exposes profiling functionality of jemalloc memory allocator
> when it is detected to be the memory allocator of the current process.
> 
> In particular, it gives developers an easy method to collect and
> access heap profiles which report which pieces of code were
> responsible for allocating memory.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c895d3ac7b9cc5ffd6c8b57ff24def866eb0213d 
>   3rdparty/libprocess/include/Makefile.am 
> cd2c3bc62df8de5b50ec2fa830b3e2634ba11e28 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 
> c36df991b6a2c120ab0562e8ff907f9fbf8630d1 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 0ce7dac5deea94623530820d173ce3ffe5b42ea4 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 
> 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65462: Used the new 'route()' overload from libprocess.

[Benno Evers]

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

(Updated April 13, 2018, 11:14 a.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/files/files.cpp c392019cde193125ecb532f222dd287b7a91abad 
  src/master/registrar.cpp a83514294010139117d983800d2819d65e8c2d9e 


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


Testing (updated)
---

`make check` on Mac OS and some linux variants.


Thanks,

Benno Evers

Re: Review Request 65462: Used the new 'route()' overload from libprocess.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On April 5, 2018, 10:28 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65462/
> ---
> 
> (Updated April 5, 2018, 10:28 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use the new 'route()' overload from libprocess.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp c392019cde193125ecb532f222dd287b7a91abad 
>   src/master/registrar.cpp a83514294010139117d983800d2819d65e8c2d9e 
> 
> 
> Diff: https://reviews.apache.org/r/65462/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65461: Provide new overload of 'ProcessBase::route()'.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Feb. 28, 2018, 7:55 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65461/
> ---
> 
> (Updated Feb. 28, 2018, 7:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide new overload of 'ProcessBase::route()'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/logging.hpp 
> f9997677d69d54f5723d4fc0a495008d3ce11cc5 
>   3rdparty/libprocess/include/process/process.hpp 
> 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/include/process/profiler.hpp 
> 2991dd2033d68802a813de91babb47679c807aa0 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 3a5bd3dfafa7d4adec26d17acad3e4f27e4046ea 
> 
> 
> Diff: https://reviews.apache.org/r/65461/diff/4/
> 
> 
> Testing
> ---
> 
> It is a common pattern in libprocess to have a single function serving as 
> endpoint that handles both authenticated and non-authenticated requests, 
> providing `None()` as a default parameter in non-authenticated contexts.
> 
> This patch adds an overload to `ProcessBase::route()` implementing that so 
> that it does not need to be re-implemented in each individual process.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>