Re: Review Request 40347: [3/4] Quota Authorization: Implemented quota request authorization.

[Jan Schlicht]

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

(Updated Nov. 20, 2015, 10:55 a.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Till 
Toenshoff.


Changes
---

Authorize using the authentification crendentials. Authorization won't work 
without authentification, but this is in line with the current other 
authorization implementations.


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


Repository: mesos


Description
---

Quota: Implemented quota request authorization.


Diffs (updated)
-

  include/mesos/quota/quota.proto 4e4d8ccc92e2bf9a8e5eae8488c0c952f82fdd6d 
  src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
  src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 

Diff: https://reviews.apache.org/r/40347/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 39449: Documented order of includes.

[Jan Schlicht]

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

(Updated Nov. 20, 2015, 12:17 p.m.)


Review request for mesos, Marco Massenzio and Michael Park.


Changes
---

Related headers should be included first.


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


Repository: mesos


Description
---

Documented order of includes.


Diffs (updated)
-

  docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 

Diff: https://reviews.apache.org/r/39449/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40371: Changed mesos-execute to add containerizer option.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40371]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 7:45 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> ---
> 
> (Updated Nov. 20, 2015, 7:45 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a  new option 'containerizer' to disambiguate the two 
> containerizers.
> 
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=docker
> 
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=mesos
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc 
> 
> Diff: https://reviews.apache.org/r/40371/diff/
> 
> 
> Testing
> ---
> 
> tested the two containerizer locally with simple commands.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 37999: Introduced an Authenticator interface and an AuthenticationRouter in libprocess.

[Ben Mahler]

> On Nov. 20, 2015, 2 p.m., Ben Mahler wrote:
> > For transparency we pulled out the libprocess integration because we 
> > realized that requests sent to the authentication router need to have 
> > authentication results satisfied in the same order in which the requests 
> > were sent. We're still thinking through how to solve this within 
> > libprocess, so for now we're just going to commit these interfaces (we 
> > don't expect these interfaces to change further for the MVP).
> > 
> > I will update the description in the commit to reflect that this no longer 
> > includes the ProcessManager integration.

We're also pulling out the remaining changes to event.hpp and process.cpp in 
this patch, as they will make more sense alongside the subsequent integration 
patches.


- Ben


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


On Nov. 20, 2015, 12:53 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Nov. 20, 2015, 12:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Authenticator interface allows us to implement different
> authenticators based on the scheme (e.g. Basic, Digest, SPNEGO).
> The AuthenticationRouter manages the authentication realms and
> the mapping from endpoints to realms. It is then used by the
> ProcessManager to route requests to the authenticator for the
> realm, if applicable.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
>   3rdparty/libprocess/include/Makefile.am 
> e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 60c615281f3810230bf6c17866f46eaa6855ca29 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fb9bd04832779ac43151f2feb3dfbf58cb996434 
>   3rdparty/libprocess/src/authentication_router.hpp PRE-CREATION 
>   3rdparty/libprocess/src/authentication_router.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 
> 7abdf21a5784920251c3627f9820c12fdc356c6e 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37999: Introduced an Authenticator interface and an AuthenticationRouter in libprocess.

[Ben Mahler]

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

Ship it!


For transparency we pulled out the libprocess integration because we realized 
that requests sent to the authentication router need to have authentication 
results satisfied in the same order in which the requests were sent. We're 
still thinking through how to solve this within libprocess, so for now we're 
just going to commit these interfaces (we don't expect these interfaces to 
change further for the MVP).

I will update the description in the commit to reflect that this no longer 
includes the ProcessManager integration.

- Ben Mahler


On Nov. 20, 2015, 12:53 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37999/
> ---
> 
> (Updated Nov. 20, 2015, 12:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Authenticator interface allows us to implement different
> authenticators based on the scheme (e.g. Basic, Digest, SPNEGO).
> The AuthenticationRouter manages the authentication realms and
> the mapping from endpoints to realms. It is then used by the
> ProcessManager to route requests to the authenticator for the
> realm, if applicable.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
>   3rdparty/libprocess/include/Makefile.am 
> e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/event.hpp 
> 60c615281f3810230bf6c17866f46eaa6855ca29 
>   3rdparty/libprocess/include/process/http.hpp 
> 90c9be122ee0c402b806d70fc818e3c03b15101a 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fb9bd04832779ac43151f2feb3dfbf58cb996434 
>   3rdparty/libprocess/src/authentication_router.hpp PRE-CREATION 
>   3rdparty/libprocess/src/authentication_router.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 
> 7abdf21a5784920251c3627f9820c12fdc356c6e 
> 
> Diff: https://reviews.apache.org/r/37999/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 40539: Removed socket from HttpEvent.

[Ben Mahler]

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

Ship it!


- Ben Mahler


On Nov. 20, 2015, 12:34 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40539/
> ---
> 
> (Updated Nov. 20, 2015, 12:34 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The socket was passed so ProcessBase could access the HttpProxy asociated with
> the socket every time the event was visited. This lead to HTTP pipelining in
> certain circumstances. Once the bug was fixed there was no more use for the
> socket being attached to the HttpEvent.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> 60c615281f3810230bf6c17866f46eaa6855ca29 
>   3rdparty/libprocess/src/process.cpp 
> 7abdf21a5784920251c3627f9820c12fdc356c6e 
> 
> Diff: https://reviews.apache.org/r/40539/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 40532: WIP: Added notion of evictable task to RunTaskMessage.

[Guangya Liu]

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

(Updated 十一月 20, 2015, 1 p.m.)


Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Added notion of evictable task to RunTaskMessage.


Diffs (updated)
-

  src/messages/messages.proto 5714087940a4bd252a649f8a1ab0194467b0db14 

Diff: https://reviews.apache.org/r/40532/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 40529: WIP: Added helper function to get stateless resources.

[Guangya Liu]

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

(Updated 十一月 20, 2015, 12:59 p.m.)


Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Added helper function to get stateless resources.


Diffs (updated)
-

  include/mesos/resources.hpp ce12a26e9cc9057d6612cf2380c07be959e2990b 
  include/mesos/v1/resources.hpp 92db123507ba3442d475c5d3bb7e1a375cf5c1b2 
  src/common/resources.cpp f1da3237724b3766a5df1cb0a4c0159fb9098e01 
  src/tests/resources_tests.cpp 0d084fd97ec108d5ec2d050eddc2e80ea81ffac0 
  src/v1/resources.cpp 6f50101538cf30ebb5a8022558108f103d62a44c 

Diff: https://reviews.apache.org/r/40529/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 40469: Update Allocator interface to support dynamic roles

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40431, 40469]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 2:02 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40469/
> ---
> 
> (Updated Nov. 20, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos, Adam B, Guangya Liu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3956
> https://issues.apache.org/jira/browse/MESOS-3956
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update Allocator interface to support dynamic roles
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp f76118bbf028610c330cebe937d81457fc67a6f3 
>   src/master/allocator/mesos/allocator.hpp 
> d2d32af227d66c4030becd4cd64b907a70d25f49 
>   src/master/allocator/mesos/hierarchical.hpp 
> 64ccf4164197e59d93d739fa2afbdee2cc2a1d23 
>   src/master/allocator/mesos/hierarchical.cpp 
> f2e3b639f210eb06c70584ee7294609d9fd978ad 
>   src/tests/allocator.hpp e0cb2e75e6cac41ae8d8ed1608f1d64e7c533e34 
> 
> Diff: https://reviews.apache.org/r/40469/diff/
> 
> 
> Testing
> ---
> 
> Make check successfully.
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 40291: Added the stye guideline for blank line after line-wrapping.

[Joerg Schad]

> On Nov. 19, 2015, 7:06 p.m., Neil Conway wrote:
> > docs/c++-style-guide.md, line 118
> > 
> >
> > The point is that the for loop condition should be defined over 
> > multiple lines, right?

Yes


> On Nov. 19, 2015, 7:06 p.m., Neil Conway wrote:
> > docs/c++-style-guide.md, line 114
> > 
> >
> > Why the <> in  ?

It is just to indicate that it is a ver long line


- Joerg


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


On Nov. 16, 2015, 5:32 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40291/
> ---
> 
> (Updated Nov. 16, 2015, 5:32 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the stye guideline for blank line after line-wrapping.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/40291/diff/
> 
> 
> Testing
> ---
> 
> viewed rendered doc.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 39614: Quota: Added Status Validation Tests.

[Joerg Schad]

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

(Updated Nov. 20, 2015, 10:05 a.m.)


Review request for mesos, Alexander Rukletsov and Bernd Mathiske.


Changes
---

Correct ordering for expect call.


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


Repository: mesos


Description
---

Quota: Added Status Validation Tests.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/39614/diff/


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 39223: Added Quota Request Validation Tests.

[Joerg Schad]

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

(Updated Nov. 20, 2015, 9:49 a.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
Remoortere.


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


Repository: mesos


Description
---

see Summary.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/39223/diff/


Testing
---

make check


Thanks,

Joerg Schad

Review Request 40532: WIP: Added notion of evictable task to RunTaskMessage.

[Guangya Liu]

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

Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Added notion of evictable task to RunTaskMessage.


Diffs
-

  src/messages/messages.proto 5714087940a4bd252a649f8a1ab0194467b0db14 

Diff: https://reviews.apache.org/r/40532/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 40529: WIP: Added helper function to get stateless resources.

[Guangya Liu]

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

(Updated 十一月 20, 2015, 11:51 a.m.)


Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Added helper function to get stateless resources.


Diffs (updated)
-

  include/mesos/resources.hpp ce12a26e9cc9057d6612cf2380c07be959e2990b 
  include/mesos/v1/resources.hpp 92db123507ba3442d475c5d3bb7e1a375cf5c1b2 
  src/common/resources.cpp f1da3237724b3766a5df1cb0a4c0159fb9098e01 
  src/tests/resources_tests.cpp 0d084fd97ec108d5ec2d050eddc2e80ea81ffac0 
  src/v1/resources.cpp 6f50101538cf30ebb5a8022558108f103d62a44c 

Diff: https://reviews.apache.org/r/40529/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 40524: WIP: Enabled resources.cpp and resources.hpp use std::string

[Guangya Liu]

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

(Updated 十一月 20, 2015, 11:51 a.m.)


Review request for mesos and Klaus Ma.


Summary (updated)
-

WIP: Enabled resources.cpp and resources.hpp use std::string


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


Repository: mesos


Description (updated)
---

Enabled resources.cpp and resources.hpp use std::string


Diffs (updated)
-

  include/mesos/resources.hpp ce12a26e9cc9057d6612cf2380c07be959e2990b 
  include/mesos/v1/resources.hpp 92db123507ba3442d475c5d3bb7e1a375cf5c1b2 
  src/common/resources.cpp f1da3237724b3766a5df1cb0a4c0159fb9098e01 
  src/v1/resources.cpp 6f50101538cf30ebb5a8022558108f103d62a44c 

Diff: https://reviews.apache.org/r/40524/diff/


Testing
---


Thanks,

Guangya Liu

Review Request 40539: Removed socket from HttpEvent.

[Alexander Rojas]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

The socket was passed so ProcessBase could access the HttpProxy asociated with
the socket every time the event was visited. This lead to HTTP pipelining in
certain circumstances. Once the bug was fixed there was no more use for the
socket being attached to the HttpEvent.


Diffs
-

  3rdparty/libprocess/include/process/event.hpp 
60c615281f3810230bf6c17866f46eaa6855ca29 
  3rdparty/libprocess/src/process.cpp 7abdf21a5784920251c3627f9820c12fdc356c6e 

Diff: https://reviews.apache.org/r/40539/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 40339: WIP: Added a flag to master to enable oversubscription for reservations.

[Guangya Liu]

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

(Updated 十一月 20, 2015, 12:59 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Klaus Ma.


Summary (updated)
-

WIP: Added a flag to master to enable oversubscription for reservations.


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


Repository: mesos


Description
---

Added a flag to master to enable optimistic offers.


Diffs (updated)
-

  include/mesos/master/allocator.hpp f76118bbf028610c330cebe937d81457fc67a6f3 
  src/master/allocator/mesos/allocator.hpp 
d2d32af227d66c4030becd4cd64b907a70d25f49 
  src/master/allocator/mesos/hierarchical.hpp 
64ccf4164197e59d93d739fa2afbdee2cc2a1d23 
  src/master/allocator/mesos/hierarchical.cpp 
f2e3b639f210eb06c70584ee7294609d9fd978ad 
  src/master/flags.hpp 5fd5d502697b2edc22ae98a5a8e361bf62bf8bb6 
  src/master/flags.cpp 806e2da6ad37a6acf76818d4c6b3c462175fd09d 
  src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
  src/tests/allocator.hpp e0cb2e75e6cac41ae8d8ed1608f1d64e7c533e34 
  src/tests/hierarchical_allocator_tests.cpp 
740cfa801ee90417c038308192d1f4f2416f8315 
  src/tests/master_allocator_tests.cpp 646cacd3e16b9e0b72c0b259eecf2760cfb530db 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
  src/tests/reservation_tests.cpp ac664ebb49e74aa28551f427ea8f39ac9ce0cfb3 
  src/tests/resource_offers_tests.cpp bf2fe3ac7b982e31d289a703ae637d8e2b3a2d8a 
  src/tests/slave_recovery_tests.cpp 2cc7132deb9b8c324aa9dbab0b81643d07377a89 

Diff: https://reviews.apache.org/r/40339/diff/


Testing
---

Ubuntu 14.04
make
make check


Thanks,

Guangya Liu

Re: Review Request 40524: WIP: Enabled resources.cpp and resources.hpp use std::string

[Guangya Liu]

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

(Updated 十一月 20, 2015, 12:59 p.m.)


Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Enabled resources.cpp and resources.hpp use std::string


Diffs (updated)
-

  include/mesos/resources.hpp ce12a26e9cc9057d6612cf2380c07be959e2990b 
  include/mesos/v1/resources.hpp 92db123507ba3442d475c5d3bb7e1a375cf5c1b2 
  src/common/resources.cpp f1da3237724b3766a5df1cb0a4c0159fb9098e01 
  src/v1/resources.cpp 6f50101538cf30ebb5a8022558108f103d62a44c 

Diff: https://reviews.apache.org/r/40524/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 40351: Quota: Added rescinding offers for set quota requests.

[Alexander Rukletsov]

> On Nov. 18, 2015, 7:51 a.m., Qian Zhang wrote:
> > src/master/quota_handler.cpp, line 180
> > 
> >
> > Why do we want to rescind the offeres that do not contribute to 
> > satisfying quota request?
> 
> Alexander Rukletsov wrote:
> Because we may rescind more than necessary to satisfy quota request 
> (remember minimal agent count). If we have a check in place, this will 
> effectively prevent us from doing so. Does it make sense to you?
> 
> Qian Zhang wrote:
> Suppose the quota request is to request 20GB disk for a role, and there 
> is an offer which only include 2 CPU & 2GB memory and has no disk resources 
> at all, so we will rescind this offer too? This seems a little unfair to me.
> And can you please clarify a little more about why we want to rescind 
> offers from at least `numF` agents? The reason is that we want to ensure each 
> framework in that role will have a chance to get an offer in next allocation 
> cycle?

That's correct, we will rescind that offer and yes, it's a bit unfair. Let me 
explain why I decided to remove this check. Suppose we a quota request is for 6 
CPUs for role with 3 frameworks. The first offer we rescind is 10 CPUs, 10GB 
MEM. Technically, we have enough resources to satisfy quota, but we would like 
to rescind offers from at least 2 more agents. Having a check in place will 
prevent us from doing so. Do you think greedy rescinding can be a problem?

Yes, we would like to facilitate allocation for each framework in the role, for 
which quota is set.


- Alexander


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


On Nov. 19, 2015, 5:15 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40351/
> ---
> 
> (Updated Nov. 19, 2015, 5:15 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3912
> https://issues.apache.org/jira/browse/MESOS-3912
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 
> 
> Diff: https://reviews.apache.org/r/40351/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40392: Added force flag to override quota capacityHeuristic check.

[Joerg Schad]

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

(Updated Nov. 20, 2015, 9:39 a.m.)


Review request for mesos, Alexander Rukletsov and Joris Van Remoortere.


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


Repository: mesos


Description
---

Added force flag to override quota capacityHeuristic check.


Diffs (updated)
-

  src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 
  src/tests/master_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/40392/diff/


Testing
---

make check.


Thanks,

Joerg Schad

Re: Review Request 39782: Add a comment for os::libraries::setPaths.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39780, 39781, 39782]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 1:27 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39782/
> ---
> 
> (Updated Nov. 20, 2015, 1:27 a.m.)
> 
> 
> Review request for mesos, Kapil Arya and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3725
> https://issues.apache.org/jira/browse/MESOS-3725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a comment for os::libraries::setPaths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467 
> 
> Diff: https://reviews.apache.org/r/39782/diff/
> 
> 
> Testing
> ---
> 
> No code changes.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 39449: Documented order of includes.

[Jan Schlicht]

> On Nov. 3, 2015, 10:53 p.m., Joerg Schad wrote:
> > docs/c++-style-guide.md, line 251
> > 
> >
> > Could we add a short comment above every new section describing the 
> > representative meaning of each? (e.g. here nested subfolder)

IMHO this could get quite redundant if we do this for every source file.


- Jan


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


On Oct. 19, 2015, 11:29 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Oct. 19, 2015, 11:29 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 37999: Introduced an Authenticator interface and an AuthenticationRouter in libprocess.

[Alexander Rojas]

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

(Updated Nov. 20, 2015, 1:53 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Removes actual authentication logic from `ProcessManager::handle()` until 
delivering pipelining to processes is cleared.


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


Repository: mesos


Description
---

The Authenticator interface allows us to implement different
authenticators based on the scheme (e.g. Basic, Digest, SPNEGO).
The AuthenticationRouter manages the authentication realms and
the mapping from endpoints to realms. It is then used by the
ProcessManager to route requests to the authenticator for the
realm, if applicable.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/Makefile.am 
e6be2c4db121585bbe7f3c0627de048adc7cfb2c 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
60c615281f3810230bf6c17866f46eaa6855ca29 
  3rdparty/libprocess/include/process/http.hpp 
90c9be122ee0c402b806d70fc818e3c03b15101a 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authentication_router.hpp PRE-CREATION 
  3rdparty/libprocess/src/authentication_router.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 7abdf21a5784920251c3627f9820c12fdc356c6e 

Diff: https://reviews.apache.org/r/37999/diff/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 40392: Added force flag to override quota capacityHeuristic check.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39211, 39018, 39102, 36913, 38059, 39285, 38110, 40342, 
40351, 38956, 40396, 39223, 40392]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 9:39 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40392/
> ---
> 
> (Updated Nov. 20, 2015, 9:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3911
> https://issues.apache.org/jira/browse/MESOS-3911
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added force flag to override quota capacityHeuristic check.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40392/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40291: Added the stye guideline for blank line after line-wrapping.

[Joerg Schad]

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

(Updated Nov. 20, 2015, 4:55 p.m.)


Review request for mesos, Bernd Mathiske and Neil Conway.


Repository: mesos


Description
---

Added the stye guideline for blank line after line-wrapping.


Diffs (updated)
-

  docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 

Diff: https://reviews.apache.org/r/40291/diff/


Testing
---

viewed rendered doc.


Thanks,

Joerg Schad

Review Request 40456: MESOS-3950: show running task count in web ui

[Ian Babrou]

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

Review request for mesos.


Repository: mesos


Description
---

MESOS-3950: show running task count in web ui


Diffs
-

  src/webui/master/static/home.html 4fc64c0aea558fca18083dc317f8370670d7a4d3 
  src/webui/master/static/js/controllers.js 
ccf5c31715e298e96f493cce58921bfe6b16b779 
  src/webui/master/static/slave.html a1446bce827944609faf10f72e788f33d275d6f9 

Diff: https://reviews.apache.org/r/40456/diff/


Testing
---


Thanks,

Ian Babrou

Re: Review Request 40345: [1/4] Quota Authorization: Added "SetQuota" message to ACL protobuf.

[Alexander Rojas]

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



include/mesos/authorizer/authorizer.proto (line 74)


Can you follow `ShutdownFramework` example and add a one to two lines 
comment of why is it here?


- Alexander Rojas


On Nov. 20, 2015, 11:02 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40345/
> ---
> 
> (Updated Nov. 20, 2015, 11:02 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
> Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Added "RequestQuota" message to ACL protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
> 
> Diff: https://reviews.apache.org/r/40345/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 40345: [1/4] Quota Authorization: Added "SetQuota" message to ACL protobuf.

[Jan Schlicht]

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

(Updated Nov. 20, 2015, 5:13 p.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

Quota: Added "SetQuota" message to ACL protobuf.


Diffs
-

  include/mesos/authorizer/authorizer.proto 
86bbb45f9d91b4098a262e3e50a793f3bb39497e 

Diff: https://reviews.apache.org/r/40345/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40345: [1/4] Quota Authorization: Added "SetQuota" message to ACL protobuf.

[Jan Schlicht]

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

(Updated Nov. 20, 2015, 4:52 p.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Address issues.


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


Repository: mesos


Description
---

Quota: Added "RequestQuota" message to ACL protobuf.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
86bbb45f9d91b4098a262e3e50a793f3bb39497e 

Diff: https://reviews.apache.org/r/40345/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40345: [1/4] Quota Authorization: Added "SetQuota" message to ACL protobuf.

[Alexander Rojas]

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

Ship it!


Ship It!

- Alexander Rojas


On Nov. 20, 2015, 4:52 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40345/
> ---
> 
> (Updated Nov. 20, 2015, 4:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
> Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Added "RequestQuota" message to ACL protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 86bbb45f9d91b4098a262e3e50a793f3bb39497e 
> 
> Diff: https://reviews.apache.org/r/40345/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 40501: Cleanup a leaked reference to a test process living in the stack.

[Benjamin Bannier]

> On Nov. 20, 2015, 2:16 a.m., Neil Conway wrote:
> > Good find. I wonder:
> > 
> > (a) is there some general advice we should give to people implementing 
> > Processes (e.g., "always provide a destructor that does terminate/wait" -- 
> > that is probably too broad though). Would be nice to add this to the 
> > libprocess README.
> > (b) does this problem occur anywhere else? and/or is there a way to detect 
> > it?
> 
> Bernd Mathiske wrote:
> [~neilc] a) I agree that we should provide documentation in this regard. 
> This kind of pattern is too easily overlooked, also by reviewers, as 
> exemplified when the bug got introduced: https://reviews.apache.org/r/27483/
> b) For test code (such as is the case here), we could put something into 
> the inherited fixture that scans for orphaned processes at TearDown.

Note that there is some assymmetry in the API: when *not giving* a `manage` arg 
to `spawn`. you get the default value `false`; but you then *do need to add 
code* to terminate and wait for the process.

I personally would naively have expected an API to either (i) require me to be 
explicitly in both places (explictly set `manage=false`, and to manual 
`terminate` and `wait`), or (ii) it just do The Right Thing if I was brief (I 
called `spawn` with default args, and do not have to worry about cleanup).

I.e., wouldn't it in the long run make more sense the change `spawn` to `PID 
spawn(T& t, bool manage = true)` than to add more documentation?


- Benjamin


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


On Nov. 19, 2015, 9:51 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40501/
> ---
> 
> (Updated Nov. 19, 2015, 9:51 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3753
> https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Two of the fetcher tests will spawn a process which is stored in the stack 
> (i.e. local variable in the test).  `spawn` will store a pointer to the 
> process in libprocess's `ProcessManager`.  When the test finishes, the 
> process goes out of scope and is therefore lost.  However, the process is 
> **not** terminated.
> 
> Failing to terminate this process will lead to an infinite loop in 
> `~ProcessManager`, which is called in `process::finalize`.  In 
> `ProcessManager` 's destructor, we will loop and try to kill all processes.  
> The process spawned in the test will be running.  However, since the pointer 
> lives in the stack, the `ProcessManager` will be unable to find the process 
> and will thereby be stuck trying to kill a process it cannot find.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 04079964b3539f555351d1444f3635c64700a1a8 
> 
> Diff: https://reviews.apache.org/r/40501/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Additional testing:
> 
> Insert a `process::finalize` in `src/test/main.cpp`.  i.e.
> ```
>   // Replace `return RUN_ALL_TESTS();` with this:
>   int ret = RUN_ALL_TESTS();
>   process::finalize();
>   return ret;
> ```
> 
> Then `make check 
> GTEST_FILTER="*FetcherTest.OSNetUriTest*:*FetcherTest.OSNetUriSpaceTest*"`.
> The test program should not stall or segfault or abort in some weird way.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40531: Added the public Mesos events calendar to the Community page.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40531]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 7:40 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40531/
> ---
> 
> (Updated Nov. 20, 2015, 7:40 a.m.)
> 
> 
> Review request for mesos and Dave Lester.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   site/source/community.html.md 81b8d3d02739fb624b599e002df6bc5274381453 
> 
> Diff: https://reviews.apache.org/r/40531/diff/
> 
> 
> Testing
> ---
> 
> Used `support/site-docker` to render the calendar locally.
> 
> 
> File Attachments
> 
> 
> Screen Shot 2015-11-19 at 11.34.17 PM.png
>   
> https://reviews.apache.org/media/uploaded/files/2015/11/20/b39ee774-96f7-4afb-b6db-adcfa8bab9aa__Screen_Shot_2015-11-19_at_11.34.17_PM.png
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 40546: MESOS-3972: fix framework cpu counters on slave page

[Ian Babrou]

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

Review request for mesos.


Repository: mesos


Description
---

MESOS-3972: fix framework cpu counters on slave page


Diffs
-

  src/webui/master/static/slave.html a1446bce827944609faf10f72e788f33d275d6f9 

Diff: https://reviews.apache.org/r/40546/diff/


Testing
---


Thanks,

Ian Babrou

Re: Review Request 40497: Add hex number support to numify()

[Benjamin Bannier]

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



3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp (line 30)


I believe giving these proper names would make e.g., the ordering here much 
easier to grasp.

What about `template  ..`?

Also space after `template`.



3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp (line 39)


Where you planning to output `c` here? Otherwise I suggest replacing this 
at least `ss.get()` (and drop the decl above).

I like H.Sutter's synopsis of `boost::lexical_cast` even better since it 
shows clearly the failure sites (modulo the throw for here), see 
http://www.gotw.ca/publications/mill19.htm:

template
Target lexical_cast(Source arg)
{
  std::stringstream interpreter;
  Target result;

  if(!(interpreter << arg) ||
 !(interpreter >> result) ||
 !(interpreter >> std::ws).eof())
throw bad_lexical_cast();

  return result;
}

You could do a similar impl just by injection a `<< std::hex`.



3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp (line 40)


I understand how the error string landed here from `numify`, but as written 
hardly anything about `hex_cast` is concerned with numbers (e.g., 
`hex_cast(1.2)` is perfectly fine).

You should probably make it more general here, and specialize the error 
message at the call site in `numify` instead.



3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp (line 51)


With the implementation you added, what keeps us from replacing this with 
`return hex_cast(s)` and dropping the Boost `#include` to simplify the deps?



3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp (line 26)


Would be nice to add a couple more positives here, e.g., to catch 
`hex_cast` impls like `boost::lexical_cast(s.substr(2)` ;)


- Benjamin Bannier


On Nov. 19, 2015, 10:56 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40497/
> ---
> 
> (Updated Nov. 19, 2015, 10:56 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> numify() in libprocess uses boost lexical_cast() to cast numbers, but it can 
> not cast a hex number. This patch adds hex support to numify().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5a0ffc38d4194b9f9c53dc1cf0c90ca7bbae2afd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> ddd3dd9e015c583e04d72ac9a9b5a5ed6f1d49d5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 14fb644b38a5cbb8cde74aab39e84305f6ab7041 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40497/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 40292: Added style guideline for writing numbers to markdown styleguide.

[Joerg Schad]

> On Nov. 17, 2015, 8:18 a.m., Alexander Rukletsov wrote:
> > What is the motivation for this? Are there any studies proving it 
> > facilitates preception?
> 
> Joerg Schad wrote:
> Motivation is the current inconsistent writing of numbers e.g. in the c++ 
> styleguide.
> 
> Alexander Rukletsov wrote:
> Sorry for not being clear. My question is more "why do you propose such 
> scheme?". For me it feels inconsistent to have both figures and spelled out 
> numbers. My intuition is that such rule is harder to remember than simple 
> ones like "use figures everywhere" or "always spell out". I assume you base 
> your proposal on common practices in american? british? English or studies 
> around human perception which are unknown to me as an ESL : ).
> 
> Alexander Rukletsov wrote:
> It looks like there are guideline on spelling out numbers versus using 
> literals, though they are not consistent:
> http://www.grammarbook.com/numbers/numbers.asp
> http://www.dailywritingtips.com/10-rules-for-writing-numbers-and-numerals/
> 
> Another question I have, is your proposal related to docs only, or for 
> code comments as well? Though I do think comments should be proper text, I'm 
> not sure they should be fiction and would rather keep only numerals there.

This is in the markdown styleguide and hence only applies to documentation.


- Joerg


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


On Nov. 17, 2015, 8:07 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40292/
> ---
> 
> (Updated Nov. 17, 2015, 8:07 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added style guideline for writing numbers to markdown styleguide.
> 
> 
> Diffs
> -
> 
>   docs/markdown-style-guide.md fe9ffefbbb2c0ce9ade39cec1de073163a9a03b2 
> 
> Diff: https://reviews.apache.org/r/40292/diff/
> 
> 
> Testing
> ---
> 
> viewed rendered version.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40507: Cleanup leaked containerizer and potentially orphaned process in SlaveTest.LaunchTaskInfoWithContainerInfo.

[Gilbert Song]

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

Ship it!


Ship It!

- Gilbert Song


On Nov. 19, 2015, 3:38 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40507/
> ---
> 
> (Updated Nov. 19, 2015, 3:38 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3753
> https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Normally, the containerizer would be cleaned and deallocated by one of the 
> helper methods in `MesosTest` or in `test::Cluster`.  However, in this case, 
> the containerizer is allocated via `MesosContainerizer::create` and then 
> passed to `MockSlave`.  `MockSlave` does not clean up the containerizer upon 
> destruction.
> 
> Note: `MockSlave` normally takes a reference to a containerizer allocated on 
> the stack, hence why it does not deallocate.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 7c9dcc6186a8cccb0eb30ff59914a41961e47293 
> 
> Diff: https://reviews.apache.org/r/40507/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Note: If you **only** add `delete containerizer.get();` to the end of the 
> test, the test suite will output:
> ```
> [--] Global test environment tear-down
> ../../src/tests/environment.cpp:488: Failure
> Failed
> Tests completed with child processes remaining:
> -+- 29543 /path/to/mesos/build/src/.libs/mesos-tests
>  --- 29558 /bin/sh /path/to/mesos/build/src/mesos-containerizer launch 
> --command={"shell":true,"value":"\/path\/to\/mesos\/build\/src\/mesos-executor"}
>  --commands={"commands":[]} --directory=/tmp --help=false --pipe_read=13 
> --pipe_write=16 --user=test
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40501: Cleanup a leaked reference to a test process living in the stack.

[Joseph Wu]

> On Nov. 19, 2015, 6:16 p.m., Neil Conway wrote:
> > Good find. I wonder:
> > 
> > (a) is there some general advice we should give to people implementing 
> > Processes (e.g., "always provide a destructor that does terminate/wait" -- 
> > that is probably too broad though). Would be nice to add this to the 
> > libprocess README.
> > (b) does this problem occur anywhere else? and/or is there a way to detect 
> > it?
> 
> Bernd Mathiske wrote:
> [~neilc] a) I agree that we should provide documentation in this regard. 
> This kind of pattern is too easily overlooked, also by reviewers, as 
> exemplified when the bug got introduced: https://reviews.apache.org/r/27483/
> b) For test code (such as is the case here), we could put something into 
> the inherited fixture that scans for orphaned processes at TearDown.
> 
> Benjamin Bannier wrote:
> Note that there is some assymmetry in the API: when *not giving* a 
> `manage` arg to `spawn`. you get the default value `false`; but you then *do 
> need to add code* to terminate and wait for the process.
> 
> I personally would naively have expected an API to either (i) require me 
> to be explicitly in both places (explictly set `manage=false`, and to manual 
> `terminate` and `wait`), or (ii) it just do The Right Thing if I was brief (I 
> called `spawn` with default args, and do not have to worry about cleanup).
> 
> I.e., wouldn't it in the long run make more sense the change `spawn` to 
> `PID spawn(T& t, bool manage = true)` than to add more documentation?

[~neilc] 
a) I agree that documentation is a good direction.   (I'll draft something.)

[~bernd-mesos]
b) We already have an orphaned process detector in `src/tests/environment.cpp` 
`Environment::TearDown`.  However, this only catches **orphaned** processes 
(whose parents have been killed :( ).  This HTTP process's parent is the test 
process.
There is an endpoint `/__processes__` which we might be able to use to check 
for extraneous processes.  But this might be messy, since we have a great deal 
of global processes.

[~bbannier]
The `manage` argument will only change ownership of the process pointer.  If 
you dig into the `GarbageCollector`, you'll see that all it does is `delete` 
the pointer when the process terminates.  The original process is still in 
charge of its own termination.


- Joseph


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


On Nov. 19, 2015, 1:51 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40501/
> ---
> 
> (Updated Nov. 19, 2015, 1:51 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3753
> https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Two of the fetcher tests will spawn a process which is stored in the stack 
> (i.e. local variable in the test).  `spawn` will store a pointer to the 
> process in libprocess's `ProcessManager`.  When the test finishes, the 
> process goes out of scope and is therefore lost.  However, the process is 
> **not** terminated.
> 
> Failing to terminate this process will lead to an infinite loop in 
> `~ProcessManager`, which is called in `process::finalize`.  In 
> `ProcessManager` 's destructor, we will loop and try to kill all processes.  
> The process spawned in the test will be running.  However, since the pointer 
> lives in the stack, the `ProcessManager` will be unable to find the process 
> and will thereby be stuck trying to kill a process it cannot find.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 04079964b3539f555351d1444f3635c64700a1a8 
> 
> Diff: https://reviews.apache.org/r/40501/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Additional testing:
> 
> Insert a `process::finalize` in `src/test/main.cpp`.  i.e.
> ```
>   // Replace `return RUN_ALL_TESTS();` with this:
>   int ret = RUN_ALL_TESTS();
>   process::finalize();
>   return ret;
> ```
> 
> Then `make check 
> GTEST_FILTER="*FetcherTest.OSNetUriTest*:*FetcherTest.OSNetUriSpaceTest*"`.
> The test program should not stall or segfault or abort in some weird way.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40247: Added HTTP endpoints for creating and destroying persistent volumes.

[Neil Conway]

> On Nov. 19, 2015, 7:09 p.m., Michael Park wrote:
> > docs/persistent-volume.md, lines 255-280
> > 
> >
> > This looks to be formatted weird, could you double check? Here and below

I previewed with mesos-website-container and it seems fine to me -- what 
weirdness did you notice?


- Neil


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


On Nov. 18, 2015, 11:28 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40247/
> ---
> 
> (Updated Nov. 18, 2015, 11:28 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP endpoints for creating and destroying persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
>   docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
>   src/tests/mesos.hpp b3f69ccb9870b17a335a2fe7dbf2802c1b709e6b 
>   src/tests/persistent_volume_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40247/diff/
> 
> 
> Testing
> ---
> 
> (1) make check, including newly added tests
> 
> (2) Manually created/removed persistent volumes via HTTP endpoints + curl.
> 
> (3) Previewed docs in Github gist.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39449: Documented order of includes.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39449]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 11:17 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Nov. 20, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 40339: WIP: Added a flag to master to enable oversubscription for reservations.

[Joseph Wu]

> On Nov. 19, 2015, 9:59 a.m., Joseph Wu wrote:
> > Overall notes:
> > * Looks like everything is in place (to my knowledge) for this change :)
> > * (Mentioned in the last working group sync) We **may** want to rename the 
> > feature to something else, like "oversubscription for reservations".
> 
> Guangya Liu wrote:
> Agree, I will update the name to "oversubscription for reservations"

Sorry!  I should have been more explicit that I didn't want you to rename the 
variable.  (At least, not until the working group has nailed down the 
public-facing name for this feature.)

You should probably stick with the original variable name in the meantime.


- Joseph


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


On Nov. 20, 2015, 4:59 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40339/
> ---
> 
> (Updated Nov. 20, 2015, 4:59 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Klaus 
> Ma.
> 
> 
> Bugs: MESOS-3887
> https://issues.apache.org/jira/browse/MESOS-3887
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a flag to master to enable optimistic offers.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp f76118bbf028610c330cebe937d81457fc67a6f3 
>   src/master/allocator/mesos/allocator.hpp 
> d2d32af227d66c4030becd4cd64b907a70d25f49 
>   src/master/allocator/mesos/hierarchical.hpp 
> 64ccf4164197e59d93d739fa2afbdee2cc2a1d23 
>   src/master/allocator/mesos/hierarchical.cpp 
> f2e3b639f210eb06c70584ee7294609d9fd978ad 
>   src/master/flags.hpp 5fd5d502697b2edc22ae98a5a8e361bf62bf8bb6 
>   src/master/flags.cpp 806e2da6ad37a6acf76818d4c6b3c462175fd09d 
>   src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
>   src/tests/allocator.hpp e0cb2e75e6cac41ae8d8ed1608f1d64e7c533e34 
>   src/tests/hierarchical_allocator_tests.cpp 
> 740cfa801ee90417c038308192d1f4f2416f8315 
>   src/tests/master_allocator_tests.cpp 
> 646cacd3e16b9e0b72c0b259eecf2760cfb530db 
>   src/tests/reservation_endpoints_tests.cpp 
> 1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
>   src/tests/reservation_tests.cpp ac664ebb49e74aa28551f427ea8f39ac9ce0cfb3 
>   src/tests/resource_offers_tests.cpp 
> bf2fe3ac7b982e31d289a703ae637d8e2b3a2d8a 
>   src/tests/slave_recovery_tests.cpp 2cc7132deb9b8c324aa9dbab0b81643d07377a89 
> 
> Diff: https://reviews.apache.org/r/40339/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 14.04
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 40532: WIP: Added notion of evictable task to RunTaskMessage.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40339, 40524, 40529, 40532]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 1 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40532/
> ---
> 
> (Updated Nov. 20, 2015, 1 p.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-3890
> https://issues.apache.org/jira/browse/MESOS-3890
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added notion of evictable task to RunTaskMessage.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto 5714087940a4bd252a649f8a1ab0194467b0db14 
> 
> Diff: https://reviews.apache.org/r/40532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39948: Remove some undocumented, commented-out code within libprocess.

[Joseph Wu]

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

(Updated Nov. 20, 2015, 11:16 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

These are remnants of commented-out code, dating back to when libprocess was 
originally open-sourced.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 8fa0594b671969c2713d1b361f2dbbb07d25b3a8 

Diff: https://reviews.apache.org/r/39948/diff/


Testing
---

None.  Comment change only.


Thanks,

Joseph Wu

Re: Review Request 40461: Changed HDFS wrapper from a struct to a class.

[Jie Yu]

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

(Updated Nov. 20, 2015, 7:23 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Changed HDFS wrapper from a struct to a class.


Diffs
-

  src/hdfs/hdfs.hpp 6604938e8ca1962db1f0159d175f52fd5c03dd3c 

Diff: https://reviews.apache.org/r/40461/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40461: Changed HDFS wrapper from a struct to a class.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 20, 2015, 7:23 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40461/
> ---
> 
> (Updated Nov. 20, 2015, 7:23 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3951
> https://issues.apache.org/jira/browse/MESOS-3951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed HDFS wrapper from a struct to a class.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp 6604938e8ca1962db1f0159d175f52fd5c03dd3c 
> 
> Diff: https://reviews.apache.org/r/40461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 40293: Applied consistent number syle in c++ styleguide.

[Joerg Schad]

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

(Updated Nov. 20, 2015, 8:42 p.m.)


Review request for mesos and Bernd Mathiske.


Repository: mesos


Description (updated)
---

Applied consistent number style in c++ styleguide.


Diffs
-

  docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 

Diff: https://reviews.apache.org/r/40293/diff/


Testing
---

viewed rendered version.


Thanks,

Joerg Schad

Re: Review Request 39949: Document and simplify libprocess initialization synchronization logic.

[Joseph Wu]

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

(Updated Nov. 20, 2015, 11:16 a.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, and Neil 
Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

* Renamed `initialized` to `initialize_started`.
* Renamed `initializing` to `initialize_complete`.
* Removed the (2) condition, described below: 

The initialization synchronization logic contains three conditions, which check:
1) Was `initialize` called and is it done?
2) Was `initialize` called and is it not done?
3) Are you the first to call `initialize`?

Condition (3) uses `compare_exchange_strong` between `initialized` and `false`. 
 This returns `true` (and sets `initialized` to true) iff the caller is the 
first to reach that expression.

The second simultaneous caller of `initialize` will either satisify condition 
(2) or (3) and then wait on `initializing`.  For the second caller, (2) and (3) 
are identical because `compare_exchange_strong` between `true` and `false` will 
always return false, thereby putting the second caller into the waiting loop.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 8fa0594b671969c2713d1b361f2dbbb07d25b3a8 

Diff: https://reviews.apache.org/r/39949/diff/


Testing
---

`make check`

Replaced `process::initialize();` in `3rdparty/libprocess/src/tests/main.cpp` 
with:
```

  const size_t numThreads = 50;

  std::thread* runningThreads[numThreads];

  // Create additional threads.
  for (size_t i = 0; i < numThreads; i++) {
runningThreads[i] = new std::thread([]() {
  process::initialize();
});
  }

  for (size_t i = 0; i < numThreads; i++) {
runningThreads[i]->join();
delete runningThreads[i];
  }
```
(Also added `#include ` to the header).

Rebuilt `libprocess-tests` with the modification and ran it a few times.
`3rdparty/libprocess/libprocess-tests`


Thanks,

Joseph Wu

Review Request 40264: Libprocess Reinitialization: Implement Clock::finalize for cleaning up timers

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


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


Repository: mesos


Description
---

Timers may deference `process_manager`, either directly or via the associated 
lambda, after `process_manager` has been freed in `process::finalize`.  
Cleaning up the `Clock` appropriately in `process::finalize` prevents that race.


Diffs
-

  3rdparty/libprocess/include/process/clock.hpp 
1107a329caf77f15901d87808eee72818601510c 
  3rdparty/libprocess/src/clock.cpp 58060985bf8ab8a1bbd78b687b4c96836e13b86a 
  3rdparty/libprocess/src/process.cpp 8fa0594b671969c2713d1b361f2dbbb07d25b3a8 

Diff: https://reviews.apache.org/r/40264/diff/


Testing
---

`make check` (--enable-libevent --enable-ssl)


Thanks,

Joseph Wu

Re: Review Request 40268: Libprocess Reinitialization: Change Socket::DEFAULT_KIND to return a non-static local value.

[Joseph Wu]

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

(Updated Nov. 20, 2015, 11:29 a.m.)


Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


Changes
---

Re-ordered this review.


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


Repository: mesos


Description
---

This is only needed for tests that utilize the test-only 
`process::reinitialize` function.


Diffs (updated)
-

  3rdparty/libprocess/include/process/socket.hpp 
ebee78909feb5a4032da68f51d08dbf11b03b332 
  3rdparty/libprocess/src/socket.cpp 6ac834e7459f5958b7c788ccdc60cbed90530183 

Diff: https://reviews.apache.org/r/40268/diff/


Testing
---

Some test cleanup issues were uncovered by this change; fixed here: 
https://reviews.apache.org/r/40453/


Thanks,

Joseph Wu

Review Request 40512: Libprocess Reinitialization: Add a test-only method to reinitialize libprocess.

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


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


Repository: mesos


Description
---

This builds upon earlier changes to complete `process::finalize`.  Tests which 
need to reconfigure libprocess, such as some SSL-related tests, can use 
`process::reinitialize` to do so.  

This method may also be useful for providing additional isolation between tests.


Diffs
-

  3rdparty/libprocess/src/openssl.cpp a2b7ab4e7ec92cc690f173134a7e8661cda77179 
  3rdparty/libprocess/src/process.cpp 8fa0594b671969c2713d1b361f2dbbb07d25b3a8 

Diff: https://reviews.apache.org/r/40512/diff/


Testing
---

Defined `process::reinitialize` in several tests, such as `HTTPTest`, 
`MetricsTest`, and `ReapTest` can called `process::reinitialize` before and 
after tests.  Confirmed that this did not have side effects on other tests (See 
https://reviews.apache.org/r/40453/ ).


Thanks,

Joseph Wu

Re: Review Request 40462: Fixed the license header in hdfs.hpp.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 19, 2015, 12:26 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40462/
> ---
> 
> (Updated Nov. 19, 2015, 12:26 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3951
> https://issues.apache.org/jira/browse/MESOS-3951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the license header in hdfs.hpp.
> 
> 
> Diffs
> -
> 
>   src/hdfs/hdfs.hpp 6604938e8ca1962db1f0159d175f52fd5c03dd3c 
> 
> Diff: https://reviews.apache.org/r/40462/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 40513: [DO NOT COMMIT] Parameterizes existing scheduler tests to run with varieties of SSL enabled.

[Joseph Wu]

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

Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Joris Van 
Remoortere.


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


Repository: mesos


Description
---

Changes the parameterization of the `SchedulerTests` to include different SSL 
configurations.  Libprocess and SSL are reinitialized in order for the 
different configurations to show up on newly launched masters.

This review adds a basic SSL configuration.

NOTE: This should be committed while/after the related components in the HTTP 
Scheduler library are fixed.


Diffs
-

  src/tests/scheduler_tests.cpp b61f5a0dc9e56ecbc98188c6361d03cd5c1db667 

Diff: https://reviews.apache.org/r/40513/diff/


Testing
---

The new tests fail because the underlying HTTP Scheduler library is hard-coded 
to use `http`.


Thanks,

Joseph Wu

Re: Review Request 40463: Moved HDFS wrapper implementation to a cpp file.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 19, 2015, 12:27 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40463/
> ---
> 
> (Updated Nov. 19, 2015, 12:27 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3951
> https://issues.apache.org/jira/browse/MESOS-3951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved HDFS wrapper implementation to a cpp file.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/hdfs/hdfs.hpp 6604938e8ca1962db1f0159d175f52fd5c03dd3c 
>   src/hdfs/hdfs.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40463/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 40557: Add documentation about using terminate/wait on Processes when deallocating them.

[Joseph Wu]

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

Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Neil Conway.


Repository: mesos


Description
---

Based on a discussion here: https://reviews.apache.org/r/40501/#review107290


Diffs
-

  3rdparty/libprocess/README.md c3f309a7dc1c94882c4cc97eeaf0736c2fca0ba5 

Diff: https://reviews.apache.org/r/40557/diff/


Testing
---

Checked README.md rendering on GitHub.


Thanks,

Joseph Wu

Review Request 40413: Libprocess Reinitialization: Move ReaperProcess instantiation into process::intialize.

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


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


Repository: mesos


Description
---

This must be unified with `process::initialize` so that it also falls under the 
scope of reinitialization.


Diffs
-

  3rdparty/libprocess/include/process/reap.hpp 
ca5acc4c8f5a62a49b7fde83946e283ea40baa65 
  3rdparty/libprocess/src/process.cpp 8fa0594b671969c2713d1b361f2dbbb07d25b3a8 
  3rdparty/libprocess/src/reap.cpp 9a994b052c7920a16557c3d880ad499a5efd43cb 

Diff: https://reviews.apache.org/r/40413/diff/


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu

Review Request 40411: Libprocess Reinitialization: Modify test to use PID instead of a process pointer.

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


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


Repository: mesos


Description
---

A related function return type was changed in the previous commit.


Diffs
-

  src/tests/scheduler_driver_tests.cpp 4be2f72839365cea1893cbf21b6bd18cb2a8fcce 

Diff: https://reviews.apache.org/r/40411/diff/


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu

Re: Review Request 40546: MESOS-3972: fix framework cpu counters on slave page

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Nov. 20, 2015, 5 p.m., Ian Babrou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40546/
> ---
> 
> (Updated Nov. 20, 2015, 5 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3972: fix framework cpu counters on slave page
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/slave.html a1446bce827944609faf10f72e788f33d275d6f9 
> 
> Diff: https://reviews.apache.org/r/40546/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Babrou
> 
>

Re: Review Request 40498: Used factory method to create HDFS client.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 19, 2015, 8:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40498/
> ---
> 
> (Updated Nov. 19, 2015, 8:22 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-3951
> https://issues.apache.org/jira/browse/MESOS-3951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used factory method to create HDFS client.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc 
>   src/hdfs/hdfs.hpp 6604938e8ca1962db1f0159d175f52fd5c03dd3c 
>   src/hdfs/hdfs.cpp PRE-CREATION 
>   src/launcher/fetcher.cpp 7ad3206c31c47b7d1ae9f89c9e9347831aa3ea39 
>   src/slave/containerizer/fetcher.cpp 
> 0b676f43e8529c6790824c8cf82e21a81d885ef1 
> 
> Diff: https://reviews.apache.org/r/40498/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 40556: Added a test filter for CURL tests.

[Jie Yu]

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

Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Added a test filter for CURL tests.


Diffs
-

  src/tests/environment.cpp c07d9f037f81bce315df7c818131efcb0b186595 
  src/tests/uri_fetcher_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/40556/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40557: Add documentation about using terminate/wait on Processes when deallocating them.

[Neil Conway]

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

Ship it!



3rdparty/libprocess/README.md (line 139)


Why is this taking about `Process*`? i.e., same issues would exist with a 
stack-allocated `Process`, for which (user code) never gets a `Process*`.


- Neil Conway


On Nov. 20, 2015, 7:04 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40557/
> ---
> 
> (Updated Nov. 20, 2015, 7:04 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based on a discussion here: https://reviews.apache.org/r/40501/#review107290
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md c3f309a7dc1c94882c4cc97eeaf0736c2fca0ba5 
> 
> Diff: https://reviews.apache.org/r/40557/diff/
> 
> 
> Testing
> ---
> 
> Checked README.md rendering on GitHub.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40268: Libprocess Reinitialization: Change Socket::DEFAULT_KIND to return a non-static local value.

[Joseph Wu]

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

(Updated Nov. 20, 2015, 11:19 a.m.)


Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


Summary (updated)
-

Libprocess Reinitialization: Change Socket::DEFAULT_KIND to return a non-static 
local value.


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


Repository: mesos


Description (updated)
---

This is only needed for tests that utilize the test-only 
`process::reinitialize` function.


Diffs (updated)
-

  3rdparty/libprocess/include/process/socket.hpp 
ebee78909feb5a4032da68f51d08dbf11b03b332 
  3rdparty/libprocess/src/socket.cpp 6ac834e7459f5958b7c788ccdc60cbed90530183 

Diff: https://reviews.apache.org/r/40268/diff/


Testing (updated)
---

Some test cleanup issues were uncovered by this change; fixed here: 
https://reviews.apache.org/r/40453/


Thanks,

Joseph Wu

Re: Review Request 40556: Added a test filter for CURL tests.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 20, 2015, 7:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40556/
> ---
> 
> (Updated Nov. 20, 2015, 7:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3924
> https://issues.apache.org/jira/browse/MESOS-3924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test filter for CURL tests.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp c07d9f037f81bce315df7c818131efcb0b186595 
>   src/tests/uri_fetcher_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40556/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 40410: Libprocess Reinitialization: Move MetricsProcess instantiation into process::initialize.

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


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


Repository: mesos


Description
---

This must be unified with `process::initialize` so that it also falls under the 
scope of reinitialization.


Diffs
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
f2e84d8e62df58812b660c858eb3b0366db4 
  3rdparty/libprocess/src/metrics/metrics.cpp 
0cf844c2f393135236e978862acee88b24581c5c 
  3rdparty/libprocess/src/process.cpp 8fa0594b671969c2713d1b361f2dbbb07d25b3a8 
  3rdparty/libprocess/src/tests/system_tests.cpp 
c5436f72a70f0a55bfe8ba282b5190873a2d9076 

Diff: https://reviews.apache.org/r/40410/diff/


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu

Re: Review Request 40501: Cleanup a leaked reference to a test process living in the stack.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40501]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 19, 2015, 9:51 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40501/
> ---
> 
> (Updated Nov. 19, 2015, 9:51 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3753
> https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Two of the fetcher tests will spawn a process which is stored in the stack 
> (i.e. local variable in the test).  `spawn` will store a pointer to the 
> process in libprocess's `ProcessManager`.  When the test finishes, the 
> process goes out of scope and is therefore lost.  However, the process is 
> **not** terminated.
> 
> Failing to terminate this process will lead to an infinite loop in 
> `~ProcessManager`, which is called in `process::finalize`.  In 
> `ProcessManager` 's destructor, we will loop and try to kill all processes.  
> The process spawned in the test will be running.  However, since the pointer 
> lives in the stack, the `ProcessManager` will be unable to find the process 
> and will thereby be stuck trying to kill a process it cannot find.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 04079964b3539f555351d1444f3635c64700a1a8 
> 
> Diff: https://reviews.apache.org/r/40501/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Additional testing:
> 
> Insert a `process::finalize` in `src/test/main.cpp`.  i.e.
> ```
>   // Replace `return RUN_ALL_TESTS();` with this:
>   int ret = RUN_ALL_TESTS();
>   process::finalize();
>   return ret;
> ```
> 
> Then `make check 
> GTEST_FILTER="*FetcherTest.OSNetUriTest*:*FetcherTest.OSNetUriSpaceTest*"`.
> The test program should not stall or segfault or abort in some weird way.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Greg Mann]

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



src/common/resources.cpp (lines 879 - 881)


What about the resource types besides `cpu`? The JIRA was filed because of 
a `cpu` error, but this same problem is just as likely for `mem` and `disk`, 
can we apply the fix to them as well?


- Greg Mann


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 40367: Added backtick usage in comments to the C++ style guide.

[Joerg Schad]

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

Ship it!


Ship It!

- Joerg Schad


On Nov. 16, 2015, 9:31 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40367/
> ---
> 
> (Updated Nov. 16, 2015, 9:31 p.m.)
> 
> 
> Review request for mesos, Michael Park and Neil Conway.
> 
> 
> Bugs: MESOS-3786
> https://issues.apache.org/jira/browse/MESOS-3786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added backtick usage in comments to the C++ style guide.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/40367/diff/
> 
> 
> Testing
> ---
> 
> Viewed with the Mesos Website Container 
> (https://github.com/mesosphere/mesos-website-container).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40497: Add hex number support to numify()

[Cong Wang]

> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 30
> > 
> >
> > I believe giving these proper names would make e.g., the ordering here 
> > much easier to grasp.
> > 
> > What about `template  ..`?
> > 
> > Also space after `template`.

Fixed, I also remove the Source type since I only need std::string there. 
hex_cast(1) is beyond what I need.


> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 40
> > 
> >
> > I understand how the error string landed here from `numify`, but as 
> > written hardly anything about `hex_cast` is concerned with numbers (e.g., 
> > `hex_cast(1.2)` is perfectly fine).
> > 
> > You should probably make it more general here, and specialize the error 
> > message at the call site in `numify` instead.

Removed the second type from template, see above.


> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 39
> > 
> >
> > Where you planning to output `c` here? Otherwise I suggest replacing 
> > this at least `ss.get()` (and drop the decl above).
> > 
> > I like H.Sutter's synopsis of `boost::lexical_cast` even better since 
> > it shows clearly the failure sites (modulo the throw for here), see 
> > http://www.gotw.ca/publications/mill19.htm:
> > 
> > template
> > Target lexical_cast(Source arg)
> > {
> >   std::stringstream interpreter;
> >   Target result;
> > 
> >   if(!(interpreter << arg) ||
> >  !(interpreter >> result) ||
> >  !(interpreter >> std::ws).eof())
> > throw bad_lexical_cast();
> > 
> >   return result;
> > }
> > 
> > You could do a similar impl just by injection a `<< std::hex`.

I replace that ss.get() with ss.eof(). And also I don't understand why checking 
the return value of operator<< could work, it always return the stream right? 
The document says we should check ss.fail().


> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp, line 51
> > 
> >
> > With the implementation you added, what keeps us from replacing this 
> > with `return hex_cast(s)` and dropping the Boost `#include` to simplify 
> > the deps?

Good point, but I only wanted to add the hex implementation here (see the next 
patch in the chain). We can do that later.


- Cong


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


On Nov. 19, 2015, 10:56 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40497/
> ---
> 
> (Updated Nov. 19, 2015, 10:56 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> numify() in libprocess uses boost lexical_cast() to cast numbers, but it can 
> not cast a hex number. This patch adds hex support to numify().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5a0ffc38d4194b9f9c53dc1cf0c90ca7bbae2afd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> ddd3dd9e015c583e04d72ac9a9b5a5ed6f1d49d5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 14fb644b38a5cbb8cde74aab39e84305f6ab7041 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40497/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 40562: Added validation for 'uuid' field being present

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

This change adds validation to ensure `uuid` field is always present in 
`Call::Update::TaskStatus` message sent by the Executor.


Diffs
-

  src/slave/validation.cpp b46559d702ccad713cfa5754815bed65e21d14e1 

Diff: https://reviews.apache.org/r/40562/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 40497: Add hex number support to numify()

[Cong Wang]

> On Nov. 20, 2015, 3:23 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp, line 26
> > 
> >
> > Would be nice to add a couple more positives here, e.g., to catch 
> > `hex_cast` impls like `boost::lexical_cast(s.substr(2)` ;)

I added some more positives, but not sure if they are 
boost::lexical_cast(s.substr(2))...


- Cong


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


On Nov. 19, 2015, 10:56 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40497/
> ---
> 
> (Updated Nov. 19, 2015, 10:56 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> numify() in libprocess uses boost lexical_cast() to cast numbers, but it can 
> not cast a hex number. This patch adds hex support to numify().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5a0ffc38d4194b9f9c53dc1cf0c90ca7bbae2afd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> ddd3dd9e015c583e04d72ac9a9b5a5ed6f1d49d5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 14fb644b38a5cbb8cde74aab39e84305f6ab7041 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40497/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 40559: Added a wait() function to Subprocess.

[Jie Yu]

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

(Updated Nov. 20, 2015, 9:06 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Added a wait() function to Subprocess. This is similar to 
`Popen.communicate(None)` in python. See the example code below:

```
Try s = subprocess(...);
if (s.isError()) {
  ...
}

return s.get().wait()
  .then([](const Subprocess::Result& result) {
if (result.status.isNone()) { ... }
if (result.status.get() != 0) { ... }

handle(result.out.get());
handle(result.err.get());
  });
```

Relevant review: https://reviews.apache.org/r/37336/


Diffs
-

  3rdparty/libprocess/include/process/subprocess.hpp 
f17816e813d5efce1d3bb1ff1e850eeda3ba 
  3rdparty/libprocess/src/subprocess.cpp 
e51f024e89594d84f1dfb7ee6f2e1d8fb33b4a08 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
ac600a551fb1a7782ff33cce204b7819497ef54a 

Diff: https://reviews.apache.org/r/40559/diff/


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40293: Applied consistent number style in c++ styleguide.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Nov. 20, 2015, 8:42 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40293/
> ---
> 
> (Updated Nov. 20, 2015, 8:42 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Applied consistent number style in c++ styleguide.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/40293/diff/
> 
> 
> Testing
> ---
> 
> viewed rendered version.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 40563: Added functionality for handling status updates from HTTP based executors

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

This change adds the ability for the agent to handle status updates from HTTP 
based executors. Previously, the existing `statusUpdate` method used to handle 
status updates sent from slave with `pid == UPID()`, sent from other 
executors/on behalf of other executors with `pid == Some()`. This change 
modifies the argument to be `Option`. This ensures that `pid == None()` 
when set, the ACK is correctly routed to the corresponding HTTP based executor.


Diffs
-

  src/slave/http.cpp ce48a0584ab18a8d95dd02619f62df18b2276639 
  src/slave/slave.hpp ec2dfa99e6b553e2bcd82d12db915ae8625075a1 
  src/slave/slave.cpp d1126f00d947fdb4823b0c495335b743254ac7ee 

Diff: https://reviews.apache.org/r/40563/diff/


Testing
---

make check + would add tests once the https://reviews.apache.org/r/39297/ chain 
is reviewed.


Thanks,

Anand Mazumdar

Re: Review Request 39417: Add --egress_flow_classifier_parent flag

[Jie Yu]

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



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 390 - 
414)


Can you introduce a static method in Handle (src/linux/routing/handle.hpp)

```
Try Handle::parse(const string& value);
```



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1416)


s/parent/egressParentHandle/



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1418)


Failed to parse egress_flow_classifier_parent: ...



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1421)


No need for a temp variable. Simply use egressParentHandle.get()



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1423)


We typically do not update global variables. Can you make it an instance 
member of the isolator class and keep the constant HOST_TX_FQ_CODEL_HANDLE



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 1425 - 
1426)


Please add a TODO(cwang) here.


- Jie Yu


On Nov. 20, 2015, 10:27 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> ---
> 
> (Updated Nov. 20, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When --egress_unique_flow_per_container is enabled, we need to install a flow 
> classifier (fq_codel) qdisc on egress side. This flag specifies where to 
> install it in the hierarchy. By default, we install it at root. But, for 
> example, if you have already installed HTB qdisc at root, you may want this 
> to be installed in other place than root, specify an HTB class ID as its 
> parent here.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
>   src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936 
>   src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c 
> 
> Diff: https://reviews.apache.org/r/39417/diff/
> 
> 
> Testing
> ---
> 
> Manual tests, with and without a pre-installed HTB qdisc and classes.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 40371: Changed mesos-execute to add containerizer option.

[Timothy Chen]

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


Thanks Jojy, I'll fix these myself and merge.

- Timothy Chen


On Nov. 20, 2015, 7:45 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40371/
> ---
> 
> (Updated Nov. 20, 2015, 7:45 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since docker_image option could be used for mesos and docker containerizer,
> introduced a  new option 'containerizer' to disambiguate the two 
> containerizers.
> 
> New usage:
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_docker --docker_image=ubuntu --containerizer=docker
> 
> mesos-execute --master=127.0.0.1:5050 --command="uname -a" \
> --name=test_mesos --docker_image=ubuntu --containerizer=mesos
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc 
> 
> Diff: https://reviews.apache.org/r/40371/diff/
> 
> 
> Testing
> ---
> 
> tested the two containerizer locally with simple commands.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 40562: Added validation for 'uuid' field being present

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 20, 2015, 10:47 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40562/
> ---
> 
> (Updated Nov. 20, 2015, 10:47 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, Marco Massenzio, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-3476
> https://issues.apache.org/jira/browse/MESOS-3476
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds validation to ensure `uuid` field is always present in 
> `Call::Update::TaskStatus` message sent by the Executor.
> 
> 
> Diffs
> -
> 
>   src/slave/validation.cpp b46559d702ccad713cfa5754815bed65e21d14e1 
> 
> Diff: https://reviews.apache.org/r/40562/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 40561: Added an overload for createStatusUpdate(...) function

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 20, 2015, 10:47 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40561/
> ---
> 
> (Updated Nov. 20, 2015, 10:47 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, Marco Massenzio, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-3476
> https://issues.apache.org/jira/browse/MESOS-3476
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds an overload for the existing `createStatusUpdate(...)` 
> function. This is later used by the Agent Executor HTTP API handler to get an 
> `StatusUpdateMessage` from an `Call::Update` message and then invoke 
> `Slave::statusUpdate` function.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 333d17e8461f53f73f724d67debdfff4adbb7fd2 
>   src/common/protobuf_utils.cpp 9a940ef9657a8174b6e5b84af6f7fcd791548c0b 
> 
> Diff: https://reviews.apache.org/r/40561/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 39490: Always create non-IP egress filters

[Jie Yu]

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

Ship it!



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 2430 - 
2432)


Please move this comments down (to where you increment the metrics)



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 2450)


Let's not increment this metrics. Otherwise, you'll have this metrics being 
increased everytime a new container is launched.

Instead, Please just add a comment and ignore this case.



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 2469)


Ditto here.


- Jie Yu


On Nov. 20, 2015, 11:36 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> ---
> 
> (Updated Nov. 20, 2015, 11:36 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When rolling out the new flag --egress_unique_flow_per_container, I noticed, 
> on some slaves, only IP egress filters were created as expected, the reset 
> were not. Looking at the code, it looks like we skipped the creation if this 
> is not the first container we create, this is wrong for this case, because 
> egress filters were not created for previous containers yet. We should always 
> create them and ignore error if they exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39490: Always create non-IP egress filters

[Jie Yu]

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


I'll fix this for you.

- Jie Yu


On Nov. 20, 2015, 11:36 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> ---
> 
> (Updated Nov. 20, 2015, 11:36 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When rolling out the new flag --egress_unique_flow_per_container, I noticed, 
> on some slaves, only IP egress filters were created as expected, the reset 
> were not. Looking at the code, it looks like we skipped the creation if this 
> is not the first container we create, this is wrong for this case, because 
> egress filters were not created for previous containers yet. We should always 
> create them and ignore error if they exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38956: Quota: Added allocator-agnostic tests.

[Alexander Rukletsov]

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

(Updated Nov. 20, 2015, 8:48 a.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
Joseph Wu, and Michael Park.


Changes
---

Corrected comments. NNTR.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
  src/tests/master_quota_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/38956/diff/


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40392: Added force flag to override quota capacityHeuristic check.

[Alexander Rukletsov]

> On Nov. 19, 2015, 11:10 p.m., Alexander Rukletsov wrote:
> > src/tests/master_quota_tests.cpp, line 460
> > 
> >
> > Could you please restore this blank line?
> 
> Joerg Schad wrote:
> Sure, but FYI this style is consistent with the the rest of the file 
> where we have one single line between TODO and first test (see line 155/156)

It makes sense to fix it where it was introduced to reduce noise. Agree?


- Alexander


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


On Nov. 20, 2015, 5:44 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40392/
> ---
> 
> (Updated Nov. 20, 2015, 5:44 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3911
> https://issues.apache.org/jira/browse/MESOS-3911
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added force flag to override quota capacityHeuristic check.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 
>   src/tests/master_quota_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40392/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40292: Added style guideline for writing numbers to markdown styleguide.

[Alexander Rukletsov]

> On Nov. 17, 2015, 8:18 a.m., Alexander Rukletsov wrote:
> > What is the motivation for this? Are there any studies proving it 
> > facilitates preception?
> 
> Joerg Schad wrote:
> Motivation is the current inconsistent writing of numbers e.g. in the c++ 
> styleguide.

Sorry for not being clear. My question is more "why do you propose such 
scheme?". For me it feels inconsistent to have both figures and spelled out 
numbers. My intuition is that such rule is harder to remember than simple ones 
like "use figures everywhere" or "always spell out". I assume you base your 
proposal on common practices in american? british? English or studies around 
human perception which are unknown to me as an ESL : ).


- Alexander


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


On Nov. 17, 2015, 8:07 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40292/
> ---
> 
> (Updated Nov. 17, 2015, 8:07 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added style guideline for writing numbers to markdown styleguide.
> 
> 
> Diffs
> -
> 
>   docs/markdown-style-guide.md fe9ffefbbb2c0ce9ade39cec1de073163a9a03b2 
> 
> Diff: https://reviews.apache.org/r/40292/diff/
> 
> 
> Testing
> ---
> 
> viewed rendered version.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40292: Added style guideline for writing numbers to markdown styleguide.

[Alexander Rukletsov]

> On Nov. 17, 2015, 8:18 a.m., Alexander Rukletsov wrote:
> > What is the motivation for this? Are there any studies proving it 
> > facilitates preception?
> 
> Joerg Schad wrote:
> Motivation is the current inconsistent writing of numbers e.g. in the c++ 
> styleguide.
> 
> Alexander Rukletsov wrote:
> Sorry for not being clear. My question is more "why do you propose such 
> scheme?". For me it feels inconsistent to have both figures and spelled out 
> numbers. My intuition is that such rule is harder to remember than simple 
> ones like "use figures everywhere" or "always spell out". I assume you base 
> your proposal on common practices in american? british? English or studies 
> around human perception which are unknown to me as an ESL : ).

It looks like there are guideline on spelling out numbers versus using 
literals, though they are not consistent:
http://www.grammarbook.com/numbers/numbers.asp
http://www.dailywritingtips.com/10-rules-for-writing-numbers-and-numerals/

Another question I have, is your proposal related to docs only, or for code 
comments as well? Though I do think comments should be proper text, I'm not 
sure they should be fiction and would rather keep only numerals there.


- Alexander


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


On Nov. 17, 2015, 8:07 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40292/
> ---
> 
> (Updated Nov. 17, 2015, 8:07 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added style guideline for writing numbers to markdown styleguide.
> 
> 
> Diffs
> -
> 
>   docs/markdown-style-guide.md fe9ffefbbb2c0ce9ade39cec1de073163a9a03b2 
> 
> Diff: https://reviews.apache.org/r/40292/diff/
> 
> 
> Testing
> ---
> 
> viewed rendered version.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 40378: Added link to upgrade guide to documentation page.

[Neil Conway]

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

(Updated Nov. 20, 2015, 10:54 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, and Joris Van Remoortere.


Changes
---

Place link in a better place.


Repository: mesos


Description
---

Added link to upgrade guide to documentation page.


Diffs (updated)
-

  docs/home.md 7aa785e9ae07f2cc14eb0f1108ae4ab4c8748599 
  docs/upgrades.md f2da680d6877dfc161bb33cc97358cbf46787faa 

Diff: https://reviews.apache.org/r/40378/diff/


Testing
---

Previewed with mesos-website-container.


Thanks,

Neil Conway

Re: Review Request 38117: Export per container SNMP statistics

[Cong Wang]

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

(Updated Nov. 20, 2015, 11:25 p.m.)


Review request for mesos, Jie Yu, Paul Brett, and Vinod Kone.


Changes
---

Rebase


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


Repository: mesos


Description
---

These stats are those we get by `netstat -s`, they are important for diagnose 
networking issues.


Diffs (updated)
-

  include/mesos/mesos.proto 5ad48bd376c34dd495399b62fa0bd37ddcc5518b 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
ae53c1b1716a7ad9c6e64f9079c972bae31e4ed2 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
  src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936 
  src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c 

Diff: https://reviews.apache.org/r/38117/diff/


Testing
---

./src/mesos-network-helper statistics --eth0_name=X --enable_snmp_statistics 
--pid=


Thanks,

Cong Wang

Re: Review Request 37336: Added `execute()` method to process::Subprocess

[Jie Yu]

> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, lines 119-130
> > 
> >
> > Why not just use a single `int status` field here. The users can use 
> > WEXITSTATUS ... to derive those information themself. This is also 
> > consistent with other places in the code place.
> 
> Marco Massenzio wrote:
> `Subprocess` does exactly that (actually, it uses an `Option`) - 
> this leaves the caller with the burned to do all the legwork: again and again 
> and again - it's all over the code base.
> 
> The point of this patch is to encapsulate that work, having it done 
> (hopefully, properly) in **one** place and avoid to have the same code 
> sprinkled all over the codebase (and you can tell, most places, by copy & 
> paste).

What if there's no returnCode (due to signal)? Should you use Option 
returnCode here? Similarly, should you use Option for signal as well. What 
will the client code be look like? Do you still need to check if 
(returnCode.isSome()) ... if (signal.isSome()) ...

Also
1) what if I want to know if WCOREDUMP is true or not, do you need to add 
another boolean?
2) what if the reaper cannot reap the subprocess (i.e., we cannot get the exit 
status)?


- Jie


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


On Nov. 10, 2015, 8:51 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37336/
> ---
> 
> (Updated Nov. 10, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The original API for `process::Subprocess` still left a lot of legwork
> to do for the caller; we have now added an `execute()` method
> that returns a `Future`.
>  
> `Subprocess::Result`, also introduced with this patch, contains useful 
> information
> about the command invocation (an `Invocation` struct); the exit code; 
> `stdout`;
> and, optionally, `stderr` too.
>  
> Once the Future completes, if successful, the caller will be able to retrieve
> stdout/stderr; whether the command was successful; and whether it received a 
> signal
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> f17816e813d5efce1d3bb1ff1e850eeda3ba 
>   3rdparty/libprocess/src/subprocess.cpp 
> efe0018d0414c4137fd833c153eb262232e712bc 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ac600a551fb1a7782ff33cce204b7819497ef54a 
> 
> Diff: https://reviews.apache.org/r/37336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> (also tested functionality with an anonymous module that exposes an 
> `/execute` endpoint and runs arbitrary commands, asynchronously,
> on an Agent)
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 39490: Always create non-IP egress filters

[Cong Wang]

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

(Updated Nov. 20, 2015, 11:36 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


Changes
---

Rebase


Repository: mesos


Description
---

When rolling out the new flag --egress_unique_flow_per_container, I noticed, on 
some slaves, only IP egress filters were created as expected, the reset were 
not. Looking at the code, it looks like we skipped the creation if this is not 
the first container we create, this is wrong for this case, because egress 
filters were not created for previous containers yet. We should always create 
them and ignore error if they exist.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 

Diff: https://reviews.apache.org/r/39490/diff/


Testing
---

Manual tests


Thanks,

Cong Wang

Re: Review Request 40506: Add stdout/tests/numify_tests.cpp into Makefile.am

[Cong Wang]

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

(Updated Nov. 20, 2015, 10:27 p.m.)


Review request for mesos, Ben Mahler and Ian Downes.


Changes
---

Rebase


Repository: mesos


Description
---

Add stdout/tests/numify_tests.cpp into Makefile.am


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
0adbe539afaf683e4a85582463a2930049a63998 

Diff: https://reviews.apache.org/r/40506/diff/


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 39417: Add --egress_flow_classifier_parent flag

[Cong Wang]

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

(Updated Nov. 20, 2015, 10:27 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


Changes
---

Rebase


Repository: mesos


Description
---

When --egress_unique_flow_per_container is enabled, we need to install a flow 
classifier (fq_codel) qdisc on egress side. This flag specifies where to 
install it in the hierarchy. By default, we install it at root. But, for 
example, if you have already installed HTB qdisc at root, you may want this to 
be installed in other place than root, specify an HTB class ID as its parent 
here.


Diffs (updated)
-

  docs/configuration.md 126f4aa8e3de2a2346336991c9b9a2ea61a8cd0a 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
  src/slave/flags.hpp 6ae7c94d2e05d81c8b970e7dcaa82d8aa4de7936 
  src/slave/flags.cpp 26f554e1a73f1f7f3d7baec7bfc1a7f456c5677c 

Diff: https://reviews.apache.org/r/39417/diff/


Testing
---

Manual tests, with and without a pre-installed HTB qdisc and classes.


Thanks,

Cong Wang

Re: Review Request 40497: Add hex number support to numify()

[Cong Wang]

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

(Updated Nov. 20, 2015, 10:26 p.m.)


Review request for mesos, Ben Mahler and Ian Downes.


Changes
---

Address review comments


Repository: mesos


Description
---

numify() in libprocess uses boost lexical_cast() to cast numbers, but it can 
not cast a hex number. This patch adds hex support to numify().


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
5a0ffc38d4194b9f9c53dc1cf0c90ca7bbae2afd 
  3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
ddd3dd9e015c583e04d72ac9a9b5a5ed6f1d49d5 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
14fb644b38a5cbb8cde74aab39e84305f6ab7041 
  3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/40497/diff/


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 40553: Enable mesos tests installation

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39780, 39781, 39782, 40553]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 6:53 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> ---
> 
> (Updated Nov. 20, 2015, 6:53 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
> 
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
> 
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none
> 
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
> 
> Current test status:
> 
>   [==] 784 tests from 106 test cases ran. (135304 ms total)
>   [  PASSED  ] 780 tests.
>   [  FAILED  ] 4 tests, listed below:
>   [  FAILED  ] ExamplesTest.TestFramework
>   [  FAILED  ] ExamplesTest.NoExecutorFramework
>   [  FAILED  ] ExamplesTest.EventCallFramework
>   [  FAILED  ] ExamplesTest.PersistentVolumeFramework
> 
> 
> Diffs
> -
> 
>   configure.ac 8b28ac78eeb3e3b5905b411b4bc0db3ccf0f543f 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/tests/containerizer/launch_tests.cpp 
> bf6b7561bf9b5c06fd5d1101e2b2511776a12010 
>   src/tests/containerizer/memory_test_helper.cpp 
> 6abd29ba6f0d208a5f10f645977fec309334af0a 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> d1ff139b0674ea266a4d554de112094045add509 
>   src/tests/containerizer/ns_tests.cpp 
> ac3c9ff1767bd34d3257bb39effce789fae47252 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 91953389f793755770ff957b675936521b0e338a 
>   src/tests/environment.cpp c07d9f037f81bce315df7c818131efcb0b186595 
>   src/tests/fetcher_tests.cpp 04079964b3539f555351d1444f3635c64700a1a8 
>   src/tests/health_check_tests.cpp ff6275b19206b49eacb6761f3aeb58dd87651ade 
>   src/tests/mesos.cpp 766a51cddc8801d5e79188f80e9ce0828598c8b9 
>   src/tests/module.cpp edab0b37dcf0bd8e15d439726354039c1bbcd51f 
>   src/tests/module_tests.cpp 28c71b0c1960bad4933f86d35fe8a0248fef326e 
>   src/tests/oversubscription_tests.cpp 
> 0d0bf7e0b9a4028ed116e00b56d59b670510c5ce 
>   src/tests/script.cpp c9b04e67ce8176798412c0c17e436bbc01a94d70 
>   src/tests/slave_tests.cpp 7c9dcc6186a8cccb0eb30ff59914a41961e47293 
>   src/tests/utils.hpp 1497013f43428269aa1e3c63e166c303af00483e 
>   src/tests/utils.cpp 0578e63bc46ea79b0508e656d0793710484b21ef 
> 
> Diff: https://reviews.apache.org/r/40553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 40245: Fixed typos in comments.

[Neil Conway]

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

(Updated Nov. 20, 2015, 10:31 p.m.)


Review request for mesos, Greg Mann and Michael Park.


Changes
---

Fix more typos, rebase.


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


Repository: mesos


Description
---

Fixed typos in comments.


Diffs (updated)
-

  src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
  src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
  src/tests/persistent_volume_tests.cpp 
8b791ac0861171b0c655307e965165d9ad7ba966 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
  src/tests/reservation_tests.cpp ac664ebb49e74aa28551f427ea8f39ac9ce0cfb3 

Diff: https://reviews.apache.org/r/40245/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 40242: Improved docs for dynamic reservation HTTP endpoints.

[Neil Conway]

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

(Updated Nov. 20, 2015, 10:32 p.m.)


Review request for mesos, Greg Mann and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Improved docs for dynamic reservation HTTP endpoints.


Diffs (updated)
-

  docs/home.md 7aa785e9ae07f2cc14eb0f1108ae4ab4c8748599 
  docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
  docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 

Diff: https://reviews.apache.org/r/40242/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 40243: Documented "role" field in Resource protobuf message.

[Neil Conway]

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

(Updated Nov. 20, 2015, 10:31 p.m.)


Review request for mesos, Greg Mann and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Documented "role" field in Resource protobuf message.


Diffs (updated)
-

  include/mesos/mesos.proto 5ad48bd376c34dd495399b62fa0bd37ddcc5518b 
  include/mesos/v1/mesos.proto e71ddda7f23f2272ce8eb00f358c66fce205c13b 

Diff: https://reviews.apache.org/r/40243/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 40246: Removed unused "using" statement from test code.

[Neil Conway]

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

(Updated Nov. 20, 2015, 10:32 p.m.)


Review request for mesos, Greg Mann and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed unused "using" statement from test code.


Diffs (updated)
-

  src/tests/containerizer/external_containerizer_test.cpp 
c6993bab736bbe300c5d84ca6ca281a4f23bdef8 
  src/tests/containerizer/isolator_tests.cpp 
01d22b46a681f40dfa923f0a5c9b9b9b084d9bcd 
  src/tests/reservation_endpoints_tests.cpp 
1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 
  src/tests/reservation_tests.cpp ac664ebb49e74aa28551f427ea8f39ac9ce0cfb3 
  src/tests/slave_tests.cpp 7c9dcc6186a8cccb0eb30ff59914a41961e47293 
  src/tests/teardown_tests.cpp 96e98bd0d134b2cf093285f37bec4c89c8f3553e 

Diff: https://reviews.apache.org/r/40246/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 37336: Added `execute()` method to process::Subprocess

[Marco Massenzio]

> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, lines 53-75
> > 
> >
> > What's the motivation of storing this? Should the caller already have 
> > those information?

not necessarily - please note that this is executed asynchronously, so the 
caller may have several of them running concurrently and having the invocation 
returned together with the invocation will greatly simplifly the caller's code.

The whole point of this patch is to make the caller's API less "low-level" and 
simplify the life of those using this facility.

I have, for example, a module that executes commands upon an HTTP request - 
having the `invocation` returned to me with the result, greatly simplifies my 
code and enables me to return a Response without waiting for execution.


> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, lines 114-117
> > 
> >
> > Ditto. I am not convinced that this is strictly necessary.

Of course it's not *necessary* - but it makes the caller's life a lot easier.
This was the whole point of "wrapping" the calls to `subprocess()` with 
something that avoided the need to do all the legwork


> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, lines 119-130
> > 
> >
> > Why not just use a single `int status` field here. The users can use 
> > WEXITSTATUS ... to derive those information themself. This is also 
> > consistent with other places in the code place.

`Subprocess` does exactly that (actually, it uses an `Option`) - this 
leaves the caller with the burned to do all the legwork: again and again and 
again - it's all over the code base.

The point of this patch is to encapsulate that work, having it done (hopefully, 
properly) in **one** place and avoid to have the same code sprinkled all over 
the codebase (and you can tell, most places, by copy & paste).


> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 328
> > 
> >
> > I don't like the name 'execute'. When you create the Subprocess 
> > instance, the subprocss is already launched and exec'ed. This is rather 
> > waiting for the subprocess to terminate.

This method most definitely does **not** wait.

This is how one can use it as a caller (code simplified):
```
  auto s = process::subprocess(commandInfo.command(), args);

  if (s.isError()) {
LOG(ERROR) << "Could not spawn subprocess: " << s.error();
return http::ServiceUnavailable(s.error());
  }

  store(s.get().pid());  // <-- needed to reconcile with GETs

  Future result_ = s->execute();
  result_.then([](const Future ) {
if (future.isFailed()) {
  // mark the command as failed
  return Nothing();
}
auto result = future.get();
// update status of job - use pid(); something equivalent to:
LOG(INFO) << "Result of '" << result.invocation.command << "'was: "
  << result.stdout();
return Nothing();
  }).after(Seconds(30), [s](const Future ) {
// update status of job to timed out; use `invocation` and `stdout`.
s.get().cleanup();
return Nothing();
  });

  http::Response response = http::OK("{\"result\": \"OK\", \"pid\": \"" +
 stringify(s.get().pid()) + "\"}");
  response.headers["Content-Type"] = "application/json";
  return response;
```


> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, lines 336-343
> > 
> >
> > Why introduce this method? I think the caller should be responsible for 
> > killing the subprocess if needed.
> > 
> > Also, os::killtree is in general not reliable and shouldn't be used in 
> > library code.

again, this is all about hiding implementation details from the caller and make 
their life easier.
thanks for heads-up about `killtree()` - what would you suggest we use instead?

when you say "unreliable" what do you mean, exactly?
that it may fail to actually kill the process or that it risks blowing up the 
app or segfaulting or whatever?


> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, lines 185-190
> > 
> >
> > What if outData is called more than once?

Great question!
I would guess they'll get an empty string (as I assume that `io::read()` is 
sitting on an empty buffer) but worth looking into (and testing!)

Thanks.


- Marco

Re: Review Request 37336: Added `execute()` method to process::Subprocess

[Jie Yu]

> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 328
> > 
> >
> > I don't like the name 'execute'. When you create the Subprocess 
> > instance, the subprocss is already launched and exec'ed. This is rather 
> > waiting for the subprocess to terminate.
> 
> Marco Massenzio wrote:
> This method most definitely does **not** wait.
> 
> This is how one can use it as a caller (code simplified):
> ```
>   auto s = process::subprocess(commandInfo.command(), args);
> 
>   if (s.isError()) {
> LOG(ERROR) << "Could not spawn subprocess: " << s.error();
> return http::ServiceUnavailable(s.error());
>   }
> 
>   store(s.get().pid());  // <-- needed to reconcile with GETs
> 
>   Future result_ = s->execute();
>   result_.then([](const Future ) {
> if (future.isFailed()) {
>   // mark the command as failed
>   return Nothing();
> }
> auto result = future.get();
> // update status of job - use pid(); something equivalent to:
> LOG(INFO) << "Result of '" << result.invocation.command << "'was: 
> "
>   << result.stdout();
> return Nothing();
>   }).after(Seconds(30), [s](const Future ) {
> // update status of job to timed out; use `invocation` and 
> `stdout`.
> s.get().cleanup();
> return Nothing();
>   });
> 
>   http::Response response = http::OK("{\"result\": \"OK\", \"pid\": \"" +
>  stringify(s.get().pid()) + "\"}");
>   response.headers["Content-Type"] = "application/json";
>   return response;
> ```
> 
> Jie Yu wrote:
> From the code above, can you just caputure commandInfo.command() in the 
> lambda and print it?
> 
> ```
> string command = commandInfo.command();
> 
> result_.then([command](...) {
>   ...
>   LOG(INFO) << command << "...";
> });
> ```

ALso, `auto s = process::subprocess(commandInfo.command(), args);` this line 
fork and exec the subprocess. So having another `s->execute()` sounds very 
confusing to me.


- Jie


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


On Nov. 10, 2015, 8:51 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37336/
> ---
> 
> (Updated Nov. 10, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The original API for `process::Subprocess` still left a lot of legwork
> to do for the caller; we have now added an `execute()` method
> that returns a `Future`.
>  
> `Subprocess::Result`, also introduced with this patch, contains useful 
> information
> about the command invocation (an `Invocation` struct); the exit code; 
> `stdout`;
> and, optionally, `stderr` too.
>  
> Once the Future completes, if successful, the caller will be able to retrieve
> stdout/stderr; whether the command was successful; and whether it received a 
> signal
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> f17816e813d5efce1d3bb1ff1e850eeda3ba 
>   3rdparty/libprocess/src/subprocess.cpp 
> efe0018d0414c4137fd833c153eb262232e712bc 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ac600a551fb1a7782ff33cce204b7819497ef54a 
> 
> Diff: https://reviews.apache.org/r/37336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> (also tested functionality with an anonymous module that exposes an 
> `/execute` endpoint and runs arbitrary commands, asynchronously,
> on an Agent)
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 40247: Added HTTP endpoints for creating and destroying persistent volumes.

[Neil Conway]

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

(Updated Nov. 20, 2015, 10:32 p.m.)


Review request for mesos, Greg Mann and Michael Park.


Changes
---

Fixed bug in /create-volume, new tests, rebase.


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


Repository: mesos


Description
---

Added HTTP endpoints for creating and destroying persistent volumes.


Diffs (updated)
-

  docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
  docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 
  src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
  src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
  src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
  src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
  src/tests/mesos.hpp b3f69ccb9870b17a335a2fe7dbf2802c1b709e6b 
  src/tests/persistent_volume_endpoints_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/40247/diff/


Testing
---

(1) make check, including newly added tests

(2) Manually created/removed persistent volumes via HTTP endpoints + curl.

(3) Previewed docs in Github gist.


Thanks,

Neil Conway

Re: Review Request 40247: Added HTTP endpoints for creating and destroying persistent volumes.

[Neil Conway]

> On Nov. 19, 2015, 7:09 p.m., Michael Park wrote:
> > src/master/http.cpp, line 612
> > 
> >
> > The `volumes.flatten()` here looks incorrect to me. For `/reserve`, we 
> > call `flatten` since the `required` resources are the same resources in 
> > their unreserved state. For `/create-volume` however, I think the 
> > `required` resources are the same resources with no `DiskInfo` information.
> > 
> > For example, suppose we're requested to perform a `CREATE` operation 
> > with a 4GB volume statically reserved for role R. The resources we're 
> > searching for is 4GB disk statically reserved for role R. As opposed to 4GB 
> > volume unreserved, which is what `flatten()` gives us.

Good catch! I fixed this in the most recent version of the patch.


- Neil


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


On Nov. 20, 2015, 10:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40247/
> ---
> 
> (Updated Nov. 20, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Bugs: MESOS-2455
> https://issues.apache.org/jira/browse/MESOS-2455
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP endpoints for creating and destroying persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 0951ccb69daaa19b959e11cf3bf972a674a58305 
>   docs/reservation.md 81f21c3755b216b0932876c1ddd9de4d3fbe814a 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
>   src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
>   src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
>   src/tests/mesos.hpp b3f69ccb9870b17a335a2fe7dbf2802c1b709e6b 
>   src/tests/persistent_volume_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40247/diff/
> 
> 
> Testing
> ---
> 
> (1) make check, including newly added tests
> 
> (2) Manually created/removed persistent volumes via HTTP endpoints + curl.
> 
> (3) Previewed docs in Github gist.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40305: Added URI fetcher interface.

[Jie Yu]

> On Nov. 18, 2015, 7:39 p.m., Vinod Kone wrote:
> > include/mesos/uri/fetcher.hpp, line 49
> > 
> >
> > Plugin might be confusing with Module.
> > 
> > Not sure what's a better alternative name is, Downloader? Schema?

Schema is not good because one Plugin can handle more than one schemes. I 
couldn't find a better name. Since Plugin is within Fetcher namespace, and will 
always be used like Fetcher::Plugin. So I think it shouldn't cause confusion to 
Module.


- Jie


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


On Nov. 19, 2015, 6:30 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40305/
> ---
> 
> (Updated Nov. 19, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-3922
> https://issues.apache.org/jira/browse/MESOS-3922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added URI fetcher interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/uri/fetcher.hpp PRE-CREATION 
>   src/CMakeLists.txt 9a2c70d40031c80a304462107758802c08411a49 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/uri/fetcher.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40305/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 40403: Added streaming and construction methods for URI.

[Jie Yu]

> On Nov. 18, 2015, 7:54 p.m., Vinod Kone wrote:
> > src/uri/schemes/http.hpp, line 38
> > 
> >
> > consider merging `uri/schemes/*` and `src/uri/utils.cpp` into a single 
> > `src/uri.cpp` file.

I thought about that before. two reasons why I didn't do that:
1) you might introduce more ane more schemes. One file each scheme sounds more 
scalable. (we might want to add parse functions as well for each scheme).
2) I don't want to introduce a src/uri.hpp which might get confused with 
mesos/uri/uri.hpp.


- Jie


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


On Nov. 18, 2015, 11:25 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40403/
> ---
> 
> (Updated Nov. 18, 2015, 11:25 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3924
> https://issues.apache.org/jira/browse/MESOS-3924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added streaming and construction methods for URI.
> 
> 
> Diffs
> -
> 
>   include/mesos/uri/uri.hpp PRE-CREATION 
>   src/CMakeLists.txt 9a2c70d40031c80a304462107758802c08411a49 
>   src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
>   src/tests/uri_tests.cpp PRE-CREATION 
>   src/uri/schemes/file.hpp PRE-CREATION 
>   src/uri/schemes/http.hpp PRE-CREATION 
>   src/uri/utils.hpp PRE-CREATION 
>   src/uri/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40403/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 40378: Added link to upgrade guide to documentation page.

[Adam B]

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


Looks good, but we should find a better place to put that link.


docs/home.md (line 31)


Thanks for adding this link, but why did you put it here? Are you trying to 
maintain a pseudo-alpha-order? We don't seem to do that well otherwise. If 
we're going in some conceptual grouping order, I'd think Upgrades would go up 
at the top, near getting-started instructions


- Adam B


On Nov. 17, 2015, 1:59 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40378/
> ---
> 
> (Updated Nov. 17, 2015, 1:59 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added link to upgrade guide to documentation page.
> 
> 
> Diffs
> -
> 
>   docs/home.md 7aa785e9ae07f2cc14eb0f1108ae4ab4c8748599 
>   docs/upgrades.md f2da680d6877dfc161bb33cc97358cbf46787faa 
> 
> Diff: https://reviews.apache.org/r/40378/diff/
> 
> 
> Testing
> ---
> 
> Previewed with mesos-website-container.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>