Re: Review Request 48923: Updated test to expect ServiceUnavailable.

[Till Toenshoff]

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

(Updated June 20, 2016, 5:51 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.


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


Repository: mesos


Description
---

StatisticsEndpointGetResourceUsageFailed was expecting an
InternalServerError but instead it will now receive a
ServiceUnavailable.


Diffs
-

  src/tests/slave_tests.cpp ad36b74c12e52435b356bced1d15bd33983949e0 

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


Testing (updated)
---

make check (OSX and some Linux distros)


Thanks,

Till Toenshoff

Re: Review Request 48914: Added GPU_RESOURCES capability to FrameworkInfo.

[Guangya Liu]

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




include/mesos/mesos.proto (line 278)


I think that this comment should be updated, as even a framework with 
`GPU_RESOURCES`, it can still get resources from agents which does not have gpu 
resources.


- Guangya Liu


On 六月 18, 2016, 10:07 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48914/
> ---
> 
> (Updated 六月 18, 2016, 10:07 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5634
> https://issues.apache.org/jira/browse/MESOS-5634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the scarce resource problem described in MESOS-5377, we are
> introducing a GPU_RESOURCES Framework capability. This capability
> allows the Mesos allocator to make better decisions about which
> frameworks should receive resources from GPU capable machines. In
> essence, the allocator ONLY allocate resources from GPU capable
> machines to frameworks that have this capability. This is necessary to
> prevent non-GPU workloads from filling up the GPU machines and
> preventing GPU workloads to run.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e4c5bd31cf035707036eb509336fe051119b4e78 
>   include/mesos/v1/mesos.proto 9be22f02861f1eb89ab547d88530faf90ebee7ab 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9c6b23abe2b0cb16412f1ed90165f8d0c14552fa 
>   src/master/allocator/mesos/hierarchical.cpp 
> 8b7b3afb5770c617918ec4864faaf8d8a7a81ef2 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> e06d107f2dcdb9b470e330c8ceee66a54220d41b 
> 
> Diff: https://reviews.apache.org/r/48914/diff/
> 
> 
> Testing
> ---
> 
> $ make -j check; sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48919: Updated HTTPProxy to return a failure message in the HTTP result.

[Till Toenshoff]

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

(Updated June 20, 2016, 5:50 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.


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


Repository: mesos


Description
---

see summary.


Diffs
-

  3rdparty/libprocess/src/process.cpp 703f673a98102958c5e2b0c1833efad2ddc53ef8 

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


Testing (updated)
---

make check (OSX and some Linux distros)


Thanks,

Till Toenshoff

Re: Review Request 48920: Updated the HTTP result returned by failures of authn/authz.

[Till Toenshoff]

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

(Updated June 20, 2016, 5:49 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.


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


Repository: mesos


Description
---

Changes authentication and authorization happening on the libprocess
level to be in line with failures possibly returned by Mesos
authorization as currently implemented by the HTTPProxy::process
function. The HTTP result returned on failures has changed from
InternalServerError (500) towards ServiceNotAvailable (503) and now
contains a message describing the problem.


Diffs
-

  3rdparty/libprocess/src/process.cpp 703f673a98102958c5e2b0c1833efad2ddc53ef8 

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


Testing (updated)
---

make check (OSX and some Linux distros) & functional testing.


Thanks,

Till Toenshoff

Re: Review Request 48268: Implemented SET_QUOTA Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 20, 2016, 5:46 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Implemented SET_QUOTA Call in v1 master API.


Diffs (updated)
-

  include/mesos/v1/master/master.proto abcf628eea7922cdb323ec135ec48e9b11209608 
  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
  src/master/quota_handler.cpp 567f1c2cc59b859227a8b48c6086ce3f8049f14d 
  src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 

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


Testing
---

On Ubuntu 16.04:
sudo GTEST_FILTER="*MasterAPITest.SetQuota*" make -j4 check

[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from ContentType/MasterAPITest
[ RUN  ] ContentType/MasterAPITest.SetQuota/0
[   OK ] ContentType/MasterAPITest.SetQuota/0 (129 ms)
[ RUN  ] ContentType/MasterAPITest.SetQuota/1
[   OK ] ContentType/MasterAPITest.SetQuota/1 (98 ms)
[--] 2 tests from ContentType/MasterAPITest (227 ms total)

[--] Global test environment tear-down
[==] 2 tests from 1 test case ran. (236 ms total)
[  PASSED  ] 2 tests.


Thanks,

Abhishek Dasgupta

Re: Review Request 48901: Fixed all source files affected by the `fromBytes()` change.

[Deshna Jain]

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

(Updated June 20, 2016, 5:41 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

This change fixes all occurrences in the existing code
of `fromBytes()` that needed revisiting after the return
type has been changed to a `Try`.


Diffs (updated)
-

  include/mesos/state/state.hpp f2fddee4fa803fa0572f6194e7f5f45a56254c00 
  src/examples/long_lived_executor.cpp 770aae2fcc7a11db9abab31262cf81967796bb23 
  src/examples/test_http_executor.cpp 1a04b75c885733b01c624e29f0bf8f68a9cd7346 
  src/exec/exec.cpp f78e8b66947b33d70c4d4aaf11f66de6a84389fb 
  src/launcher/executor.cpp 877b0fdbcacf92d36f85f16457c38edb204f489b 
  src/master/master.cpp 1971e9bf3875ceb117130b84533dc37873ca60df 
  src/messages/messages.cpp 9274b1b3964a52a7929c2f1e9cea55e84b7549a1 
  src/slave/slave.cpp 4bf01f2b020f5e975fb57cffcd19865d7431eac2 
  src/slave/state.cpp 04c3d42040f186507a0e484a3ee616f1b1a77ea8 
  src/slave/status_update_manager.cpp 0e9891c2e8219f901610b8ec711fd45919ae6fbe 
  src/state/in_memory.cpp c50f46dba45199ca4f3060039ad094a93a8a2e18 
  src/state/leveldb.cpp 6cd86551d88018ee6a044491f78c21a90abf83e4 
  src/state/log.cpp 2a55913c4a246ef09b73eb8205826e88c1963171 
  src/state/zookeeper.cpp 1ba700ef4a1f650cb8c5d9090bf004398f2729e0 
  src/tests/slave_recovery_tests.cpp 2238355e9459cf4598a7aa96d3c6153a2dfcaa13 

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


Testing
---

make check


Thanks,

Deshna Jain

Re: Review Request 48613: Added validation logic for UUID's.

[Deshna Jain]

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

(Updated June 20, 2016, 5:41 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated desc.


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


Repository: mesos


Description (updated)
---

This change adds validation logic to `UUID::fromBytes()` for uuids.
The modified return type of this function is now `Try`.


Diffs (updated)
-

  3rdparty/stout/include/stout/uuid.hpp 
cde3bdbbdcc428410c83a7724389056f7aaf63a1 
  3rdparty/stout/tests/uuid_tests.cpp 50295ad25843a3938a5a58f5ba7f082f8ffeba45 

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


Testing
---

make check


Thanks,

Deshna Jain

Re: Review Request 48614: Added validation for UUID's to master/agent validation helpers.

[Deshna Jain]

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

(Updated June 20, 2016, 5:41 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Run review bot.


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


Repository: mesos


Description
---

This change adds logic to the master/agent validation helpers
to see if the status update acknowledgments have a valid UUID
set conforming to one of the supported versions.


Diffs (updated)
-

  src/master/validation.cpp 7f139fb0a67281c56fcb2bc2afe086c3382dff30 
  src/slave/validation.cpp 86e95a26fc7b7ee156a17f3ab3ffa2393e40f98a 
  src/tests/scheduler_http_api_tests.cpp 
c12205f90e4f2da2c6ad7a0ea75777601eb0ef13 

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


Testing
---


Thanks,

Deshna Jain

Re: Review Request 48937: Added a TODO about possible security issues due to misspelled ACLs.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48936, 48937]

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

- Mesos ReviewBot


On June 20, 2016, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48937/
> ---
> 
> (Updated June 20, 2016, 12:12 a.m.)
> 
> 
> Review request for mesos, Joerg Schad and Till Toenshoff.
> 
> 
> Bugs: MESOS-5588
> https://issues.apache.org/jira/browse/MESOS-5588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> 60dfe10dbefa0e82c8f3c1012ef632d786109d1b 
> 
> Diff: https://reviews.apache.org/r/48937/diff/
> 
> 
> Testing
> ---
> 
> None: Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 48268: Implemented SET_QUOTA Call in v1 master API.

[Anand Mazumdar]

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




include/mesos/v1/master/master.proto (lines 164 - 165)


s/SET_QUOTA call to set/Sets



include/mesos/v1/master/master.proto (lines 167 - 168)


Kill this, this is redundant. The reader can see the documentation of 
`QuotaRequest` message from it's corresponding protobuf.



src/Makefile.am (line 718)


Can we do this change in a separate patch? This modification is outside the 
scope of implementing the `SET_QUOTA` call.

For now, just include the quota header pb in this patch and post another 
patch thereafter.



src/master/master.hpp (line 959)


hmm.. there is already an existing overload called `set()`. Can we just use 
`set()` as the function name?

Also, since this function is already inside `QuotaHandler`, `setQuota` is 
redundant.



src/master/master.hpp (line 962)


What do we need the argument `ContentType` for if we just return a `OK()`? 
I see that you don't use this parameter in the definition at all?

I wonder how this escaped the `-Wunused-parameter warning`.



src/master/master.hpp (line 1038)


s/_setQuota/_set



src/master/master.hpp (line 1039)


const reference please.



src/master/quota_handler.cpp (lines 272 - 274)


4 spaces indent please



src/master/quota_handler.cpp (line 277)


Nit: Kill this new line and group these checks together?



src/master/quota_handler.cpp (lines 280 - 282)


Can we just inline the previous statement?

```cpp
return _setQuota(call.set_quota().quota_request, principal)
```

Also, it's a good practice to capture aliases by const reference to avoid 
the overhead of copying. (`const QuotaRequest&`)



src/master/quota_handler.cpp (line 290)


Not yours: I am fine with removing this self explanatory comment.



src/master/quota_handler.cpp (line 304)


no space between request and ':'



src/tests/api_tests.cpp (line 17)


Kill this, you already included this 2 lines later?



src/tests/api_tests.cpp (line 48)


Kill this



src/tests/api_tests.cpp (line 64)


Do you need it?



src/tests/api_tests.cpp (line 753)


Why is this scoped in its own block? Can we just build it in the main block 
now that we don't have more tests?



src/tests/api_tests.cpp (line 758)


Nit: Newline before.



src/tests/api_tests.cpp (lines 762 - 763)


Why not just?

```cpp
 quotaRequest.mutable_guarantee()->CopyFrom(quotaResources);
```


- Anand Mazumdar


On June 19, 2016, 5:46 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48268/
> ---
> 
> (Updated June 19, 2016, 5:46 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5509
> https://issues.apache.org/jira/browse/MESOS-5509
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented SET_QUOTA Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master/master.proto 
> abcf628eea7922cdb323ec135ec48e9b11209608 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
>   src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
>   src/master/quota_handler.cpp 567f1c2cc59b859227a8b48c6086ce3f8049f14d 
>   src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 
> 
> Diff: https://reviews.apache.org/r/48268/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo GTEST_FILTER="*MasterAPITest.SetQuota*" make -j4 check
> 
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from ContentType/MasterAPITest
> [ RUN  ] 

Re: Review Request 48934: Removed redundant expectations for `connected`/`disconnected` callback.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48935, 48934]

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

- Mesos ReviewBot


On June 19, 2016, 10:47 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48934/
> ---
> 
> (Updated June 19, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos, Jose Guilherme Vanz and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a follow up patch to `625b08` with the same reasoning i.e.,
> the `master` object is now destroyed _after_ the scheduler
> library thereby ensuring that there cannot be any pending callbacks
> that can be triggered. Hence, we can get rid of these redundant
> expectations.
> 
> 
> Diffs
> -
> 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/scheduler_tests.cpp b1831052ef2ef4bc4f8f6ecabbb95f309378fa25 
> 
> Diff: https://reviews.apache.org/r/48934/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48925: Implement GET_WEIGHTS Call in v1 master API.

[zhou xing]

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

(Updated 六月 20, 2016, 3:35 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

modify code indent


Bugs: mesos-5495
https://issues.apache.org/jira/browse/mesos-5495


Repository: mesos


Description
---

Add getWeights method in weights_handler.


Diffs (updated)
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
  src/master/weights_handler.cpp 25075357c5ac8450d2fa1790844802f121908e9e 
  src/tests/api_tests.cpp d9596f35c2adfbb223c831136546bd854b19c320 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 48924: Extract public logic of collecting weights info into _getWeights.

[zhou xing]

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

(Updated 六月 20, 2016, 3:33 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Bugs: mesos-5495
https://issues.apache.org/jira/browse/mesos-5495


Repository: mesos


Description
---

This patch refined the logic of collecting weights info in
WeightsHandler. Extracted the public code into new _getWeights
method so that the new operator API can reuse this method to
collect weight info.


Diffs (updated)
-

  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
  src/master/weights_handler.cpp 25075357c5ac8450d2fa1790844802f121908e9e 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 48438: Implement GET_AGENTS Call in v1 master API.

[zhou xing]

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

(Updated 六月 20, 2016, 2:47 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

rebase and change var name


Bugs: mesos-5491
https://issues.apache.org/jira/browse/mesos-5491


Repository: mesos


Description
---

Implement the basic getAgents method for v1 operator master API.


Diffs (updated)
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
  src/tests/api_tests.cpp d9596f35c2adfbb223c831136546bd854b19c320 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 48928: Added info about external `libelf` dependence to `upgrades.md`.

[Kevin Klues]

> On June 20, 2016, 1:59 a.m., haosdent huang wrote:
> > docs/upgrades.md, line 257
> > 
> >
> > I saw the `getting-started.md` use `elfutils-libelf-devel.x86_64`, 
> > seems not match here?

The getting started guide describes what to install as a developer (i.e. the 
headers + the libs). The upgrade guide describes what to install as a user 
(i.e. just the libs).


- Kevin


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


On June 19, 2016, 7:30 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48928/
> ---
> 
> (Updated June 19, 2016, 7:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, Vinod Kone, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-5630
> https://issues.apache.org/jira/browse/MESOS-5630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added info about external `libelf` dependence to `upgrades.md`.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 61b7cd3ab45d09f3e4785e00bb8622421952d253 
> 
> Diff: https://reviews.apache.org/r/48928/diff/
> 
> 
> Testing
> ---
> 
> Ran the site docker to verify everything looked OK.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48931: Implemented LIST_FILES Call in v1 master API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48704, 48705, 48931]

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

- Mesos ReviewBot


On June 19, 2016, 8:37 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48931/
> ---
> 
> (Updated June 19, 2016, 8:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5487
> https://issues.apache.org/jira/browse/MESOS-5487
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented LIST_FILES Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master/master.proto 
> abcf628eea7922cdb323ec135ec48e9b11209608 
>   src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
>   src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
>   src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 
> 
> Diff: https://reviews.apache.org/r/48931/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 16.04:
> sudo GTEST_FILTER="*AgentAPITest.ListFiles*" make -j4 check
> sudo GTEST_FILTER="*MasterAPITest.ListFiles*" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 48927: Implemented DESTROY_VOLUMES Call in v1 master API.

[Shuai Lin]

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

(Updated June 20, 2016, 1:59 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Implemented DESTROY_VOLUMES Call in v1 master API.


Diffs
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 

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


Testing (updated)
---

Please check https://reviews.apache.org/r/48938/ , which adds the test case for 
both create/destroy volumes api.


Thanks,

Shuai Lin

Re: Review Request 48926: Implemented CREATE_VOLUMES Call in v1 master API.

[Shuai Lin]

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

(Updated June 20, 2016, 1:59 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Implemented CREATE_VOLUMES Call in v1 master API.


Diffs
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 

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


Testing (updated)
---

Please check https://reviews.apache.org/r/48938/ , which adds the test case for 
both create/destroy volumes api.


Thanks,

Shuai Lin

Re: Review Request 48928: Added info about external `libelf` dependence to `upgrades.md`.

[haosdent huang]

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


Fix it, then Ship it!





docs/upgrades.md (line 257)


I saw the `getting-started.md` use `elfutils-libelf-devel.x86_64`, seems 
not match here?


- haosdent huang


On June 19, 2016, 7:30 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48928/
> ---
> 
> (Updated June 19, 2016, 7:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, Vinod Kone, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-5630
> https://issues.apache.org/jira/browse/MESOS-5630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added info about external `libelf` dependence to `upgrades.md`.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 61b7cd3ab45d09f3e4785e00bb8622421952d253 
> 
> Diff: https://reviews.apache.org/r/48928/diff/
> 
> 
> Testing
> ---
> 
> Ran the site docker to verify everything looked OK.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48927: Implemented DESTROY_VOLUMES Call in v1 master API.

[Shuai Lin]

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

(Updated June 20, 2016, 1:54 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Implemented DESTROY_VOLUMES Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 

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


Testing
---

Would follow up with a test case for both create/destroy volumes api soon.


Thanks,

Shuai Lin

Review Request 48938: Added test case `MasterAPITest.CreateAndDetroyVolumes`.

[Shuai Lin]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.CreateAndDetroyVolumes`.


Diffs
-

  src/internal/evolve.hpp 7ce62d92a87f4885d8c4faab542c49d5bfb251d3 
  src/internal/evolve.cpp 67c550342c06acdfcd0ed5cbe860ad29e41e6846 
  src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 

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


Testing
---

"make check" on ubuntu 14.04 64bit with gcc.


Thanks,

Shuai Lin

Re: Review Request 48926: Implemented CREATE_VOLUMES Call in v1 master API.

[Shuai Lin]

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

(Updated June 20, 2016, 1:53 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Implemented CREATE_VOLUMES Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 

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


Testing
---

Would follow up with a test case for both create/destroy volumes api soon.


Thanks,

Shuai Lin

Re: Review Request 48928: Added info about external `libelf` dependence to `upgrades.md`.

[Kevin Klues]

> On June 20, 2016, 12:34 a.m., Vinod Kone wrote:
> > docs/upgrades.md, line 255
> > 
> >
> > s/dependence/dependency/ ?
> > 
> > esp the last occurence in this para.

I can make the change if you like, but what you propose (although colloquially 
used) is not technically correct english in the way I'm using it. A 
"dependence" is the thing you depend on. A "dependency" is the state of being 
dependent on something.

I could say "Mesos 1.0 adds an external dependency to libmesos for `libelf` on 
Linux". And then "Please install this library before upgrading". Does that 
sound better?


- Kevin


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


On June 19, 2016, 7:30 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48928/
> ---
> 
> (Updated June 19, 2016, 7:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, Vinod Kone, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-5630
> https://issues.apache.org/jira/browse/MESOS-5630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added info about external `libelf` dependence to `upgrades.md`.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 61b7cd3ab45d09f3e4785e00bb8622421952d253 
> 
> Diff: https://reviews.apache.org/r/48928/diff/
> 
> 
> Testing
> ---
> 
> Ran the site docker to verify everything looked OK.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48928: Added info about external `libelf` dependence to `upgrades.md`.

[Kevin Klues]

> On June 20, 2016, 12:34 a.m., Vinod Kone wrote:
> > docs/upgrades.md, line 262
> > 
> >
> > Is libelf a run-time dep as well? I thought it was a build-time dep 
> > only? I'm thinking more and more that we should guard this dep on a config 
> > or run time flag. cc @bmahler.

Yes, it's a runtime dependence.  The headers are a build-time dependence and 
then the library itself is a runtime dependence. Honestly, for this release 
it's not even necessary to pull in this dependence yet, as none of the 
committed code uses it yet (other than a test). It is only used for anything 
substantial by stuff that won't make it until the next release.  If you want me 
to submit a patch to just remove it for now so we can think about the best way 
forward for the next release, let me know.


- Kevin


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


On June 19, 2016, 7:30 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48928/
> ---
> 
> (Updated June 19, 2016, 7:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, Vinod Kone, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-5630
> https://issues.apache.org/jira/browse/MESOS-5630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added info about external `libelf` dependence to `upgrades.md`.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 61b7cd3ab45d09f3e4785e00bb8622421952d253 
> 
> Diff: https://reviews.apache.org/r/48928/diff/
> 
> 
> Testing
> ---
> 
> Ran the site docker to verify everything looked OK.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48920: Updated the HTTP result returned by failures of authn/authz.

[Till Toenshoff]

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

(Updated June 20, 2016, 1:02 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.


Changes
---

addressed comments.


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


Repository: mesos


Description
---

Changes authentication and authorization happening on the libprocess
level to be in line with failures possibly returned by Mesos
authorization as currently implemented by the HTTPProxy::process
function. The HTTP result returned on failures has changed from
InternalServerError (500) towards ServiceNotAvailable (503) and now
contains a message describing the problem.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 703f673a98102958c5e2b0c1833efad2ddc53ef8 

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


Testing
---

make check & functional testing.


Thanks,

Till Toenshoff

Re: Review Request 48919: Updated HTTPProxy to return a failure message in the HTTP result.

[Till Toenshoff]

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

(Updated June 20, 2016, 1:01 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.


Changes
---

addressed comments.


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 703f673a98102958c5e2b0c1833efad2ddc53ef8 

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


Testing
---

make check & functional testing.


Thanks,

Till Toenshoff

Re: Review Request 48438: Implement GET_AGENTS Call in v1 master API.

[Vinod Kone]

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




src/master/http.cpp (line 1706)


s/agent/slave/

calling Slave* agent is a bit confusing.



src/master/http.cpp (line 1707)


s/agentObject/agent/



src/master/http.cpp (lines 1733 - 1742)


you can kill this.



src/master/http.cpp (lines 1744 - 1746)


you can kill this.


- Vinod Kone


On June 18, 2016, 2:02 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48438/
> ---
> 
> (Updated June 18, 2016, 2:02 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5491
> https://issues.apache.org/jira/browse/mesos-5491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement the basic getAgents method for v1 operator master API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a6beb1721958a77886f0aa535346e2ff33bd5d04 
>   src/master/master.hpp 618d928ef1398e7397df968c7cd47acbc987d1e3 
>   src/tests/api_tests.cpp afa5ffab8729dfc5b102968ec64c6a3bcd832dfd 
> 
> Diff: https://reviews.apache.org/r/48438/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 48937: Added a TODO about possible security issues due to misspelled ACLs.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On June 20, 2016, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48937/
> ---
> 
> (Updated June 20, 2016, 12:12 a.m.)
> 
> 
> Review request for mesos, Joerg Schad and Till Toenshoff.
> 
> 
> Bugs: MESOS-5588
> https://issues.apache.org/jira/browse/MESOS-5588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> 60dfe10dbefa0e82c8f3c1012ef632d786109d1b 
> 
> Diff: https://reviews.apache.org/r/48937/diff/
> 
> 
> Testing
> ---
> 
> None: Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 48928: Added info about external `libelf` dependence to `upgrades.md`.

[Vinod Kone]

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




docs/upgrades.md (line 255)


s/dependence/dependency/ ?

esp the last occurence in this para.



docs/upgrades.md (line 262)


Is libelf a run-time dep as well? I thought it was a build-time dep only? 
I'm thinking more and more that we should guard this dep on a config or run 
time flag. cc @bmahler.


- Vinod Kone


On June 19, 2016, 7:30 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48928/
> ---
> 
> (Updated June 19, 2016, 7:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, Vinod Kone, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-5630
> https://issues.apache.org/jira/browse/MESOS-5630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added info about external `libelf` dependence to `upgrades.md`.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 61b7cd3ab45d09f3e4785e00bb8622421952d253 
> 
> Diff: https://reviews.apache.org/r/48928/diff/
> 
> 
> Testing
> ---
> 
> Ran the site docker to verify everything looked OK.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48614: Added validation for UUID's to master/agent validation helpers.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 18, 2016, 9:16 p.m., Deshna Jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48614/
> ---
> 
> (Updated June 18, 2016, 9:16 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5224
> https://issues.apache.org/jira/browse/MESOS-5224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds logic to the master/agent validation helpers
> to see if the status update acknowledgments have a valid UUID
> set conforming to one of the supported versions.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 7f139fb0a67281c56fcb2bc2afe086c3382dff30 
>   src/slave/validation.cpp 86e95a26fc7b7ee156a17f3ab3ffa2393e40f98a 
>   src/tests/scheduler_http_api_tests.cpp 
> c12205f90e4f2da2c6ad7a0ea75777601eb0ef13 
> 
> Diff: https://reviews.apache.org/r/48614/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deshna Jain
> 
>

Review Request 48937: Added a TODO about possible security issues due to misspelled ACLs.

[Alexander Rukletsov]

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

Review request for mesos, Joerg Schad and Till Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/authorizer/local/authorizer.cpp 60dfe10dbefa0e82c8f3c1012ef632d786109d1b 

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


Testing
---

None: Not a functional change.


Thanks,

Alexander Rukletsov

Review Request 48936: Explained why fields in the acts.Entity must be required.

[Alexander Rukletsov]

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

Review request for mesos, Alexander Rojas and Till Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/authorizer/acls.proto f8e866b876c49e62a07f29edc66fc205ff33d618 

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


Testing
---

None: Not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 48613: Added validation logic for UUID's.

[Vinod Kone]

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


Ship it!




Can you update the description?

- Vinod Kone


On June 18, 2016, 7:58 a.m., Deshna Jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48613/
> ---
> 
> (Updated June 18, 2016, 7:58 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5224
> https://issues.apache.org/jira/browse/MESOS-5224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a `validateFromBytes` function for validating
> UUID's by checking if they are one of the supported UUID
> versions.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/uuid.hpp 
> cde3bdbbdcc428410c83a7724389056f7aaf63a1 
>   3rdparty/stout/tests/uuid_tests.cpp 
> 50295ad25843a3938a5a58f5ba7f082f8ffeba45 
> 
> Diff: https://reviews.apache.org/r/48613/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Deshna Jain
> 
>

Re: Review Request 48934: Removed redundant expectations for `connected`/`disconnected` callback.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 19, 2016, 10:47 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48934/
> ---
> 
> (Updated June 19, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos, Jose Guilherme Vanz and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a follow up patch to `625b08` with the same reasoning i.e.,
> the `master` object is now destroyed _after_ the scheduler
> library thereby ensuring that there cannot be any pending callbacks
> that can be triggered. Hence, we can get rid of these redundant
> expectations.
> 
> 
> Diffs
> -
> 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/scheduler_tests.cpp b1831052ef2ef4bc4f8f6ecabbb95f309378fa25 
> 
> Diff: https://reviews.apache.org/r/48934/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48935: Fixed flakiness in some of the tests using scheduler library.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 19, 2016, 10:47 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48935/
> ---
> 
> (Updated June 19, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos, Jose Guilherme Vanz and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests piggyback on the old `connected` future. If the new
> connection to the master takes some time to setup, we might end
> up sending a call to the master before the connection is set up
> since the `connected` future is already set from the last
> invocation. While this succeeds now, but this might show more
> flakiness once MESOS-5359 is committed.
> 
> 
> Diffs
> -
> 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/scheduler_tests.cpp b1831052ef2ef4bc4f8f6ecabbb95f309378fa25 
> 
> Diff: https://reviews.apache.org/r/48935/diff/
> 
> 
> Testing
> ---
> 
> make check (gtest_repeat=1000)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

> On June 19, 2016, 11:56 p.m., Jose Guilherme Vanz wrote:
> > src/scheduler/constants.hpp, line 30
> > 
> >
> > Sorry my my fail, but what's "Nit" means? 
> > I suppose I should remove the extra line, right?

+1:  
http://stackoverflow.com/questions/27810522/what-does-nit-mean-in-hacker-speak


- Anand


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


On June 19, 2016, 7:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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




src/scheduler/constants.hpp (line 30)


Sorry my my fail, but what's "Nit" means? 
I suppose I should remove the extra line, right?


- Jose Guilherme Vanz


On June 19, 2016, 4:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48056: Windows: Added build and run instructions.

[Joseph Wu]

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



Possibly missing some steps related to OpenSSL.  Here's what I got while 
building the curl dependency (for example):

```
1>  -- Could NOT find OpenSSL, try to set the path to OpenSSL root folder in 
the system variable OPENSSL_ROOT_DIR (missing:  OPENSSL_LIBRARIES 
OPENSSL_INCLUDE_DIR)
1>  CMake Warning at CMakeLists.txt:462 (find_package):
1>By not providing "FindLibSSH2.cmake" in CMAKE_MODULE_PATH this project has
1>asked CMake to find a package configuration file provided by "LibSSH2", 
but
1>CMake did not find one.
1>
1>Could not find a package configuration file provided by "LibSSH2" with any
1>of the following names:
1>
1>  LibSSH2Config.cmake
1>  libssh2-config.cmake
1>
1>Add the installation prefix of "LibSSH2" to CMAKE_PREFIX_PATH or set
1>"LibSSH2_DIR" to a directory containing one of the above files.  If
1>"LibSSH2" provides a separate development package or SDK, be sure it has
1>been installed.
1>
1>
1>  CMake Error at CMake/OtherTests.cmake:86 (message):
1>Unable to link function recv
1>  Call Stack (most recent call first):
1>CMakeLists.txt:988 (include)
1>
1>
1>  -- Configuring incomplete, errors occurred!
```


docs/getting-started.md (line 207)


Add a note here:

This can be found by opening VS2015 and looking under the "tools" menu for 
"Visual Studio Command Prompt".



docs/getting-started.md (line 215)


s/-DENABLE_LIB_EVENT/-DENABLE_LIBEVENT/

Looks like we also need to specify:
`cmake -G "Visual Studio 14 2015 Win64" ..`



docs/getting-started.md (line 227)


Space at the beginning, period at the end.



docs/getting-started.md (line 230)


Minor nits (spacing, capitalization, period).


- Joseph Wu


On June 15, 2016, 11:19 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48056/
> ---
> 
> (Updated June 15, 2016, 11:19 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added build and run instructions.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 8c72e5ec1263adb8792c8c49ede50bbf649e1ef3 
> 
> Diff: https://reviews.apache.org/r/48056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 48918: Removed explicit authorization results in Mesos.

[Till Toenshoff]

> On June 19, 2016, 10:40 p.m., Alexander Rukletsov wrote:
> > src/slave/http.cpp, lines 1089-1093
> > 
> >
> > How about this one?

Nope, that one will not get invoked if the authorizer failed - `containers` 
will already return the failed future, `_containers` then never gets invoked.


- Till


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


On June 19, 2016, 3:02 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48918/
> ---
> 
> (Updated June 19, 2016, 3:02 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, 
> Benjamin Mahler, Greg Mann, and Kapil Arya.
> 
> 
> Bugs: MESOS-5637
> https://issues.apache.org/jira/browse/MESOS-5637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removing the explicit authorization results as formerly returned by
> some authorizer invocations in Mesos, the returned future falls
> through/back to the HTTP proxy handler of libprocess. This change
> allows us to reduce the amount of failure result handlers.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a6beb1721958a77886f0aa535346e2ff33bd5d04 
>   src/slave/http.cpp 77834111bf51b58ba5e12f262b725202f7d3a7bf 
> 
> Diff: https://reviews.apache.org/r/48918/diff/
> 
> 
> Testing
> ---
> 
> make check & functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

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


Fix it, then Ship it!




Thanks for your patience. This looks good!

Some other suggestions:
- Regarding the changes in tests, we can just resume the clock. This is much 
more simpler/intuitive.
- The newly introduced delay makes the tests run a lot slower sometimes ~1 sec! 
Let's default the delay to `0ms` for the tests.

I have included the changes from the above suggestions in this diff: 
https://gist.github.com/hatred/171deb74c3b28b13fc5e681fb697a64c

I would be happy to commit the changes once you post the updated diff from the 
gist + the other minor issues I raised below.


src/scheduler/constants.hpp (line 30)


Nit: Extra new line



src/scheduler/flags.hpp (line 49)


Nit: Extra new line



src/scheduler/scheduler.cpp (line 141)


Pass by const ref. `const Flags&`



src/scheduler/scheduler.cpp (line 307)


It is possible to compare a `Option` directly with `T`. You can just do 
`connectionId != _connectionId`.



src/scheduler/scheduler.cpp (line 461)


Nit: Kill this extra new line.



src/scheduler/scheduler.cpp (line 470)


Nit: new line before. We typically have a newline after a statement 
spanning multiple lines.



src/scheduler/scheduler.cpp (lines 747 - 751)


Kill this comment. It is no longer relevant. We are no longer loading from 
`local::Flags`.



src/scheduler/scheduler.cpp (line 765)


Nit: Kill this new line.


- Anand Mazumdar


On June 19, 2016, 7:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 44498: Forbid the executor to inherit from slave environment.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 19, 2016, 8:34 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44498/
> ---
> 
> (Updated June 19, 2016, 8:34 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Forbid the executor to inherit from slave environment.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 6e4632f784607bb0b969adafa2d710098a9fd1c7 
>   src/slave/containerizer/containerizer.hpp 
> ff78b4d0fd4a3b862f6019fc757c16b7367cd3cf 
>   src/slave/containerizer/containerizer.cpp 
> faa0c789dda8a6f36fdb6217b0bae270b6b8f2e2 
>   src/slave/containerizer/docker.hpp 311dca23ac17fb533866aba1de2b81d750bbe6df 
>   src/slave/containerizer/docker.cpp bd4f5930684208d532f4ad1721aae353d309c0a7 
>   src/slave/flags.cpp 44c217dd6d96bf28d1aaa1e7849c0328727fd8fb 
> 
> Diff: https://reviews.apache.org/r/44498/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48387]

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

- Mesos ReviewBot


On June 19, 2016, 7:08 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 19, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48932: Updated CHANGELOG to include note about '503' on authorizer failures.

[Alexander Rukletsov]

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




CHANGELOG (lines 160 - 161)


I would like us to mention two more things: the purpose of the change and 
what is being substituted. How about the following:

[MESOS-5637] - Authorized endpoints consistently return `503 (Service 
Unavailable)` instead of `500 (Internal Server Error)` when the authenticator 
or the authorizer fails to process the request.


- Alexander Rukletsov


On June 19, 2016, 9 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48932/
> ---
> 
> (Updated June 19, 2016, 9 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.
> 
> 
> Bugs: MESOS-5637
> https://issues.apache.org/jira/browse/MESOS-5637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   CHANGELOG aed56300f1090c816ab2893c20e87fd37230a9a7 
> 
> Diff: https://reviews.apache.org/r/48932/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 48920: Updated the HTTP result returned by failures of authn/authz.

[Alexander Rukletsov]

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




3rdparty/libprocess/src/process.cpp (line 3327)


As in r/48919/, let's not create an empty body.



3rdparty/libprocess/src/process.cpp (line 3387)


Ditto.


- Alexander Rukletsov


On June 19, 2016, 9:01 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48920/
> ---
> 
> (Updated June 19, 2016, 9:01 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.
> 
> 
> Bugs: MESOS-5637
> https://issues.apache.org/jira/browse/MESOS-5637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes authentication and authorization happening on the libprocess
> level to be in line with failures possibly returned by Mesos
> authorization as currently implemented by the HTTPProxy::process
> function. The HTTP result returned on failures has changed from
> InternalServerError (500) towards ServiceNotAvailable (503) and now
> contains a message describing the problem.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 703f673a98102958c5e2b0c1833efad2ddc53ef8 
> 
> Diff: https://reviews.apache.org/r/48920/diff/
> 
> 
> Testing
> ---
> 
> make check & functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 48935: Fixed flakiness in some of the tests using scheduler library.

[Anand Mazumdar]

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

Review request for mesos, Jose Guilherme Vanz and Vinod Kone.


Repository: mesos


Description
---

These tests piggyback on the old `connected` future. If the new
connection to the master takes some time to setup, we might end
up sending a call to the master before the connection is set up
since the `connected` future is already set from the last
invocation. While this succeeds now, but this might show more
flakiness once MESOS-5359 is committed.


Diffs
-

  src/tests/http_fault_tolerance_tests.cpp 
baa07395b9bd588daa5438369954584787d7952a 
  src/tests/scheduler_tests.cpp b1831052ef2ef4bc4f8f6ecabbb95f309378fa25 

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


Testing
---

make check (gtest_repeat=1000)


Thanks,

Anand Mazumdar

Review Request 48934: Removed redundant expectations for `connected`/`disconnected` callback.

[Anand Mazumdar]

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

Review request for mesos, Jose Guilherme Vanz and Vinod Kone.


Repository: mesos


Description
---

This is a follow up patch to `625b08` with the same reasoning i.e.,
the `master` object is now destroyed _after_ the scheduler
library thereby ensuring that there cannot be any pending callbacks
that can be triggered. Hence, we can get rid of these redundant
expectations.


Diffs
-

  src/tests/http_fault_tolerance_tests.cpp 
baa07395b9bd588daa5438369954584787d7952a 
  src/tests/scheduler_tests.cpp b1831052ef2ef4bc4f8f6ecabbb95f309378fa25 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 48919: Updated HTTPProxy to return a failure message in the HTTP result.

[Alexander Rukletsov]

> On June 19, 2016, 10:26 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 1217-1218
> > 
> >
> > In r/48918 you removed logging of the failed future, so now those 
> > failures are not captured in the log. I would strongly suggest to restore 
> > logging here. My preference would be `VLOG(3)`.
> 
> Till Toenshoff wrote:
> I would like to go with VLOG(1) for now as that is how libprocess' own 
> authorization / authentication already does it - so it is consistent with 
> that. Sounds good?

VLOG(1) is fine.


- Alexander


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


On June 19, 2016, 9:02 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48919/
> ---
> 
> (Updated June 19, 2016, 9:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.
> 
> 
> Bugs: MESOS-5637
> https://issues.apache.org/jira/browse/MESOS-5637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 703f673a98102958c5e2b0c1833efad2ddc53ef8 
> 
> Diff: https://reviews.apache.org/r/48919/diff/
> 
> 
> Testing
> ---
> 
> make check & functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 48918: Removed explicit authorization results in Mesos.

[Alexander Rukletsov]

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




src/slave/http.cpp 


Technically, this relates not only to authz failures, but to every future 
that may fail on the way.

I think it is fine to remove it here for now, because the log message does 
not really bring any value (assuming failure is logged by libprocess). However, 
we should come up with a "template" for HTTP request handlers, so that error 
handling is not chosen arbitrary. It would be great to have several stages 
(e.g., authn, authz, parsing, validation) and standardize on error codes 
returning in case of failures in each case. Could you please create a ticket 
for this?



src/slave/http.cpp (lines 1084 - 1088)


How about this one?


- Alexander Rukletsov


On June 19, 2016, 3:02 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48918/
> ---
> 
> (Updated June 19, 2016, 3:02 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, 
> Benjamin Mahler, Greg Mann, and Kapil Arya.
> 
> 
> Bugs: MESOS-5637
> https://issues.apache.org/jira/browse/MESOS-5637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removing the explicit authorization results as formerly returned by
> some authorizer invocations in Mesos, the returned future falls
> through/back to the HTTP proxy handler of libprocess. This change
> allows us to reduce the amount of failure result handlers.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a6beb1721958a77886f0aa535346e2ff33bd5d04 
>   src/slave/http.cpp 77834111bf51b58ba5e12f262b725202f7d3a7bf 
> 
> Diff: https://reviews.apache.org/r/48918/diff/
> 
> 
> Testing
> ---
> 
> make check & functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 48919: Updated HTTPProxy to return a failure message in the HTTP result.

[Till Toenshoff]

> On June 19, 2016, 10:26 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1218
> > 
> >
> > In case future is not failed, you create a response with an empty body, 
> > but with some headers ("Content-Length", "Content-Type") set. I believe 
> > this is not what you want. How about:
> > ```
> > Response response = future.isFailed() ? 
> >   ServiceUnavailable(future.failure() :
> >   ServiceUnavailable();
> >   
> > socket_manager->send(response, request, socket);
> > ```

Good catch - indeed. ty!


> On June 19, 2016, 10:26 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 1217-1218
> > 
> >
> > In r/48918 you removed logging of the failed future, so now those 
> > failures are not captured in the log. I would strongly suggest to restore 
> > logging here. My preference would be `VLOG(3)`.

I would like to go with VLOG(1) for now as that is how libprocess' own 
authorization / authentication already does it - so it is consistent with that. 
Sounds good?


- Till


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


On June 19, 2016, 9:02 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48919/
> ---
> 
> (Updated June 19, 2016, 9:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.
> 
> 
> Bugs: MESOS-5637
> https://issues.apache.org/jira/browse/MESOS-5637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 703f673a98102958c5e2b0c1833efad2ddc53ef8 
> 
> Diff: https://reviews.apache.org/r/48919/diff/
> 
> 
> Testing
> ---
> 
> make check & functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 48919: Updated HTTPProxy to return a failure message in the HTTP result.

[Alexander Rukletsov]

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




3rdparty/libprocess/src/process.cpp (lines 1217 - 1218)


In r/48918 you removed logging of the failed future, so now those failures 
are not captured in the log. I would strongly suggest to restore logging here. 
My preference would be `VLOG(3)`.



3rdparty/libprocess/src/process.cpp (line 1218)


In case future is not failed, you create a response with an empty body, but 
with some headers ("Content-Length", "Content-Type") set. I believe this is not 
what you want. How about:
```
Response response = future.isFailed() ? 
  ServiceUnavailable(future.failure() :
  ServiceUnavailable();
  
socket_manager->send(response, request, socket);
```



3rdparty/libprocess/src/process.cpp (lines 1220 - 1221)


We tend to insert a blank line in this case : ).


- Alexander Rukletsov


On June 19, 2016, 9:02 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48919/
> ---
> 
> (Updated June 19, 2016, 9:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.
> 
> 
> Bugs: MESOS-5637
> https://issues.apache.org/jira/browse/MESOS-5637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 703f673a98102958c5e2b0c1833efad2ddc53ef8 
> 
> Diff: https://reviews.apache.org/r/48919/diff/
> 
> 
> Testing
> ---
> 
> make check & functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 48094: Implemented GET_ROLES Call in v1 master API.

[Anand Mazumdar]

> On June 19, 2016, 9:27 p.m., Kevin Klues wrote:
> > src/tests/api_tests.cpp, lines 419-421
> > 
> >
> > It's not here in the review, but the final committed version of this 
> > patch contains these lines as:
> > ```
> > slave::Flags slaveFlags = CreateSlaveFlags();
> > slaveFlags.resources =
> >   "cpus(role1):0.5;mem(role1):512;ports(role1):[31000-31001];"
> >   "disk(role1):1024;gpus(role1):1";
> > ```
> > 
> > This breaks the unit tests on machines that *actually* contain GPUs. 
> > Was there a reason for adding the `gpus(role1):1` resource?

Nopes, we can easily get away with not setting the GPU resource but just 
defaulting it to `0`. Posted a patch to fix it: 
https://reviews.apache.org/r/48933/


- Anand


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


On June 18, 2016, 4:21 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48094/
> ---
> 
> (Updated June 18, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5494
> https://issues.apache.org/jira/browse/MESOS-5494
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_ROLES Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto fa92240 
>   include/mesos/mesos.proto e4c5bd3 
>   include/mesos/v1/master/master.proto 59e978f 
>   include/mesos/v1/mesos.proto 9be22f0 
>   src/master/http.cpp a6beb17 
>   src/master/master.hpp 618d928 
>   src/tests/api_tests.cpp afa5ffa 
> 
> Diff: https://reviews.apache.org/r/48094/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="*MasterAPITest.*" make -j2 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 48933: Fixed build on machines that contain GPU's.

[Kevin Klues]

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


Ship it!




Looks good:

Ran `GTEST_FILTER="*GetRoles*" make -j check`

on my GPU machine and it all passes now.

```
[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from ContentType/MasterAPITest
[ RUN  ] ContentType/MasterAPITest.GetRoles/0
[   OK ] ContentType/MasterAPITest.GetRoles/0 (249 ms)
[ RUN  ] ContentType/MasterAPITest.GetRoles/1
[   OK ] ContentType/MasterAPITest.GetRoles/1 (177 ms)
[--] 2 tests from ContentType/MasterAPITest (427 ms total)

[--] Global test environment tear-down
[==] 2 tests from 1 test case ran. (458 ms total)
[  PASSED  ] 2 tests.
```

- Kevin Klues


On June 19, 2016, 9:47 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48933/
> ---
> 
> (Updated June 19, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-5494
> https://issues.apache.org/jira/browse/MESOS-5494
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change modifies a test that was explicitly setting the GPU
> resource. This broke the build on machines that actually
> had a GPU device present.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 
> 
> Diff: https://reviews.apache.org/r/48933/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 48933: Fixed build on machines that contain GPU's.

[Anand Mazumdar]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

This change modifies a test that was explicitly setting the GPU
resource. This broke the build on machines that actually
had a GPU device present.


Diffs
-

  src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 48094: Implemented GET_ROLES Call in v1 master API.

[Kevin Klues]

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




src/tests/api_tests.cpp (lines 419 - 421)


It's not here in the review, but the final committed version of this patch 
contains these lines as:
```
slave::Flags slaveFlags = CreateSlaveFlags();
slaveFlags.resources =
  "cpus(role1):0.5;mem(role1):512;ports(role1):[31000-31001];"
  "disk(role1):1024;gpus(role1):1";
```

This breaks the unit tests on machines that *actually* contain GPUs. Was 
there a reason for adding the `gpus(role1):1` resource?


- Kevin Klues


On June 18, 2016, 4:21 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48094/
> ---
> 
> (Updated June 18, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5494
> https://issues.apache.org/jira/browse/MESOS-5494
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_ROLES Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto fa92240 
>   include/mesos/mesos.proto e4c5bd3 
>   include/mesos/v1/master/master.proto 59e978f 
>   include/mesos/v1/mesos.proto 9be22f0 
>   src/master/http.cpp a6beb17 
>   src/master/master.hpp 618d928 
>   src/tests/api_tests.cpp afa5ffa 
> 
> Diff: https://reviews.apache.org/r/48094/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="*MasterAPITest.*" make -j2 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 48915: Added an example framework for consuming GPUs.

[Kevin Klues]

> On June 19, 2016, 7:13 a.m., Guangya Liu wrote:
> > It is better re-implement your framework with v1 API, you can take a look 
> > at 
> > https://github.com/apache/mesos/blob/master/src/examples/long_lived_framework.cpp
> >  as an referene.

The `MesosSchedulerDriver` is implemented in terms of the v1 API, and covers 
alot more corner cases than the `long_lived_framework` does. I realize it's 
important to implement *some* example frameworks in terms of the new API, but 
it feels odd to me to have to implement all new framework in terms of the raw 
HTTP API since wrappers for C++ exist.


> On June 19, 2016, 7:13 a.m., Guangya Liu wrote:
> > src/examples/gpu_framework.cpp, line 51
> > 
> >
> > s/TestScheduler/GPUScheduler

Updated


- Kevin


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


On June 19, 2016, 9:01 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48915/
> ---
> 
> (Updated June 19, 2016, 9:01 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5649
> https://issues.apache.org/jira/browse/MESOS-5649
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This framework is designed to show how to build a GPU capable
> framework that can accept offers with GPUs and launch tasks that use
> them. The key thing to remember is that the GPU_RESOURCES capability
> must be set in `FrameworkInfo` in order for a framework to receive
> resource offers from agents that contain GPUs.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/examples/gpu_framework.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48915/diff/
> 
> 
> Testing
> ---
> 
> Run a master and an agent capable of handing out GPUs:
> ```
> $ sudo bin/mesos-master.sh --ip=127.0.0.1 --log_dir=/var/log/mesos 
> --work_dir=/var/lib/mesos
> $ sudo bin/mesos-agent.sh --master=127.0.0.1:5050 --ip=127.0.0.1 
> --log_dir=/var/log/mesos --work_dir=/var/lib/mesos
>   --isolation="cgroups/devices,gpu/nvidia"
> ```
> 
> Run a couple of instances of the framework and verify the correct output:
> ```
> $ ./src/gpu-framework --master=127.0.0.1:5050 --num_gpus=0
> $ ./src/gpu-framework --master=127.0.0.1:5050 --num_gpus=1
> $ ./src/gpu-framework --master=127.0.0.1:5050 --num_gpus=4
> $ ./src/gpu-framework --master=127.0.0.1:5050 --no-allow_gpus --num_gpus=1
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48915: Added an example framework for consuming GPUs.

[Kevin Klues]

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

(Updated June 19, 2016, 9:01 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

s/TestScheduler/GpuScheduler


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


Repository: mesos


Description
---

This framework is designed to show how to build a GPU capable
framework that can accept offers with GPUs and launch tasks that use
them. The key thing to remember is that the GPU_RESOURCES capability
must be set in `FrameworkInfo` in order for a framework to receive
resource offers from agents that contain GPUs.


Diffs (updated)
-

  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/examples/gpu_framework.cpp PRE-CREATION 

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


Testing
---

Run a master and an agent capable of handing out GPUs:
```
$ sudo bin/mesos-master.sh --ip=127.0.0.1 --log_dir=/var/log/mesos 
--work_dir=/var/lib/mesos
$ sudo bin/mesos-agent.sh --master=127.0.0.1:5050 --ip=127.0.0.1 
--log_dir=/var/log/mesos --work_dir=/var/lib/mesos
  --isolation="cgroups/devices,gpu/nvidia"
```

Run a couple of instances of the framework and verify the correct output:
```
$ ./src/gpu-framework --master=127.0.0.1:5050 --num_gpus=0
$ ./src/gpu-framework --master=127.0.0.1:5050 --num_gpus=1
$ ./src/gpu-framework --master=127.0.0.1:5050 --num_gpus=4
$ ./src/gpu-framework --master=127.0.0.1:5050 --no-allow_gpus --num_gpus=1
```


Thanks,

Kevin Klues

Review Request 48932: Updated CHANGELOG to include note about '503' on authorizer failures.

[Till Toenshoff]

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

Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, Benjamin Mahler, Greg Mann, and Kapil Arya.


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


Repository: mesos


Description
---

see summary.


Diffs
-

  CHANGELOG aed56300f1090c816ab2893c20e87fd37230a9a7 

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


Testing
---


Thanks,

Till Toenshoff

Re: Review Request 48268: Implemented SET_QUOTA Call in v1 master API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48268]

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

- Mesos ReviewBot


On June 19, 2016, 5:46 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48268/
> ---
> 
> (Updated June 19, 2016, 5:46 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5509
> https://issues.apache.org/jira/browse/MESOS-5509
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented SET_QUOTA Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master/master.proto 
> abcf628eea7922cdb323ec135ec48e9b11209608 
>   src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
>   src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
>   src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
>   src/master/quota_handler.cpp 567f1c2cc59b859227a8b48c6086ce3f8049f14d 
>   src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 
> 
> Diff: https://reviews.apache.org/r/48268/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo GTEST_FILTER="*MasterAPITest.SetQuota*" make -j4 check
> 
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from ContentType/MasterAPITest
> [ RUN  ] ContentType/MasterAPITest.SetQuota/0
> [   OK ] ContentType/MasterAPITest.SetQuota/0 (129 ms)
> [ RUN  ] ContentType/MasterAPITest.SetQuota/1
> [   OK ] ContentType/MasterAPITest.SetQuota/1 (98 ms)
> [--] 2 tests from ContentType/MasterAPITest (227 ms total)
> 
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (236 ms total)
> [  PASSED  ] 2 tests.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 48705: Implemented LIST_FILES Call in v1 agent API.

[Abhishek Dasgupta]

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

(Updated June 19, 2016, 8:38 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented LIST_FILES Call in v1 agent API.


Diffs (updated)
-

  include/mesos/v1/agent/agent.proto 5ac6d4c93a826f0e7a3654d7dcc02b16424f90ce 
  src/slave/http.cpp 77834111bf51b58ba5e12f262b725202f7d3a7bf 
  src/slave/slave.hpp 58ff2bfac6918d989ab36b67cf6ba2f3657c8356 

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


Testing (updated)
---


Thanks,

Abhishek Dasgupta

Review Request 48931: Implemented LIST_FILES Call in v1 master API.

[Abhishek Dasgupta]

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

Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented LIST_FILES Call in v1 master API.


Diffs
-

  include/mesos/v1/master/master.proto abcf628eea7922cdb323ec135ec48e9b11209608 
  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
  src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 

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


Testing
---

Ubuntu 16.04:
sudo GTEST_FILTER="*AgentAPITest.ListFiles*" make -j4 check
sudo GTEST_FILTER="*MasterAPITest.ListFiles*" make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 44498: Forbid the executor to inherit from slave environment.

[Gilbert Song]

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

(Updated June 19, 2016, 1:34 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


Repository: mesos


Description
---

Forbid the executor to inherit from slave environment.


Diffs (updated)
-

  docs/configuration.md 6e4632f784607bb0b969adafa2d710098a9fd1c7 
  src/slave/containerizer/containerizer.hpp 
ff78b4d0fd4a3b862f6019fc757c16b7367cd3cf 
  src/slave/containerizer/containerizer.cpp 
faa0c789dda8a6f36fdb6217b0bae270b6b8f2e2 
  src/slave/containerizer/docker.hpp 311dca23ac17fb533866aba1de2b81d750bbe6df 
  src/slave/containerizer/docker.cpp bd4f5930684208d532f4ad1721aae353d309c0a7 
  src/slave/flags.cpp 44c217dd6d96bf28d1aaa1e7849c0328727fd8fb 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48914: Added GPU_RESOURCES capability to FrameworkInfo.

[Kevin Klues]

> On June 19, 2016, 2:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1277-1279
> > 
> >
> > This logic seems also allow agents without gpu resources offered to the 
> > framework with gpuCapble, is it expected behavior?
> 
> Kevin Klues wrote:
> Yes, that was my intended behaviour as I wrote it. If we think there is 
> some problem with this, we should discuss it further.
> 
> Guangya Liu wrote:
> So what is the use of the agent without GPU for the framework which 
> request gpu resources?
> 
> Klaus Ma wrote:
> As far as I known, some GPU application can also run on CPU.

Not all tasks launched by a GPU capable framework will necessarily require GPU 
resources. Some will, some wont -- but at least this capability gives these 
frameworks a chance to be offered GPUs before non-GPU-capable frameworks starve 
them out.  In other words, this capability is more about making sure that 
frameworks that definitely *never* need GPU resources, don't fill up GPU 
capable machines with non-GPU workloads. It shouldn't restrict frameworks that 
have this capability from receiving their fair share of resources in the 
cluster.


> On June 19, 2016, 2:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1274
> > 
> >
> > s/slaves/agents
> 
> Kevin Klues wrote:
> I thought about this as I was writing it, but I left it as slave because 
> all of the other comments in the file use slave. I figured it was better to 
> stay consistent and wait for a full pass to change it than have one coment 
> refering to agent and the next referring to slave.
> 
> Guangya Liu wrote:
> I think that we should use `agent` but not `slave` for the new code as we 
> need finally update all `slave` to `agent`, you can take a look at 
> https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp
>  , all of the latest code are using `agent` but not `slave`

I'm still not convinced. In the case of adding a new test, I agree -- 
everything should be worded as `agent`. This is because you typically consume 
each test individually, and so long as the terminology within the test is 
self-consistent, everything looks good.

I just think its weird -- within the same function -- to have one comment say 
`slave`, the next say `agent`, and then another say `slave`, all within 10 
lines of each other.


- Kevin


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


On June 18, 2016, 10:07 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48914/
> ---
> 
> (Updated June 18, 2016, 10:07 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5634
> https://issues.apache.org/jira/browse/MESOS-5634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the scarce resource problem described in MESOS-5377, we are
> introducing a GPU_RESOURCES Framework capability. This capability
> allows the Mesos allocator to make better decisions about which
> frameworks should receive resources from GPU capable machines. In
> essence, the allocator ONLY allocate resources from GPU capable
> machines to frameworks that have this capability. This is necessary to
> prevent non-GPU workloads from filling up the GPU machines and
> preventing GPU workloads to run.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e4c5bd31cf035707036eb509336fe051119b4e78 
>   include/mesos/v1/mesos.proto 9be22f02861f1eb89ab547d88530faf90ebee7ab 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9c6b23abe2b0cb16412f1ed90165f8d0c14552fa 
>   src/master/allocator/mesos/hierarchical.cpp 
> 8b7b3afb5770c617918ec4864faaf8d8a7a81ef2 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> e06d107f2dcdb9b470e330c8ceee66a54220d41b 
> 
> Diff: https://reviews.apache.org/r/48914/diff/
> 
> 
> Testing
> ---
> 
> $ make -j check; sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 48928: Added info about external `libelf` dependence to `upgrades.md`.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler, haosdent huang, Vinod Kone, and 
Zhitao Li.


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


Repository: mesos


Description
---

Added info about external `libelf` dependence to `upgrades.md`.


Diffs
-

  docs/upgrades.md 61b7cd3ab45d09f3e4785e00bb8622421952d253 

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


Testing
---

Ran the site docker to verify everything looked OK.


Thanks,

Kevin Klues

Re: Review Request 45086: Enable cgroups unified isolator in isolation.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [45086, 46300, 43284, 45574, 45573, 45472, 45363, 46299, 
45362, 45352, 45087, 46158, 45085, 46043, 45084, 45083]

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

Error:
2016-06-19 19:24:48 URL:https://reviews.apache.org/r/45087/diff/raw/ 
[7151/7151] -> "45087.patch" [1]
error: patch failed: 
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp:148
error: src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp: patch does 
not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13894/console

- Mesos ReviewBot


On June 19, 2016, 4:50 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45086/
> ---
> 
> (Updated June 19, 2016, 4:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable cgroups unified isolator in isolation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d6a91113da322dc58e1d3dd3e03353631b6dafee 
> 
> Diff: https://reviews.apache.org/r/45086/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48705: Implemented LIST_FILES Call in v1 agent API.

[Abhishek Dasgupta]

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

(Updated June 19, 2016, 7:11 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented LIST_FILES Call in v1 agent API.


Diffs (updated)
-

  src/slave/http.cpp 77834111bf51b58ba5e12f262b725202f7d3a7bf 
  src/slave/slave.hpp 58ff2bfac6918d989ab36b67cf6ba2f3657c8356 
  src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 

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


Testing
---

Ubuntu 16.04:
sudo GTEST_FILTER="*AgentAPITest.ListFiles*" make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 19, 2016, 4:08 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

Reorder using statements


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


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs (updated)
-

  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
  src/tests/http_fault_tolerance_tests.cpp 
baa07395b9bd588daa5438369954584787d7952a 
  src/tests/master_maintenance_tests.cpp 
a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 

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


Testing
---

mesos-tests


Thanks,

Jose Guilherme Vanz

Re: Review Request 48921: Added a check of namespaces in Posix launcher.

[Avinash sridharan]

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




src/slave/containerizer/mesos/launcher.cpp (line 94)


Typo: 
Meant to ask "Is there a valid case where we would be calling `fork` with 
`namespaces = 0` ?


- Avinash sridharan


On June 19, 2016, 5:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48921/
> ---
> 
> (Updated June 19, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, 
> and Kapil Arya.
> 
> 
> Bugs: MESOS-5533
> https://issues.apache.org/jira/browse/MESOS-5533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Posix launcher does not support namespaces. We should reject the launch
> if namespaces are desired (e.g., from isolators). The currently behavior
> of silently ignoring it will cause weird issues.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> b1b323b4040fd72a7f01e60781eaf992e4ef766a 
> 
> Diff: https://reviews.apache.org/r/48921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 48921: Added a check of namespaces in Posix launcher.

[Avinash sridharan]

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




src/slave/containerizer/mesos/launcher.cpp (line 94)


Is there a valid case where we would be calling `fork` with `namespaces = ` 
? If not isn't checking namespaces.isSome() enough?


- Avinash sridharan


On June 19, 2016, 5:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48921/
> ---
> 
> (Updated June 19, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, 
> and Kapil Arya.
> 
> 
> Bugs: MESOS-5533
> https://issues.apache.org/jira/browse/MESOS-5533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Posix launcher does not support namespaces. We should reject the launch
> if namespaces are desired (e.g., from isolators). The currently behavior
> of silently ignoring it will cause weird issues.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> b1b323b4040fd72a7f01e60781eaf992e4ef766a 
> 
> Diff: https://reviews.apache.org/r/48921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 48927: Implemented DESTROY_VOLUMES Call in v1 master API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48926, 48927]

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

- Mesos ReviewBot


On June 19, 2016, 4:43 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48927/
> ---
> 
> (Updated June 19, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5502
> https://issues.apache.org/jira/browse/MESOS-5502
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented DESTROY_VOLUMES Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
>   src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
> 
> Diff: https://reviews.apache.org/r/48927/diff/
> 
> 
> Testing
> ---
> 
> Would follow up with a test case for both create/destroy volumes api soon.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 48268: Implemented SET_QUOTA Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 19, 2016, 5:46 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Implemented SET_QUOTA Call in v1 master API.


Diffs (updated)
-

  include/mesos/v1/master/master.proto abcf628eea7922cdb323ec135ec48e9b11209608 
  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
  src/master/quota_handler.cpp 567f1c2cc59b859227a8b48c6086ce3f8049f14d 
  src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 

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


Testing
---

On Ubuntu 16.04:
sudo GTEST_FILTER="*MasterAPITest.SetQuota*" make -j4 check

[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from ContentType/MasterAPITest
[ RUN  ] ContentType/MasterAPITest.SetQuota/0
[   OK ] ContentType/MasterAPITest.SetQuota/0 (129 ms)
[ RUN  ] ContentType/MasterAPITest.SetQuota/1
[   OK ] ContentType/MasterAPITest.SetQuota/1 (98 ms)
[--] 2 tests from ContentType/MasterAPITest (227 ms total)

[--] Global test environment tear-down
[==] 2 tests from 1 test case ran. (236 ms total)
[  PASSED  ] 2 tests.


Thanks,

Abhishek Dasgupta

Re: Review Request 48925: Implement GET_WEIGHTS Call in v1 master API.

[haosdent huang]

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



LGTM except the style issue @tillt mentioned.


src/master/weights_handler.cpp (lines 88 - 89)


Suggest to capture exactly here as well.
```
  return _getWeights(principal)
.then([contentType](const vector& weightInfos)
  -> Future {
```


- haosdent huang


On June 19, 2016, 12:01 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48925/
> ---
> 
> (Updated June 19, 2016, 12:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5495
> https://issues.apache.org/jira/browse/mesos-5495
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add getWeights method in weights_handler.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
>   src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
>   src/master/weights_handler.cpp 25075357c5ac8450d2fa1790844802f121908e9e 
>   src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 
> 
> Diff: https://reviews.apache.org/r/48925/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 48924: Extract public logic of collecting weights info into _getWeights.

[haosdent huang]

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


Fix it, then Ship it!





src/master/master.hpp (line 1078)


Should be `_filterWeights` here?



src/master/weights_handler.cpp (lines 66 - 68)


I suggest to capture exactly here.
```
  return _getWeights(principal)
.then([request](const vector& weightInfos)
  -> Future {
  RepeatedPtrField filteredWeightInfos;
```


- haosdent huang


On June 19, 2016, 12:01 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48924/
> ---
> 
> (Updated June 19, 2016, 12:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5495
> https://issues.apache.org/jira/browse/mesos-5495
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch refined the logic of collecting weights info in
> WeightsHandler. Extracted the public code into new _getWeights
> method so that the new operator API can reuse this method to
> collect weight info.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
>   src/master/weights_handler.cpp 25075357c5ac8450d2fa1790844802f121908e9e 
> 
> Diff: https://reviews.apache.org/r/48924/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 48773: Added docs on aufs provisioning backend.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48773]

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

- Mesos ReviewBot


On June 19, 2016, 2:10 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48773/
> ---
> 
> (Updated June 19, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Jie Yu.
> 
> 
> Bugs: MESOS-5549
> https://issues.apache.org/jira/browse/MESOS-5549
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs on aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 50a0fa799d7a179f3124e61619e28ffb3845d531 
>   src/slave/flags.cpp 69828ae43c2e5432aaa27cae3dc683cd90f10db9 
> 
> Diff: https://reviews.apache.org/r/48773/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[haosdent huang]

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


Fix it, then Ship it!




LGTM! Thank you for your patch!


src/tests/http_fault_tolerance_tests.cpp (line 63)


Move this above like
```
using mesos::v1::scheduler::Call;
using mesos::v1::scheduler::DEFAULT_CONNECTION_DELAY_MAX;
using mesos::v1::scheduler::Event;
```


- haosdent huang


On June 18, 2016, 5:07 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 18, 2016, 5:07 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 90b6c033054a09e9fbad9066e0763113a13a4d09 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 45086: Enable cgroups unified isolator in isolation.

[haosdent huang]

> On June 1, 2016, 3:09 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 265
> > 
> >
> > I think you need to handle `cgroups/devices/gpus/nvidia` which also 
> > starts with `cgroups` :-)
> 
> haosdent huang wrote:
> Yes, when I post the first version of this. The gpu have not been 
> submitted, I think it's time to add it now. :)
> 
> Jie Yu wrote:
> I think for GPU, the plan is to rename it to gpu/nvidia.
> 
> haosdent huang wrote:
> Does this mean we not need to consider gpu isolators when we implement 
> this?
> 
> Qian Zhang wrote:
> In https://reviews.apache.org/r/48478 , I see 
> `cgroups/devices/gpus/nvidia` isolator has been divided into 2 isolators: 
> `gpu/nvidia` and `cgroups/devices`, so I think we still need to handle 
> `cgruops/devices` which starts with `cgroups`.

Create a ticket here for `DevicesSubsystem` at 
https://issues.apache.org/jira/browse/MESOS-5651.


- haosdent


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


On June 19, 2016, 4:50 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45086/
> ---
> 
> (Updated June 19, 2016, 4:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable cgroups unified isolator in isolation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d6a91113da322dc58e1d3dd3e03353631b6dafee 
> 
> Diff: https://reviews.apache.org/r/45086/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45086: Enable cgroups unified isolator in isolation.

[haosdent huang]

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

(Updated June 19, 2016, 4:50 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, Kevin 
Klues, and Qian Zhang.


Changes
---

Address @qianzhang's comment.


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


Repository: mesos


Description
---

Enable cgroups unified isolator in isolation.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
d6a91113da322dc58e1d3dd3e03353631b6dafee 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45574: Add `PerfEventSubsystem` for cgroups unified isolator.

[haosdent huang]

> On June 11, 2016, 2:27 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp, lines 37-38
> > 
> >
> > So even after we introduce the unified cgroups isolator, we still need 
> > to keep the code of the original `cgroups/xxx` isolators? But I think it 
> > does not make sense since we have consolidated their code into the unified 
> > cgroups isolator, so ideally under 
> > `src/slave/containerizer/mesos/isolators/cgroups`, we will only have 
> > `cgroups.hpp/cpp` and `subsystem.hpp/cpp`, right?

Yes, those legacy files would be clean up, include `cgroups/perf_event.hpp`. 
Now I include it because I don't want to move or copy `PerfEventHandleManager` 
in this patch.


> On June 11, 2016, 2:27 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, lines 57-75
> > 
> >
> > In this method, I think you missed the part that we need to ensure no 
> > other subsystem is attached to the hierarchy, please check the the 
> > following code as a reference, and actually  
> > `CgroupsCpushareIsolatorProcess::create()`, 
> > `CgroupsNetClsIsolatorProcess::create()` also have the similar logic.
> > 
> > https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L96:L105

We not need check this because the purpose of this epic is to allow multiple 
subsystems attached to the same hierarchy.


- haosdent


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


On June 19, 2016, 10:32 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45574/
> ---
> 
> (Updated June 19, 2016, 10:32 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5047
> https://issues.apache.org/jira/browse/MESOS-5047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `PerfEventSubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp 674d5f886ea41b939a8e48832ee6595a78b2f6ce 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45574/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48927: Implemented DESTROY_VOLUMES Call in v1 master API.

[Shuai Lin]

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

(Updated June 19, 2016, 4:43 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Implemented DESTROY_VOLUMES Call in v1 master API.


Diffs
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 

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


Testing
---

Would follow up with a test case for both create/destroy volumes api soon.


Thanks,

Shuai Lin

Review Request 48927: Implemented DESTROY_VOLUMES Call in v1 master API.

[Shuai Lin]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Implemented DESTROY_VOLUMES Call in v1 master API.


Diffs
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 

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


Testing
---

Would follow up with a test case for both create/destroy volumes api soon.


Thanks,

Shuai Lin

Review Request 48926: Implemented CREATE_VOLUMES Call in v1 master API.

[Shuai Lin]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Implemented CREATE_VOLUMES Call in v1 master API.


Diffs
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 

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


Testing
---

Would follow up with a test case for both create/destroy volumes api soon.


Thanks,

Shuai Lin

Re: Review Request 45472: Add `NetClsSubsystem` for cgroups unified isolator.

[haosdent huang]

> On June 12, 2016, 1:48 a.m., Qian Zhang wrote:
> > I think in `NetClsSubsystem`, you still need a stub for the `update()` 
> > method, please check the following code:
> > https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp#L461:L465
> > 
> > If we do not have the stub for the `update()` method, then the TODO in the 
> > above code will be lost.

Good catch!


- haosdent


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


On June 19, 2016, 4:39 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45472/
> ---
> 
> (Updated June 19, 2016, 4:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5046
> https://issues.apache.org/jira/browse/MESOS-5046
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `NetClsSubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45363: Add `MemorySubsystem` for cgroups unified isolator.

[haosdent huang]

> On June 12, 2016, 1:09 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, lines 674-677
> > 
> >
> > I see here we call `lambda::bind()`, but in the original `cgroups/mem` 
> > isolator, we call defer():
> > 
> > https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L605:L609
> > 
> > That means `MemorySubsystem::oomWaited()` will be executed in a 
> > separate thread. So my concern is there may be some multi-thread race 
> > condition issue for your way, e.g., what if `MemorySubsystem::oomWaited()` 
> > is triggered after the container exits (i.e, we have cleaned it up)? I 
> > think in the original `cgroups/mem` isolator, we have already considered 
> > this issue, please refer to this comment for details:
> > 
> > https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L634:L637

For
```
// It is likely that process exited is executed before this
// function (e.g.  The kill and OOM events happen at the same
// time, and the process exit event arrives first.) Therefore, we
// should not report a fatal error here.
```
It is because eventfd would be triggered when hierarchy destruction. It should 
be fixed in https://reviews.apache.org/r/46299/


- haosdent


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


On June 19, 2016, 10:31 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45363/
> ---
> 
> (Updated June 19, 2016, 10:31 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5045
> https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `MemorySubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45363/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45363: Add `MemorySubsystem` for cgroups unified isolator.

[haosdent huang]

> On June 15, 2016, 8:18 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 505
> > 
> >
> > I can see two issues here:
> > 1. `updated` is a field in subsystem level (`MemorySubsystem` class) 
> > rather than container level (`Info` class), that means once we set it to 
> > `true` (e.g., when we update resources for the first container), it will be 
> > `true` for all the subsequent container, I do not think that is what we 
> > expect. Actually what we need is a flag in container level with which we 
> > can know whether it is the first time that we update resources for this 
> > container. I think that's the purpose of 
> > `CgroupsMemIsolatorProcess::Info::pid` in the original `cgroups/mem` 
> > isolator.
> > 2. How to recover this `updated` field when agent is restarted? I think 
> > when agent is restarted, this field will be always set back to `false` 
> > though it may be `true` before agent is restarted. Actually I see the same 
> > issue in the original `cgroups/mem` isolator, in that isolator, I do not 
> > see we recover `CgroupsMemIsolatorProcess::Info::pid` in 
> > `CgroupsMemIsolatorProcess::recover`. So it is a bug in the `cgroups/mem` 
> > isolator?
> > 
> > We probably need to follow the way of the `cgroups/mem` isolator (use 
> > `CgroupsMemIsolatorProcess::Info::pid` as the flag) and recover 
> > `CgroupsMemIsolatorProcess::Info::pid` during recovery (I think we can get 
> > pid from `ContainerState`).

For first issue, because different containers have different MemorySubsystem 
objects, so it would not be an issue.


- haosdent


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


On June 19, 2016, 10:31 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45363/
> ---
> 
> (Updated June 19, 2016, 10:31 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5045
> https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `MemorySubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45363/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 18, 2016, 4:38 a.m., Mesos ReviewBot wrote:
> > Patch looks great!
> > 
> > Reviews applied: [48387]
> > 
> > Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' 
> > COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 
> > MESOS_VERBOSE=1'; ./support/docker_build.sh

Finally! I've submitted the patch. Sorry for the delay. ;)


- Jose Guilherme


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


On June 18, 2016, 2:07 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 18, 2016, 2:07 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 90b6c033054a09e9fbad9066e0763113a13a4d09 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_maintenance_tests.cpp 
> a8649fdf3cbd4996d1528393506b1b0ed0a7cf5c 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> mesos-tests
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 45363: Add `MemorySubsystem` for cgroups unified isolator.

[haosdent huang]

> On June 11, 2016, 2:12 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, lines 650-656
> > 
> >
> > I see this method is different from the original one: 
> > https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L66:L69
> > 
> > Can you please let me know why you want to do this change?

I think it looks same?
```
static const vector levels()
{
  return {Level::LOW, Level::MEDIUM, Level::CRITICAL};
}
```
I move the scope to `MemorySubsystem` because we put all `XXXSubsystem` in 
`subsystem.cpp` currently. Define a global `levels()` looks not exactly.


- haosdent


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


On June 19, 2016, 10:31 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45363/
> ---
> 
> (Updated June 19, 2016, 10:31 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5045
> https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `MemorySubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45363/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[haosdent huang]

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

(Updated June 19, 2016, 3:55 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, Kevin 
Klues, and Qian Zhang.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Completed implementation of the cgroups unified isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45085: Added stubs for the cgroups unified isolator.

[haosdent huang]

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

(Updated June 19, 2016, 3:55 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, Kevin 
Klues, and Qian Zhang.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Added stubs for the cgroups unified isolator.


Diffs (updated)
-

  src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 
  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[haosdent huang]

> On June 15, 2016, 1:12 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 74
> > 
> >
> > Can you please clarfiy why we want to have `user` as a field of `Info` 
> > class? Basically, I do not think `Info` class should have such field.

I think a complete container info structure for cgroup should indicate which 
user it belongs to, so I add this before. Could remove it here.


> On June 15, 2016, 1:12 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 425
> > 
> >
> > Can you please clarify why we need the `pid` field in the `Info` class? 
> > This is the only place I see we assign value to `Info::pid`, but it seems 
> > we never use this field? Basically, I do not think we should have such 
> > field in `Info` class.

I think a complete container info structure for cgroup should contain the pid 
isolated, so I add this before. Could remove it here.


> On June 15, 2016, 1:12 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 511
> > 
> >
> > Can you please clarify why we need the `resources` field in the `Info` 
> > class? This is the only place I see we assign value to `Info::resources`, 
> > but it seems we never use this field? Basically, I do not think we should 
> > have such field in `Info` class.

I think a complete container info structure for cgroup should contain the 
resources allocated, so I add this before. Could remove it here.


- haosdent


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


On June 19, 2016, 3:34 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated June 19, 2016, 3:34 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[haosdent huang]

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

(Updated June 19, 2016, 3:34 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, Kevin 
Klues, and Qian Zhang.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Completed implementation of the cgroups unified isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[haosdent huang]

> On June 12, 2016, 2:51 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 351-363
> > 
> >
> > Why do we prepare hierarchy here? I think preparing hierarchy for a 
> > specific container is a low-level work which should be handled in 
> > `Subsystem::prepare()` method rather than here. And in that way, we only 
> > need one `for` loop (the following one) rather than 2 here.

We delegate the create/destory operations of hierarchy to 
`CgroupsIsolatorProcess`. Because if we do the create/destroy operations of 
hierarchy in `Subsystem::prepare()` and `Subsystem::cleanup()`, it would create 
the hierarchy or destroy the hierarchy multiple times when differnt subsystems 
mount in same hierarchies.


> On June 12, 2016, 2:51 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 428-445
> > 
> >
> > Again I think assigning container to its own cgroup should be handled 
> > in `Subsystem::isolate()` method. In that way, we only need one `for` loop 
> > (the following one) rather than 2 here.

Same reason as above, suppose we mount multiple subsystems into same hierarchy, 
isolate would be executed multiple times.


> On June 12, 2016, 2:51 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 592-600
> > 
> >
> > Again I think destroying container's cgroup should be handled in 
> > `Subsystem::cleanup()` method.

Same reason as above, suppose we mount multiple subsystems into same hierarchy, 
the hierarchy would be destroyed multiple times.


- haosdent


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


On June 19, 2016, 3:34 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated June 19, 2016, 3:34 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45085: Added stubs for the cgroups unified isolator.

[haosdent huang]

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

(Updated June 19, 2016, 3:34 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, Kevin 
Klues, and Qian Zhang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added stubs for the cgroups unified isolator.


Diffs (updated)
-

  src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 
  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45085: Added stubs for the cgroups unified isolator.

[haosdent huang]

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

(Updated June 19, 2016, 3:28 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, Kevin 
Klues, and Qian Zhang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added stubs for the cgroups unified isolator.


Diffs (updated)
-

  src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 
  src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48925: Implement GET_WEIGHTS Call in v1 master API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48924, 48925]

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

- Mesos ReviewBot


On June 19, 2016, 12:01 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48925/
> ---
> 
> (Updated June 19, 2016, 12:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5495
> https://issues.apache.org/jira/browse/mesos-5495
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add getWeights method in weights_handler.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
>   src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
>   src/master/weights_handler.cpp 25075357c5ac8450d2fa1790844802f121908e9e 
>   src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 
> 
> Diff: https://reviews.apache.org/r/48925/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[haosdent huang]

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

(Updated June 19, 2016, 2:55 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, Kevin 
Klues, and Qian Zhang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Completed implementation of the cgroups unified isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48773: Added docs on aufs provisioning backend.

[Shuai Lin]

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

(Updated June 19, 2016, 2:10 p.m.)


Review request for mesos, Guangya Liu and Jie Yu.


Changes
---

Adress review comments.


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


Repository: mesos


Description
---

Added docs on aufs provisioning backend.


Diffs (updated)
-

  docs/container-image.md 50a0fa799d7a179f3124e61619e28ffb3845d531 
  src/slave/flags.cpp 69828ae43c2e5432aaa27cae3dc683cd90f10db9 

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


Testing
---


Thanks,

Shuai Lin

Re: Review Request 48773: Added docs on aufs provisioning backend.

[Shuai Lin]

> On June 16, 2016, 6:22 a.m., Guangya Liu wrote:
> > docs/container-image.md, line 305
> > 
> >
> > Community link here?

Emm, I don't think it's necessary. The paragraph is copied from the docker aufs 
driver documents.


> On June 16, 2016, 6:22 a.m., Guangya Liu wrote:
> > docs/container-image.md, line 307
> > 
> >
> > Can you clarify which linux distribution do not support AUFS instead of 
> > "some Linux distributions"?

The same as above.


- Shuai


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


On June 16, 2016, 5:55 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48773/
> ---
> 
> (Updated June 16, 2016, 5:55 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Jie Yu.
> 
> 
> Bugs: MESOS-5549
> https://issues.apache.org/jira/browse/MESOS-5549
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs on aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 50a0fa799d7a179f3124e61619e28ffb3845d531 
>   src/slave/flags.cpp ce2aa336f779ee6746d5f62136251af2c7191f9d 
> 
> Diff: https://reviews.apache.org/r/48773/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 48923: Updated test to expect ServiceUnavailable.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48918, 48919, 48920, 48923]

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

- Mesos ReviewBot


On June 19, 2016, 11:37 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48923/
> ---
> 
> (Updated June 19, 2016, 11:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, 
> Benjamin Mahler, Greg Mann, and Kapil Arya.
> 
> 
> Bugs: MESOS-5637
> https://issues.apache.org/jira/browse/MESOS-5637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> StatisticsEndpointGetResourceUsageFailed was expecting an
> InternalServerError but instead it will now receive a
> ServiceUnavailable.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp ad36b74c12e52435b356bced1d15bd33983949e0 
> 
> Diff: https://reviews.apache.org/r/48923/diff/
> 
> 
> Testing
> ---
> 
> make check & functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 48925: Implement GET_WEIGHTS Call in v1 master API.

[Till Toenshoff]

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



Quick drive-by style review...


src/master/weights_handler.cpp (lines 94 - 95)


Remove the whitespaces here please - then, to make it fit into 80 chars, 
break at the argument.

```
response.mutable_get_weights()->add_weight_infos()->CopyFrom(
weightInfo);
```


- Till Toenshoff


On June 19, 2016, 12:01 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48925/
> ---
> 
> (Updated June 19, 2016, 12:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5495
> https://issues.apache.org/jira/browse/mesos-5495
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add getWeights method in weights_handler.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
>   src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
>   src/master/weights_handler.cpp 25075357c5ac8450d2fa1790844802f121908e9e 
>   src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 
> 
> Diff: https://reviews.apache.org/r/48925/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 45086: Enable cgroups unified isolator in isolation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45083, 45084, 46043, 45085, 46158, 45087, 45352, 45362, 
46299, 45363, 45472, 45573, 45574, 43284, 46300, 45086]

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

- Mesos ReviewBot


On June 19, 2016, 10:39 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45086/
> ---
> 
> (Updated June 19, 2016, 10:39 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable cgroups unified isolator in isolation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d6a91113da322dc58e1d3dd3e03353631b6dafee 
> 
> Diff: https://reviews.apache.org/r/45086/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 48925: Implement GET_WEIGHTS Call in v1 master API.

[zhou xing]

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

Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Bugs: mesos-5495
https://issues.apache.org/jira/browse/mesos-5495


Repository: mesos


Description
---

Add getWeights method in weights_handler.


Diffs
-

  src/master/http.cpp 148324e2c9d52f8211df4cead783e5a2979a45fe 
  src/master/master.hpp 50230397dcf52219fd4ed98dd1107bf53790478d 
  src/master/weights_handler.cpp 25075357c5ac8450d2fa1790844802f121908e9e 
  src/tests/api_tests.cpp 5d10533858cf1c512e00dbe6d5fab1f6401687bf 

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


Testing
---

make
make check


Thanks,

zhou xing