Review Request 41078: Fixed tests to call socket accept before sending response.

[Jojy Varghese]

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

By calling accept before sending a response, the server would be ready for
client's next request.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
c63bf53fee40ef12536a16e11f4d5224c4e4278e 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 40998: Added CHECK's for various executor states in Command Executor to verify if messages are delivered in order.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40998]

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

- Mesos ReviewBot


On Dec. 7, 2015, 11:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40998/
> ---
> 
> (Updated Dec. 7, 2015, 11:19 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3851
> https://issues.apache.org/jira/browse/MESOS-3851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp f90ea01131e6fa28e42f0c00a317d66a49f81ffa 
> 
> Diff: https://reviews.apache.org/r/40998/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 41077: Fixed the license header in src/linux/ns.hpp.

[Jie Yu]

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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

Fixed the license header in src/linux/ns.hpp.


Diffs
-

  src/linux/ns.hpp 1ebf2dbf9bbb350e91fc52925bd424b0a44c34d5 

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


Testing
---

trivial


Thanks,

Jie Yu

Re: Review Request 41078: Fixed tests to call socket accept before sending response.

[Jojy Varghese]

> On Dec. 8, 2015, 7:08 a.m., Timothy Chen wrote:
> > Can you elaborate more without this fix what happened and why?

This fix moves "Accept" call before sending a response back to the client so 
that when the client sends the next request, the server is ready to accept it. 
Before this fix, there was a race condition between server sending the response 
and client sending the next request and server missing that request because the 
"accept" was not called yet.


- Jojy


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


On Dec. 8, 2015, 7:03 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41078/
> ---
> 
> (Updated Dec. 8, 2015, 7:03 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By calling accept before sending a response, the server would be ready for
> client's next request.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/41078/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41075: Added support for implicit roles.

[Yong Qiao Wang]

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



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


When allocator initialize, their should be no active frameworks in mesos, 
can we consider to remove this parmeter from initialize()? and can initialize 
role-related information in addFramework().


- Yong Qiao Wang


On Dec. 8, 2015, 5:41 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41075/
> ---
> 
> (Updated Dec. 8, 2015, 5:41 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Hindman, Greg 
> Mann, and Yong Qiao Wang.
> 
> 
> Bugs: MESOS-4085
> https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the behavior of the master when the `--roles` flag is NOT
> specified. Previously, this would allow only the `*` role to be used. Now,
> omitting `--roles` means that any role can be used. This is called "implicit
> roles". Configuring which principals can perform operations as which roles
> should be done using ACLs in the authorization system.
> 
> Note that this changes the behavior of the system when `--roles` is not
> specified. This is likely acceptable: if the operator didn't specify `--roles`
> in prior versions of Mesos, they were likely not using roles or authorization 
> at
> that time.
> 
> Another minor behavioral change is that the "/roles" endpoint will now only
> return results for currently "active" roles (those with one or more registered
> frameworks).
> 
> The `--roles` flag is now considered deprecated and will be removed in a 
> future
> version of Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   include/mesos/master/allocator.proto 
> 702f56f56c3b1331613cecf26522986f6b572f8c 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 99c742906874c30c39c159e58a65277ade3c07fd 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
>   src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
>   src/tests/hierarchical_allocator_tests.cpp 
> fb214a829a57529d3f5c49730ae9733f53e622ca 
> 
> Diff: https://reviews.apache.org/r/41075/diff/
> 
> 
> Testing
> ---
> 
> "make check" on OSX 10.10 and Ubuntu 15.10; `--gtest_repeat=1000` for the 
> more likely role-related tests.
> 
> TODOs:
> 
> * Update documentation
> * Add tests for allocation behavior for weights + implicit roles
> * Add tests for quota + implicit roles?
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41078: Fixed tests to call socket accept before sending response.

[Timothy Chen]

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


Can you elaborate more without this fix what happened and why?

- Timothy Chen


On Dec. 8, 2015, 7:03 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41078/
> ---
> 
> (Updated Dec. 8, 2015, 7:03 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By calling accept before sending a response, the server would be ready for
> client's next request.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/41078/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41075: Added support for implicit roles.

[Neil Conway]

> On Dec. 8, 2015, 7:02 a.m., Yong Qiao Wang wrote:
> > include/mesos/master/allocator.hpp, line 101
> > 
> >
> > When allocator initialize, their should be no active frameworks in 
> > mesos, can we consider to remove this parmeter from initialize()? and can 
> > initialize role-related information in addFramework().

Hmmm -- I think it is better as written. Right now, weights are static and set 
at initialization-time, so I think it makes more sense for them to be 
parameters of `initialize`, not `addFramework`. (When we do dynamic weights 
we'll need to reconsider this, but even then, I don't think passing weights to 
`addFramework` will be the right API).


- Neil


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


On Dec. 8, 2015, 5:41 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41075/
> ---
> 
> (Updated Dec. 8, 2015, 5:41 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Hindman, Greg 
> Mann, and Yong Qiao Wang.
> 
> 
> Bugs: MESOS-4085
> https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the behavior of the master when the `--roles` flag is NOT
> specified. Previously, this would allow only the `*` role to be used. Now,
> omitting `--roles` means that any role can be used. This is called "implicit
> roles". Configuring which principals can perform operations as which roles
> should be done using ACLs in the authorization system.
> 
> Note that this changes the behavior of the system when `--roles` is not
> specified. This is likely acceptable: if the operator didn't specify `--roles`
> in prior versions of Mesos, they were likely not using roles or authorization 
> at
> that time.
> 
> Another minor behavioral change is that the "/roles" endpoint will now only
> return results for currently "active" roles (those with one or more registered
> frameworks).
> 
> The `--roles` flag is now considered deprecated and will be removed in a 
> future
> version of Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   include/mesos/master/allocator.proto 
> 702f56f56c3b1331613cecf26522986f6b572f8c 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 99c742906874c30c39c159e58a65277ade3c07fd 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
>   src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
>   src/tests/hierarchical_allocator_tests.cpp 
> fb214a829a57529d3f5c49730ae9733f53e622ca 
> 
> Diff: https://reviews.apache.org/r/41075/diff/
> 
> 
> Testing
> ---
> 
> "make check" on OSX 10.10 and Ubuntu 15.10; `--gtest_repeat=1000` for the 
> more likely role-related tests.
> 
> TODOs:
> 
> * Update documentation
> * Add tests for allocation behavior for weights + implicit roles
> * Add tests for quota + implicit roles?
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41026: libevent ssl: Added check for buffer length before swapping request.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41026]

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

- Mesos ReviewBot


On Dec. 8, 2015, 1:22 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41026/
> ---
> 
> (Updated Dec. 8, 2015, 1:22 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4069
> https://issues.apache.org/jira/browse/MESOS-4069
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> recv_callback could be called from libevents receive callback and Socket::recv
> for the same buffer event and different requests. There is a check for buffer
> length at Socket::recv but not at libevent's receive callback. This could lead
> to the incoming request for Socket::recv being swapped out even though the
> buffer length is zero. This change adds a check for buffer length before
> swapping out the receive request object.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 55b91dd47bb5bd5e97147d0af91c7899fd42702c 
> 
> Diff: https://reviews.apache.org/r/41026/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41075: Added support for implicit roles.

[Yong Qiao Wang]

> On Dec. 8, 2015, 7:02 a.m., Yong Qiao Wang wrote:
> >

I have talked with our shepherd Adam B yesterday, and we all agree to improve 
/roles endpoint to update/remove/list active roles, and Implicit Roles will 
focus on removing the static role list(specified by --roles flag) and let 
framework can register with any role. In addition, RoleInfo is still needed, 
and will be persisted when the default values are changed by /roles endpoint. 
Currently I also have posted some patches for Dynamic Roles, and there are some 
serious comflicts between our patches. Can you have a talk with our shepherd? I 
think we should reach a consensus before coding.


> On Dec. 8, 2015, 7:02 a.m., Yong Qiao Wang wrote:
> > include/mesos/master/allocator.hpp, line 101
> > 
> >
> > When allocator initialize, their should be no active frameworks in 
> > mesos, can we consider to remove this parmeter from initialize()? and can 
> > initialize role-related information in addFramework().
> 
> Neil Conway wrote:
> Hmmm -- I think it is better as written. Right now, weights are static 
> and set at initialization-time, so I think it makes more sense for them to be 
> parameters of `initialize`, not `addFramework`. (When we do dynamic weights 
> we'll need to reconsider this, but even then, I don't think passing weights 
> to `addFramework` will be the right API).

For long term solution in Implicit Roles, --roles and --weights should be 
removed, so weights parameter will only have one default value(*,1). so it is 
make sence to remove this parameter from initialize (),and when framework 
register, we can add its role and the corresponding weight into allocator, then 
they can become active.


- Yong Qiao


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


On Dec. 8, 2015, 5:41 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41075/
> ---
> 
> (Updated Dec. 8, 2015, 5:41 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Hindman, Greg 
> Mann, and Yong Qiao Wang.
> 
> 
> Bugs: MESOS-4085
> https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the behavior of the master when the `--roles` flag is NOT
> specified. Previously, this would allow only the `*` role to be used. Now,
> omitting `--roles` means that any role can be used. This is called "implicit
> roles". Configuring which principals can perform operations as which roles
> should be done using ACLs in the authorization system.
> 
> Note that this changes the behavior of the system when `--roles` is not
> specified. This is likely acceptable: if the operator didn't specify `--roles`
> in prior versions of Mesos, they were likely not using roles or authorization 
> at
> that time.
> 
> Another minor behavioral change is that the "/roles" endpoint will now only
> return results for currently "active" roles (those with one or more registered
> frameworks).
> 
> The `--roles` flag is now considered deprecated and will be removed in a 
> future
> version of Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   include/mesos/master/allocator.proto 
> 702f56f56c3b1331613cecf26522986f6b572f8c 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 99c742906874c30c39c159e58a65277ade3c07fd 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
>   src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
>   src/tests/hierarchical_allocator_tests.cpp 
> fb214a829a57529d3f5c49730ae9733f53e622ca 
> 
> Diff: https://reviews.apache.org/r/41075/diff/
> 
> 
> Testing
> ---
> 
> "make check" on OSX 10.10 and Ubuntu 15.10; `--gtest_repeat=1000` for the 
> more likely role-related tests.
> 
> TODOs:
> 
> * Update documentation
> * Add tests for allocation behavior for weights + implicit roles
> * Add tests for quota + implicit roles?
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rojas]

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

(Updated Dec. 7, 2015, 3:18 p.m.)


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


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39043: Added support for HTTP Authentication in Mesos.

[Alexander Rojas]

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

(Updated Dec. 7, 2015, 3:22 p.m.)


Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till 
Toenshoff.


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


Repository: mesos


Description
---

1. Adds a flag to load an HTTP Authenticator module from the flags.
2. If provided, uses the credentials file to initialize the default HTTP 
Authenticator.
3. Updates the existing endpoints which implement their own basic HTTP 
authenticator with the libprocess one.
4. Updates one test which expected the wrong results since now credentials are 
checked before the body of the request.


Diffs (updated)
-

  src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759 
  src/master/constants.cpp 98ea7c8f1e7c3057cfe941f82f1b71f7731a8f32 
  src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
  src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
  src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
  src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Till Toenshoff]

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



3rdparty/libprocess/include/process/authenticator.hpp (line 20)


Add `#include ` since you are using it.



3rdparty/libprocess/src/authenticator.cpp (line 1)


License missing - please make sure you include the Apache License Version 
2.0 header;

```
   // Licensed under the Apache License, Version 2.0 (the "License");  
   // you may not use this file except in compliance with the License.  
   // You may obtain a copy of the License at  
   //  
   // http://www.apache.org/licenses/LICENSE-2.0  
   //  
   // Unless required by applicable law or agreed to in writing, software  
   // distributed under the License is distributed on an "AS IS" BASIS,  
   // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.  
   // See the License for the specific language governing permissions and  
   // limitations under the License
```



3rdparty/libprocess/src/authenticator.cpp (line 28)


You had added a `using std::string` already, we can kill those `std::` 
prefixes for references of `string`.



3rdparty/libprocess/src/authenticator.cpp (line 30)


We commonly derive from clang-format in these cases, the brackets move up;

```
BasicAuthenticator::BasicAuthenticator(
const string& realm,
const hashmap& credentials)
  : realm_(realm), credentials_(credentials) {}
```



3rdparty/libprocess/src/authenticator.cpp (lines 51 - 56)


Shall we combine those two clauses just like you did below with the 
credentials check?

```
  if (components.size() < 2 || components[0] != "Basic") {
return result;
  }
```



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


We are `using http::Response` already, kill the namespace.


- Till Toenshoff


On Dec. 7, 2015, 2:18 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 2:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38000: Introduced support for user interaction with HTTP AuthenticationRouter.

[Alexander Rojas]

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

(Updated Dec. 7, 2015, 3:14 p.m.)


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


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


Repository: mesos


Description
---

Adds functions which allow libprocess users to register HTTP authenticators.
Overloads `ProcesBase::route()` to allow for registering of authenticating 
endpoints.
Includes tests.


Diffs (updated)
-

  3rdparty/libprocess/include/process/event.hpp 
a03824c061c4a0eb865b163999a763635e56744c 
  3rdparty/libprocess/include/process/process.hpp 
81c094414d4d5ac5eb593df2a6d14aaacb19a826 
  3rdparty/libprocess/src/process.cpp e93709d3bb4ac588457bb9331fc05ec5ab539f6d 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rukletsov]

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



3rdparty/libprocess/include/process/authenticator.hpp (line 26)


Why do you need to include `hashset` here? I don't see any uses of it in 
the ".hpp" file.



3rdparty/libprocess/include/process/authenticator.hpp (lines 94 - 95)


Love trailing underscores for class members!



3rdparty/libprocess/src/authenticator.cpp (line 1)


Do you need a license here?



3rdparty/libprocess/src/authenticator.cpp (lines 25 - 26)


I think `std::*`s go first.



3rdparty/libprocess/src/authenticator.cpp (line 30)


You can remove `std::` prefixes.



3rdparty/libprocess/src/authenticator.cpp (lines 39 - 40)


I see you use the same error message in case something is wrong. Is it done 
on purpose for security reasons? Or do you think it makes sense to extend the 
message with specific note in each case?



3rdparty/libprocess/src/authenticator.cpp (lines 67 - 68)


Let's format this with each condition on a separate line, so it's easier to 
read.



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


Could you please adjust the order?



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1336 - 1337)


Let's either s/Basic/basic if it's a description, or use the class name 
(`BasicAuthenticator` with backticks) instead.



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1345 - 1346)


const?



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


We usually tend to put a verb first, e.g. "Provide no credentials", 
"Provide wrong credentials". Do you think it makes sense to fix it for 
consistency?



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1352 - 1360)


One thing we did in quota tests is we put each scenario in a scope. This 
way it's more clear to the user where are the boundaries of each scenario, and 
also it's more clear that a comment prepending the scope is for the whole scope 
and not for a single following line



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1361 - 1363)


Do you want to add a test case for wrong principal (maybe even with 
"testpassword")?


- Alexander Rukletsov


On Dec. 7, 2015, 2:18 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 2:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38950: Http Authenticators can be loaded as modules from mesos.

[Alexander Rojas]

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

(Updated Dec. 7, 2015, 3:21 p.m.)


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


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


Repository: mesos


Description
---

Adds support for modularization of HTTP Authenticators. 

It includes an example of how to do it with the Basic HTTP Authenticator.


Diffs (updated)
-

  include/mesos/authentication/http/basic_authenticator_factory.hpp 
PRE-CREATION 
  include/mesos/module/http_authenticator.hpp PRE-CREATION 
  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
  src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
  src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION 
  src/examples/test_http_authenticator_module.cpp PRE-CREATION 
  src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759 
  src/master/constants.cpp 98ea7c8f1e7c3057cfe941f82f1b71f7731a8f32 
  src/module/manager.cpp 1f04790510a2ab9ccd6907fd01be192f52ee90c6 
  src/tests/http_authentication_tests.cpp PRE-CREATION 
  src/tests/module.hpp e46ed12c80707bf44ceef3ed1a4eb2f321ce10f6 
  src/tests/module.cpp e272bf0eccb61ae54440ec79adac8efad804c828 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

[Bartek Plotka]

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

(Updated Dec. 7, 2015, 3:05 p.m.)


Review request for mesos and Niklas Nielsen.


Repository: mesos


Description (updated)
---

Added Load QoS Controller for the simple eviction when system load is above 
configured system load threshold for 5min and 15min:
- Made os::loadavg called from the lambda and passed via the contructor.
- Added unit test.


Diffs
-

  src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0 
  src/slave/qos_controllers/load.hpp PRE-CREATION 
  src/slave/qos_controllers/load.cpp PRE-CREATION 
  src/tests/oversubscription_tests.cpp 0333281c247dd182860a49f39be791c00679bf6b 

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


Testing
---

make check


Thanks,

Bartek Plotka

Re: Review Request 38950: Http Authenticators can be loaded as modules from mesos.

[Alexander Rojas]

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

(Updated Dec. 7, 2015, 2:33 p.m.)


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


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


Repository: mesos


Description
---

Adds support for modularization of HTTP Authenticators. 

It includes an example of how to do it with the Basic HTTP Authenticator.


Diffs (updated)
-

  include/mesos/authentication/http/basic_authenticator_factory.hpp 
PRE-CREATION 
  include/mesos/module/http_authenticator.hpp PRE-CREATION 
  src/CMakeLists.txt 9a2c70d40031c80a304462107758802c08411a49 
  src/Makefile.am 5aac0ee8657dfca17a232438fd561cb00fd5e752 
  src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION 
  src/examples/test_http_authenticator_module.cpp PRE-CREATION 
  src/master/constants.hpp 51d477444c4d1a2c9dd2f32164ebffbb1fd5c8c2 
  src/master/constants.cpp 2b66b27783f18930010ee912e9977ea1647eba09 
  src/module/manager.cpp f9a0643a70bc9de1484599629041650493842c69 
  src/tests/http_authentication_tests.cpp PRE-CREATION 
  src/tests/module.hpp 04f42dc4f687e55c4e6758ef2f7c7096f1b1ec47 
  src/tests/module.cpp edab0b37dcf0bd8e15d439726354039c1bbcd51f 

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


Testing
---

make check


Thanks,

Alexander Rojas

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

[Jan Schlicht]

> On Dec. 2, 2015, 4:47 p.m., Alexander Rukletsov wrote:
> > src/master/quota_handler.cpp, lines 236-240
> > 
> >
> > Can we re-write it using a lambda? This way you do not need to inject 
> > `authorized` into the continuation.
> > 
> > ```
> > authorize(quotaInfo)
> >   .then(defer(master->self(), [=](bool authorized) -> Future {
> > if (!authorized) {
> >   return Unauthorized("Mesos master");
> > }
> > 
> > _set(quotaInfo);
> >   }));
> > ```
> 
> Jan Schlicht wrote:
> Sure, but the rest of the function already has a similar lamdba 
> continuation (i.e. will have after rebasing). This will result in a lambda 
> inside a lambda which is too much nesting IMHO. Therefore I would prefer to 
> keep the continuation.

Oh, sorry, I skipped your example code. Thought that I ought to put everything 
into a lambda and remove the continuation completly. Yes, using the lambda this 
way makes much more sense, will implement it that way.


- Jan


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


On Dec. 7, 2015, 2:03 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40347/
> ---
> 
> (Updated Dec. 7, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
> Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Implemented quota request authorization.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 
> 
> Diff: https://reviews.apache.org/r/40347/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

[Bartek Plotka]

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

(Updated Dec. 7, 2015, 3:02 p.m.)


Review request for mesos and Niklas Nielsen.


Changes
---

- Added few descriptions and comments.
- Moved wrapping function to c++11 lambda.
- Removed simple typedef.
- Moved isstring to numify.
- Added proper unit test.


Repository: mesos


Description
---

Added Load QoS Controller for the simple eviction when system load is above 
configured threshold:
- Made os::loadavg called from internal method. This method is then passed as 
lambda to the Load QoS Controller process.
- Added MockLoadQoSController
- Added tests

Tests still WIP.


Diffs (updated)
-

  src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0 
  src/slave/qos_controllers/load.hpp PRE-CREATION 
  src/slave/qos_controllers/load.cpp PRE-CREATION 
  src/tests/oversubscription_tests.cpp 0333281c247dd182860a49f39be791c00679bf6b 

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


Testing
---

make check


Thanks,

Bartek Plotka

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rojas]

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

(Updated Dec. 7, 2015, 4:02 p.m.)


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


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rojas]

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

(Updated Dec. 7, 2015, 4:11 p.m.)


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


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Dec. 7, 2015, 3:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

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

[Jan Schlicht]

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

(Updated Dec. 7, 2015, 3:34 p.m.)


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


Changes
---

Added lambda to simplify continuation.


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


Repository: mesos


Description
---

Quota: Implemented quota request authorization.


Diffs (updated)
-

  src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
  src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 39043: Added support for HTTP Authentication in Mesos.

[Alexander Rojas]

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

(Updated Dec. 7, 2015, 2:33 p.m.)


Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till 
Toenshoff.


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


Repository: mesos


Description
---

1. Adds a flag to load an HTTP Authenticator module from the flags.
2. If provided, uses the credentials file to initialize the default HTTP 
Authenticator.
3. Updates the existing endpoints which implement their own basic HTTP 
authenticator with the libprocess one.
4. Updates one test which expected the wrong results since now credentials are 
checked before the body of the request.


Diffs (updated)
-

  src/master/constants.hpp 51d477444c4d1a2c9dd2f32164ebffbb1fd5c8c2 
  src/master/constants.cpp 2b66b27783f18930010ee912e9977ea1647eba09 
  src/master/flags.hpp 5fd5d502697b2edc22ae98a5a8e361bf62bf8bb6 
  src/master/flags.cpp 806e2da6ad37a6acf76818d4c6b3c462175fd09d 
  src/master/http.cpp 1c4f1406f5d917f5d655a7d61d311365f8999ce0 
  src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
  src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rojas]

> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 25-26
> > 
> >
> > I think `std::*`s go first.

Not according to all the examples I checked, see `process.cpp` or all the 
`main.cpp`


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 39-40
> > 
> >
> > I see you use the same error message in case something is wrong. Is it 
> > done on purpose for security reasons? Or do you think it makes sense to 
> > extend the message with specific note in each case?

It is actually not an error message but the challenge(-s) to be emited to the 
client in authentication fails (See the constructor of 
[Unauthorized](https://github.com/apache/mesos/blob/49296b9a80ec26bf77bc9191fff7b2f5e143b1d2/3rdparty/libprocess/include/process/http.hpp#L521)
 which takes a vector). 

Still, the reason why you don't give detailed error messages is because with 
authentication you want to be quite vague. When you failed to authenticate to a 
site, it tells you that either your username doesn't exist or your password was 
wrong, since you rather don't tell which one of the two failed.


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, line 50
> > 
> >
> > Could you please adjust the order?

Not relevant anymore, since the authentication code got its own namespace.


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, lines 1336-1337
> > 
> >
> > Let's either s/Basic/basic if it's a description, or use the class name 
> > (`BasicAuthenticator` with backticks) instead.

I would rather not, since you find _Basic_ in pretty much all the literature on 
the topic (See [RFC 26217](https://tools.ietf.org/html/rfc2617)).


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, lines 1352-1360
> > 
> >
> > One thing we did in quota tests is we put each scenario in a scope. 
> > This way it's more clear to the user where are the boundaries of each 
> > scenario, and also it's more clear that a comment prepending the scope is 
> > for the whole scope and not for a single following line

Great idea.


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, line 1352
> > 
> >
> > We usually tend to put a verb first, e.g. "Provide no credentials", 
> > "Provide wrong credentials". Do you think it makes sense to fix it for 
> > consistency?

I didn't even know there was a rule about it (which I find rather extreme). In 
any case, I think putting a verb first violates the _subject-verb-object_ 
syntax of english and makes it sound imperative (which it is not). In this case 
by use of the passive tense I transform the _object_ into the _subject_ and 
still have a correctly formed english sentence.


- Alexander


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


On Dec. 7, 2015, 3:18 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rojas]

> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 25-26
> > 
> >
> > I think `std::*`s go first.
> 
> Alexander Rojas wrote:
> Not according to all the examples I checked, see `process.cpp` or all the 
> `main.cpp`
> 
> Alexander Rukletsov wrote:
> Yeah, we are inconsistent, but I think that's where we want to be : ).

Well, after checking more than 20 files taken at random from the list producing 
after grepping for `^using +std::.*;$` we seem to be really consistent about 
the ordering, just `libprocess/src/http.cpp` has the ordering you want.

On top of that, the styleguide says nothing on the topic.


- Alexander


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


On Dec. 7, 2015, 4:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 4:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 41042: Added description of the LoadQoSController in the oversubscription.md

[Niklas Nielsen]

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

Ship it!



docs/oversubscription.md (line 201)


s/QoS/quality of service for regular tasks/ or something in the manner :)



docs/oversubscription.md (line 202)


How do you 'perform' QoS? :) It's ensuring the total system load doesn't 
exceed a threshold and thereby, simplistically, try to avoid cpu congestion.



docs/oversubscription.md (line 203)


It comes as a module parameter, right? That can live in a file as well.



docs/oversubscription.md (line 316)


We should maybe describe that these are standard unix load averages.



docs/oversubscription.md (line 329)


Let's expand to s/5min/5 minutes/



docs/oversubscription.md (line 330)


Same here for 15min


- Niklas Nielsen


On Dec. 7, 2015, 9:18 a.m., Bartek Plotka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41042/
> ---
> 
> (Updated Dec. 7, 2015, 9:18 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-4076
> https://issues.apache.org/jira/browse/MESOS-4076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added description to _Writing a custom QoS controller_ and _Configuring  
> oversubscription_ sections.
> 
> 
> Diffs
> -
> 
>   docs/oversubscription.md 7d1415a712f818f7664ed8322ddcdc57d3a1fb1f 
> 
> Diff: https://reviews.apache.org/r/41042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rukletsov]

> On Dec. 7, 2015, 2:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 25-26
> > 
> >
> > I think `std::*`s go first.
> 
> Alexander Rojas wrote:
> Not according to all the examples I checked, see `process.cpp` or all the 
> `main.cpp`

Yeah, we are inconsistent, but I think that's where we want to be : ).


- Alexander


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


On Dec. 7, 2015, 3:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 40056: Make hook execution order deterministic.

[Niklas Nielsen]

> On Dec. 4, 2015, 1:15 p.m., Niklas Nielsen wrote:
> > Hi Haosdent!
> > 
> > I apologize the tardy reply. The patch looks good but needs rebasing.
> > Also, have you thought of a way to test this?
> > 
> > With a test (maybe by just ensuring the existing ordering of the test 
> > modules are indeed run in order), we should be able to land this. Kapil, 
> > any concerns on this?
> 
> Guangya Liu wrote:
> @nnielsen, does this test help? Thanks! 
> https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp#L93-L106

That should be fine - rebase and let's get it in. (Kapil, any objections?)


- Niklas


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


On Nov. 8, 2015, 4:58 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40056/
> ---
> 
> (Updated Nov. 8, 2015, 4:58 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Kapil Arya, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3485
> https://issues.apache.org/jira/browse/MESOS-3485
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make hook execution order deterministic.
> 
> 
> Diffs
> -
> 
>   src/hook/manager.cpp d9e660a3b6f6d13d9f6b59f53bfc8a4f65af6df4 
> 
> Diff: https://reviews.apache.org/r/40056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[James Peach]

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

(Updated Dec. 7, 2015, 5:17 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 (updated)
-

  src/tests/module_tests.cpp b5cfbcd4885a4f2c20b19e00958fedda81afa6e0 

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


Testing
---

make check


Thanks,

James Peach

Re: Review Request 37540: Add perf event API

[Niklas Nielsen]

> On Dec. 2, 2015, 10:13 a.m., Niklas Nielsen wrote:
> > Ping - Cong, is there anything you need to close the last issues? :)
> 
> Cong Wang wrote:
> I was blocked at the second to the last issue above and switched to 
> something else, now I can think more about it. Thanks!

Sweet! Let us know when you are ready for new review rounds :)


- Niklas


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


On Sept. 29, 2015, 5:12 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> ---
> 
> (Updated Sept. 29, 2015, 5:12 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Abstract Linux kernel perf event API and provide API to collect schedule 
> events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
>   src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
>   src/tests/containerizer/perf_tests.cpp 
> ed5212ee31b8aa47339b8b8fab184bbdf85be82a 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 41042: Added description of the LoadQoSController in the oversubscription.md

[Bartek Plotka]

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

(Updated Dec. 7, 2015, 9:18 a.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


Summary (updated)
-

Added description of the LoadQoSController in the oversubscription.md


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


Repository: mesos


Description
---

Added description to _Writing a custom QoS controller_ and _Configuring  
oversubscription_ sections.


Diffs
-

  docs/oversubscription.md 7d1415a712f818f7664ed8322ddcdc57d3a1fb1f 

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


Testing
---


Thanks,

Bartek Plotka

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rukletsov]

> On Dec. 7, 2015, 2:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 39-40
> > 
> >
> > I see you use the same error message in case something is wrong. Is it 
> > done on purpose for security reasons? Or do you think it makes sense to 
> > extend the message with specific note in each case?
> 
> Alexander Rojas wrote:
> It is actually not an error message but the challenge(-s) to be emited to 
> the client in authentication fails (See the constructor of 
> [Unauthorized](https://github.com/apache/mesos/blob/49296b9a80ec26bf77bc9191fff7b2f5e143b1d2/3rdparty/libprocess/include/process/http.hpp#L521)
>  which takes a vector). 
> 
> Still, the reason why you don't give detailed error messages is because 
> with authentication you want to be quite vague. When you failed to 
> authenticate to a site, it tells you that either your username doesn't exist 
> or your password was wrong, since you rather don't tell which one of the two 
> failed.

Got it, mind adding a comment about this for the next reader?


- Alexander


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


On Dec. 7, 2015, 3:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

[Bartek Plotka]

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

(Updated Dec. 7, 2015, 4:40 p.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


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


Repository: mesos


Description
---

Added Load QoS Controller for the simple eviction when system load is above 
configured system load threshold for 5min and 15min:
- Made os::loadavg called from the lambda and passed via the contructor.
- Added unit test.


Diffs
-

  src/Makefile.am a57e46d06c8c26a32a9444be4f0e1269d775f8b0 
  src/slave/qos_controllers/load.hpp PRE-CREATION 
  src/slave/qos_controllers/load.cpp PRE-CREATION 
  src/tests/oversubscription_tests.cpp 0333281c247dd182860a49f39be791c00679bf6b 

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


Testing (updated)
---

make check
run mesos with org_apache_mesos_LoadQoSController module included.


Thanks,

Bartek Plotka

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

[Niklas Nielsen]

> On Nov. 30, 2015, 12:51 a.m., Benjamin Bannier wrote:
> >

James - are you blocked by anything? Can we help? It looks like we need one 
more review iteration.


- Niklas


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


On Nov. 26, 2015, 6:04 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39781/
> ---
> 
> (Updated Nov. 26, 2015, 6:04 a.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 40431: Move RoleInfo message out of allocator.proto

[Yong Qiao Wang]

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

(Updated Dec. 7, 2015, 8:41 a.m.)


Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.


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


Repository: mesos


Description
---

Currently role protobuf is defined in allocator.proto due to only the 
traditional DRF allocator uses roles as it’s first level of hierarchy, I think 
we should move it out and define it in a separated file as quota had in dynamic 
roles project, because role protobuf will also be used by master to persist.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
  include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c 
  include/mesos/role/role.hpp PRE-CREATION 
  include/mesos/role/role.proto PRE-CREATION 
  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
  src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
  src/master/allocator/mesos/allocator.hpp 
97ee80726ad155917811265a983258b0165d3451 
  src/master/allocator/mesos/hierarchical.hpp 
99c742906874c30c39c159e58a65277ade3c07fd 
  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 
  src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
  src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
  src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
  src/tests/hierarchical_allocator_tests.cpp 
fb214a829a57529d3f5c49730ae9733f53e622ca 

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


Testing
---

1. Make Check successfully;

2. $ curl http://9.110.48.168:5050/roles
{"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}


Thanks,

Yong Qiao Wang

Re: Review Request 40431: Move RoleInfo message out of allocator.proto

[Yong Qiao Wang]

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

(Updated Dec. 7, 2015, 10:12 a.m.)


Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.


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


Repository: mesos


Description
---

Currently role protobuf is defined in allocator.proto due to only the 
traditional DRF allocator uses roles as it’s first level of hierarchy, I think 
we should move it out and define it in a separated file as quota had in dynamic 
roles project, because role protobuf will also be used by master to persist.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
  include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c 
  include/mesos/role/role.hpp PRE-CREATION 
  include/mesos/role/role.proto PRE-CREATION 
  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
  src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
  src/master/allocator/mesos/allocator.hpp 
97ee80726ad155917811265a983258b0165d3451 
  src/master/allocator/mesos/hierarchical.hpp 
99c742906874c30c39c159e58a65277ade3c07fd 
  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 
  src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
  src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
  src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
  src/tests/hierarchical_allocator_tests.cpp 
fb214a829a57529d3f5c49730ae9733f53e622ca 

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


Testing
---

1. Make Check successfully;

2. $ curl http://9.110.48.168:5050/roles
{"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}


Thanks,

Yong Qiao Wang

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

[Yong Qiao Wang]

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

(Updated Dec. 7, 2015, 10:12 a.m.)


Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.


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


Repository: mesos


Description
---

Update Allocator interface to support dynamic roles


Diffs (updated)
-

  include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
  src/master/allocator/mesos/allocator.hpp 
97ee80726ad155917811265a983258b0165d3451 
  src/master/allocator/mesos/hierarchical.hpp 
99c742906874c30c39c159e58a65277ade3c07fd 
  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 
  src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 

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


Testing
---

Make check successfully.


Thanks,

Yong Qiao Wang

Re: Review Request 40346: [2/4] Quota Authorization: Implemented authorization of quota requests in the authorizer.

[Jan Schlicht]

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

(Updated Dec. 7, 2015, 11:32 a.m.)


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


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

Quota: Implemented authorization of quota requests in the authorizer.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
ec3d3e9c9bae461c8cd136f17cfda58e0cbd4e1b 
  src/authorizer/local/authorizer.hpp 965658b53d21734a2dbf3713d02d44d728fd6cca 
  src/authorizer/local/authorizer.cpp 1ac6a41d20454515a3ef2b84e296494a4592b3ea 
  src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8 
  src/tests/mesos.cpp a501f04dfbf9c93c2196e83b512519e925a67c97 

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


Testing
---

make check


Thanks,

Jan Schlicht

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

[Jan Schlicht]

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

(Updated Dec. 7, 2015, 2:03 p.m.)


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


Changes
---

Rebased, refactored & issues addressed: Instead of authorizing using 
`QuotaInfo` fields, `principal` and `role` are now used directly.


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


Repository: mesos


Description
---

Quota: Implemented quota request authorization.


Diffs (updated)
-

  src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
  src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 

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


Testing
---

make check


Thanks,

Jan Schlicht

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

[Jan Schlicht]

> On Dec. 2, 2015, 4:47 p.m., Alexander Rukletsov wrote:
> > src/master/quota_handler.cpp, lines 236-240
> > 
> >
> > Can we re-write it using a lambda? This way you do not need to inject 
> > `authorized` into the continuation.
> > 
> > ```
> > authorize(quotaInfo)
> >   .then(defer(master->self(), [=](bool authorized) -> Future {
> > if (!authorized) {
> >   return Unauthorized("Mesos master");
> > }
> > 
> > _set(quotaInfo);
> >   }));
> > ```

Sure, but the rest of the function already has a similar lamdba continuation 
(i.e. will have after rebasing). This will result in a lambda inside a lambda 
which is too much nesting IMHO. Therefore I would prefer to keep the 
continuation.


> On Dec. 2, 2015, 4:47 p.m., Alexander Rukletsov wrote:
> > include/mesos/quota/quota.proto, lines 43-45
> > 
> >
> > Do you think `principal` should be required? Does it mean quota cannot 
> > be requested without providing `Authorization` header? I think you do so in 
> > `authorize()` below.
> > 
> > I think it's also fine to prohibit setting quota without prinicipal, 
> > but let's be consitent and explicit about it.

Actually, after refactoring `principal` can be removed here. It will be needed 
when the `remove quota` is implemented and will be implemented there.


> On Dec. 2, 2015, 4:47 p.m., Alexander Rukletsov wrote:
> > src/master/quota_handler.cpp, lines 94-96
> > 
> >
> > I think this may be buggy: `principal` is required, however we do not 
> > ensure it is present here, means we may create a protbuf message, that we 
> > cannot deserialize.

The `principal` field will be removed and implemented later for `remove quota`.


- Jan


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


On Dec. 7, 2015, 2:03 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40347/
> ---
> 
> (Updated Dec. 7, 2015, 2:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
> Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Implemented quota request authorization.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 
> 
> Diff: https://reviews.apache.org/r/40347/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 41070: Made v1 API in mesos.proto equivalent to non-v1.

[Till Toenshoff]

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

Review request for mesos, Bernd Mathiske, Joris Van Remoortere, and Michael 
Park.


Repository: mesos


Description
---

Now in parity also for master.


Diffs
-

  include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 

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


Testing
---

make check (OSX)


Thanks,

Till Toenshoff

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1 for 0.26.0.

[Till Toenshoff]

> On Dec. 8, 2015, 1 a.m., Michael Park wrote:
> > include/mesos/v1/mesos.proto, lines 1507-1508
> > 
> >
> > It looks like we're also missing this from `include/mesos/mesos.proto`?
> > ```
> > 1510 // The name of volume driver plugin.
> > 1511 optional string volume_driver = 7;
> > ```
> 
> Till Toenshoff wrote:
> That one was brought in by commit 
> fc7e25d8c05ee3c226e4d45819c047009ddd71c0, which is RR 
> https://reviews.apache.org/r/38338, backed by 
> https://issues.apache.org/jira/browse/MESOS-3392.
> 
> It got in after the release-cut, so it is not a problem for 0.26.0 but 
> following releases.
> 
> Will drop this issue for this one as it was marked as based on 0.26.0-rc3 
> but add a new RR which is a complete fix for the current master.

See https://reviews.apache.org/r/41070/


- Till


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


On Dec. 8, 2015, 2:06 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41060/
> ---
> 
> (Updated Dec. 8, 2015, 2:06 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied missing feature updates to v1 from include/mesos/mesos.proto.
> 
> This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
> V1 API again as it has been intensionally left out (as being Slave only).
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 
> 
> Diff: https://reviews.apache.org/r/41060/diff/
> 
> 
> Testing
> ---
> 
> For achieving this, the following was done within the tagged 0.26.0-rc3:
> 
> `diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
> `diff include/mesos/executor.proto include/mesos/v1/executor.proto`
> `diff include/mesos/scheduler/scheduler.proto 
> include/mesos/v1/scheduler/scheduler.proto`
> 
> These diffs do show a couple of "slave"->"agent" renames which were ignored 
> for this udate.
> 
> The results showed that only mesos.proto was not en par with V1. We listed 
> out all responsible review-requests;
> https://reviews.apache.org/r/31915
> https://reviews.apache.org/r/38253
> https://reviews.apache.org/r/38367
> 
> Parity on message definitions that are not purely slave related should now be 
> achieved by this RR.
> 
> We did however notice a irregularity on the non v1-proto, documented by 
> https://issues.apache.org/jira/browse/MESOS-4091
> 
> make check (on OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1 for 0.26.0.

[Till Toenshoff]

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

(Updated Dec. 8, 2015, 2:23 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van Remoortere.


Repository: mesos


Description
---

Copied missing feature updates to v1 from include/mesos/mesos.proto.

This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
V1 API again as it has been intensionally left out (as being Slave only).


Diffs (updated)
-

  include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 

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


Testing
---

For achieving this, the following was done within the tagged 0.26.0-rc3:

`diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
`diff include/mesos/executor.proto include/mesos/v1/executor.proto`
`diff include/mesos/scheduler/scheduler.proto 
include/mesos/v1/scheduler/scheduler.proto`

These diffs do show a couple of "slave"->"agent" renames which were ignored for 
this udate.

The results showed that only mesos.proto was not en par with V1. We listed out 
all responsible review-requests;
https://reviews.apache.org/r/31915
https://reviews.apache.org/r/38253
https://reviews.apache.org/r/38367

Parity on message definitions that are not purely slave related should now be 
achieved by this RR.

We did however notice a irregularity on the non v1-proto, documented by 
https://issues.apache.org/jira/browse/MESOS-4091

make check (on OSX)


Thanks,

Till Toenshoff

Re: Review Request 36610: Add explicit syscall header file to linux fs

[Zhiwei Chen]

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

Ship it!


Ship It!

- Zhiwei Chen


On July 20, 2015, 12:21 p.m., Jihun Kang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36610/
> ---
> 
> (Updated July 20, 2015, 12:21 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3085
> https://issues.apache.org/jira/browse/MESOS-3085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add explicit syscall header file to linux fs
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp ea0891e320154b85a21ed2d138c192821efae9cd 
> 
> Diff: https://reviews.apache.org/r/36610/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jihun Kang
> 
>

Re: Review Request 41026: libevent ssl: Added check for buffer length before swapping request.

[Jojy Varghese]

> On Dec. 8, 2015, 1:43 a.m., Joseph Wu wrote:
> > Would it be plausible to write a repro/test (in the libprocess level) for 
> > this?
> > 
> > Presumably, we should be able to write a process that does a "long running 
> > streaming download" (which causes the bug, according to your diagnosis).

The steps described in the bug is a good way to test. I was able to repro the 
issue 1/10 times. Since this is a race condition, will be hard to write test.


- Jojy


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


On Dec. 8, 2015, 1:22 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41026/
> ---
> 
> (Updated Dec. 8, 2015, 1:22 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4069
> https://issues.apache.org/jira/browse/MESOS-4069
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> recv_callback could be called from libevents receive callback and Socket::recv
> for the same buffer event and different requests. There is a check for buffer
> length at Socket::recv but not at libevent's receive callback. This could lead
> to the incoming request for Socket::recv being swapped out even though the
> buffer length is zero. This change adds a check for buffer length before
> swapping out the receive request object.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 55b91dd47bb5bd5e97147d0af91c7899fd42702c 
> 
> Diff: https://reviews.apache.org/r/41026/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1 for 0.26.0.

[Till Toenshoff]

> On Dec. 8, 2015, 1 a.m., Michael Park wrote:
> > include/mesos/v1/mesos.proto, lines 1507-1508
> > 
> >
> > It looks like we're also missing this from `include/mesos/mesos.proto`?
> > ```
> > 1510 // The name of volume driver plugin.
> > 1511 optional string volume_driver = 7;
> > ```
> 
> Till Toenshoff wrote:
> That one was brought in by commit 
> fc7e25d8c05ee3c226e4d45819c047009ddd71c0, which is RR 
> https://reviews.apache.org/r/38338, backed by 
> https://issues.apache.org/jira/browse/MESOS-3392.
> 
> It got in after the release-cut, so it is not a problem for 0.26.0 but 
> following releases.
> 
> Will drop this issue for this one as it was marked as based on 0.26.0-rc3 
> but add a new RR which is a complete fix for the current master.
> 
> Till Toenshoff wrote:
> See https://reviews.apache.org/r/41070/

Actually, scratch 41070 - that partity is brought in by 
https://reviews.apache.org/r/40888


- Till


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


On Dec. 8, 2015, 2:23 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41060/
> ---
> 
> (Updated Dec. 8, 2015, 2:23 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied missing feature updates to v1 from include/mesos/mesos.proto.
> 
> This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
> V1 API again as it has been intensionally left out (as being Slave only).
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 
> 
> Diff: https://reviews.apache.org/r/41060/diff/
> 
> 
> Testing
> ---
> 
> For achieving this, the following was done within the tagged 0.26.0-rc3:
> 
> `diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
> `diff include/mesos/executor.proto include/mesos/v1/executor.proto`
> `diff include/mesos/scheduler/scheduler.proto 
> include/mesos/v1/scheduler/scheduler.proto`
> 
> These diffs do show a couple of "slave"->"agent" renames which were ignored 
> for this udate.
> 
> The results showed that only mesos.proto was not en par with V1. We listed 
> out all responsible review-requests;
> https://reviews.apache.org/r/31915
> https://reviews.apache.org/r/38253
> https://reviews.apache.org/r/38367
> 
> Parity on message definitions that are not purely slave related should now be 
> achieved by this RR.
> 
> We did however notice a irregularity on the non v1-proto, documented by 
> https://issues.apache.org/jira/browse/MESOS-4091
> 
> make check (on OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 40888: Add volume driver plugin to mesos v1 proto.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Dec. 2, 2015, 11:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40888/
> ---
> 
> (Updated Dec. 2, 2015, 11:48 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add volume driver plugin to mesos v1 proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto 9acefd55603a5a4f3f08a879a380ff927fd1e0dd 
> 
> Diff: https://reviews.apache.org/r/40888/diff/
> 
> 
> Testing
> ---
> 
> make check(ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41066: Add containerId to ResourceUsage in v1 API

[Guangya Liu]

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


Just FYI, https://reviews.apache.org/r/41046/ is syncing up all V1 APIs

- Guangya Liu


On 十二月 8, 2015, 1:40 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41066/
> ---
> 
> (Updated 十二月 8, 2015, 1:40 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Missed containerId in v1 API when fixing MESOS-2875.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 
> 
> Diff: https://reviews.apache.org/r/41066/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41066: Add containerId to ResourceUsage in v1 API

[Till Toenshoff]

> On Dec. 8, 2015, 3:03 a.m., Guangya Liu wrote:
> > Just FYI, https://reviews.apache.org/r/41046/ is syncing up all V1 APIs

In fact, that is done by https://reviews.apache.org/r/41060/ now, sorry for the 
confusion caused, had to update that RR but am not the original owner.


- Till


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


On Dec. 8, 2015, 1:40 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41066/
> ---
> 
> (Updated Dec. 8, 2015, 1:40 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Missed containerId in v1 API when fixing MESOS-2875.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 
> 
> Diff: https://reviews.apache.org/r/41066/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41050: Added a paragraph to the release guide that handles API updates.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Dec. 7, 2015, 1:06 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41050/
> ---
> 
> (Updated Dec. 7, 2015, 1:06 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 6a05a2600154c70228946cf13b86d5aa85ee45d4 
> 
> Diff: https://reviews.apache.org/r/41050/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 32505: Added SHUTDOWN scheduler call.

[Vinod Kone]

> On Nov. 20, 2015, 6:36 p.m., Zhitao Li wrote:
> > src/master/master.cpp, line 3498
> > 
> >
> > Hi [~vinodkone], can you please explain what it takes to "repliabily 
> > forward" this message? Do you mean we should perform some kind of retry 
> > action, or send this message at a different message path?
> > 
> > I'm working on 
> > [MESOS-313](https://issues.apache.org/jira/browse/MESOS-313) and would like 
> > to get it to the last mile.
> > 
> > Thanks!

I haven't thought through on how to solve this yet, but yes, there needs to be 
some kind of retry and acknowledgement mechanism involved.


- Vinod


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


On April 22, 2015, 9:41 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32505/
> ---
> 
> (Updated April 22, 2015, 9:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1127 and MESOS-330
> https://issues.apache.org/jira/browse/MESOS-1127
> https://issues.apache.org/jira/browse/MESOS-330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a new call added to explicitly shutdown an executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 783a63ad1cc0edd86605d638046fb959cb6e97e8 
>   src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15 
>   src/master/master.cpp f3462d15005e24ab28e8265484b0d3810f21bd47 
>   src/messages/messages.proto bdf474b6614fad6eebdd79c636e820f9c30cf0c9 
>   src/scheduler/scheduler.cpp 584b042e32865fdf875bf41ebcfb7f9c327d882a 
>   src/slave/slave.hpp d214ddb168298071dd90ad6d7f9bc6fd3d78d986 
>   src/slave/slave.cpp 60345ec0d05a8150e61d71a6510824a7f2ca9524 
>   src/tests/scheduler_tests.cpp 4a89a7a88b50bb8c254f5076661ce07ac9fc7657 
> 
> Diff: https://reviews.apache.org/r/32505/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 41046: Made v1 API in mesos.proto equivalent to non-v1.

[Till Toenshoff]

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



include/mesos/v1/mesos.proto (line 1506)


I must be missing something, but where did index 6 go?



include/mesos/v1/mesos.proto (lines 1585 - 1615)


Isn't this slave specific only?


- Till Toenshoff


On Dec. 7, 2015, 6:28 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41046/
> ---
> 
> (Updated Dec. 7, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied all missing feature updates (and 1 comment, all verbatim) to v1 from 
> include/mesos.proto in these RRs:
> https://reviews.apache.org/r/31915
> https://reviews.apache.org/r/38253
> https://reviews.apache.org/r/38367
> https://reviews.apache.org/r/38367
> https://reviews.apache.org/r/37307/
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto e71ddda7f23f2272ce8eb00f358c66fce205c13b 
> 
> Diff: https://reviews.apache.org/r/41046/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 39923: Cleaned up configuration.md.

[Neil Conway]

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


What is the status of this patch -- can someone (Till) commit it?

- Neil Conway


On Nov. 7, 2015, 12:20 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39923/
> ---
> 
> (Updated Nov. 7, 2015, 12:20 a.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Joris Van Remoortere, and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited configuration.md for style and consistency. Made consistent use of 
>  for commands, flags, and flag values. Edited the libprocess 
> flags for consistency and proper use of `=DIR` or `[=DIR]` depending on 
> whether or not the directory is an optional parameter. Note that the 
> libprocess `--with-*` flags have two different descriptions depending upon 
> whether or not the library is bundled with Mesos.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 195814cf918e018d8287113299163415b94ab09f 
> 
> Diff: https://reviews.apache.org/r/39923/diff/
> 
> 
> Testing
> ---
> 
> Viewed with the mesos website container 
> (https://github.com/mesosphere/mesos-website-container).
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Alexander Rukletsov]

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

Ship it!


Ship It!

- Alexander Rukletsov


On Dec. 7, 2015, 2:34 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40347/
> ---
> 
> (Updated Dec. 7, 2015, 2:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
> Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Implemented quota request authorization.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 
> 
> Diff: https://reviews.apache.org/r/40347/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

[Till Toenshoff]

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



include/mesos/mesos.proto (line 1466)


Why are we missing index 6 here?


- Till Toenshoff


On Sept. 16, 2015, 10 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38367/
> ---
> 
> (Updated Sept. 16, 2015, 10 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the frameworks to specify an intent to enable ip-per-container. 
> The
> IP information is supplied back to the framework as well as state.json 
> endpoints
> by including NetworkInfo inside TaskStatus.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
>   src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
>   src/tests/slave_tests.cpp 447c43c96ce0c043e37319bb213723ed33820f8a 
> 
> Diff: https://reviews.apache.org/r/38367/diff/
> 
> 
> Testing
> ---
> 
> Added new tests and ran make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 41046: Made v1 API in mesos.proto equivalent to non-v1.

[Till Toenshoff]

> On Dec. 7, 2015, 9:32 p.m., Till Toenshoff wrote:
> > include/mesos/v1/mesos.proto, line 1506
> > 
> >
> > I must be missing something, but where did index 6 go?

Appears to be a bug in Mesos master, filed 
https://issues.apache.org/jira/browse/MESOS-4091.

Given that we want to create parity, I think leaving this in for the HTTP API 
makes sense so it gets solved for V0 and V1 together, if needed.


- Till


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


On Dec. 7, 2015, 6:28 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41046/
> ---
> 
> (Updated Dec. 7, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied all missing feature updates (and 1 comment, all verbatim) to v1 from 
> include/mesos.proto in these RRs:
> https://reviews.apache.org/r/31915
> https://reviews.apache.org/r/38253
> https://reviews.apache.org/r/38367
> https://reviews.apache.org/r/38367
> https://reviews.apache.org/r/37307/
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto e71ddda7f23f2272ce8eb00f358c66fce205c13b 
> 
> Diff: https://reviews.apache.org/r/41046/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 35668: Report "unevictable" memory in container statistics.

[Till Toenshoff]

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



include/mesos/mesos.proto (line 605)


We missed to update the V1 API with this change, it seems.


- Till Toenshoff


On June 19, 2015, 9:03 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35668/
> ---
> 
> (Updated June 19, 2015, 9:03 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.
> 
> 
> Bugs: mesos-2798
> https://issues.apache.org/jira/browse/mesos-2798
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Report "unevictable" memory in container statistics.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 81008ed85daea798ed19d1031de8421a7dc3fb19 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> 9d65bf5b64ce72c1ca9a7a50e65a357e098af63e 
> 
> Diff: https://reviews.apache.org/r/35668/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 40951: Initial set of source files missing for cmake agent binary.

[Alex Clemmer]

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

Ship it!



src/CMakeLists.txt (line 222)


Perhaps there should be a blank line above this block?



src/CMakeLists.txt (lines 224 - 227)


Our list style is: if a list append can fit on one line, we usually try to 
fit it.



src/CMakeLists.txt (line 269)


Our style is: we don't put a space between `set` and `(`.



src/CMakeLists.txt (lines 274 - 280)


Hmm, I might be crazy, but does `isolators` come before `linux_launcher`? :)



src/CMakeLists.txt (line 289)


There's an extra newline here.


- Alex Clemmer


On Dec. 5, 2015, 3:35 a.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40951/
> ---
> 
> (Updated Dec. 5, 2015, 3:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial set of source files missing for cmake agent binary.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
> 
> Diff: https://reviews.apache.org/r/40951/diff/
> 
> 
> Testing
> ---
> 
> Tested to make sure library builds successfully.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

[Till Toenshoff]

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



include/mesos/mesos.proto (line 1165)


We missed to update the V1 API with this change, it seems.



include/mesos/mesos.proto (lines 1470 - 1477)


We missed to update the V1 API with this change, it seems.


- Till Toenshoff


On Sept. 16, 2015, 10 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38367/
> ---
> 
> (Updated Sept. 16, 2015, 10 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the frameworks to specify an intent to enable ip-per-container. 
> The
> IP information is supplied back to the framework as well as state.json 
> endpoints
> by including NetworkInfo inside TaskStatus.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
>   src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
>   src/tests/slave_tests.cpp 447c43c96ce0c043e37319bb213723ed33820f8a 
> 
> Diff: https://reviews.apache.org/r/38367/diff/
> 
> 
> Testing
> ---
> 
> Added new tests and ran make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

[Till Toenshoff]

> On Dec. 7, 2015, 10:07 p.m., Till Toenshoff wrote:
> > include/mesos/mesos.proto, line 1466
> > 
> >
> > Why are we missing index 6 here?

Also missing in the V1 API.


- Till


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


On Sept. 16, 2015, 10 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38367/
> ---
> 
> (Updated Sept. 16, 2015, 10 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the frameworks to specify an intent to enable ip-per-container. 
> The
> IP information is supplied back to the framework as well as state.json 
> endpoints
> by including NetworkInfo inside TaskStatus.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
>   src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
>   src/tests/slave_tests.cpp 447c43c96ce0c043e37319bb213723ed33820f8a 
> 
> Diff: https://reviews.apache.org/r/38367/diff/
> 
> 
> Testing
> ---
> 
> Added new tests and ran make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 38000: Introduced support for user interaction with HTTP AuthenticationRouter.

[Alexander Rukletsov]

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



3rdparty/libprocess/include/process/process.hpp (lines 261 - 263)


I'm not sure how the comment is related to the typedef. Do you mean one of 
the parameter is `principal`? Mind explaining it then?



3rdparty/libprocess/include/process/process.hpp (line 264)


Given the second argument is optional (and assuming it's principal), I 
believe `AuthenticatedHttpRequestHandler` may also be responsible for requests 
that do not require authentication (e.g. no authenticator for the realm). If my 
understanding is correct, naming the handler with `Authenticated*` can be 
misleading.



3rdparty/libprocess/src/process.cpp (lines 3218 - 3220)


If this is an internal invariant, how about adding a `CHECK` here?



3rdparty/libprocess/src/tests/http_tests.cpp (lines 20 - 23)


Sort alphabetically



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1314 - 1315)


Blank line or swap.



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1357 - 1358)


Blank line or swap.



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


Let's definitely have a comment here why there is sleep. Also, why not 
`os::sleep()`?


- Alexander Rukletsov


On Dec. 7, 2015, 2:14 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38000/
> ---
> 
> (Updated Dec. 7, 2015, 2:14 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3233
> https://issues.apache.org/jira/browse/MESOS-3233
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds functions which allow libprocess users to register HTTP authenticators.
> Overloads `ProcesBase::route()` to allow for registering of authenticating 
> endpoints.
> Includes tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> a03824c061c4a0eb865b163999a763635e56744c 
>   3rdparty/libprocess/include/process/process.hpp 
> 81c094414d4d5ac5eb593df2a6d14aaacb19a826 
>   3rdparty/libprocess/src/process.cpp 
> e93709d3bb4ac588457bb9331fc05ec5ab539f6d 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38000/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rukletsov]

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



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


Why do you use `http::Connection.send()` in the previous RR and 
`http::get()` here?


- Alexander Rukletsov


On Dec. 7, 2015, 3:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 40732: Enabling ResourcesTest.Precision

[Avinash sridharan]

> On Nov. 26, 2015, 3:35 p.m., Neil Conway wrote:
> > src/tests/resources_tests.cpp, line 1526
> > 
> >
> > If we're going to enable this test, the comment should be removed.

Based on Klaus Ma's comments will either drop this commit, or if he agrees will 
remove this comment and keep it enabled.


- Avinash


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


On Nov. 26, 2015, 6:52 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40732/
> ---
> 
> (Updated Nov. 26, 2015, 6:52 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
> ---
> 
> This is an existing test case to check precision of resource reservations, 
> but would have always failed due to double precision errors. Forcing the test 
> case to use EXPECT_DOUBE_EQ instead of EXPECT_EQ (strict equality)
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12 
> 
> Diff: https://reviews.apache.org/r/40732/diff/
> 
> 
> Testing
> ---
> 
> Ran make check against 40730
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 40732: Enabling ResourcesTest.Precision

[Avinash sridharan]

> On Nov. 26, 2015, 3:48 p.m., Klaus Ma wrote:
> > src/tests/resources_tests.cpp, line 1534
> > 
> >
> > We can not change this to `EXPECT_DOUBLE_EQ` because it's used to check 
> > `operator==` in `Resources`. I think we can check the source code of 
> > `CHECK_NEAR` and re-use it in `Scalar::operator==`.

Maybe I am getting this wrong but in TEST(ResourcesTest, Precision) I thought 
we are explicitly checking cpu resources which are set to double? Hence the 
change to EXPECT_DOUBLE_EQ


- Avinash


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


On Nov. 26, 2015, 6:52 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40732/
> ---
> 
> (Updated Nov. 26, 2015, 6:52 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
> ---
> 
> This is an existing test case to check precision of resource reservations, 
> but would have always failed due to double precision errors. Forcing the test 
> case to use EXPECT_DOUBE_EQ instead of EXPECT_EQ (strict equality)
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12 
> 
> Diff: https://reviews.apache.org/r/40732/diff/
> 
> 
> Testing
> ---
> 
> Ran make check against 40730
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 40998: Added CHECK's for various executor states in Command Executor to verify if messages are delivered in order.

[Anand Mazumdar]

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

(Updated Dec. 7, 2015, 11:19 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Changes based on feedback from BenM around not using an unused variable 
`executorInfo` but introduing an `State` enum.


Summary (updated)
-

Added CHECK's for various executor states in Command Executor to verify if 
messages are delivered in order.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  src/launcher/executor.cpp f90ea01131e6fa28e42f0c00a317d66a49f81ffa 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41026: libevent ssl: Added check for buffer length before swapping request.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41026]

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

- Mesos ReviewBot


On Dec. 7, 2015, 6:55 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41026/
> ---
> 
> (Updated Dec. 7, 2015, 6:55 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4069
> https://issues.apache.org/jira/browse/MESOS-4069
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> recv_callback could be called from libevents receive callback and Socket::recv
> for the same buffer event and different requests. There is a check for buffer
> length at Socket::recv but not at libevent's receive callback. This could lead
> to the incoming request for Socket::recv being swapped out even though the
> buffer length is zero. This change adds a check for buffer length before
> swapping out the receive request object.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 55b91dd47bb5bd5e97147d0af91c7899fd42702c 
> 
> Diff: https://reviews.apache.org/r/41026/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41046: Made v1 API in mesos.proto equivalent to non-v1.

[Bernd Mathiske]

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

(Updated Dec. 7, 2015, 10:24 a.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, and Till Toenshoff.


Changes
---

In addition to added code, there was also one change required->optional. 
Covering this as well now.


Repository: mesos


Description
---

Copied all missing feature updates (and 1 comment, all verbatim) to v1 from 
include/mesos.proto in these RRs:
https://reviews.apache.org/r/31915
https://reviews.apache.org/r/38253
https://reviews.apache.org/r/38367
https://reviews.apache.org/r/38367


Diffs (updated)
-

  include/mesos/v1/mesos.proto e71ddda7f23f2272ce8eb00f358c66fce205c13b 

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


Testing
---

make check


Thanks,

Bernd Mathiske

Re: Review Request 40995: Added test cases for role behavior.

[Joseph Wu]

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

Ship it!


LGTM!

One question: Is the positive case (correct role -> successful operation) not 
interesting or just tested in other files?


src/Makefile.am (line 1720)


Nit: Off by one tab.


- Joseph Wu


On Dec. 7, 2015, 1:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated Dec. 7, 2015, 1:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1.

[Till Toenshoff]

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

(Updated Dec. 8, 2015, 12:24 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van Remoortere.


Changes
---

Added mention of https://issues.apache.org/jira/browse/MESOS-4091


Repository: mesos


Description
---

Copied missing feature updates to v1 from include/mesos/mesos.proto.

This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
V1 API again as it has been intensionally left out (as being Slave only).


Diffs
-

  include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 

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


Testing (updated)
---

For achieving this, the following was done within the tagged 0.26.0-rc3:

`diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
`diff include/mesos/executor.proto include/mesos/v1/executor.proto`
`diff include/mesos/scheduler/scheduler.proto 
include/mesos/v1/scheduler/scheduler.proto`

These diffs do show a couple of "slave"->"agent" renames which were ignored for 
this udate.

The results showed that only mesos.proto was not en par with V1. We listed out 
all responsible review-requests;
https://reviews.apache.org/r/31915
https://reviews.apache.org/r/38253
https://reviews.apache.org/r/38367

Parity on message definitions that are not purely slave related should now be 
achieved by this RR.

We did however notice a irregularity on the non v1-proto, documented by 
https://issues.apache.org/jira/browse/MESOS-4091


Thanks,

Till Toenshoff

Review Request 41046: Made v1 API in mesos.proto equivalent to non-v1.

[Bernd Mathiske]

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

Review request for mesos, Ben Mahler, Joris Van Remoortere, and Till Toenshoff.


Repository: mesos


Description
---

Copied all missing feature updates (and 1 comment, all verbatim) to v1 from 
include/mesos.proto in these RRs:
https://reviews.apache.org/r/31915
https://reviews.apache.org/r/38253
https://reviews.apache.org/r/38367
https://reviews.apache.org/r/38367


Diffs
-

  include/mesos/v1/mesos.proto e71ddda7f23f2272ce8eb00f358c66fce205c13b 

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


Testing
---

make check


Thanks,

Bernd Mathiske

Re: Review Request 41046: Made v1 API in mesos.proto equivalent to non-v1.

[Bernd Mathiske]

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

(Updated Dec. 7, 2015, 10:28 a.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, and Till Toenshoff.


Changes
---

Added the RR that changed the required->optional field to the description.


Repository: mesos


Description (updated)
---

Copied all missing feature updates (and 1 comment, all verbatim) to v1 from 
include/mesos.proto in these RRs:
https://reviews.apache.org/r/31915
https://reviews.apache.org/r/38253
https://reviews.apache.org/r/38367
https://reviews.apache.org/r/38367
https://reviews.apache.org/r/37307/


Diffs
-

  include/mesos/v1/mesos.proto e71ddda7f23f2272ce8eb00f358c66fce205c13b 

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


Testing
---

make check


Thanks,

Bernd Mathiske

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1 for 0.26.0.

[Michael Park]

> On Dec. 8, 2015, 1 a.m., Michael Park wrote:
> > include/mesos/v1/mesos.proto, lines 1507-1508
> > 
> >
> > It looks like we're also missing this from `include/mesos/mesos.proto`?
> > ```
> > 1510 // The name of volume driver plugin.
> > 1511 optional string volume_driver = 7;
> > ```
> 
> Till Toenshoff wrote:
> That one was brought in by commit 
> fc7e25d8c05ee3c226e4d45819c047009ddd71c0, which is RR 
> https://reviews.apache.org/r/38338, backed by 
> https://issues.apache.org/jira/browse/MESOS-3392.
> 
> It got in after the release-cut, so it is not a problem for 0.26.0 but 
> following releases.
> 
> Will drop this issue for this one as it was marked as based on 0.26.0-rc3 
> but add a new RR which is a complete fix for the current master.
> 
> Till Toenshoff wrote:
> See https://reviews.apache.org/r/41070/
> 
> Till Toenshoff wrote:
> Actually, scratch 41070 - that partity is brought in by 
> https://reviews.apache.org/r/40888

Saw that. Thank you!


- Michael


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


On Dec. 8, 2015, 2:23 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41060/
> ---
> 
> (Updated Dec. 8, 2015, 2:23 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied missing feature updates to v1 from include/mesos/mesos.proto.
> 
> This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
> V1 API again as it has been intensionally left out (as being Slave only).
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 
> 
> Diff: https://reviews.apache.org/r/41060/diff/
> 
> 
> Testing
> ---
> 
> For achieving this, the following was done within the tagged 0.26.0-rc3:
> 
> `diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
> `diff include/mesos/executor.proto include/mesos/v1/executor.proto`
> `diff include/mesos/scheduler/scheduler.proto 
> include/mesos/v1/scheduler/scheduler.proto`
> 
> These diffs do show a couple of "slave"->"agent" renames which were ignored 
> for this udate.
> 
> The results showed that only mesos.proto was not en par with V1. We listed 
> out all responsible review-requests;
> https://reviews.apache.org/r/31915
> https://reviews.apache.org/r/38253
> https://reviews.apache.org/r/38367
> 
> Parity on message definitions that are not purely slave related should now be 
> achieved by this RR.
> 
> We did however notice a irregularity on the non v1-proto, documented by 
> https://issues.apache.org/jira/browse/MESOS-4091
> 
> make check (on OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 40995: Added test cases for role behavior.

[Guangya Liu]

> On 十二月 8, 2015, 12:51 a.m., Guangya Liu wrote:
> > src/tests/role_tests.cpp, lines 24-27
> > 
> >
> > I think that we should always use std first?
> > 
> > using std::vector;
> > 
> > using process::Future;
> > using process::PID;
> 
> Neil Conway wrote:
> All the test cases I looked at place `process` before `std`: e.g., 
> `fault_tolerance_tests.cpp`, `master_tests.cpp`, `reservation_tests.cpp`.
> 
> Guangya Liu wrote:
> I think that there are bugs here, you can also refer to 
> master_quota_test.cpp , I recalled that I discussed with alex-mesos for this 
> issue and his comments is we should put std first.
> 
> Neil Conway wrote:
> Can you open a JIRA for this? ISTM we aren't consistent right now, and as 
> far as I can tell, the style guide doesn't say anything about the order of 
> `using` directives.

Filed a JIRA here https://issues.apache.org/jira/browse/MESOS-4093 Thanks!


- Guangya


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


On 十二月 7, 2015, 9:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated 十二月 7, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40759: [WIP] Command executor can overcommit the slave

[Klaus Ma]

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

(Updated Dec. 8, 2015, 12:03 p.m.)


Review request for Ben Mahler, Ian Downes and Vinod Kone.


Changes
---

`make check` passed without `sudo`


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


Repository: mesos


Description
---

Currently we give a small amount of resources to the command executor, in 
addition to resources used by the command task: 
https://github.com/apache/mesos/blob/0.20.0-rc1/src/slave/slave.cpp#L2448

```
ExecutorInfo Slave::getExecutorInfo(
const FrameworkID& frameworkId,
const TaskInfo& task)
{
  ...
// Add an allowance for the command executor. This does lead to a
// small overcommit of resources.
executor.mutable_resources()->MergeFrom(
Resources::parse(
  "cpus:" + stringify(DEFAULT_EXECUTOR_CPUS) + ";" +
  "mem:" + stringify(DEFAULT_EXECUTOR_MEM.megabytes())).get());
  ...
}
```

This leads to an overcommit of the slave. Ideally, for command tasks we can 
"transfer" all of the task resources to the executor at the slave / isolation 
level.


Diffs (updated)
-

  include/mesos/mesos.proto 3a10c8a 
  src/docker/executor.cpp 4042cec 
  src/launcher/executor.cpp f90ea01 
  src/master/constants.hpp cc38dfc 
  src/master/master.hpp 4683fa5 
  src/master/master.cpp 953fa4f 
  src/slave/constants.hpp bcbb140 
  src/slave/slave.hpp b7586ce 
  src/slave/slave.cpp 9bd86e1 
  src/tests/containerizer/docker_containerizer_tests.cpp 3f199e6 
  src/tests/master_tests.cpp 865fa4a 
  src/tests/monitor_tests.cpp a848d14 
  src/tests/reservation_endpoints_tests.cpp c3833d0 
  src/tests/slave_recovery_tests.cpp c0e4ff7 
  src/tests/slave_tests.cpp 4975bea 

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


Testing
---

only `make` passwd for now.


Thanks,

Klaus Ma

Re: Review Request 40995: Added test cases for role behavior.

[Guangya Liu]

> On 十二月 8, 2015, 12:51 a.m., Guangya Liu wrote:
> > src/tests/role_tests.cpp, lines 24-27
> > 
> >
> > I think that we should always use std first?
> > 
> > using std::vector;
> > 
> > using process::Future;
> > using process::PID;
> 
> Neil Conway wrote:
> All the test cases I looked at place `process` before `std`: e.g., 
> `fault_tolerance_tests.cpp`, `master_tests.cpp`, `reservation_tests.cpp`.

I think that there are bugs here, you can also refer to master_quota_test.cpp , 
I recalled that I discussed with alex-mesos for this issue and his comments is 
we should put std first.


- Guangya


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


On 十二月 7, 2015, 9:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated 十二月 7, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41073: Fixed a typo in a comment in ReservationTest.ACLMultipleOperations.

[Greg Mann]

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

Ship it!


Ship It!

- Greg Mann


On Dec. 8, 2015, 3:38 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41073/
> ---
> 
> (Updated Dec. 8, 2015, 3:38 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in a comment in ReservationTest.ACLMultipleOperations.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
> 
> Diff: https://reviews.apache.org/r/41073/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40995: Added test cases for role behavior.

[Neil Conway]

> On Dec. 8, 2015, 12:51 a.m., Guangya Liu wrote:
> > src/tests/role_tests.cpp, lines 24-27
> > 
> >
> > I think that we should always use std first?
> > 
> > using std::vector;
> > 
> > using process::Future;
> > using process::PID;
> 
> Neil Conway wrote:
> All the test cases I looked at place `process` before `std`: e.g., 
> `fault_tolerance_tests.cpp`, `master_tests.cpp`, `reservation_tests.cpp`.
> 
> Guangya Liu wrote:
> I think that there are bugs here, you can also refer to 
> master_quota_test.cpp , I recalled that I discussed with alex-mesos for this 
> issue and his comments is we should put std first.

Can you open a JIRA for this? ISTM we aren't consistent right now, and as far 
as I can tell, the style guide doesn't say anything about the order of `using` 
directives.


- Neil


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


On Dec. 7, 2015, 9:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated Dec. 7, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41073: Fixed a typo in a comment in ReservationTest.ACLMultipleOperations.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Dec. 8, 2015, 3:38 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41073/
> ---
> 
> (Updated Dec. 8, 2015, 3:38 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in a comment in ReservationTest.ACLMultipleOperations.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
> 
> Diff: https://reviews.apache.org/r/41073/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41050: Added a paragraph to the release guide that handles API updates.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41050]

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

- Mesos ReviewBot


On Dec. 7, 2015, 9:06 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41050/
> ---
> 
> (Updated Dec. 7, 2015, 9:06 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 6a05a2600154c70228946cf13b86d5aa85ee45d4 
> 
> Diff: https://reviews.apache.org/r/41050/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 40995: Added test cases for role behavior.

[Neil Conway]

> On Dec. 8, 2015, 12:17 a.m., Joseph Wu wrote:
> > LGTM!
> > 
> > One question: Is the positive case (correct role -> successful operation) 
> > not interesting or just tested in other files?

There are tests for successful cases in other files. Maybe not exhaustive 
though...


- Neil


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


On Dec. 7, 2015, 9:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated Dec. 7, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1.

[Till Toenshoff]

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

(Updated Dec. 8, 2015, 12:47 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van Remoortere.


Repository: mesos


Description
---

Copied missing feature updates to v1 from include/mesos/mesos.proto.

This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
V1 API again as it has been intensionally left out (as being Slave only).


Diffs
-

  include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 

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


Testing (updated)
---

For achieving this, the following was done within the tagged 0.26.0-rc3:

`diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
`diff include/mesos/executor.proto include/mesos/v1/executor.proto`
`diff include/mesos/scheduler/scheduler.proto 
include/mesos/v1/scheduler/scheduler.proto`

These diffs do show a couple of "slave"->"agent" renames which were ignored for 
this udate.

The results showed that only mesos.proto was not en par with V1. We listed out 
all responsible review-requests;
https://reviews.apache.org/r/31915
https://reviews.apache.org/r/38253
https://reviews.apache.org/r/38367

Parity on message definitions that are not purely slave related should now be 
achieved by this RR.

We did however notice a irregularity on the non v1-proto, documented by 
https://issues.apache.org/jira/browse/MESOS-4091

make check (on OSX)


Thanks,

Till Toenshoff

Re: Review Request 40995: Added test cases for role behavior.

[Guangya Liu]

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



src/tests/role_tests.cpp (lines 24 - 27)


I think that we should always use std first?

using std::vector;

using process::Future;
using process::PID;


- Guangya Liu


On 十二月 7, 2015, 9:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated 十二月 7, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40431]

Failed command: ./support/apply-review.sh -n -r 40431

Error:
 2015-12-08 01:04:32 URL:https://reviews.apache.org/r/40431/diff/raw/ 
[12531/12531] -> "40431.patch" [1]
Successfully applied: Move RoleInfo message out of allocator.proto

Currently role protobuf is defined in allocator.proto due to only the 
traditional DRF allocator uses roles as it’s first level of hierarchy, I think 
we should move it out and define it in a separated file as quota had in dynamic 
roles project, because role protobuf will also be used by master to persist.


Review: https://reviews.apache.org/r/40431
include/mesos/role/role.hpp:1:  A license header should appear on the file's  
first line starting with '// Licensed'.: /**
Total errors found: 1
Checking 9 files
Failed to commit patch

- Mesos ReviewBot


On Dec. 7, 2015, 10:12 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40469/
> ---
> 
> (Updated Dec. 7, 2015, 10:12 a.m.)
> 
> 
> Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.
> 
> 
> Bugs: MESOS-3956
> https://issues.apache.org/jira/browse/MESOS-3956
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update Allocator interface to support dynamic roles
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 99c742906874c30c39c159e58a65277ade3c07fd 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
> 
> Diff: https://reviews.apache.org/r/40469/diff/
> 
> 
> Testing
> ---
> 
> Make check successfully.
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 40873: RegistryClientTests: Created separate server to serve blobs.

[Timothy Chen]

> On Dec. 3, 2015, 6:21 p.m., Mesos ReviewBot wrote:
> > Patch looks great!
> > 
> > Reviews applied: [40872, 40873]
> > 
> > Passed command: export OS=ubuntu:14.04;export 
> > CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

What does simulates the real world more closely mean here?


- Timothy


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


On Dec. 3, 2015, 5:20 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40873/
> ---
> 
> (Updated Dec. 3, 2015, 5:20 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4025
> https://issues.apache.org/jira/browse/MESOS-4025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By having a separate blob server, it simulates the real world more closely. 
> It also allows the test server to be in accept mode early.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/40873/diff/
> 
> 
> Testing
> ---
> 
> make check (600 count).
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41026: libevent ssl: Added check for buffer length before swapping request.

[Jojy Varghese]

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

(Updated Dec. 8, 2015, 1:22 a.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

variable name fixed.


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


Repository: mesos


Description
---

recv_callback could be called from libevents receive callback and Socket::recv
for the same buffer event and different requests. There is a check for buffer
length at Socket::recv but not at libevent's receive callback. This could lead
to the incoming request for Socket::recv being swapped out even though the
buffer length is zero. This change adds a check for buffer length before
swapping out the receive request object.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
55b91dd47bb5bd5e97147d0af91c7899fd42702c 

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


Testing
---

make check


Thanks,

Jojy Varghese

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

[Klaus Ma]

> On Dec. 8, 2015, 6:34 a.m., Till Toenshoff wrote:
> > include/mesos/mesos.proto, line 821
> > 
> >
> > We missed to update the V1 API with this change, it seems.

Posted a new RR#41066 to address.


- Klaus


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


On Sept. 18, 2015, 1:41 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38253/
> ---
> 
> (Updated Sept. 18, 2015, 1:41 a.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2875
> https://issues.apache.org/jira/browse/MESOS-2875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should ensure that we are addressing the container which the QoS 
> controller intended to kill. Without this check, we may run into a scenario 
> where the executor has terminated and one with the same id has started in the 
> interim i.e. running in a different container than the one the QoS controller 
> targeted.
> 
> This most likely requires us to add containerId to the ResourceUsage message 
> and encode the containerID in the QoS Correction message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 899d52f 
>   include/mesos/slave/oversubscription.proto fa69a95 
>   src/slave/slave.cpp 93353a1 
>   src/tests/oversubscription_tests.cpp 0c5edaf 
> 
> Diff: https://reviews.apache.org/r/38253/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 41066: Add containerId to ResourceUsage in v1 API

[Klaus Ma]

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

Review request for mesos, Niklas Nielsen and Till Toenshoff.


Repository: mesos


Description
---

Missed containerId in v1 API when fixing MESOS-2875.


Diffs
-

  include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 41026: libevent ssl: Added check for buffer length before swapping request.

[Joseph Wu]

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


Would it be plausible to write a repro/test (in the libprocess level) for this?

Presumably, we should be able to write a process that does a "long running 
streaming download" (which causes the bug, according to your diagnosis).

- Joseph Wu


On Dec. 7, 2015, 5:22 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41026/
> ---
> 
> (Updated Dec. 7, 2015, 5:22 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4069
> https://issues.apache.org/jira/browse/MESOS-4069
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> recv_callback could be called from libevents receive callback and Socket::recv
> for the same buffer event and different requests. There is a check for buffer
> length at Socket::recv but not at libevent's receive callback. This could lead
> to the incoming request for Socket::recv being swapped out even though the
> buffer length is zero. This change adds a check for buffer length before
> swapping out the receive request object.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 55b91dd47bb5bd5e97147d0af91c7899fd42702c 
> 
> Diff: https://reviews.apache.org/r/41026/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39043: Added support for HTTP Authentication in Mesos.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [40539, 37999, 38000, 38094, 38950]

Failed command: ./support/apply-review.sh -n -r 38950

Error:
 2015-12-08 01:54:53 URL:https://reviews.apache.org/r/38950/diff/raw/ 
[19796/19796] -> "38950.patch" [1]
Successfully applied: Http Authenticators can be loaded as modules from mesos.

Adds support for modularization of HTTP Authenticators. 

It includes an example of how to do it with the Basic HTTP Authenticator.


Review: https://reviews.apache.org/r/38950
src/examples/test_http_authenticator_module.cpp:1:  A license header should 
appear on the file's  first line starting with '// Licensed'.: /**
src/authentication/http/basic_authenticator_factory.cpp:1:  A license header 
should appear on the file's  first line starting with '// Licensed'.: /**
include/mesos/authentication/http/basic_authenticator_factory.hpp:1:  A license 
header should appear on the file's  first line starting with '// Licensed'.: /**
include/mesos/module/http_authenticator.hpp:1:  A license header should appear 
on the file's  first line starting with '// Licensed'.: /**
src/tests/http_authentication_tests.cpp:1:  A license header should appear on 
the file's  first line starting with '// Licensed'.: /**
Total errors found: 5
Checking 10 files
Failed to commit patch

- Mesos ReviewBot


On Dec. 7, 2015, 2:22 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> ---
> 
> (Updated Dec. 7, 2015, 2:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3756
> https://issues.apache.org/jira/browse/MESOS-3756
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 1. Adds a flag to load an HTTP Authenticator module from the flags.
> 2. If provided, uses the credentials file to initialize the default HTTP 
> Authenticator.
> 3. Updates the existing endpoints which implement their own basic HTTP 
> authenticator with the libprocess one.
> 4. Updates one test which expected the wrong results since now credentials 
> are checked before the body of the request.
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759 
>   src/master/constants.cpp 98ea7c8f1e7c3057cfe941f82f1b71f7731a8f32 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
> 
> Diff: https://reviews.apache.org/r/39043/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1.

[Michael Park]

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

Ship it!


Looks good to me overall! The non-issues are just me leaving notes. Thanks!


include/mesos/v1/mesos.proto (line 1317)


`s/AppC/Appc/` according to https://github.com/apache/mesos/commit/40638c5



include/mesos/v1/mesos.proto (line 1325)


Updated by https://github.com/apache/mesos/commit/e90c448



include/mesos/v1/mesos.proto (lines 1360 - 1363)


Updated by 
https://github.com/apache/mesos/commit/f55e36a5a7e0e00186bdcb3c1dc7601f9ba90bb0



include/mesos/v1/mesos.proto (lines 1408 - 1411)


Perhaps simply an oversight on 
https://github.com/apache/mesos/commit/5520587, since both of these seemed to 
have been committed together.



include/mesos/v1/mesos.proto (lines 1507 - 1508)


It looks like we're also missing this from `include/mesos/mesos.proto`?
```
1510 // The name of volume driver plugin.
1511 optional string volume_driver = 7;
```


- Michael Park


On Dec. 8, 2015, 12:47 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41060/
> ---
> 
> (Updated Dec. 8, 2015, 12:47 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied missing feature updates to v1 from include/mesos/mesos.proto.
> 
> This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
> V1 API again as it has been intensionally left out (as being Slave only).
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 
> 
> Diff: https://reviews.apache.org/r/41060/diff/
> 
> 
> Testing
> ---
> 
> For achieving this, the following was done within the tagged 0.26.0-rc3:
> 
> `diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
> `diff include/mesos/executor.proto include/mesos/v1/executor.proto`
> `diff include/mesos/scheduler/scheduler.proto 
> include/mesos/v1/scheduler/scheduler.proto`
> 
> These diffs do show a couple of "slave"->"agent" renames which were ignored 
> for this udate.
> 
> The results showed that only mesos.proto was not en par with V1. We listed 
> out all responsible review-requests;
> https://reviews.apache.org/r/31915
> https://reviews.apache.org/r/38253
> https://reviews.apache.org/r/38367
> 
> Parity on message definitions that are not purely slave related should now be 
> achieved by this RR.
> 
> We did however notice a irregularity on the non v1-proto, documented by 
> https://issues.apache.org/jira/browse/MESOS-4091
> 
> make check (on OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 40995: Added test cases for role behavior.

[Neil Conway]

> On Dec. 8, 2015, 12:51 a.m., Guangya Liu wrote:
> > src/tests/role_tests.cpp, lines 24-27
> > 
> >
> > I think that we should always use std first?
> > 
> > using std::vector;
> > 
> > using process::Future;
> > using process::PID;

All the test cases I looked at place `process` before `std`: e.g., 
`fault_tolerance_tests.cpp`, `master_tests.cpp`, `reservation_tests.cpp`.


- Neil


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


On Dec. 7, 2015, 9:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated Dec. 7, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1.

[Till Toenshoff]

> On Dec. 8, 2015, 1 a.m., Michael Park wrote:
> > include/mesos/v1/mesos.proto, lines 1507-1508
> > 
> >
> > It looks like we're also missing this from `include/mesos/mesos.proto`?
> > ```
> > 1510 // The name of volume driver plugin.
> > 1511 optional string volume_driver = 7;
> > ```

That one was brought in by commit fc7e25d8c05ee3c226e4d45819c047009ddd71c0, 
which is RR https://reviews.apache.org/r/38338, backed by 
https://issues.apache.org/jira/browse/MESOS-3392.

It got in after the release-cut, so it is not a problem for 0.26.0 but 
following releases.

Will drop this issue for this one as it was marked as based on 0.26.0-rc3 but 
add a new RR which is a complete fix for the current master.


- Till


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


On Dec. 8, 2015, 12:47 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41060/
> ---
> 
> (Updated Dec. 8, 2015, 12:47 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied missing feature updates to v1 from include/mesos/mesos.proto.
> 
> This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
> V1 API again as it has been intensionally left out (as being Slave only).
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 
> 
> Diff: https://reviews.apache.org/r/41060/diff/
> 
> 
> Testing
> ---
> 
> For achieving this, the following was done within the tagged 0.26.0-rc3:
> 
> `diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
> `diff include/mesos/executor.proto include/mesos/v1/executor.proto`
> `diff include/mesos/scheduler/scheduler.proto 
> include/mesos/v1/scheduler/scheduler.proto`
> 
> These diffs do show a couple of "slave"->"agent" renames which were ignored 
> for this udate.
> 
> The results showed that only mesos.proto was not en par with V1. We listed 
> out all responsible review-requests;
> https://reviews.apache.org/r/31915
> https://reviews.apache.org/r/38253
> https://reviews.apache.org/r/38367
> 
> Parity on message definitions that are not purely slave related should now be 
> achieved by this RR.
> 
> We did however notice a irregularity on the non v1-proto, documented by 
> https://issues.apache.org/jira/browse/MESOS-4091
> 
> make check (on OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 40873: RegistryClientTests: Created separate server to serve blobs.

[Jojy Varghese]

> On Dec. 3, 2015, 6:21 p.m., Mesos ReviewBot wrote:
> > Patch looks great!
> > 
> > Reviews applied: [40872, 40873]
> > 
> > Passed command: export OS=ubuntu:14.04;export 
> > CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
> 
> Timothy Chen wrote:
> What does simulates the real world more closely mean here?

The change introduces a new server to serve the blobs like how the docker 
registry has a blob server (where the blob request is rediercted to).


- Jojy


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


On Dec. 3, 2015, 5:20 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40873/
> ---
> 
> (Updated Dec. 3, 2015, 5:20 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4025
> https://issues.apache.org/jira/browse/MESOS-4025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By having a separate blob server, it simulates the real world more closely. 
> It also allows the test server to be in accept mode early.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c63bf53fee40ef12536a16e11f4d5224c4e4278e 
> 
> Diff: https://reviews.apache.org/r/40873/diff/
> 
> 
> Testing
> ---
> 
> make check (600 count).
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41060: Made v1 API in mesos.proto equivalent to non-v1 for 0.26.0.

[Till Toenshoff]

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

(Updated Dec. 8, 2015, 2:06 a.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, and Joris Van Remoortere.


Summary (updated)
-

Made v1 API in mesos.proto equivalent to non-v1 for 0.26.0.


Repository: mesos


Description
---

Copied missing feature updates to v1 from include/mesos/mesos.proto.

This is a revised version of RR 41046 - removing the `AppcImageManifest` from 
V1 API again as it has been intensionally left out (as being Slave only).


Diffs (updated)
-

  include/mesos/v1/mesos.proto be3d61e9ec2f092019d6111d6e08b06fcc6dd068 

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


Testing
---

For achieving this, the following was done within the tagged 0.26.0-rc3:

`diff include/mesos/mesos.proto include/mesos/v1/mesos.proto`
`diff include/mesos/executor.proto include/mesos/v1/executor.proto`
`diff include/mesos/scheduler/scheduler.proto 
include/mesos/v1/scheduler/scheduler.proto`

These diffs do show a couple of "slave"->"agent" renames which were ignored for 
this udate.

The results showed that only mesos.proto was not en par with V1. We listed out 
all responsible review-requests;
https://reviews.apache.org/r/31915
https://reviews.apache.org/r/38253
https://reviews.apache.org/r/38367

Parity on message definitions that are not purely slave related should now be 
achieved by this RR.

We did however notice a irregularity on the non v1-proto, documented by 
https://issues.apache.org/jira/browse/MESOS-4091

make check (on OSX)


Thanks,

Till Toenshoff

Re: Review Request 40431: Move RoleInfo message out of allocator.proto

[Yong Qiao Wang]

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

(Updated Dec. 8, 2015, 5:15 a.m.)


Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.


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


Repository: mesos


Description
---

Currently role protobuf is defined in allocator.proto due to only the 
traditional DRF allocator uses roles as it’s first level of hierarchy, I think 
we should move it out and define it in a separated file as quota had in dynamic 
roles project, because role protobuf will also be used by master to persist.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
  include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c 
  include/mesos/role/role.hpp PRE-CREATION 
  include/mesos/role/role.proto PRE-CREATION 
  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
  src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
  src/master/allocator/mesos/allocator.hpp 
97ee80726ad155917811265a983258b0165d3451 
  src/master/allocator/mesos/hierarchical.hpp 
99c742906874c30c39c159e58a65277ade3c07fd 
  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 
  src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
  src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
  src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
  src/tests/hierarchical_allocator_tests.cpp 
fb214a829a57529d3f5c49730ae9733f53e622ca 

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


Testing
---

1. Make Check successfully;

2. $ curl http://9.110.48.168:5050/roles
{"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}


Thanks,

Yong Qiao Wang

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

[Yong Qiao Wang]

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

(Updated Dec. 8, 2015, 5:20 a.m.)


Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.


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


Repository: mesos


Description
---

Update Allocator interface to support dynamic roles


Diffs (updated)
-

  include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
  src/master/allocator/mesos/allocator.hpp 
97ee80726ad155917811265a983258b0165d3451 
  src/master/allocator/mesos/hierarchical.hpp 
99c742906874c30c39c159e58a65277ade3c07fd 
  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 
  src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 

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


Testing
---

Make check successfully.


Thanks,

Yong Qiao Wang

Re: Review Request 40431: Move RoleInfo message out of allocator.proto

[Yong Qiao Wang]

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

(Updated Dec. 8, 2015, 5:20 a.m.)


Review request for mesos, Adam B, Guangya Liu, Qian Zhang, and Jian Qiu.


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


Repository: mesos


Description
---

Currently role protobuf is defined in allocator.proto due to only the 
traditional DRF allocator uses roles as it’s first level of hierarchy, I think 
we should move it out and define it in a separated file as quota had in dynamic 
roles project, because role protobuf will also be used by master to persist.


Diffs (updated)
-

  include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
  include/mesos/master/allocator.proto 702f56f56c3b1331613cecf26522986f6b572f8c 
  include/mesos/role/role.hpp PRE-CREATION 
  include/mesos/role/role.proto PRE-CREATION 
  src/CMakeLists.txt c0d77c745eb5b12dd6d9d7afaba7e820f8d848ef 
  src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
  src/master/allocator/mesos/allocator.hpp 
97ee80726ad155917811265a983258b0165d3451 
  src/master/allocator/mesos/hierarchical.hpp 
99c742906874c30c39c159e58a65277ade3c07fd 
  src/master/allocator/mesos/hierarchical.cpp 
5da825a1d578a9ee40b4985378fddb3c5fb3b416 
  src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
  src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
  src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
  src/tests/hierarchical_allocator_tests.cpp 
fb214a829a57529d3f5c49730ae9733f53e622ca 

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


Testing
---

1. Make Check successfully;

2. $ curl http://9.110.48.168:5050/roles
{"roles":[{"frameworks":[],"name":"*","resources":{"cpus":0,"disk":0,"mem":0},"weight":1.0}]}


Thanks,

Yong Qiao Wang

Re: Review Request 36610: Add explicit syscall header file to linux fs

[Cong Wang]

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


Isn't SYS_xxx in sys/syscall.h instead of syscall.h?

- Cong Wang


On July 20, 2015, 4:21 a.m., Jihun Kang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36610/
> ---
> 
> (Updated July 20, 2015, 4:21 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3085
> https://issues.apache.org/jira/browse/MESOS-3085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add explicit syscall header file to linux fs
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp ea0891e320154b85a21ed2d138c192821efae9cd 
> 
> Diff: https://reviews.apache.org/r/36610/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jihun Kang
> 
>

Re: Review Request 40995: Added test cases for role behavior.

[Yong Qiao Wang]

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

Ship it!


Ship It!

- Yong Qiao Wang


On Dec. 7, 2015, 9:22 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40995/
> ---
> 
> (Updated Dec. 7, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Yong 
> Qiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for role behavior.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e96e0ec41e11acff00fbfb3e86427b48a0625bd2 
>   src/tests/reservation_tests.cpp 3fdf5e121840fe99057e917cca48f1425eff6624 
>   src/tests/role_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40995/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>