Re: Review Request 40118: [1/7] Added 'principal' field to 'Resource.DiskInfo.Persistence'.

[Greg Mann]

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

(Updated Nov. 30, 2015, 9:40 p.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added 'principal' field to 'Resource.DiskInfo.Persistence'.


Diffs (updated)
-

  include/mesos/mesos.proto 27971fea6935b82d7034397edaa7a37edb1f6f38 
  include/mesos/v1/mesos.proto 9acefd55603a5a4f3f08a879a380ff927fd1e0dd 

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


Testing
---

This is the first in a chain of 7 patches. `make check` was used to test after 
all patches were applied.

Note that this chain of patches touches many of the same files as another chain 
beginning with Review #39985 and ending with Review #39989, which is currently 
in review as well. To avoid conflicts, the beginning of this chain begins on 
top of Review #39989.

One additional patch with documentation is forthcoming.


Thanks,

Greg Mann

Re: Review Request 39989: [5/5] Added framework authorization for dynamic reservation.

[Jie Yu]

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

Ship it!



src/master/master.cpp (line 3191)


Do you also need to add the comment about the ordering in `authorizations` 
is important?


- Jie Yu


On Nov. 30, 2015, 9:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39989/
> ---
> 
> (Updated Nov. 30, 2015, 9:20 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added framework authorization for dynamic reservation.
> Note: this review is continued from https://reviews.apache.org/r/37127/
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 
>   src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 
> 
> Diff: https://reviews.apache.org/r/39989/diff/
> 
> 
> Testing
> ---
> 
> This is the fifth in a chain of 5 patches. New reservation tests were added 
> to `reservation_tests.cpp` to validate the authentication of framework 
> reserve and unreserve operations using ACLs. `make check` was run to test 
> after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39297: Added support for recovering RunState for HTTP based executors

[Anand Mazumdar]

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

(Updated Nov. 30, 2015, 9:28 p.m.)


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


Changes
---

Review comments from Vinod


Repository: mesos


Description
---

This change adds support for recovering the `RunState` for `HTTP` based 
executors. Upon agent recovery, it checks if the marker file for HTTP exists 
and populates `RunState.http` based on that. This is later used by the Agent 
for recovering `HTTP` based executors.


Diffs (updated)
-

  src/slave/state.hpp eb6b06cfcb5cd638f659035726f4e4e41fc5e017 
  src/slave/state.cpp bc46cc65af533639fcd556a8a65c509318702948 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

[Jie Yu]

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



src/master/master.cpp (lines 2776 - 2778)


Let's not drop the operation here. Let the caller do it if Error is 
returned.



src/master/master.cpp (line 2780)


return Failure(error.get().message);



src/master/master.cpp (line 2810)


Instead of passing in the Framework pointer, can we pass in the optional 
role instead?


- Jie Yu


On Nov. 30, 2015, 8:35 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39987/
> ---
> 
> (Updated Nov. 30, 2015, 8:35 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.
> Note: this review is continued from https://reviews.apache.org/r/37125/
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
>   src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 
> 
> Diff: https://reviews.apache.org/r/39987/diff/
> 
> 
> Testing
> ---
> 
> This is the third in a chain of 5 patches. `make check` was used to test 
> after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Jie Yu]

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



src/master/http.cpp (line 2255)


s/authorized/future/

Since it could be the future returned from `_unreserve`



src/master/http.cpp (line 2307)


Instead of adding a new parameter to `_operation`. Can we put the logics of 
checking authorization result to `_reserve` and `_unreserve`? It's wiered to me 
that `_operation` needs to check authorization result.

```
auto _unreserve = [=](bool authorized) {
  if (!authorized) {
return Unauthorized("Mesos master");
  }
  
  return _operation(slaveId, resources, operation);
};

return master->authorizeUnreserveResources(...)
  .then(defer(master->self(), _unreserve))
  .repair([](const Future& future) {
return BadRequest("Invalid UNRESERVE operation: " + future.failure());
  });
```



src/master/http.cpp (lines 2309 - 2321)


As I suggested above, no need for this anymore.


- Jie Yu


On Nov. 30, 2015, 8:46 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39988/
> ---
> 
> (Updated Nov. 30, 2015, 8:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for dynamic reservation master endpoints.
> Note: this review is continued from https://reviews.apache.org/r/37126/
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
>   src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
>   src/tests/reservation_endpoints_tests.cpp 
> f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 
> 
> Diff: https://reviews.apache.org/r/39988/diff/
> 
> 
> Testing
> ---
> 
> This is the fourth in a chain of 5 patches. Added new reservation endpoints 
> tests to validate authorization of reserve and unreserve operations using 
> ACLs. `make check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mei Wan]

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

(Updated Nov. 30, 2015, 10:01 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.


Changes
---

Fixed bug.


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


Repository: mesos


Description
---

Implemented the overlay filesystem backend by layering the images as a 
read-only filesystem.


Diffs (updated)
-

  src/CMakeLists.txt cfe9d26 
  src/Makefile.am fd38cfa 
  src/slave/containerizer/mesos/provisioner/backend.cpp 01d06eb 
  src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 25b28ef 

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


Testing
---

Basic test in provisioner_backend_tests.cpp.


Thanks,

Mei Wan

Re: Review Request 39989: [5/5] Added framework authorization for dynamic reservation.

[Greg Mann]

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

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


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Addressed comments, refactored validation into master authorization helpers.


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


Repository: mesos


Description
---

Added framework authorization for dynamic reservation.
Note: this review is continued from https://reviews.apache.org/r/37127/


Diffs (updated)
-

  src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 
  src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 

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


Testing
---

This is the fifth in a chain of 5 patches. New reservation tests were added to 
`reservation_tests.cpp` to validate the authentication of framework reserve and 
unreserve operations using ACLs. `make check` was run to test after all patches 
were applied.


Thanks,

Greg Mann

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. 30, 2015, 9:58 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 addcc7262584c19f9e5afe4b796c5fc4ecdb53b7 
  src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04 
  src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a 
  src/linux/routing/handle.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
09f819685a96cb4785423a5b2303d147719e273e 
  src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36 
  src/slave/flags.cpp 31700b4f80cf346ab68f9e35d33d7cbb5ddc04cc 

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. 30, 2015, 9:57 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.


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 
440181c25d9d132f48c7a4888dc1dbb0157dc6c8 
  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 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. 30, 2015, 9:58 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.


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 40167: [2/7] Added ACL protobuf messages 'CreateVolume' and 'DestroyVolume'.

[Greg Mann]

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

(Updated Nov. 30, 2015, 9:41 p.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added ACL protobuf messages 'CreateVolume' and 'DestroyVolume'.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
8e72003f405770f00c5d87f318a9e1a8ed7430ee 

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


Testing
---

This is the second in a chain of 7 patches. `make check` was used to test after 
all patches were applied.

Note that this chain of patches touches many of the same files as another chain 
beginning with Review #39985 and ending with Review #39989, which is currently 
in review as well. To avoid conflicts, the beginning of this chain begins on 
top of Review #39989.


Thanks,

Greg Mann

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40586, 40551, 40332, 40795, 39450]

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

- Mesos ReviewBot


On Nov. 30, 2015, 3:32 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39450/
> ---
> 
> (Updated Nov. 30, 2015, 3:32 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-3984
> https://issues.apache.org/jira/browse/MESOS-3984
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> a45b3dd0a8237fdd080536cb9ab600e71ad939d3 
> 
> Diff: https://reviews.apache.org/r/39450/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39780: Update OversubscriptionTest to not assume dynamic dlopen search.

[Benjamin Bannier]

> On Nov. 30, 2015, 8:51 a.m., Benjamin Bannier wrote:
> > src/tests/oversubscription_tests.cpp, line 117
> > 
> >
> > This isn't really too nice, and e.g. cmake does not put build libraries 
> > into these paths (i.e. what you write will only work with automake).
> > 
> > Like I already pointed out on #40553, you should really lift the level 
> > of abstraction here, and use a function to get this path.
> 
> James Peach wrote:
> The intent of the original code was clearly to use this path. When 
> running in the context of the automake wrapper scripts, automake constructs 
> LD_LIBRARY_PATH so that we don't really need to do anything here. What does 
> cmake do? Can you point me to any documentation of how it handles library 
> search paths when executing artifacts from the build directory?

You are of course right that this wasn't yours.

My point is that in #40553 you introduce abstractions like `findModule` that 
should be perfectly good to capture what was written here.


- Benjamin


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


On Nov. 26, 2015, 2:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39780/
> ---
> 
> (Updated Nov. 26, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-3725
> https://issues.apache.org/jira/browse/MESOS-3725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update OversubscriptionTest to not assume dynamic dlopen search.
> 
> 
> Diffs
> -
> 
>   src/tests/oversubscription_tests.cpp 
> 0d0bf7e0b9a4028ed116e00b56d59b670510c5ce 
> 
> Diff: https://reviews.apache.org/r/39780/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 40454: Add calls to parent teardown methods in child teardown methods.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Nov. 18, 2015, 9:52 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40454/
> ---
> 
> (Updated Nov. 18, 2015, 9:52 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
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 01d22b46a681f40dfa923f0a5c9b9b9b084d9bcd 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> edfbb6fe932173dcbb15133e0eb685d86dd09c00 
>   src/tests/mesos.cpp 766a51cddc8801d5e79188f80e9ce0828598c8b9 
>   src/tests/module_tests.cpp 28c71b0c1960bad4933f86d35fe8a0248fef326e 
> 
> Diff: https://reviews.apache.org/r/40454/diff/
> 
> 
> Testing
> ---
> 
> Picked https://reviews.apache.org/r/40268/ which exposes the bad cleanup 
> after SSL-reinitializing tests.
> 
> Then:
> `make check GTEST_FILTER="*RegistryClientTest*:*ExecutorHttpApiTest*"`
> 
> The `RegistryClientTest` involve some reinitialization of SSL variables.  The 
> `ExecutorHttpApiTest` spawns sockets.  If the SSL variables are not cleaned 
> up, the second test will try to use SSL sockets against non-SSL endpoints.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39780: Update OversubscriptionTest to not assume dynamic dlopen search.

[James Peach]

> On Nov. 30, 2015, 8:51 a.m., Benjamin Bannier wrote:
> > src/tests/oversubscription_tests.cpp, line 117
> > 
> >
> > This isn't really too nice, and e.g. cmake does not put build libraries 
> > into these paths (i.e. what you write will only work with automake).
> > 
> > Like I already pointed out on #40553, you should really lift the level 
> > of abstraction here, and use a function to get this path.

The intent of the original code was clearly to use this path. When running in 
the context of the automake wrapper scripts, automake constructs 
LD_LIBRARY_PATH so that we don't really need to do anything here. What does 
cmake do? Can you point me to any documentation of how it handles library 
search paths when executing artifacts from the build directory?


- James


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


On Nov. 26, 2015, 2:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39780/
> ---
> 
> (Updated Nov. 26, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-3725
> https://issues.apache.org/jira/browse/MESOS-3725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update OversubscriptionTest to not assume dynamic dlopen search.
> 
> 
> Diffs
> -
> 
>   src/tests/oversubscription_tests.cpp 
> 0d0bf7e0b9a4028ed116e00b56d59b670510c5ce 
> 
> Diff: https://reviews.apache.org/r/39780/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 38899: Added handler for Executor->Framework message for the Executor HTTP API

[Anand Mazumdar]

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

(Updated Nov. 30, 2015, 5:01 p.m.)


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


Changes
---

Rebased


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


Repository: mesos


Description
---

Trivial change to call the `executorMessage(...)` function that already existed 
in `src/slave/slave.cpp` to handle ExecutorToFrameworkMessage.


Diffs (updated)
-

  src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 

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


Testing
---

make check. Would add tests later after the MESOS-3480 review chain gets 
committed.


Thanks,

Anand Mazumdar

Review Request 40797: Filtered non-whitelisted and deactivated agents once per allocation.

[Alexander Rukletsov]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
31ed62efb5b1a2edb567f43d37559c5914e0665e 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40767: Take possible isNone() into account when comparing two Option CPU resource numbers.

[Ben Mahler]

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



src/common/resources.cpp (lines 962 - 964)


Do you know about Option's getOrElse? You can avoid these ternary 
expressions.

Also the ? needs a space in the second line here.


- Ben Mahler


On Nov. 27, 2015, 1:44 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40767/
> ---
> 
> (Updated Nov. 27, 2015, 1:44 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Avinash sridharan, Klaus Ma, 
> Mandeep Chadha, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addendum to https://reviews.apache.org/r/40730. Not squeeky clean yet, but 
> good enough IMHO until we bring about MESOS-3997, which will change every 
> piece of code like this.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 5cf04ea93e7567107e6664ed56733b7b54dd321b 
> 
> Diff: https://reviews.apache.org/r/40767/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 40797: Filtered non-whitelisted and deactivated agents once per allocation.

[Alexander Rukletsov]

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

(Updated Nov. 30, 2015, 5:14 p.m.)


Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
31ed62efb5b1a2edb567f43d37559c5914e0665e 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40453: Add a TearDownTestCase method for cleaning up after tests that reinitialize SSL configuration.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Nov. 18, 2015, 9:52 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40453/
> ---
> 
> (Updated Nov. 18, 2015, 9:52 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
> ---
> 
> Tests that reinitiailize SSL configuration only change environment variables 
> upon setup.  This means the changed environment will stay intact after the 
> test.  This may lead to unexpected results in subsequent tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 54885ec359df947f6d31375d4c2a32cae587f845 
> 
> Diff: https://reviews.apache.org/r/40453/diff/
> 
> 
> Testing
> ---
> 
> See next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40551: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Alexander Rukletsov]

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

(Updated Nov. 30, 2015, 3:24 p.m.)


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


Changes
---

Minor corrections.


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


Repository: mesos


Description
---

Quota is satisfied in a separate loop over agents. Running total is maintained 
as an exit criterion for the WDRF allocation stage.

Precursory version: https://reviews.apache.org/r/39401/


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
31ed62efb5b1a2edb567f43d37559c5914e0665e 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

[Alexander Rukletsov]

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

(Updated Nov. 30, 2015, 3:32 p.m.)


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


Changes
---

Improved event flow.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
a45b3dd0a8237fdd080536cb9ab600e71ad939d3 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Review Request 40795: Quota: Properly initialized the sorter for quota'ed roles in the allocator.

[Alexander Rukletsov]

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

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


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
31ed62efb5b1a2edb567f43d37559c5914e0665e 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40332: Quota: Implemented recovery in hierarchical allocator.

[Alexander Rukletsov]

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

(Updated Nov. 30, 2015, 3:32 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1cd8d16661568010901e74705375e7719cdfb8a0 
  src/master/allocator/mesos/hierarchical.cpp 
31ed62efb5b1a2edb567f43d37559c5914e0665e 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40553: Enable mesos tests installation

[James Peach]

> On Nov. 25, 2015, 2:36 p.m., Benjamin Bannier wrote:
> > src/Makefile.am, line 1753
> > 
> >
> > This would be a perfect opportunity to add a config header, and not 
> > pass any more strings as command line flags. Are we sure really we liked 
> > the approach taken previously with`SOURCE_DIR` and `BUILD_DIR`? OK if you 
> > disagree, not an issue.

I'd be happy to do that as a separate Jira, but I think that this change is 
intrusive enough already.


> On Nov. 25, 2015, 2:36 p.m., Benjamin Bannier wrote:
> > src/tests/utils.cpp, line 71
> > 
> >
> > Should this be a JIRA issue?

I don't know whether there is even supposed to be a module directory. I removed 
the XXX because this seems to be intentional.


> On Nov. 25, 2015, 2:36 p.m., Benjamin Bannier wrote:
> > src/tests/utils.hpp, line 38
> > 
> >
> > These functions cannot promise to return *valid* paths; they should 
> > probably all return `Try` instead. Please also check their impls.

Well they may return a valid path to something that doesn't exist, which is 
fine and will cause the test to fail. It might be reasonable to return a 
```Try``` and change the callers to do ```findX().get()```, which would 
```ABORT``` at the call-site rather than some way into the test.


> On Nov. 25, 2015, 2:36 p.m., Benjamin Bannier wrote:
> > src/tests/utils.hpp, line 37
> > 
> >
> > I think `getX` would be more in line with other places in the code base.

Agreed.


> On Nov. 25, 2015, 2:36 p.m., Benjamin Bannier wrote:
> > src/tests/containerizer/memory_test_helper.cpp, line 197
> > 
> >
> > Should probably be passed as an arg (`bool` or even better an `enum` 
> > value). You can always pick a default.

I am not sure that I follow what you mean here. Are you suggesting that the 
caller should specify where to search, based on the ```MESOS_INSTALL_TESTS``` 
define? If so, I don't think the callers should need to know that. The default 
needs to be consistent across a test run, which implies that specific tests 
should not be toggling this in an ad-hoc fashion.


> On Nov. 25, 2015, 2:36 p.m., Benjamin Bannier wrote:
> > src/tests/utils.cpp, line 56
> > 
> >
> > All the `findX` functions here (but maybe `findModuleDir`) seem very 
> > easy to use incorrectly. We might e.g., depending on what is in our build 
> > directory or in the install prefix load a mix of different versions.
> > 
> > I would much prefer if we'd either load from one place or the other.
> > 
> > Going down that path would probably also lead us to remove a lot of the 
> > duplication here, by e.g., calling once into `findModuleDir` and building 
> > up the more specialized functions from there.

Yes, forcing a blanket policy is exactly why I needed the ```findX``` family. 
There are only 2 scenarios that can cause inconsistent paths to tbe used: (1) 
if you run the tests from an installation and do not specify --build_dir=/none 
you could run tests from installed version A with helpers from built version B, 
and (2) if you nuke some parts of the build directory before running the tests.

I avoided forcing either one path or the other because it seemed reasonable to 
run "make check" before installing. It would be simpler to always force the 
tests to be installed before running in the ```--enable-tests-install``` case 
if we could agree that "make check" would fail in that case.

In general, I don't see these functions as easily composable. Some of them 
happen to produce the same path string, but there's no real relationship 
between the launcher directory and the test helper directory, for example.


> On Nov. 25, 2015, 2:36 p.m., Benjamin Bannier wrote:
> > src/tests/utils.cpp, line 64
> > 
> >
> > Here and in the following: I have the feeling explicitly passing in a 
> > flag (e.g., `bool searchInstallationDirectory`, or even clearer with an 
> > `enum` value) would make this easier to digest. All of these could have a 
> > default.

How would a test know whether to specify ```searchInstallationDirectory```? A 
global command-line flag?


- James


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


On Nov. 25, 2015, 2:09 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated 

Re: Review Request 40553: Enable mesos tests installation

[James Peach]

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

(Updated Nov. 30, 2015, 6:43 p.m.)


Review request for mesos, Benjamin Bannier 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 (updated)
-

  configure.ac b30a8d30076f3068fd7d5fc8ccea1982639e998a 
  src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04 
  src/tests/containerizer/launch_tests.cpp 
c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
  src/tests/containerizer/memory_test_helper.cpp 
4a3de2e3c887aa6afc604588850e1386f92d8c11 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
fe679354d04d68b68e168cd8c4eab23898f6532f 
  src/tests/containerizer/ns_tests.cpp 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
  src/tests/containerizer/port_mapping_tests.cpp 
9bcf05ec071b44156b57d8515f47ee6a8bbfdfa0 
  src/tests/environment.cpp 36f0ad0b739559e5c883d72585731944da4283cf 
  src/tests/fetcher_tests.cpp 99c494a2167ea9ec94e63d92980df17a4089f95a 
  src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
  src/tests/mesos.cpp a75182ec7f6d0c6063bbb36a6ccf4e8a4a81c8dd 
  src/tests/module.cpp e272bf0eccb61ae54440ec79adac8efad804c828 
  src/tests/module_tests.cpp b5cfbcd4885a4f2c20b19e00958fedda81afa6e0 
  src/tests/oversubscription_tests.cpp 0333281c247dd182860a49f39be791c00679bf6b 
  src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
  src/tests/slave_tests.cpp 4975bea8a7a701e0414426760692720f73dea7f5 
  src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
  src/tests/utils.cpp 877139e97249761658dce3b1058cdc2e2a52367b 

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


Testing
---


Thanks,

James Peach

Review Request 40799: Removed non-ASCII characters from test case comment.

[Neil Conway]

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

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


Repository: mesos


Description
---

Removed non-ASCII characters from test case comment.


Diffs
-

  src/tests/master_quota_tests.cpp 481298361574008b2cf9d213c908deb633393e31 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 40667: Fixed flakiness in reservation endpoint tests

[Anand Mazumdar]

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

(Updated Nov. 30, 2015, 6:55 p.m.)


Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description (updated)
---

1. Fixed some flakiness in the reservation tests around `resourceOffers` 
expectation being set after `driver.start()` leading to a race condition 
sometimes.

```GMOCK WARNING:
Uninteresting mock function call - returning directly.
Function call: resourceOffers(0x7fffe27c3830, @0x2ae03c9ce9d0 { 144-byte 
object  })
S```

2. Added `AWAIT_READY` for the returned future from 1.
3. Removed the initial call to `reviveOffers` after `driver.start()` since that 
was redundant.

-- Not to be included in commit description --
Still does not fixes MESOS-4002 completely though. The test kept looping for 
300 minutes meaning there is some future that was never ready and we invoked a 
`.get()` blocking forever or a deadlock somewhere else. Would keep digging for 
the root cause and for future failures on the ASF CI leading to more clues.


Diffs
-

  src/tests/reservation_endpoints_tests.cpp 
f4e332327049944000baccd3e607201240a8c5f4 

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


Testing
---

make check + ran the tests in a loop. They did not fail for me before the fix 
too :-(


Thanks,

Anand Mazumdar

Re: Review Request 40788: Added lambda function in resources.cpp.

[Neil Conway]

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

Ship it!


Ship It!


src/common/resources.cpp (line 1149)


I wonder if we could clarify this by replacing lambda::bind with a lambda 
expression?


- Neil Conway


On Nov. 30, 2015, 6:48 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40788/
> ---
> 
> (Updated Nov. 30, 2015, 6:48 p.m.)
> 
> 
> Review request for mesos, Michael Park and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added lambda function in resources.cpp.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 98804a456b0faecc4aa56929bd1b83e5900db5ae 
> 
> Diff: https://reviews.apache.org/r/40788/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40746: Quota: Removed quota from registry for remove request.

[Joseph Wu]

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

Ship it!


LGTM.


src/master/quota_handler.cpp (lines 393 - 394)


`RemoveQuota` appears idempotent, so what is the problem with trying to 
remove a quota that is currently being removed?

Is it the fact that you check this?
```
  // Check that we are removing an existing quota.
  if (!master->quotas.contains(role)) { ... }
```


- Joseph Wu


On Nov. 26, 2015, 4:28 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40746/
> ---
> 
> (Updated Nov. 26, 2015, 4:28 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-4021
> https://issues.apache.org/jira/browse/MESOS-4021
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp e9c8791d4bd00e6c1be1844f27d9bee26f722a9b 
> 
> Diff: https://reviews.apache.org/r/40746/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 40788: Added lambda function in resources.cpp.

[Greg Mann]

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

Review request for mesos, Michael Park and Neil Conway.


Repository: mesos


Description
---

Added lambda function in resources.cpp.


Diffs
-

  src/common/resources.cpp 98804a456b0faecc4aa56929bd1b83e5900db5ae 

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


Testing
---

`make check`


Thanks,

Greg Mann

Review Request 40787: Removed obsolete TODO in mesos.proto.

[Greg Mann]

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Removed obsolete TODO in mesos.proto.


Diffs
-

  include/mesos/mesos.proto 27971fea6935b82d7034397edaa7a37edb1f6f38 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 40114: Windows: Began adding Windows support to `process/future.hpp`

[Joseph Wu]

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


Note: You need to include the two new files in 
`3rdparty/libprocess/include/Makefile.am`.

- Joseph Wu


On Nov. 24, 2015, 3:04 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40114/
> ---
> 
> (Updated Nov. 24, 2015, 3:04 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Windows & Posix defines for several parameter list used in future.hpp. 
> Provided a new parameter list for VC2015.
> Move the existing parameter list into posix.hpp. 
> 
> The change has been due to: 
> 
>1. VS 2015 won't support C++14 result_of SFINAE until Update 2,  so
>   result_of must be replaced with decltype(invoke).
>2. VS 2015 won't support C++14 std::function SFINAE until Update 2, so
>   converting _Deferred to std::function must be done by explicitly
>   calling _Deferred's conversion function.
>3. The Standard (C++11 through 17) does not require bind's function call
>   operator to SFINAE, and VS 2015's doesn't.  is_bind_expression can be
>   used to manually reroute bind expressions to the 1-arg overload, where
>   (conveniently) the argument will be ignored if necessary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 3916150691e9a0880b8b826734fa74bd33d18cfd 
>   3rdparty/libprocess/include/process/posix/future.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/windows/future.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40114/diff/
> 
> 
> Testing
> ---
> 
> OSX make check, Windows 10 make, Ubuntu 15.1 make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 39780: Update OversubscriptionTest to not assume dynamic dlopen search.

[James Peach]

> On Nov. 30, 2015, 8:51 a.m., Benjamin Bannier wrote:
> > src/tests/oversubscription_tests.cpp, line 117
> > 
> >
> > This isn't really too nice, and e.g. cmake does not put build libraries 
> > into these paths (i.e. what you write will only work with automake).
> > 
> > Like I already pointed out on #40553, you should really lift the level 
> > of abstraction here, and use a function to get this path.
> 
> James Peach wrote:
> The intent of the original code was clearly to use this path. When 
> running in the context of the automake wrapper scripts, automake constructs 
> LD_LIBRARY_PATH so that we don't really need to do anything here. What does 
> cmake do? Can you point me to any documentation of how it handles library 
> search paths when executing artifacts from the build directory?
> 
> Benjamin Bannier wrote:
> You are of course right that this wasn't yours.
> 
> My point is that in #40553 you introduce abstractions like `findModule` 
> that should be perfectly good to capture what was written here.

Do you recommend rebasing this onto the subsequent patches? Or adding a new 
```getX``` helper here and rebasing the later patches on to it? Or something 
else that will work for both automake and cmake?


- James


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


On Nov. 26, 2015, 2:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39780/
> ---
> 
> (Updated Nov. 26, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-3725
> https://issues.apache.org/jira/browse/MESOS-3725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update OversubscriptionTest to not assume dynamic dlopen search.
> 
> 
> Diffs
> -
> 
>   src/tests/oversubscription_tests.cpp 
> 0d0bf7e0b9a4028ed116e00b56d59b670510c5ce 
> 
> Diff: https://reviews.apache.org/r/39780/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 39456: Documentation: added containerizer internals

[Jojy Varghese]

> On Nov. 21, 2015, 2:19 a.m., Timothy Chen wrote:
> > Are you still working on this? This doesn't look complete to me?

Thanks for taking a look Tim.Since the area is too big to document all in one 
go, do you have any particular topics you would like to be in the first 
iteration?


- Jojy


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


On Oct. 21, 2015, 9:25 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39456/
> ---
> 
> (Updated Oct. 21, 2015, 9:25 p.m.)
> 
> 
> Review request for mesos, Connor Doyle and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation: added containerizer internals
> 
> 
> Diffs
> -
> 
>   docs/containerizer-internals.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 40454: Add calls to parent teardown methods in child teardown methods.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Nov. 18, 2015, 9:52 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40454/
> ---
> 
> (Updated Nov. 18, 2015, 9:52 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
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 01d22b46a681f40dfa923f0a5c9b9b9b084d9bcd 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> edfbb6fe932173dcbb15133e0eb685d86dd09c00 
>   src/tests/mesos.cpp 766a51cddc8801d5e79188f80e9ce0828598c8b9 
>   src/tests/module_tests.cpp 28c71b0c1960bad4933f86d35fe8a0248fef326e 
> 
> Diff: https://reviews.apache.org/r/40454/diff/
> 
> 
> Testing
> ---
> 
> Picked https://reviews.apache.org/r/40268/ which exposes the bad cleanup 
> after SSL-reinitializing tests.
> 
> Then:
> `make check GTEST_FILTER="*RegistryClientTest*:*ExecutorHttpApiTest*"`
> 
> The `RegistryClientTest` involve some reinitialization of SSL variables.  The 
> `ExecutorHttpApiTest` spawns sockets.  If the SSL variables are not cleaned 
> up, the second test will try to use SSL sockets against non-SSL endpoints.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40453: Add a TearDownTestCase method for cleaning up after tests that reinitialize SSL configuration.

[Joseph Wu]

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

(Updated Nov. 30, 2015, 10:15 a.m.)


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


Changes
---

Rebase!


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


Repository: mesos


Description
---

Tests that reinitiailize SSL configuration only change environment variables 
upon setup.  This means the changed environment will stay intact after the 
test.  This may lead to unexpected results in subsequent tests.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
95e19845f90f648b148a39e7791eb4d4df939876 

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


Testing
---

See next review.


Thanks,

Joseph Wu

Re: Review Request 40454: Add calls to parent teardown methods in child teardown methods.

[Joseph Wu]

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

(Updated Nov. 30, 2015, 10:15 a.m.)


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


Changes
---

Rebase (1 conflict)!


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
b98461e65c2d6e97bce8e7906b4cba87d8bfd35c 
  src/tests/mesos.cpp a75182ec7f6d0c6063bbb36a6ccf4e8a4a81c8dd 
  src/tests/module_tests.cpp b5cfbcd4885a4f2c20b19e00958fedda81afa6e0 

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


Testing
---

Picked https://reviews.apache.org/r/40268/ which exposes the bad cleanup after 
SSL-reinitializing tests.

Then:
`make check GTEST_FILTER="*RegistryClientTest*:*ExecutorHttpApiTest*"`

The `RegistryClientTest` involve some reinitialization of SSL variables.  The 
`ExecutorHttpApiTest` spawns sockets.  If the SSL variables are not cleaned up, 
the second test will try to use SSL sockets against non-SSL endpoints.


Thanks,

Joseph Wu

Re: Review Request 40453: Add a TearDownTestCase method for cleaning up after tests that reinitialize SSL configuration.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Nov. 18, 2015, 9:52 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40453/
> ---
> 
> (Updated Nov. 18, 2015, 9:52 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
> ---
> 
> Tests that reinitiailize SSL configuration only change environment variables 
> upon setup.  This means the changed environment will stay intact after the 
> test.  This may lead to unexpected results in subsequent tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 54885ec359df947f6d31375d4c2a32cae587f845 
> 
> Diff: https://reviews.apache.org/r/40453/diff/
> 
> 
> Testing
> ---
> 
> See next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40630: Upgrade pip to 7.1.2

[Joseph Wu]

> On Nov. 25, 2015, 10:11 a.m., Joseph Wu wrote:
> > Verified `make distcheck` on Centos7, Ubunut 14, and Debian 8.
> > 
> > Our usage of `pip` is fairly innocuous (it installs the python 
> > scheduler/executor drivers), so this change should be fine.
> > 
> > Note: The CMake does not support `distcheck` yet.
> > For now, we should also open a PR to add the new tarball to [this 
> > repository](https://github.com/3rdparty/mesos-3rdparty).
> 
> Till Toenshoff wrote:
> Thanks a bunch Joseph, very assuring :).

Note: I'll coordinate with [~hausdorff] and update the 3rdparty repository 
(https://github.com/3rdparty/mesos-3rdparty/pull/2).


- Joseph


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


On Nov. 24, 2015, 12:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40630/
> ---
> 
> (Updated Nov. 24, 2015, 12:45 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3969
> https://issues.apache.org/jira/browse/MESOS-3969
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade pip to 7.1.2
> 
> 
> Diffs
> -
> 
>   3rdparty/pip-1.5.6.tar.gz b2111358d013fb40390a60b5d55bac08639f3efc 
>   3rdparty/pip-7.1.2.tar.gz PRE-CREATION 
>   3rdparty/versions.am a3e17dfc935f640a7377ae118334fd303b52fc33 
> 
> Diff: https://reviews.apache.org/r/40630/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 40453: Add a TearDownTestCase method for cleaning up after tests that reinitialize SSL configuration.

[Jan Schlicht]

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

Ship it!


Ship It!

- Jan Schlicht


On Nov. 30, 2015, 7:15 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40453/
> ---
> 
> (Updated Nov. 30, 2015, 7:15 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3975
> https://issues.apache.org/jira/browse/MESOS-3975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests that reinitiailize SSL configuration only change environment variables 
> upon setup.  This means the changed environment will stay intact after the 
> test.  This may lead to unexpected results in subsequent tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 95e19845f90f648b148a39e7791eb4d4df939876 
> 
> Diff: https://reviews.apache.org/r/40453/diff/
> 
> 
> Testing
> ---
> 
> See next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39634: FreeBSD: Enable mesos build and start fixing some tests

[Ian Downes]

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

Ship it!


Very minor things. I'll just make the changes when I commit the patch. Thanks!


configure.ac (line 619)


End with fullstop.



src/Makefile.am (line 1756)


Can you add a comment that FreeBSD includes the  necessary functionality in 
libc?



src/tests/attributes_tests.cpp (line 38)


These are unrelated to the FreeBSD support so can you split them out? Else, 
I'll do it when I commit this patch.


- Ian Downes


On Nov. 27, 2015, 8:26 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39634/
> ---
> 
> (Updated Nov. 27, 2015, 8:26 p.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: MESOS-1563
> https://issues.apache.org/jira/browse/MESOS-1563
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> FreeBSD: Enable mesos build and start fixing some tests
> 
> 
> Diffs
> -
> 
>   configure.ac b30a8d30076f3068fd7d5fc8ccea1982639e998a 
>   src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04 
>   src/tests/attributes_tests.cpp d8c84d25a37f9cf1b38a97193d5b3b3001fd54ff 
>   src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12 
>   src/tests/values_tests.cpp fb7f982a50df8274ad29dab9e55157c39acdc104 
> 
> Diff: https://reviews.apache.org/r/39634/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 39635: FreeBSD: Enable libprocess build and disable failing test

[Ian Downes]

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

Ship it!


Minor nits on comments. Will make the changes myself when I commit. Thanks!


3rdparty/libprocess/3rdparty/Makefile.am (line 243)


Full stop.



3rdparty/libprocess/configure.ac (line 835)


Full stop.



3rdparty/libprocess/src/tests/http_tests.cpp (line 897)


Full stop.


- Ian Downes


On Nov. 27, 2015, 8:26 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39635/
> ---
> 
> (Updated Nov. 27, 2015, 8:26 p.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: MESOS-1563
> https://issues.apache.org/jira/browse/MESOS-1563
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> FreeBSD: Enable libprocess build and disable failing test
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 0adbe539afaf683e4a85582463a2930049a63998 
>   3rdparty/libprocess/configure.ac 40801653a7fb9a943dfe33913161d28ef24040c3 
>   3rdparty/libprocess/src/config.hpp 8444a6018f9bc911d59a751b5a763f73d86ab1e6 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/39635/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 40454: Add calls to parent teardown methods in child teardown methods.

[Jan Schlicht]

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

Ship it!


Ship It!

- Jan Schlicht


On Nov. 30, 2015, 7:15 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40454/
> ---
> 
> (Updated Nov. 30, 2015, 7:15 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3975
> https://issues.apache.org/jira/browse/MESOS-3975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> b98461e65c2d6e97bce8e7906b4cba87d8bfd35c 
>   src/tests/mesos.cpp a75182ec7f6d0c6063bbb36a6ccf4e8a4a81c8dd 
>   src/tests/module_tests.cpp b5cfbcd4885a4f2c20b19e00958fedda81afa6e0 
> 
> Diff: https://reviews.apache.org/r/40454/diff/
> 
> 
> Testing
> ---
> 
> Picked https://reviews.apache.org/r/40268/ which exposes the bad cleanup 
> after SSL-reinitializing tests.
> 
> Then:
> `make check GTEST_FILTER="*RegistryClientTest*:*ExecutorHttpApiTest*"`
> 
> The `RegistryClientTest` involve some reinitialization of SSL variables.  The 
> `ExecutorHttpApiTest` spawns sockets.  If the SSL variables are not cleaned 
> up, the second test will try to use SSL sockets against non-SSL endpoints.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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/#review108376
---

Ship it!


Ship It!

- Jie Yu


On Nov. 23, 2015, 10:33 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> ---
> 
> (Updated Nov. 23, 2015, 10:33 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/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0 
>   src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a 
>   src/linux/routing/handle.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 09f819685a96cb4785423a5b2303d147719e273e 
>   src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36 
>   src/slave/flags.cpp 6e8872e435c9a5d895539e3cd47b41f66bc4eb89 
> 
> 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 38899: Added handler for Executor->Framework message for the Executor HTTP API

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Nov. 30, 2015, 5:01 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38899/
> ---
> 
> (Updated Nov. 30, 2015, 5:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-2295
> https://issues.apache.org/jira/browse/MESOS-2295
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Trivial change to call the `executorMessage(...)` function that already 
> existed in `src/slave/slave.cpp` to handle ExecutorToFrameworkMessage.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 
> 
> Diff: https://reviews.apache.org/r/38899/diff/
> 
> 
> Testing
> ---
> 
> make check. Would add tests later after the MESOS-3480 review chain gets 
> committed.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 39985: [1/5] Introduced ACL protobuf definitions for dynamic reservation.

[Greg Mann]

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

(Updated Nov. 30, 2015, 8:32 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Introduced ACL protobuf definitions for dynamic reservation.
Note: this review is continued from https://reviews.apache.org/r/37002/


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
8e72003f405770f00c5d87f318a9e1a8ed7430ee 

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


Testing
---

This is the first in a chain of 5 patches. `make check` was used to test at the 
end of the chain.

Note: documentation for these changes is included in 
https://reviews.apache.org/r/40271/


Thanks,

Greg Mann

Re: Review Request 39986: [2/5] Enabled the Authorizer to handle Reserve/Unreserve ACLs.

[Greg Mann]

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

(Updated Nov. 30, 2015, 8:33 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Enabled the Authorizer to handle Reserve/Unreserve ACLs.
Note: this review is continued from https://reviews.apache.org/r/37110/


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
c228c2b40ff5fdb007153101938f91ff269370a5 
  src/authorizer/local/authorizer.hpp bc3e4e12a35f222783159e35c4662e182ba0c107 
  src/authorizer/local/authorizer.cpp 853678b8fd3151d02fc77d5eadaf9948f33caebb 
  src/tests/authorization_tests.cpp 32bcac731a6e8f8adb7298ed6c7174f6eb0499cb 
  src/tests/mesos.hpp f097c683fed02240e9f54ac4491ab007f4652736 
  src/tests/mesos.cpp a75182ec7f6d0c6063bbb36a6ccf4e8a4a81c8dd 

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


Testing
---

The is the second in a chain of 5 patches. This diff includes new tests for the 
authorization of reserve and unreserve operations via ACLs. `make check` was 
used to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

[Greg Mann]

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

(Updated Nov. 30, 2015, 8:35 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Address comments, refactor to include validation.


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


Repository: mesos


Description
---

Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.
Note: this review is continued from https://reviews.apache.org/r/37125/


Diffs (updated)
-

  src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
  src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 

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


Testing
---

This is the third in a chain of 5 patches. `make check` was used to test after 
all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39988: [4/5] Added authorization for dynamic reservation master endpoints.

[Greg Mann]

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

(Updated Nov. 30, 2015, 8:46 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Refactored validation into master's authorization helpers.


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


Repository: mesos


Description
---

Added authorization for dynamic reservation master endpoints.
Note: this review is continued from https://reviews.apache.org/r/37126/


Diffs (updated)
-

  src/master/http.cpp 9d729ef7f7d7ad6185934648f833e4f8a4f0a123 
  src/master/master.hpp 96951e766de32842197506504e5ac67a2caa3efe 
  src/tests/reservation_endpoints_tests.cpp 
f30ff8bc6a3e9773437fa7fd7c8f569b7d7e2d9d 

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


Testing
---

This is the fourth in a chain of 5 patches. Added new reservation endpoints 
tests to validate authorization of reserve and unreserve operations using ACLs. 
`make check` was run to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 39297: Added support for recovering RunState for HTTP based executors

[Vinod Kone]

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

Ship it!



src/slave/state.cpp (lines 532 - 534)


Kill this. I don't think we do this for any other states that we recover?



src/slave/state.cpp (lines 546 - 547)


ditto. kill.


- Vinod Kone


On Nov. 10, 2015, 2:24 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39297/
> ---
> 
> (Updated Nov. 10, 2015, 2:24 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for recovering the `RunState` for `HTTP` based 
> executors. Upon agent recovery, it checks if the marker file for HTTP exists 
> and populates `RunState.http` based on that. This is later used by the Agent 
> for recovering `HTTP` based executors.
> 
> 
> Diffs
> -
> 
>   src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
>   src/slave/state.cpp d14159f5e8ca9957cbdcce53050b00a00dba2135 
> 
> Diff: https://reviews.apache.org/r/39297/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 39989: [5/5] Added framework authorization for dynamic reservation.

[Greg Mann]

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

(Updated Nov. 30, 2015, 8:48 p.m.)


Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.


Changes
---

Addressed comments, refactored validation into master's authorization helpers.


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


Repository: mesos


Description
---

Added framework authorization for dynamic reservation.
Note: this review is continued from https://reviews.apache.org/r/37127/


Diffs (updated)
-

  src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 
  src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 

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


Testing
---

This is the fifth in a chain of 5 patches. New reservation tests were added to 
`reservation_tests.cpp` to validate the authentication of framework reserve and 
unreserve operations using ACLs. `make check` was run to test after all patches 
were applied.


Thanks,

Greg Mann

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

[Joseph Wu]

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



include/mesos/resources.hpp (lines 155 - 156)


Can you add a comment here to explain that this is the same as 
`!isPersistentVolume`?  

i.e. This function is not redundant.  There is only one type of stateful 
resource currently.  But if more are added in future, this function should be 
extended accordingly.

---
Note: You may want to hold back on copy-pasting the comment all over this 
patch (which we will eventually want).  We will likely iterate over the wording 
a bit first :)


- Joseph Wu


On Nov. 24, 2015, 11:31 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40529/
> ---
> 
> (Updated Nov. 24, 2015, 11:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> 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
> -
> 
>   include/mesos/resources.hpp 51c63f8305f6c0313e0b34f912f5e53396a1591d 
>   include/mesos/v1/resources.hpp b481a9e0177b5253acc2585d5dfc6228ab63876c 
>   src/common/resources.cpp b4abf5405039d7d0a5028ccf034ad2e9623d064c 
>   src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12 
>   src/v1/resources.cpp 8488c318a987a150fc5fde26b54246e8effb0428 
> 
> Diff: https://reviews.apache.org/r/40529/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 40797: Filtered non-whitelisted and deactivated agents once per allocation.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40586, 40551, 40332, 40795, 39450, 40797]

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

- Mesos ReviewBot


On Nov. 30, 2015, 5:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40797/
> ---
> 
> (Updated Nov. 30, 2015, 5:14 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
> 
> Diff: https://reviews.apache.org/r/40797/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Joseph Wu]

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



src/messages/messages.proto (lines 326 - 329)


The design doc comment doesn't quite work without the context surrounding 
it.  :)

---
Suggestion:

When a task is launched on reserved resources, this list contains some 
executors that have oversubscribed the same reserved resources.  The agent can 
evict these executors to make room for the task.

This list is always empty when the task launch does not involve reserved 
resources (oversubscribed reserved resources are considered "revocable 
resources").


- Joseph Wu


On Nov. 24, 2015, 11:31 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40532/
> ---
> 
> (Updated Nov. 24, 2015, 11:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> 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 178d757a76f14829da6daab97ec678843717cf5a 
> 
> Diff: https://reviews.apache.org/r/40532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 40524: Used string directly in resources.cpp.

[Guangya Liu]

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

(Updated 十二月 1, 2015, 2:13 a.m.)


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


Repository: mesos


Description
---

Used string directly in resources.cpp.


Diffs (updated)
-

  src/common/resources.cpp 98804a456b0faecc4aa56929bd1b83e5900db5ae 
  src/v1/resources.cpp 8488c318a987a150fc5fde26b54246e8effb0428 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 40787: Removed obsolete TODO in mesos.proto.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40787]

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

- Mesos ReviewBot


On Nov. 30, 2015, 6:49 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40787/
> ---
> 
> (Updated Nov. 30, 2015, 6:49 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed obsolete TODO in mesos.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 27971fea6935b82d7034397edaa7a37edb1f6f38 
> 
> Diff: https://reviews.apache.org/r/40787/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Nov. 30, 2015, 9:58 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40506/
> ---
> 
> (Updated Nov. 30, 2015, 9:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add stdout/tests/numify_tests.cpp into Makefile.am
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 0adbe539afaf683e4a85582463a2930049a63998 
> 
> Diff: https://reviews.apache.org/r/40506/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 40806: Made HDFS::exists asynchronous.

[Jie Yu]

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

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


Repository: mesos


Description
---

Made HDFS::exists asynchronous.

This also fixed a bug in the orignal code: the original code never returns 
false.


Diffs
-

  src/cli/execute.cpp ac933b6a8636af3356f3d6a99656c0185077c262 
  src/hdfs/hdfs.hpp 6bdeedf8934a681b32fca8edc251488f4213dd07 
  src/hdfs/hdfs.cpp c32c2ae9d2ace5deb09f56dcec2bd366c2d07f6b 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Joseph Wu]

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



src/slave/slave.cpp (lines 4464 - 4466)


Can you add a comment about why this is purposefully and unilaterally done 
here (potentially overwriting what the ResourceEstimator has given you)?

It may also be a good idea to update the documentation in 
"include/mesos/slave/resource_estimator.hpp".


- Joseph Wu


On Nov. 24, 2015, 10:41 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Nov. 24, 2015, 10:41 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 9055f2a 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40787: Removed obsolete TODO in mesos.proto.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Nov. 30, 2015, 6:49 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40787/
> ---
> 
> (Updated Nov. 30, 2015, 6:49 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed obsolete TODO in mesos.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 27971fea6935b82d7034397edaa7a37edb1f6f38 
> 
> Diff: https://reviews.apache.org/r/40787/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40788: Added lambda function in resources.cpp.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40788]

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

- Mesos ReviewBot


On Nov. 30, 2015, 6:48 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40788/
> ---
> 
> (Updated Nov. 30, 2015, 6:48 p.m.)
> 
> 
> Review request for mesos, Michael Park and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added lambda function in resources.cpp.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 98804a456b0faecc4aa56929bd1b83e5900db5ae 
> 
> Diff: https://reviews.apache.org/r/40788/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 40808: Made the order of `EXPECT_CALL()`s match the expected order of events.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Similar to 5820cd3627ed01e5ede7; unfortunately, that commit missed a few
instances of this pattern.


Diffs
-

  src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

[Joseph Wu]

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



include/mesos/mesos.proto (lines 644 - 650)


Can you expand a little on these enum descriptions?

Ideally, these should include:
* A brief definition of each resource type.
* Which components control the resource and briefly how they exert control.



include/mesos/mesos.proto (line 653)


Can you note that this is effectively required?

i.e. Internally, we will ensure a `type` exists in all valid resources.  
(This may simplify some other code.)



src/common/resources.cpp (lines 98 - 110)


Note: When you specify a default, it is safe to call `protobuf.type()` 
without checking if it exists first.

See: https://developers.google.com/protocol-buffers/docs/proto#optional

So minimally, you could just `return left.type() == right.type();`.  (But 
if you do this, you need to comment about the default.)



src/common/resources.cpp (lines 1402 - 1405)


What are the backwards compatibility requirements for this?



src/tests/resources_tests.cpp (line 804)


This shouldn't pass after your modifications.


- Joseph Wu


On Nov. 24, 2015, 10:16 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> ---
> 
> (Updated Nov. 24, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3888
> https://issues.apache.org/jira/browse/MESOS-3888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3888: We need to distinguish revocable resource for usage slack and 
> allocation slack.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 27971fe 
>   include/mesos/v1/mesos.proto 9acefd5 
>   src/common/resources.cpp b4abf54 
>   src/tests/resources_tests.cpp dbd39cd 
>   src/v1/resources.cpp 8488c31 
> 
> Diff: https://reviews.apache.org/r/40375/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40797: Filtered non-whitelisted and deactivated agents once per allocation.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十一月 30, 2015, 5:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40797/
> ---
> 
> (Updated 十一月 30, 2015, 5:14 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
> 
> Diff: https://reviews.apache.org/r/40797/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40529: 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 十二月 1, 2015, 2:14 a.m.)


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


Changes
---

Remove depdency


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


Repository: mesos


Description
---

Added helper function to get stateless resources.


Diffs
-

  include/mesos/resources.hpp 51c63f8305f6c0313e0b34f912f5e53396a1591d 
  include/mesos/v1/resources.hpp b481a9e0177b5253acc2585d5dfc6228ab63876c 
  src/common/resources.cpp b4abf5405039d7d0a5028ccf034ad2e9623d064c 
  src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12 
  src/v1/resources.cpp 8488c318a987a150fc5fde26b54246e8effb0428 

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


Testing
---


Thanks,

Guangya Liu

Review Request 40811: Fixed flakiness in tests using the Scheduler library

[Anand Mazumdar]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Previously, the `event` queue object was defined after the `callbacks` object. 
However, the `callbacks` object held on a reference to the queue object. This 
led to a crash in some scenarios when we received another event from the master 
after the `event` object was destroyed. (Note that objects on the stack are 
destroyed in the reverse order of definition)

The short term fix is to move the `event` queue object above the `callbacks` 
object. This ensures that it is destroyed after the `callbacks` object. The 
long term fix would be worked on via `MESOS-3339` that moves the tests to a 
callback interface model similar to what we have for the driver tests.


Diffs
-

  src/tests/master_maintenance_tests.cpp 
ade05f932020013ced19c1573be756a029396fac 
  src/tests/scheduler_tests.cpp 67432109c7df6be0aa76e94a03bd5b2e9c96d14e 

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


Testing
---

make check + Reproduced this by introducing a sleep in the `Mesos` destructor + 
did not filter out the `HEARTBEAT` event.


Thanks,

Anand Mazumdar

Re: Review Request 37703: Add docker exec command.

[Timothy Chen]

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



src/tests/containerizer/docker_tests.cpp (line 297)


This isn't enough still, when the future is ready doesn't mean the 
container is actually running yet.

What you could do is to run docker->inspect(containerName, 
Miliseconds(500)) and it will retry inspect until it's running, then you AWAIT 
that inspect future.


- Timothy Chen


On Nov. 29, 2015, 4:37 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37703/
> ---
> 
> (Updated Nov. 29, 2015, 4:37 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3291
> https://issues.apache.org/jira/browse/MESOS-3291
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add docker exec command.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
>   src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
>   src/tests/containerizer/docker_tests.cpp 
> d9e1345aea0ef9db0e50360e3993ef2708970298 
> 
> Diff: https://reviews.apache.org/r/37703/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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 Dec. 1, 2015, 12:18 a.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 addcc7262584c19f9e5afe4b796c5fc4ecdb53b7 
  src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04 
  src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a 
  src/linux/routing/handle.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
09f819685a96cb4785423a5b2303d147719e273e 
  src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36 
  src/slave/flags.cpp 31700b4f80cf346ab68f9e35d33d7cbb5ddc04cc 

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 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 Dec. 1, 2015, 12:18 a.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.


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 38287: Check if the futrue is failed before dispatch in freeze()

[Ben Mahler]

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


See the comment below about the issue with this approach. I will commit a fix 
without the race.


src/linux/cgroups.cpp (lines 2431 - 2440)


The bug here is that when we pass a Process pointer to a managed=true call 
to spawn, this is passing ownership and we must not access the pointer any 
further.

The fix you've proposed here is racy, the future failure may occur at any 
time. If it occurs between your if condition and the dispatch, this will still 
crash.

I would suggest instead that we obtain the PID before we spawn, so that we 
can dispatch using the PID instead of a process pointer.


- Ben Mahler


On Nov. 23, 2015, 5:37 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38287/
> ---
> 
> (Updated Nov. 23, 2015, 5:37 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Paul Brett.
> 
> 
> Bugs: MESOS-3272
> https://issues.apache.org/jira/browse/MESOS-3272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer, the 
> freezer process is terminated in it initilize method, which causes the core 
> dump when cgroups::freezer::freeze() calls dispatch(freezer, 
> ::Freezer::freeze). 
> 
> This check is added in cgroups::freezer::freeze() to avoid calling dispatch 
> if the freezer process is terminated.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp f67633e31884b21e943a06100191d2228a637b1a 
> 
> Diff: https://reviews.apache.org/r/38287/diff/
> 
> 
> Testing
> ---
> 
> ./mesos-tests.sh 
> --gtest_filter="CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer"
>  --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

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 Dec. 1, 2015, 12:17 a.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.


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 
440181c25d9d132f48c7a4888dc1dbb0157dc6c8 
  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 40524: Used string directly in resources.cpp.

[Joseph Wu]

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

Ship it!


This is some nice code cleanup, but isn't related to optimistic offers.  
Can you move it out of the review chain?  (We can review/commit it quicker 
separately.)

- Joseph Wu


On Nov. 24, 2015, 10:41 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40524/
> ---
> 
> (Updated Nov. 24, 2015, 10:41 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> and Klaus Ma.
> 
> 
> Bugs: MESOS-3955
> https://issues.apache.org/jira/browse/MESOS-3955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used string directly in resources.cpp.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp b4abf5405039d7d0a5028ccf034ad2e9623d064c 
>   src/v1/resources.cpp 8488c318a987a150fc5fde26b54246e8effb0428 
> 
> Diff: https://reviews.apache.org/r/40524/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Jie Yu]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp (lines 42 - 47)


I would write the logics here in the following way:

```
if (!ss.fail() && ss.eof()) {
  return result;
}

return Error("...");
```



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


We typically use 10u instead of 10U.



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


Please use EXPECT_ERROR here. Ditto for the followings.



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


Kill extra lines here.



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


Can you also add a floating point test here:

```
EXPECT_ERROR(numify("0x10.9"));
```


- Jie Yu


On Nov. 30, 2015, 9:57 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40497/
> ---
> 
> (Updated Nov. 30, 2015, 9:57 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.
> 
> 
> 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 
> 440181c25d9d132f48c7a4888dc1dbb0157dc6c8 
>   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 40339: Added a flag to master to enable oversubscription for reservations.

[Joseph Wu]

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



include/mesos/master/allocator.hpp (line 96)


Suggestion: rename the variable to `enableReservationOversubscription`.



src/master/flags.cpp (lines 439 - 443)


Suggestion:

Whether to enable oversubscription for reservations. If enabled, the 
allocator will optimistically offer reserved-but-unallocated resources to other 
frameworks as revocable resources.  Tasks running on revocable resources will 
be evicted when the reserved resources are allocated.


- Joseph Wu


On Nov. 23, 2015, 9:40 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40339/
> ---
> 
> (Updated Nov. 23, 2015, 9:40 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
> 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 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 2a21364fdcaa4ec5e5567b9f367c14a1579b9a49 
>   src/master/allocator/mesos/hierarchical.cpp 
> aee8ced1fbfec8cf30cb939ff67fadfc6b08f37a 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp 2d58fcd56317897786a8b832c4ccf7f3d7b566e5 
>   src/master/master.cpp 370980edfc80d1e52134fdaf3ce49177b6528b02 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
>   src/tests/hierarchical_allocator_tests.cpp 
> a45b3dd0a8237fdd080536cb9ab600e71ad939d3 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_quota_tests.cpp 330e591f81c7ece7f401042ad159bd6b55881a84 
>   src/tests/reservation_endpoints_tests.cpp 
> f4e332327049944000baccd3e607201240a8c5f4 
>   src/tests/reservation_tests.cpp 1d0a65d5b4c2cb03e49f302176084ef5d602569f 
>   src/tests/resource_offers_tests.cpp 
> 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
> 
> Diff: https://reviews.apache.org/r/40339/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 14.04
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39417: Add --egress_flow_classifier_parent flag

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40497, 40506, 39415, 39416, 39417]

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

- Mesos ReviewBot


On Dec. 1, 2015, 12:18 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> ---
> 
> (Updated Dec. 1, 2015, 12:18 a.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 addcc7262584c19f9e5afe4b796c5fc4ecdb53b7 
>   src/Makefile.am fd38cfa73d81a98c819378f99a766e2ddb7e1a04 
>   src/linux/routing/handle.hpp 6deff85b52daa6b8771f127fea0d661cb5cd5e6a 
>   src/linux/routing/handle.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0865a71b1d79adcf62ca0f12d7ea0a23ebd0e786 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 09f819685a96cb4785423a5b2303d147719e273e 
>   src/slave/flags.hpp 0858bdfa93f0c82c9654f9a11d1a251df2d41c36 
>   src/slave/flags.cpp 31700b4f80cf346ab68f9e35d33d7cbb5ddc04cc 
> 
> 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 37853: Overlay filesystem provisioning backend

[Mei Wan]

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

(Updated Dec. 1, 2015, 5:26 a.m.)


Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

Implemented the overlay filesystem backend by layering the images as a 
read-only filesystem.


Diffs (updated)
-

  src/CMakeLists.txt cfe9d26 
  src/Makefile.am fd38cfa 
  src/slave/containerizer/mesos/provisioner/backend.cpp 01d06eb 
  src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 25b28ef 

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


Testing
---

Basic test in provisioner_backend_tests.cpp.


Thanks,

Mei Wan

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40339]

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

Error:
 + : ubuntu:14.04
+ : gcc
+ : --verbose
+++ dirname ./support/docker_build.sh
++ cd ./support/..
++ pwd
+ MESOS_DIRECTORY=/home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ cd /home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ DOCKERFILE=Dockerfile
+ rm -f Dockerfile
+ case $OS in
+ append_dockerfile 'FROM ubuntu:14.04'
+ echo FROM ubuntu:14.04
+ append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
+ echo RUN rm -rf 
/var/lib/apt/lists/extras.ubuntu.com_ubuntu_dists_precise_main_binary-amd64_Packages
 
/var/lib/apt/lists/extras.ubuntu.com_ubuntu_dists_precise_main_binary-i386_Packages
 /var/lib/apt/lists/extras.ubuntu.com_ubuntu_dists_precise_Release 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_main_binary-amd64_Packages 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_main_binary-i386_Packages 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_Release 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_Release.gpg 
/var/lib/apt/lists/lock 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_main_binary-amd64_Packages
 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_main_binary-i386_Packages
 /var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_Release 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_Release.gpg 
/var/lib/apt/lists/partial 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_t
 rusty_main_binary-amd64_Packages 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_binary-i386_Packages
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_i18n_Translation-en
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_source_Sources
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_Release
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_source_Sources
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_binary-i386_Packages
 /var/lib/apt/lists
 
/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_source_Sources
 /var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_Release 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_Release.gpg 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_source_Sources
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_binary-i386_Packages
 /var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_univers
 e_i18n_Translation-en 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_main_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_main_binary-i386_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_main_i18n_Translation-en
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_multiverse_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_multiverse_binary-i386_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_multiverse_i18n_Translation-en
 /var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_Release 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_Release.gpg 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_restricted_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_restricted_binary-i386_Packages
 /var
 
/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_precise_restricted_i18n_Translation-en
 

Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

[Klaus Ma]

> On Dec. 1, 2015, 9:57 a.m., Joseph Wu wrote:
> > include/mesos/mesos.proto, line 653
> > 
> >
> > Can you note that this is effectively required?
> > 
> > i.e. Internally, we will ensure a `type` exists in all valid resources. 
> >  (This may simplify some other code.)

Yes; to keep the compatibility when serializing, I changed it to `optional`.


> On Dec. 1, 2015, 9:57 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 1402-1405
> > 
> >
> > What are the backwards compatibility requirements for this?

I update the `operator<<` to return `{REV}` when `has_type()` return false.


> On Dec. 1, 2015, 9:57 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 98-110
> > 
> >
> > Note: When you specify a default, it is safe to call `protobuf.type()` 
> > without checking if it exists first.
> > 
> > See: https://developers.google.com/protocol-buffers/docs/proto#optional
> > 
> > So minimally, you could just `return left.type() == right.type();`.  
> > (But if you do this, you need to comment about the default.)

Agree, use `return left.type() == right.type()` for `operator==`.


> On Dec. 1, 2015, 9:57 a.m., Joseph Wu wrote:
> > include/mesos/mesos.proto, lines 644-650
> > 
> >
> > Can you expand a little on these enum descriptions?
> > 
> > Ideally, these should include:
> > * A brief definition of each resource type.
> > * Which components control the resource and briefly how they exert 
> > control.

Sure, I'll update comments accordingly.


- Klaus


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


On Nov. 25, 2015, 2:16 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> ---
> 
> (Updated Nov. 25, 2015, 2:16 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3888
> https://issues.apache.org/jira/browse/MESOS-3888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3888: We need to distinguish revocable resource for usage slack and 
> allocation slack.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 27971fe 
>   include/mesos/v1/mesos.proto 9acefd5 
>   src/common/resources.cpp b4abf54 
>   src/tests/resources_tests.cpp dbd39cd 
>   src/v1/resources.cpp 8488c31 
> 
> Diff: https://reviews.apache.org/r/40375/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

[Klaus Ma]

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

(Updated Dec. 1, 2015, 3:31 p.m.)


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


Changes
---

Address comments


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


Repository: mesos


Description
---

MESOS-3888: We need to distinguish revocable resource for usage slack and 
allocation slack.


Diffs (updated)
-

  include/mesos/mesos.proto 27971fe 
  include/mesos/v1/mesos.proto 9acefd5 
  src/common/resources.cpp 98804a4 
  src/tests/resources_tests.cpp dbd39cd 
  src/v1/resources.cpp 8488c31 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 40756: Updated codebase to use `nonRevocable()` where appropriate.

[Alexander Rukletsov]

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

(Updated Nov. 30, 2015, 10:45 a.m.)


Review request for mesos, Ian Downes and Michael Park.


Changes
---

Comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/common/http.cpp 692ea3fba2e48e8f8b5257a6bb6ea8a40609013b 
  src/master/allocator/mesos/hierarchical.cpp 
31ed62efb5b1a2edb567f43d37559c5914e0665e 
  src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 
  src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 40756: Updated codebase to use `nonRevocable()` where appropriate.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40755, 40756]

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

- Mesos ReviewBot


On Nov. 30, 2015, 10:45 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40756/
> ---
> 
> (Updated Nov. 30, 2015, 10:45 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Michael Park.
> 
> 
> Bugs: MESOS-4020
> https://issues.apache.org/jira/browse/MESOS-4020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 692ea3fba2e48e8f8b5257a6bb6ea8a40609013b 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
>   src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 
>   src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 
> 
> Diff: https://reviews.apache.org/r/40756/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40756: Updated codebase to use `nonRevocable()` where appropriate.

[Alexander Rukletsov]

> On Nov. 28, 2015, 3:39 a.m., Guangya Liu wrote:
> > https://github.com/apache/mesos/blob/master/src/master/master.cpp#L4322 
> > also needs an update
> 
> Alexander Rukletsov wrote:
> I'm not sure about that one. I think it's more readable how it's now.
> 
> Guangya Liu wrote:
> I think that you already did some similiar update in hierarchical.cpp, 
> this makes the code more simple to understand.
> 
>  // First remove the old oversubscribed resources from the total.
>   slaves[slaveId].total = slaves[slaveId].total.nonRevocable();

Done.


- Alexander


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


On Nov. 30, 2015, 10:45 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40756/
> ---
> 
> (Updated Nov. 30, 2015, 10:45 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Michael Park.
> 
> 
> Bugs: MESOS-4020
> https://issues.apache.org/jira/browse/MESOS-4020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 692ea3fba2e48e8f8b5257a6bb6ea8a40609013b 
>   src/master/allocator/mesos/hierarchical.cpp 
> 31ed62efb5b1a2edb567f43d37559c5914e0665e 
>   src/master/master.cpp b918ae4a0e7dc3cd41165fc4b683ae7b6f031821 
>   src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 
> 
> Diff: https://reviews.apache.org/r/40756/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39781: Update ModuleTest to not assume dynamic dlopen search.

[Benjamin Bannier]

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



src/tests/module_tests.cpp (line 54)


Since there is only one item left, this needs to be reworded, e.g.

> .., we do th `dlopen()` the examplemodule library and ..



src/tests/module_tests.cpp (line 70)


Not yours, but this should really be a `static_cast`.



src/tests/module_tests.cpp (line 77)


Since `TearDownTestCase` is effectively a fixture's dtr, both zeroing out 
`moduleBase` (a raw ptr) and clearing `libraryDirectory` (just a `string`) do 
nothing; please consider dropping.



src/tests/module_tests.cpp (line 112)


None of these functions need to be (mutating!) member functions if you 
simply inject `libraryDirectory`; please consider converting them to free 
functions, e.g. implemented in an anon namespace in this translation unit.



src/tests/module_tests.cpp 


Good catch! Looks like we already call `expandName` in most places.


- Benjamin Bannier


On Nov. 26, 2015, 2:04 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39781/
> ---
> 
> (Updated Nov. 26, 2015, 2:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-3725
> https://issues.apache.org/jira/browse/MESOS-3725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update ModuleTest to not assume dynamic dlopen search.
> 
> 
> Diffs
> -
> 
>   src/tests/module_tests.cpp 28c71b0c1960bad4933f86d35fe8a0248fef326e 
> 
> Diff: https://reviews.apache.org/r/39781/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 39780: Update OversubscriptionTest to not assume dynamic dlopen search.

[Benjamin Bannier]

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



src/tests/oversubscription_tests.cpp (line 106)


This isn't really too nice, and e.g. cmake does not put build libraries 
into these paths (i.e. what you write will only work with automake).

Like I already pointed out on #40553, you should really lift the level of 
abstraction here, and use a function to get this path.


- Benjamin Bannier


On Nov. 26, 2015, 2:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39780/
> ---
> 
> (Updated Nov. 26, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-3725
> https://issues.apache.org/jira/browse/MESOS-3725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update OversubscriptionTest to not assume dynamic dlopen search.
> 
> 
> Diffs
> -
> 
>   src/tests/oversubscription_tests.cpp 
> 0d0bf7e0b9a4028ed116e00b56d59b670510c5ce 
> 
> Diff: https://reviews.apache.org/r/39780/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mei Wan]

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

(Updated Nov. 30, 2015, 1:46 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.


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


Repository: mesos


Description
---

Implemented the overlay filesystem backend by layering the images as a 
read-only filesystem.


Diffs (updated)
-

  src/CMakeLists.txt cfe9d26 
  src/Makefile.am fd38cfa 
  src/slave/containerizer/mesos/provisioner/backend.cpp 01d06eb 
  src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 25b28ef 

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


Testing (updated)
---

Basic test in provisioner_backend_tests.cpp.


Thanks,

Mei Wan

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mei Wan]

> On Nov. 23, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/backends/overlay.hpp, line 36
> > 
> >
> > What's N.B.?
> > To me this is not really a desirable condition for users that has to 
> > prep all images to have sandbox_directory created. Can we at least add a 
> > TODO to say we want to create it here or somewhere upstream?

To be honest, I'm not too sure what this is. I was actually working off the 
bind backend code.


> On Nov. 23, 2015, 9:50 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/backends/overlay.cpp, line 97
> > 
> >
> > Should we do this check in create?

Makes sense. Moved it to create.


- Mei


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


On Nov. 30, 2015, 1:46 p.m., Mei Wan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37853/
> ---
> 
> (Updated Nov. 30, 2015, 1:46 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.
> 
> 
> Bugs: MESOS-2971
> https://issues.apache.org/jira/browse/MESOS-2971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the overlay filesystem backend by layering the images as a 
> read-only filesystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt cfe9d26 
>   src/Makefile.am fd38cfa 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 01d06eb 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 25b28ef 
> 
> Diff: https://reviews.apache.org/r/37853/diff/
> 
> 
> Testing
> ---
> 
> Basic test in provisioner_backend_tests.cpp.
> 
> 
> Thanks,
> 
> Mei Wan
> 
>

Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

[Bernd Mathiske]

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



src/tests/reservation_tests.cpp (line 168)


s/double/a double

But the whole sentence is warped. Suggestion:

"This involves a value for CPUs of type double, which triggers a 
problematic floating point comparison."



src/tests/reservation_tests.cpp (line 172)


Please write a comment in the code below that explains where the comparison 
occurs.



src/tests/reservation_tests.cpp (line 173)


s/with/with an/



src/tests/reservation_tests.cpp (line 176)


Also see the indentations in various other statements involving argument 
passing, etc. Best to sit down with one of us at the same screen and then clean 
up all these indentations in one swoop. Respectively see: 
http://mesos.apache.org/documentation/latest/c++-style-guide/



src/tests/reservation_tests.cpp (line 198)


This can be made less ambigous:

"We apply a filter that causes these resources not to be filtered (default 
would be 5 seconds).



src/tests/reservation_tests.cpp (line 208)


s/We use this to/

Redundant.



src/tests/reservation_tests.cpp (line 219)


This comment pertains to line 225, not 220.



src/tests/reservation_tests.cpp (line 227)


If you use "next" here and below for two different offers, it gets 
confusing. Better "second", "third".



src/tests/reservation_tests.cpp (line 235)


This comment seems to belong to line 241.



src/tests/reservation_tests.cpp (line 239)


Better to use freah variables like offer 2, offer3. It will be easier to 
grasp where in the algorithm we are then.



src/tests/reservation_tests.cpp (line 248)


s/RESERVE/UNRESERVE/ ?

What is the intention here? Please explain.



src/tests/reservation_tests.cpp (line 251)


This comment pertains to line 257.

s/reserved/unreserved/ ?


- Bernd Mathiske


On Nov. 30, 2015, 5:39 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Nov. 30, 2015, 5:39 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding the test framework submitted by Mandeep (@mchadha) 
> https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/

[Avinash sridharan]

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

(Updated Nov. 30, 2015, 5:39 a.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.


Changes
---

Fixed the link to the JIRA issue.


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


Repository: mesos


Description
---

Adding the test framework submitted by Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs
-

  src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan