Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43816, 43822, 43823, 43817, 43910, 43911, 44047]

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

- Mesos ReviewBot


On Feb. 25, 2016, 10:59 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44047/
> ---
> 
> (Updated Feb. 25, 2016, 10:59 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-4667
> https://issues.apache.org/jira/browse/MESOS-4667
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows operators to list all the dynamic reservations and persistent
> volumes in a cluster. This is important in itself; it also makes it easier to
> use the `/unreserve` and `/destroy-volumes` endpoints.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 2a794a572ff930aa1f95706b89fef9243be627de 
>   docs/reservation.md b98ebe6df0739b48c5fa58e087fd64b1c6c5d456 
>   src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 6069ca1e9ed278459c5182e438417e95955b1924 
> 
> Diff: https://reviews.apache.org/r/44047/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 44058: Add metrics for {RESERVE, UNRESERVE} and {CREATE, DESTROY} offer operation

[fan du]

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

Review request for mesos, Greg Mann, haosdent huang, and Jie Yu.


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


Repository: mesos


Description
---

Add metrics for {RESERVE, UNRESERVE} and {CREATE, DESTROY} offer operation


Diffs
-

  src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 

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


Testing
---

1. make check on Centos-7 (3.10.0-123.el7.x86_640
2. Verify its functionality with 'reserve' http endpoint as an test case

# curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
grep reserve
"master/messages_reserve_resource": 0.0,
"master/messages_unreserve_resource": 0.0,


# curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ { 
"name": "cpus", "type": "SCALAR","scalar": { "value": 1 
},"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
ipdc02-kvm-guest2:5050/master/reserve
HTTP/1.1 200 OK
Date: Fri, 26 Feb 2016 19:59:01 GMT
Content-Length: 0

# curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
grep reserve
"master/messages_reserve_resource": 1.0,
"master/messages_unreserve_resource": 0.0,


Thanks,

fan du

Re: Review Request 44045: Style fixes to `stout/include/Makefile.am`.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44045, 42036]

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

Error:
2016-02-26 06:45:46 URL:https://reviews.apache.org/r/42036/diff/raw/ [570/570] 
-> "42036.patch" [1]
error: patch failed: src/CMakeLists.txt:197
error: src/CMakeLists.txt: patch does not apply

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

- Mesos ReviewBot


On Feb. 25, 2016, 10:28 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44045/
> ---
> 
> (Updated Feb. 25, 2016, 10:28 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Style fixes to `stout/include/Makefile.am`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 03eff5a831283f6d298e9a1feecfdc7369cacfe7 
> 
> Diff: https://reviews.apache.org/r/44045/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44044: CMake: src CMakeLists-Added source to build master.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43997, 44001, 44003, 44005, 44007, 44008, 44044]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' 
CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker_build.sh 2>&1 | tee build_44044"]

Error:
+ : ubuntu:14.04
+ : gcc
+ : --verbose
+ : GLOG_v=1 MESOS_VERBOSE=1
+++ dirname ./support/docker_build.sh
++ cd ./support/..
++ pwd
+ MESOS_DIRECTORY=/home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ cd /home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ DOCKERFILE=Dockerfile
+ rm -f Dockerfile
+ case $OS in
+ append_dockerfile 'FROM ubuntu:14.04'
+ echo FROM ubuntu:14.04
+ append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
+ echo RUN rm -rf 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_main_binary-amd64_Packages 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_main_binary-i386_Packages 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_Release 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_Release.gpg 
/var/lib/apt/lists/lock 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_main_binary-amd64_Packages
 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_main_binary-i386_Packages
 /var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_Release 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_Release.gpg 
/var/lib/apt/lists/partial 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_binary-amd64_Packages
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_binary-i386_Packages
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_i18n_Translation-en
 /va
 
r/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_source_Sources
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_Release
 /var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_InRelease 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_source_Sources
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_i18n_Translation-en
 /var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiver
 se_source_Sources 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_source_Sources
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_InRelease
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_binary-amd64_Packages
 /var/lib/apt/lists/u
 s.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_binary-i386_Packages 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_i18n_Translation-en
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_binary-i386_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_i18n_Translation-en
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_binary-i386_Packages
 

Re: Review Request 43407: CMake: Force GMock and libevent to build and link statically.

[Alex Clemmer]

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

(Updated Feb. 26, 2016, 5:59 a.m.)


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


Repository: mesos


Description
---

CMake: Force GMock to build and link statically.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
9b61376ea6aad304607c20c9823d9ef19013eca0 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 43416: Windows: Removed ambiguous call to `::write`.

[Alex Clemmer]

> On Feb. 25, 2016, 12:22 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp, line 52
> > 
> >
> > Why is the cast to `size_t` needed here?
> 
> Alex Clemmer wrote:
> It is required, and (though it has been awhile) I believe it's because on 
> 64-bit machines, without this type information, 1 could be one of several 
> integral types. The cast disambiguates.
> 
> Michael Park wrote:
> I only see one definition of `::write` in 
> `3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp`, which has the 
> signature: `inline auto write(int fd, const void* buffer, size_t count)`.
> 
> Doesn't seem like there should be any ambiguity here, are there other 
> defintions of `write` that we're contending with? Because `1` is clearly 
> convertible to `size_t`.

There is also `::write` in the Windows implementation of the C Runtime. To 
confirm that this is the problem, we can delete the `inline auto write` you 
mention, and the compile problem disappears. (I just confirmed this is true.)

So, practically speaking, there seem to be two choices:

(1) Delete the `::write` in windows.hpp.
(2) Add a cast here.

Note that if we choose (1) we should probably then fork every file where where 
we call `::write` in the code, and replace those calls with `::_write`, as the 
former is deprecated.


- Alex


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


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43416/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Removed ambiguous call to `::write`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp 
> 88b355e09f76f0412c74ad69556572f0079deb8f 
> 
> Diff: https://reviews.apache.org/r/43416/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43777: Removed unnecessary parameter from validation function.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On Feb. 24, 2016, 6:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43777/
> ---
> 
> (Updated Feb. 24, 2016, 6:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
> https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unnecessary parameter from validation function.
> 
> Now that Reserve operations are authorized for particular roles, it is 
> unnecessary to pass the framework's role into the validation function for 
> Reserve operations. The function previously ensured that a framework could 
> only reserve resources for its own role, but this check has been removed. 
> Since this check has been removed, the test 
> `ReserveOperationValidationTest.NonMatchingRole` is no longer needed and has 
> also been removed.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/master/validation.hpp 6ec883e7e84162b648c2f04583cc776948b36c0c 
>   src/master/validation.cpp b0cc7f7ec75b66246686d1b50a61902f1455e8b6 
>   src/tests/master_validation_tests.cpp 
> ab2df22f73052f6bd77653e56e7b460b17e7b0be 
> 
> Diff: https://reviews.apache.org/r/43777/diff/
> 
> 
> Testing
> ---
> 
> This patch removes the test `ReserveOperationValidationTest.NonMatchingRole`, 
> as the condition it was testing for is no longer relevant.
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl 
> && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44042: Windows: Fixed `UUID::random` and added utime/touch to stout/os.hpp.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44042, 41632, 40620, 40115, 40939, 40938, 40937, 40936, 
40718, 40676, 40340, 40195, 40131]

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

Error:
2016-02-26 05:45:55 URL:https://reviews.apache.org/r/40340/diff/raw/ 
[14026/14026] -> "40340.patch" [1]
error: .gitignore-template: does not exist in index
error: patch failed: support/mesos-style.py:8
error: support/mesos-style.py: patch does not apply

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

- Mesos ReviewBot


On Feb. 25, 2016, 9:16 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44042/
> ---
> 
> (Updated Feb. 25, 2016, 9:16 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Fixed `UUID::random` and added utime/touch to stout/os.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> f1d38029eef7e89dfdb8915158fba17865e6855b 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 84a2a021859d4e5c8547ad2a509eebda428a8255 
>   3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp 
> 625636525ee0fb35214cc3df3a304c40c9a0b0a6 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> d3ba0b4ff5d2d125252389a0f8f618c78aa6d948 
> 
> Diff: https://reviews.apache.org/r/44042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43776: Changed object of `ReserveResources` ACL to `roles`.

[Neil Conway]

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




src/master/master.cpp (line 2857)


I might add a note here to remind the reader that authorization only 
succeeds if the principal is allowed to make reservations for the roles 
included in the resources.



src/master/master.cpp (line 2858)


"an element"



src/master/master.cpp (line 2859)


Is there a reason to prefer `std::set` over `hashset`? I would typically 
use `hashset` unless we care about ordered iteration.



src/tests/master_validation_tests.cpp (line 236)


I'd rephrase this comment slightly: "even if the `role`". i.e., as written, 
it seems to imply that resource-role != framework-role is sufficient for the 
request to validate successfully.


- Neil Conway


On Feb. 24, 2016, 6:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43776/
> ---
> 
> (Updated Feb. 24, 2016, 6:39 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
> https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed object of the `ReserveResources` ACL to `roles`.
> 
> This solves a problem in which any principal could reserve resources for any 
> role using the '/reserve' operator endpoint. A new test, 
> `ReserveOperationValidationTest.DisallowReserveForStarRole`, was added.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
>   src/authorizer/local/authorizer.cpp 
> 9557bbdf68ff182c4538bbf70cee576d717abc05 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/master/validation.cpp b0cc7f7ec75b66246686d1b50a61902f1455e8b6 
>   src/tests/authorization_tests.cpp 9d046e8d53cbb6c065a23ca3f7832021ec7faadc 
>   src/tests/master_validation_tests.cpp 
> ab2df22f73052f6bd77653e56e7b460b17e7b0be 
>   src/tests/reservation_endpoints_tests.cpp 
> 32b2af4115211b58a5127a14dd19152c2eca120c 
>   src/tests/reservation_tests.cpp b8878d51767ac0d95e346c44c0a4d5c060e565ef 
> 
> Diff: https://reviews.apache.org/r/43776/diff/
> 
> 
> Testing
> ---
> 
> Tests were altered to accomodate the new ACL object, and the test 
> `ReserveOperationValidationTest.DisallowReserveForStarRole` was added.
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl 
> && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43778: Added '/create-volumes' tests with multiple roles.

[Neil Conway]

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




src/tests/persistent_volume_endpoints_tests.cpp (line 839)


`const`



src/tests/persistent_volume_endpoints_tests.cpp (line 874)


Unused variable.



src/tests/persistent_volume_endpoints_tests.cpp (line 1066)


Unused variable.



src/tests/persistent_volume_endpoints_tests.cpp (line 1082)


`response` seems a bit more concise.


- Neil Conway


On Feb. 24, 2016, 6:42 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43778/
> ---
> 
> (Updated Feb. 24, 2016, 6:42 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
> https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '/create-volumes' tests with multiple roles.
> 
> Operators may create volumes for multiple roles in the same operation; this 
> patch adds tests to confirm correct behavior of authorization in this case. 
> The tests `ReservationEndpointsTest.GoodReserveACLMultipleRoles` and 
> `ReservationEndpointsTest.BadReserveACLMultipleRoles` were added.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 6069ca1e9ed278459c5182e438417e95955b1924 
> 
> Diff: https://reviews.apache.org/r/43778/diff/
> 
> 
> Testing
> ---
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl 
> && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43779: Added '/reserve' tests with multiple roles.

[Neil Conway]

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




src/tests/reservation_endpoints_tests.cpp (line 1069)


I'd inline this into the lone use-site.



src/tests/reservation_endpoints_tests.cpp (line 1228)


Mark this and the following string `const`.



src/tests/reservation_endpoints_tests.cpp (line 1270)


I'd omit the variable and just call `createBasicAuthHeaders` in the args to 
`post()`.



src/tests/reservation_endpoints_tests.cpp (line 1273)


The analogous test for volumes names these variables differently 
(`volume1`, `volume2`, and `volumes`). We should be consistent


- Neil Conway


On Feb. 26, 2016, 5:31 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43779/
> ---
> 
> (Updated Feb. 26, 2016, 5:31 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
> https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '/reserve' tests with multiple roles.
> 
> Operators may reserve resources for multiple roles in the same operation; 
> this patch adds tests to confirm correct behavior of authorization in this 
> case. The tests `ReservationEndpointsTest.GoodReserveACLMultipleRoles` and 
> `ReservationEndpointsTest.BadReserveACLMultipleRoles` were added.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> 32b2af4115211b58a5127a14dd19152c2eca120c 
> 
> Diff: https://reviews.apache.org/r/43779/diff/
> 
> 
> Testing
> ---
> 
> Ran `configure && make check` and `configure --enable-libevent --enable-ssl 
> && make check` on OSX; all tests passed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43800: Updated docs for reservation, volumes, and authZ.

[Neil Conway]

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



This needs a prominent note in `upgrades.md` about the change to the ACL 
format. Thinking about it, the ACL isn't stored anywhere, so there's no issue 
with incompatibility of stored state. Similarly, rolling upgrades should be 
okay -- a mixed cluster would behave in a strange way in the event of master 
failover, but that's probably to be expected.


docs/authorization.md (line 37)


Not yours, but I feel like we need a better way to organize this 
information. Maybe a table/matrix, showing the action X is used with subjects Y 
and objects Z?



docs/persistent-volume.md (line 39)


I'd say "appropriate" rather than "desired".



docs/reservation.md (line 54)


I'd say "appropriate" rather than "desired".


- Neil Conway


On Feb. 25, 2016, 5:45 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43800/
> ---
> 
> (Updated Feb. 25, 2016, 5:45 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
> https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docs for reservation, volumes, and authZ.
> 
> This updates the authorization documentation to include the new `roles` 
> object for the `CreateVolume` and `ReserveResources` ACLs. The docs for 
> persistent volumes and dynamic reservations are also updated to reflect the 
> new authorization behavior.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md bbb4f2adc9348cb1686e6af78f5604d8cf7651ab 
>   docs/persistent-volume.md 2a794a572ff930aa1f95706b89fef9243be627de 
>   docs/reservation.md b98ebe6df0739b48c5fa58e087fd64b1c6c5d456 
> 
> Diff: https://reviews.apache.org/r/43800/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43855: Added Appc fetcher support to store.

[Jie Yu]

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




src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 254)


Why we need this parameter? What does that mean? To me, it's clear that 
'fetchImage' just take 'appc'.



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 267)


Why do you need to call validate? Any image in the store should already 
been validated.

You do have to check if the image id exists in the store or not 
(cache->get(...)?)



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 278)


I would put this to the end, so the overall logic of this method should be 
look like the following:

1) see if the image id in the cache or not
2) if not, fetch it using fetcher
3) once fetch is done (i.e., moved to the store), removed the staging dir
4) fetch dependencies, get a vector of image ids
5) merge the ids get from (4) and merge it with the image id get from 
either 1) or 3)



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 298 - 323)


Any reason you need three lambdas? Looks like they are synchronous 
functions.


- Jie Yu


On Feb. 26, 2016, 4:07 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43855/
> ---
> 
> (Updated Feb. 26, 2016, 4:07 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change allows store to fetch an image using Appc image fetcher when an
> image is not found in the cache. It also recursively fetches the dependencies
> for the image.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 4b3829175f57fb9aea2478040d96f2f127cbc551 
>   src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e 
> 
> Diff: https://reviews.apache.org/r/43855/diff/
> 
> 
> Testing
> ---
> 
> make check; local image server.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 43779: Added '/reserve' tests with multiple roles.

[Greg Mann]

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

(Updated Feb. 26, 2016, 5:31 a.m.)


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


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


Repository: mesos


Description
---

Added '/reserve' tests with multiple roles.

Operators may reserve resources for multiple roles in the same operation; this 
patch adds tests to confirm correct behavior of authorization in this case. The 
tests `ReservationEndpointsTest.GoodReserveACLMultipleRoles` and 
`ReservationEndpointsTest.BadReserveACLMultipleRoles` were added.


Diffs (updated)
-

  src/tests/reservation_endpoints_tests.cpp 
32b2af4115211b58a5127a14dd19152c2eca120c 

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


Testing
---

Ran `configure && make check` and `configure --enable-libevent --enable-ssl && 
make check` on OSX; all tests passed.


Thanks,

Greg Mann

Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43908, 43907, 43906]

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

Error:
2016-02-26 05:06:40 URL:https://reviews.apache.org/r/43906/diff/raw/ 
[3197/3197] -> "43906.patch" [1]
error: patch failed: src/CMakeLists.txt:223
error: src/CMakeLists.txt: patch does not apply

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

- Mesos ReviewBot


On Feb. 25, 2016, 7:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43908/
> ---
> 
> (Updated Feb. 25, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Added significant test coverage of `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 756aa29ed134bf10a645af7f6562f86dc8e488f5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> d0592ef8a774d380e9df66b7e623eb72b29a28b3 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 
> 
> Diff: https://reviews.apache.org/r/43908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43826: Added 'Synchronized Statement in Mesos' blog post.

[Neil Conway]

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




site/source/blog/2016-02-16-synchronized-statements-in-mesos.md (line 3)


in Mesos, or in libprocess?


- Neil Conway


On Feb. 22, 2016, 7:48 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43826/
> ---
> 
> (Updated Feb. 22, 2016, 7:48 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joerg Schad, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   site/source/blog/2016-02-16-synchronized-statements-in-mesos.md 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43826/diff/
> 
> 
> Testing
> ---
> 
> Locally rendered with `rake && rake dev` from `/site`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43855: Added Appc fetcher support to store.

[Jojy Varghese]

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

(Updated Feb. 26, 2016, 4:07 a.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


Repository: mesos


Description
---

This change allows store to fetch an image using Appc image fetcher when an
image is not found in the cache. It also recursively fetches the dependencies
for the image.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
4b3829175f57fb9aea2478040d96f2f127cbc551 
  src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e 

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


Testing
---

make check; local image server.


Thanks,

Jojy Varghese

Re: Review Request 43905: Windows: Removed `user` launcher flag, preventing `su`.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43905, 43904, 43903, 43709, 43708, 43707, 43699, 43698, 
43697, 43700, 43689, 43695, 43694, 43693, 43692, 43691, 42579, 43418, 43417, 
43416, 43414, 43413, 43415, 43411, 43410, 43409, 43407, 40340, 40195, 40131]

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

Error:
2016-02-26 03:50:16 URL:https://reviews.apache.org/r/40340/diff/raw/ 
[14026/14026] -> "40340.patch" [1]
error: .gitignore-template: does not exist in index
error: patch failed: support/mesos-style.py:8
error: support/mesos-style.py: patch does not apply

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

- Mesos ReviewBot


On Feb. 25, 2016, 7:17 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43905/
> ---
> 
> (Updated Feb. 25, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `su` does not exist on Windows. Unfortunately, the launcher also depends
> on it. In this commit, we remove Windows support for the launcher flag
> `user`, which controls whether we use `su` in the launcher. This
> allows us to divest ourselves of `su` altogether on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> d917a99a46841156dc1e359c44010938cc45e943 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 129406abdff715e321f683911e404c46676b6daf 
>   src/slave/containerizer/mesos/launch.hpp 
> 7e29ca2b8bec1c20aef122472cff60f6003603ad 
>   src/slave/containerizer/mesos/launch.cpp 
> 6b3bf163e2a577e6318a4a62f96d6bfd98ef9ae9 
> 
> Diff: https://reviews.apache.org/r/43905/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43920: Added a helper function to stout : os/which.hpp.

[Disha Singh]

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

(Updated Feb. 26, 2016, 2:27 a.m.)


Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Added a helper function to stout : os/which.hpp.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp PRE-CREATION 

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


Testing
---


Thanks,

Disha  Singh

Re: Review Request 43999: Use relative path to create libraries symbolic link.

[Zhiwei Chen]

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

(Updated Feb. 26, 2016, 9:59 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Use relative path to create libraries symbolic link.


Diffs
-

  src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5 

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


Testing (updated)
---

After make install, the library links will be relative path.


Thanks,

Zhiwei Chen

Re: Review Request 44026: Moved future tests into future_tests.cpp.

[Anand Mazumdar]

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


Fix it, then Ship it!




LGTM, Just one minor query around moving one more additional test.

Thanks for resolving this tech debt.


3rdparty/libprocess/src/tests/process_tests.cpp (lines 1300 - 1312)


Should this be moved to `future_tests.cpp` too?


- Anand Mazumdar


On Feb. 25, 2016, 5:59 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44026/
> ---
> 
> (Updated Feb. 25, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved future tests into future_tests.cpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> ded1aaed25876d45e33b4a27fffc6a5c46ca92f5 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e9bf80ee69f4add299cb828ed3245ac07398943c 
> 
> Diff: https://reviews.apache.org/r/44026/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44029: Fetcher::basename should ignore query strings and fragments.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44029]

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

- Mesos ReviewBot


On Feb. 25, 2016, 6:32 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44029/
> ---
> 
> (Updated Feb. 25, 2016, 6:32 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4779
> https://issues.apache.org/jira/browse/MESOS-4779
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignore URL query parameters and fragments when determining this
> base name. This enables the fetcher to subsequently examine the
> file extension and extract archives correctly.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/44029/diff/
> 
> 
> Testing
> ---
> 
> ``make check`` on OS X.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44004: Add 'name' field into NetworkInfo.

[Avinash sridharan]

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




include/mesos/mesos.proto (line 1577)


In order to expose this field in state.json, I think you need to add the 
filed to the corresponding `model` function in src/common/http.cpp as well. 

We won't need this once we move to the new JSONIFY api, but I think we 
would need it in the immediate future.


- Avinash sridharan


On Feb. 25, 2016, 3:15 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44004/
> ---
> 
> (Updated Feb. 25, 2016, 3:15 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-4758
> https://issues.apache.org/jira/browse/MESOS-4758
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'name' field into NetworkInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 33f6b0838360b61db70a247e5d6dfb16af15aa06 
>   include/mesos/v1/mesos.proto 1b0e709e76f3f6b44ab0434c649c064e8866c8a1 
> 
> Diff: https://reviews.apache.org/r/44004/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 43416: Windows: Removed ambiguous call to `::write`.

[Michael Park]

> On Feb. 25, 2016, 12:22 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp, line 52
> > 
> >
> > Why is the cast to `size_t` needed here?
> 
> Alex Clemmer wrote:
> It is required, and (though it has been awhile) I believe it's because on 
> 64-bit machines, without this type information, 1 could be one of several 
> integral types. The cast disambiguates.

I only see one definition of `::write` in 
`3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp`, which has the 
signature: `inline auto write(int fd, const void* buffer, size_t count)`.

Doesn't seem like there should be any ambiguity here, are there other 
defintions of `write` that we're contending with? Because `1` is clearly 
convertible to `size_t`.


- Michael


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


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43416/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Removed ambiguous call to `::write`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp 
> 88b355e09f76f0412c74ad69556572f0079deb8f 
> 
> Diff: https://reviews.apache.org/r/43416/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Joseph Wu]

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

(Updated Feb. 25, 2016, 4:41 p.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Rebase and update two flaky tests fixes.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Includes the following changes:

* Added the `` header where appropriate.
* Added the namespace `using process::Owned;` where appropriate.
* Generally replaced `Try` with `Owned`.  And 
`Try` with `Owned`.
* Added the (now required) `MasterDetector` argument to all slaves.  Before, 
this was fetched from the first master in `Cluster`.
* Removed `Shutdown();` from all tests.
* Replaced `Stop(...)` with the appropriate master/slave destruction calls.
* Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
launchers, etc).
* Replace `CHECK` in tests with `ASSERT`.


Diffs (updated)
-

  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/command_executor_tests.cpp 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
  src/tests/container_logger_tests.cpp 00f4129e46aa9268fbb66da25b34e61004fa87b2 
  src/tests/containerizer/docker_containerizer_tests.cpp 
6aecd912fc84b72d2b64f7a842891fddcbc469ac 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
6a60962b4593b3521c182c7320331743ccffd4ba 
  src/tests/containerizer/isolator_tests.cpp 
7b257de2afbc66f63c47a80c1f828e3e95bd602d 
  src/tests/containerizer/memory_pressure_tests.cpp 
79f134996c4b80bf49cbb8bee28eab5e6b4f5822 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
  src/tests/containerizer/port_mapping_tests.cpp 
983a6be160aefe5a32acb6111bb3c85230ec 
  src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
  src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
  src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
  src/tests/executor_http_api_tests.cpp 
2fc0893f5f5e80a783296fb31b30abe86d92df1b 
  src/tests/fault_tolerance_tests.cpp 982468f851cd9d95eb6cde7c57f2d737d46a827c 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_maintenance_tests.cpp 
356015ebd8d5d55b656e9116eafdb04f01f99039 
  src/tests/master_quota_tests.cpp 8357ec911b2a158632a708ae3adff6eabc536697 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
  src/tests/master_validation_tests.cpp 
ab2df22f73052f6bd77653e56e7b460b17e7b0be 
  src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
  src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_endpoints_tests.cpp 
6069ca1e9ed278459c5182e438417e95955b1924 
  src/tests/persistent_volume_tests.cpp 
e169e1b141a38dc389eefd42c11a078c413123d5 
  src/tests/rate_limiting_tests.cpp caced732ded05a334861a53488ef6391885b2263 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
  src/tests/registrar_zookeeper_tests.cpp 
3df9779ee5d076e16f6a538326693a36f986b6d0 
  src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
  src/tests/reservation_endpoints_tests.cpp 
32b2af4115211b58a5127a14dd19152c2eca120c 
  src/tests/reservation_tests.cpp b8878d51767ac0d95e346c44c0a4d5c060e565ef 
  src/tests/resource_offers_tests.cpp 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
  src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
  src/tests/scheduler_driver_tests.cpp f6dc25d82ae5f1e77fc6ede7ff2660ed0d9ea039 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
428e12646d80b45daec30cfe607b97f36170fdf5 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 322f3ddaf11885d7e61e0e9232c0342e97d8bfa1 
  src/tests/status_update_manager_tests.cpp 
d64d3b8c96270478f6b681c038de77c3a9eb68fe 
  src/tests/teardown_tests.cpp 

Re: Review Request 43410: Windows: Added support for dynamic library loading.

[Michael Park]

> On Feb. 24, 2016, 11:57 p.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp,
> >  lines 67-68
> > 
> >
> > Fits in one line.
> 
> Alex Clemmer wrote:
> Hmm, does it? I don't think so, am I missing something obvious?

I meant to wrap it like this:

```
if (!FreeLibrary(handle_)) {
  return WindowsError(
  "Could not close library '" + (path_.isSome() ? path_.get() : ""));
}
```

I've committed this patch with this change.


- Michael


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


On Feb. 25, 2016, 5:15 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43410/
> ---
> 
> (Updated Feb. 25, 2016, 5:15 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Originally review #40583, this was resurrected when Dario went on
> paternity leave. Minor changes were made, the review itself remains
> largely in tact.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 
> 166c97d3e4b07451b8b4c01092cecb69315c691a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43410/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44026: Moved future tests into future_tests.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44026]

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

- Mesos ReviewBot


On Feb. 25, 2016, 5:59 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44026/
> ---
> 
> (Updated Feb. 25, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved future tests into future_tests.cpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> ded1aaed25876d45e33b4a27fffc6a5c46ca92f5 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e9bf80ee69f4add299cb828ed3245ac07398943c 
> 
> Diff: https://reviews.apache.org/r/44026/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 43932: Added overlayfs provisioning backend.

[Jie Yu]

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


Fix it, then Ship it!




Thanks! This is great!


src/slave/containerizer/mesos/provisioner/backend.cpp (line 50)


Kill this line.



src/slave/containerizer/mesos/provisioner/backend.cpp (line 52)


Add a new line above.



src/slave/containerizer/mesos/provisioner/backends/overlay.hpp (lines 30 - 31)


can you wrap comments in 70 char width. It's less jagged IMO.



src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 28)


Let's use 'using process::XXX' explicitly. We try to avoid using 'using 
namespace'.



src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (lines 124 - 129)


To be safe, can you do the same thing to mark the mount as slave+shared 
(like we did in the bind backend).

So the goal of doing that is: we want to make sure when slave fork a 
subprocess with a new mount namespace, it does not create an extra reference to 
the mount so that rmdir might fail later.



src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (lines 132 - 133)


Can you align the error message:
```
return Failure(
"Failed to remount rootfs '" + rootfs +
"' read-only: " + mount.error());
```



src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 143)


Kill this line.



src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 148)


You can use mountTable->entries



src/tests/containerizer/provisioner_backend_tests.cpp (line 50)


kill this line.



src/tests/containerizer/provisioner_backend_tests.cpp (line 51)


We should add an TearDown method to unmount anything under sandbox. You can 
take a look at fs::unmountAll.



src/tests/containerizer/provisioner_backend_tests.cpp (lines 78 - 79)


You can do EXPECT_SOME_EQ("1", read);



src/tests/containerizer/provisioner_backend_tests.cpp (lines 83 - 84)


Ditto.



src/tests/containerizer/provisioner_backend_tests.cpp (lines 87 - 88)


Ditto.



src/tests/environment.cpp (line 489)


`s/_OVERLAYFS_/OVERLAYFS_/`


- Jie Yu


On Feb. 24, 2016, 4:38 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43932/
> ---
> 
> (Updated Feb. 24, 2016, 4:38 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2971
> https://issues.apache.org/jira/browse/MESOS-2971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added overlayfs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt b13fb23219ebb23bcfd6db062e1c814ca2114aa4 
>   src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> 01d06ebc67e259272ee57ea5c75bf7077ede65c4 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
>   src/slave/flags.cpp 1c6a87b670efde2deab4d6e3f24fd6eb3704a47d 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 25b28ef8fa5aae81e8dd0c9e33df4160dd912ce8 
>   src/tests/environment.cpp 6cd295f76496770774d090e0485ff87be378f74c 
> 
> Diff: https://reviews.apache.org/r/43932/diff/
> 
> 
> Testing
> ---
> 
> sudo modprobe overlayfs
> sudo make check -j4 
> GTEST_FILTER='OverlayBackendTest.ROOT_OVERLAYFS_OverlayFSBackend'
> 
> - OS: ubuntu 14.04 64bit vm
> - Kernel: 4.2.0-27-generic
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43800: Updated docs for reservation, volumes, and authZ.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43776, 43777, 43782, 43778, 43779, 43800]

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

- Mesos ReviewBot


On Feb. 25, 2016, 5:45 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43800/
> ---
> 
> (Updated Feb. 25, 2016, 5:45 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4591
> https://issues.apache.org/jira/browse/MESOS-4591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docs for reservation, volumes, and authZ.
> 
> This updates the authorization documentation to include the new `roles` 
> object for the `CreateVolume` and `ReserveResources` ACLs. The docs for 
> persistent volumes and dynamic reservations are also updated to reflect the 
> new authorization behavior.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md bbb4f2adc9348cb1686e6af78f5604d8cf7651ab 
>   docs/persistent-volume.md 2a794a572ff930aa1f95706b89fef9243be627de 
>   docs/reservation.md b98ebe6df0739b48c5fa58e087fd64b1c6c5d456 
> 
> Diff: https://reviews.apache.org/r/43800/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

[Neil Conway]

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

Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
---

This allows operators to list all the dynamic reservations and persistent
volumes in a cluster. This is important in itself; it also makes it easier to
use the `/unreserve` and `/destroy-volumes` endpoints.


Diffs
-

  docs/persistent-volume.md 2a794a572ff930aa1f95706b89fef9243be627de 
  docs/reservation.md b98ebe6df0739b48c5fa58e087fd64b1c6c5d456 
  src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
  src/tests/persistent_volume_endpoints_tests.cpp 
6069ca1e9ed278459c5182e438417e95955b1924 

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


Testing
---

make check


Thanks,

Neil Conway

Review Request 44045: Style fixes to `stout/include/Makefile.am`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Style fixes to `stout/include/Makefile.am`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
03eff5a831283f6d298e9a1feecfdc7369cacfe7 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 43889: CMAKE: Add leveldb library to 3rdparty external builds.

[Diana Arroyo]

> On Feb. 23, 2016, 6:22 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/cmake/macros/External.cmake, line 38
> > 
> >
> > And why we need change `EXTERNAL` macro here? I prososal add a version 
> > to our leveldb.tar.gz
> 
> Diana Arroyo wrote:
> I expect we will need to get an agreement with the folks who maintain the 
> make build.  Who is the person that need to be pulled into this discussion?
> 
> haosdent huang wrote:
> I think maybe @tillt

I added @tillt to the reviewers.  Will await response.


- Diana


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


On Feb. 25, 2016, 7:42 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43889/
> ---
> 
> (Updated Feb. 25, 2016, 7:42 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4746
> https://issues.apache.org/jira/browse/MESOS-4746
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMAKE: Add leveldb library to 3rdparty external builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> 0c80fb8d799ea1252492cd98ac0780f1228aadcd 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 3a2e0999722007475c023ade75719093e35cfc80 
>   3rdparty/libprocess/cmake/macros/External.cmake 
> e3901b67048f1c028216ae8323ee1c318a46f3cc 
> 
> Diff: https://reviews.apache.org/r/43889/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Review Request 44044: CMake: src CMakeLists-Added source to build master.

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: src CMakeLists-Added source to build master.


Diffs
-

  src/CMakeLists.txt 5cf0ec8c475839ad8717192a37f01546cbcccd7a 

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


Testing
---

"mesos-master" builds on Ubuntu.


Thanks,

Diana Arroyo

Re: Review Request 44004: Add 'name' field into NetworkInfo.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44004]

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

- Mesos ReviewBot


On Feb. 25, 2016, 3:15 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44004/
> ---
> 
> (Updated Feb. 25, 2016, 3:15 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-4758
> https://issues.apache.org/jira/browse/MESOS-4758
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'name' field into NetworkInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 33f6b0838360b61db70a247e5d6dfb16af15aa06 
>   include/mesos/v1/mesos.proto 1b0e709e76f3f6b44ab0434c649c064e8866c8a1 
> 
> Diff: https://reviews.apache.org/r/44004/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 44042: Windows: Fixed `UUID::random` and added utime/touch to stout/os.hpp.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Windows: Fixed `UUID::random` and added utime/touch to stout/os.hpp.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
f1d38029eef7e89dfdb8915158fba17865e6855b 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
84a2a021859d4e5c8547ad2a509eebda428a8255 
  3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp 
625636525ee0fb35214cc3df3a304c40c9a0b0a6 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
d3ba0b4ff5d2d125252389a0f8f618c78aa6d948 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 44001: CMake: Add MasterConfigure for master executable build.

[Diana Arroyo]

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

(Updated Feb. 25, 2016, 8:53 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


Changes
---

Added check to call FindApr and FindSvn in non-windows build environment.


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


Repository: mesos


Description
---

CMake: Add MasterConfigure for master executable build.


Diffs (updated)
-

  src/master/cmake/MasterConfigure.cmake PRE-CREATION 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Re: Review Request 43963: Fixed flakiness in DockerContainerizerTest.ROOT_DOCKER_Logs.

[Joseph Wu]

> On Feb. 25, 2016, 3:36 a.m., Bernd Mathiske wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp, line 1855
> > 
> >
> > Why the extra image? This test seems to run just fine with the other 
> > changes and simply "alpine".
> > 
> > What is the significance of "Expect" here?

The plain `alpine` image doesn't have the `unbuffer` command.

I originally tried modifying the double-echo command with `apk add --update 
expect`.  But the alpine package repos are flaky (or rate-limited), so that 
step would fail every ~100 or so iterations.

Therefore, to not "fix" flakiness with more flakiness, an `alpine` with 
`expect` pre-installed was the next best solution.


- Joseph


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


On Feb. 24, 2016, 9:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43963/
> ---
> 
> (Updated Feb. 24, 2016, 9:43 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Timothy Chen.
> 
> 
> Bugs: MESOS-4676
> https://issues.apache.org/jira/browse/MESOS-4676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the `unbuffer` utility in front of each `echo` in the test.  Since 
> Docker appears to handle simultaneous stdout/stderr in a non-robust fashion, 
> this mitigates the amount of overlap the two streams will have in the test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
> 
> Diff: https://reviews.apache.org/r/43963/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> On Centos7: sudo bin/mesos-tests.sh --gtest_filter="\*ROOT_DOCKER_Logs" 
> --gtest_repeat=1200 --gtest_break_on_failure
> 
> ---
> 
> CI results: 
> sudo make check
> 
>  | OSX | CentOS 7 | CentOS 6 | Debian 8 | Ubuntu 15.10 | Ubuntu 14 | 
> Ubuntu 12 |
>  Non-SSL |  :) |:)| *|:)|   :) |:) |  
>:)|
> With-SSL |  :) |:)|&*| &|^ |:) |  
>:)|
> 
>  :) = Passed.
> 
> Known flaky tests:
>   * = DockerContainerizerTest.ROOT_DOCKER_LaunchWithPersistentVolumes
>   & = MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery
>   ^ = LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 44008: CMake: Top CMakeLists-add master to cmake module path.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43997, 44001, 44003, 44005, 44007, 44008]

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

- Mesos ReviewBot


On Feb. 25, 2016, 3:29 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44008/
> ---
> 
> (Updated Feb. 25, 2016, 3:29 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4773
> https://issues.apache.org/jira/browse/MESOS-4773
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Top CMakeLists-add master to cmake module path.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 7f83dc84997d3b824d1f63012894bd9fc5284053 
> 
> Diff: https://reviews.apache.org/r/44008/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 44001: CMake: Add MasterConfigure for master executable build.

[Diana Arroyo]

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

(Updated Feb. 25, 2016, 8:13 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


Changes
---

Added a check for non-windows before making the call to FindApr and FindSvn.


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


Repository: mesos


Description
---

CMake: Add MasterConfigure for master executable build.


Diffs (updated)
-

  src/master/cmake/MasterConfigure.cmake PRE-CREATION 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Re: Review Request 43718: Added fs::supported() function.

[Jie Yu]

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


Fix it, then Ship it!




Thanks!


src/linux/fs.cpp (line 51)


I would just be more specific:
```
Failed to read /proc/filesystems:
```



src/linux/fs.cpp (line 60)


Should we return an Error here?


- Jie Yu


On Feb. 22, 2016, 2:10 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> ---
> 
> (Updated Feb. 22, 2016, 2:10 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added fs::supported() function.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595 
>   src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e 
>   src/tests/containerizer/fs_tests.cpp 
> 29e43877612fa151e6f6d79268a7411272a7bfeb 
> 
> Diff: https://reviews.apache.org/r/43718/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04 64bit vm
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43889: CMAKE: Add leveldb library to 3rdparty external builds.

[Diana Arroyo]

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

(Updated Feb. 25, 2016, 7:42 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, Joseph Wu, and Till Toenshoff.


Changes
---

Corrected LEVELDB_LIB to point to leveldb library.


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


Repository: mesos


Description
---

CMAKE: Add leveldb library to 3rdparty external builds.


Diffs (updated)
-

  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
0c80fb8d799ea1252492cd98ac0780f1228aadcd 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
3a2e0999722007475c023ade75719093e35cfc80 
  3rdparty/libprocess/cmake/macros/External.cmake 
e3901b67048f1c028216ae8323ee1c318a46f3cc 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Review Request 43907: Stout:[1/2] Fix error reporting bug in `os::rmdir`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Stout:[1/2] Fix error reporting bug in `os::rmdir`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Stout:[2/2] Added significant test coverage of `os::rmdir`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
756aa29ed134bf10a645af7f6562f86dc8e488f5 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
3c65d0422dc6e198180d53d1c9e6cb2839137434 
  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
d0592ef8a774d380e9df66b7e623eb72b29a28b3 
  3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 43906: CMake: Added files to be built as part of libmesos.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

CMake: Added files to be built as part of libmesos.


Diffs
-

  src/CMakeLists.txt 5cf0ec8c475839ad8717192a37f01546cbcccd7a 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 43884: Added allocator metrics for used quotas.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43884, 43883, 43882, 43881, 43880, 43879]

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

Error:
2016-02-25 19:18:03 URL:https://reviews.apache.org/r/43884/diff/raw/ 
[3997/3997] -> "43884.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.hpp:382
error: src/master/allocator/mesos/hierarchical.hpp: patch does not apply

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

- Mesos ReviewBot


On Feb. 25, 2016, 11:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43884/
> ---
> 
> (Updated Feb. 25, 2016, 11:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4723
> https://issues.apache.org/jira/browse/MESOS-4723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for used quotas.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43884/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 43905: Windows: Removed `user` launcher flag, preventing `su`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

`su` does not exist on Windows. Unfortunately, the launcher also depends
on it. In this commit, we remove Windows support for the launcher flag
`user`, which controls whether we use `su` in the launcher. This
allows us to divest ourselves of `su` altogether on Windows.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
d917a99a46841156dc1e359c44010938cc45e943 
  src/slave/containerizer/mesos/containerizer.cpp 
129406abdff715e321f683911e404c46676b6daf 
  src/slave/containerizer/mesos/launch.hpp 
7e29ca2b8bec1c20aef122472cff60f6003603ad 
  src/slave/containerizer/mesos/launch.cpp 
6b3bf163e2a577e6318a4a62f96d6bfd98ef9ae9 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 43904: Windows: Removed `rootfs` launcher flag, preventing `chroot`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

`chroot` is does not exist on Windows. Unfortunately, the launcher also
depends on it. In this commit, we remove Windows support for the
launcher flag `rootfs`, which controls whether we use `chroot` in the
launcher. This allows us to divest ourselves of `chroot` altogether on
Windows.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
129406abdff715e321f683911e404c46676b6daf 
  src/slave/containerizer/mesos/launch.hpp 
7e29ca2b8bec1c20aef122472cff60f6003603ad 
  src/slave/containerizer/mesos/launch.cpp 
6b3bf163e2a577e6318a4a62f96d6bfd98ef9ae9 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 43903: Stout: Add `WindowsError` constructor to `Result`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Stout: Add `WindowsError` constructor to `Result`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 
577c8e459f505b7b9021153d5e42e9d5a892ec33 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 44029: Fetcher::basename should ignore query strings and fragments.

[James Peach]

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

Review request for mesos, Bernd Mathiske, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Ignore URL query parameters and fragments when determining this
base name. This enables the fetcher to subsequently examine the
file extension and extract archives correctly.


Diffs
-

  src/slave/containerizer/fetcher.cpp 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
  src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 

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


Testing
---

``make check`` on OS X.


Thanks,

James Peach

Re: Review Request 43995: CMake: CompilationConfigure-Creating MACROS to get time and date.

[Diana Arroyo]

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

(Updated Feb. 25, 2016, 6:04 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


Changes
---

Added escaped quotes to return value in macros.


Summary (updated)
-

CMake: CompilationConfigure-Creating MACROS to get time and date.


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


Repository: mesos


Description (updated)
---

CMake: CompilationConfigure-Creating MACROS to get time and date.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake ab503b23f054ebc9a3877a3eca27b1b4190aa51b 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Review Request 44026: Moved future tests into future_tests.cpp.

[Cong Wang]

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

Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.


Repository: mesos


Description
---

Moved future tests into future_tests.cpp.


Diffs
-

  3rdparty/libprocess/src/tests/future_tests.cpp 
ded1aaed25876d45e33b4a27fffc6a5c46ca92f5 
  3rdparty/libprocess/src/tests/process_tests.cpp 
e9bf80ee69f4add299cb828ed3245ac07398943c 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 43800: Updated docs for reservation, volumes, and authZ.

[Greg Mann]

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

(Updated Feb. 25, 2016, 5:45 p.m.)


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


Changes
---

Updated persistent volume and dynamic reservation docs.


Summary (updated)
-

Updated docs for reservation, volumes, and authZ.


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


Repository: mesos


Description (updated)
---

Updated docs for reservation, volumes, and authZ.

This updates the authorization documentation to include the new `roles` object 
for the `CreateVolume` and `ReserveResources` ACLs. The docs for persistent 
volumes and dynamic reservations are also updated to reflect the new 
authorization behavior.


Diffs (updated)
-

  docs/authorization.md bbb4f2adc9348cb1686e6af78f5604d8cf7651ab 
  docs/persistent-volume.md 2a794a572ff930aa1f95706b89fef9243be627de 
  docs/reservation.md b98ebe6df0739b48c5fa58e087fd64b1c6c5d456 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 43410: Windows: Added support for dynamic library loading.

[Alex Clemmer]

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

(Updated Feb. 25, 2016, 5:15 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Originally review #40583, this was resurrected when Dario went on
paternity leave. Minor changes were made, the review itself remains
largely in tact.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 
166c97d3e4b07451b8b4c01092cecb69315c691a 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp 
PRE-CREATION 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 43411: Windows: Added dynamic library loading tests to build.

[Alex Clemmer]

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

(Updated Feb. 25, 2016, 5:15 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Windows: Added dynamic library loading tests to build.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
3c65d0422dc6e198180d53d1c9e6cb2839137434 
  3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
27626ae28db090f1a002239ff5c674b82e8fc9a8 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 43416: Windows: Removed ambiguous call to `::write`.

[Alex Clemmer]

> On Feb. 25, 2016, 12:22 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp, line 52
> > 
> >
> > Why is the cast to `size_t` needed here?

It is required, and (though it has been awhile) I believe it's because on 
64-bit machines, without this type information, 1 could be one of several 
integral types. The cast disambiguates.


- Alex


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


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43416/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Removed ambiguous call to `::write`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp 
> 88b355e09f76f0412c74ad69556572f0079deb8f 
> 
> Diff: https://reviews.apache.org/r/43416/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43411: Windows: Added dynamic library loading tests to build.

[Alex Clemmer]

> On Feb. 25, 2016, 12:09 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp, lines 
> > 73-74
> > 
> >
> > We're only testing that `loadSymbol` fails if we don't call `open` 
> > first, not `close`. Right? Seems like this was copy/pasted from above test.

Yep. I copied it. My bad. :(


- Alex


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


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dynamic library loading tests to build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
> 27626ae28db090f1a002239ff5c674b82e8fc9a8 
> 
> Diff: https://reviews.apache.org/r/43411/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43410: Windows: Added support for dynamic library loading.

[Alex Clemmer]

> On Feb. 24, 2016, 11:57 p.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp,
> >  lines 67-68
> > 
> >
> > Fits in one line.

Hmm, does it? I don't think so, am I missing something obvious?


- Alex


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


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43410/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Originally review #40583, this was resurrected when Dario went on
> paternity leave. Minor changes were made, the review itself remains
> largely in tact.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 
> 166c97d3e4b07451b8b4c01092cecb69315c691a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43410/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43882: Added allocation metrics for allocation time.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.cpp (line 1207)


It seems a general functional class, can we move it into `process/metrics`?


- Klaus Ma


On Feb. 25, 2016, 7:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43882/
> ---
> 
> (Updated Feb. 25, 2016, 7:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4721
> https://issues.apache.org/jira/browse/MESOS-4721
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocation metrics for allocation time.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43882/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43881: Added allocator metric for the number of allocations to a framework.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.hpp (line 376)


We also need to remove counter in `removeFramework`; or we'll see metrics 
of removed framework.



src/master/allocator/mesos/hierarchical.cpp (line 1496)


I'd suggest to do this in `addFramework` & `removeFramework`?



src/tests/hierarchical_allocator_tests.cpp (line 2447)


I think `1.0` is easier to read for other contributor.



src/tests/hierarchical_allocator_tests.cpp (line 2479)


ditto


- Klaus Ma


On Feb. 25, 2016, 7:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43881/
> ---
> 
> (Updated Feb. 25, 2016, 7:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4719
> https://issues.apache.org/jira/browse/MESOS-4719
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metric for the number of allocations to a framework.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43881/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43662: Added support for pipelining calls to the scheduler library.

[Anand Mazumdar]

> On Feb. 24, 2016, 3:03 a.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, lines 239-244
> > 
> >
> > Shouldn't we do the same for executor library?
> > 
> > More importnatly, what if a previous subscribe is in progress but 
> > `subscribed` is not set?

As per our discussion introduced two more states, SUBSCRIBING/SUBSCRIBED to 
take care of this. 

In the initial version, we would have invoked the `disconnected` callback if we 
noticed the race which would not have been ideal.


> On Feb. 24, 2016, 3:03 a.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, lines 290-292
> > 
> >
> > s/i.e./e.g.,/ ?
> > 
> > s/we failed over to a new master/master failed over/
> > 
> > Also, not sure what you wanted to say in this comment? Are you saying 
> > it is OK even if a new master is enqueued because?

Removed this block of code.


> On Feb. 24, 2016, 3:03 a.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, line 309
> > 
> >
> > This is the current master as detected by the detector. What is the 
> > guarantee that the connection was made with this master and not an old 
> > master? IOW, should this method take `master` as an argument?
> > 
> > Can we guarantee that the master hasn't changed because a new master 
> > detected causes these futures to be discarded synchronously? Looking at 
> > detected() that doesn't seem to be the case.
> > 
> > Do we want to enforce the invariant that there is ever only one 
> > connection attempt in progress?

As per our offline discussion, we would be assigning a new UUID for the 
connection instance before we initiate the connection and not after the 
connection has been established.


> On Feb. 24, 2016, 3:03 a.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, line 387
> > 
> >
> > is it possible for this callback to be invoked on the scheduler after 
> > they get a connected callback from this new connection attempt?

Not quite. As per our discussion earlier, the mutex gurrantees that the calls 
are serialized in order.


> On Feb. 24, 2016, 3:03 a.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, line 474
> > 
> >
> > For executor library, we did a close() here of the previous reader. Why 
> > was it required there and not here?

Good catch. Since with pipelining, its now not possible to send two `Subscribe` 
calls on the same connection. We would need a similar change for the Executor 
library to not invoke `close()`.


> On Feb. 24, 2016, 3:03 a.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, line 209
> > 
> >
> > I think for both scheduler::Mesos and executor::Mesos we need to add 
> > more comments around send() semantics. 
> > 
> > For example, we should mention that clients should send() only after 
> > connected() callback has been called.

As per our discussion and also my comment on r43661, I would add the comments 
in a separate patch for both the scheduler/executor as they both have the same 
semantics.


- Anand


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


On Feb. 25, 2016, 4:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43662/
> ---
> 
> (Updated Feb. 25, 2016, 4:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3570
> https://issues.apache.org/jira/browse/MESOS-3570
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the scheduler library used to chain calls on previous call 
> responses. This was inherently slow. This change adds support for pipelining 
> all calls to the master on a single connection via the `http::Connection` 
> abstraction in libprocess.
> 
> This change also adds support for handling various error scenarios when we 
> notice a disconnection instead of just relying on the master detector for 
> invoking the `disconnected` callback.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
> 
> Diff: https://reviews.apache.org/r/43662/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43662: Added support for pipelining calls to the scheduler library.

[Anand Mazumdar]

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

(Updated Feb. 25, 2016, 4:27 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments from Vinod


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


Repository: mesos


Description
---

Previously, the scheduler library used to chain calls on previous call 
responses. This was inherently slow. This change adds support for pipelining 
all calls to the master on a single connection via the `http::Connection` 
abstraction in libprocess.

This change also adds support for handling various error scenarios when we 
notice a disconnection instead of just relying on the master detector for 
invoking the `disconnected` callback.


Diffs (updated)
-

  src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 43271: Modify subprocess to deal with LIBPROCESS_PORT specially.

[Shuai Lin]

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




3rdparty/libprocess/src/subprocess.cpp (line 345)


Nitpick: would the warning always be logged when forking mesos-fetcher? 
Because it would use a copy of `os::environment()`, and #43272 removes the 
logic that removes `LIBPROCESS_PORT` from it?


- Shuai Lin


On Feb. 19, 2016, 6:54 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43271/
> ---
> 
> (Updated Feb. 19, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4609
> https://issues.apache.org/jira/browse/MESOS-4609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Adds a helper method for getting the current environment plus 
> considerations for libprocess.
> * Changes the default behavior of `process::subprocess` to use the above 
> helper when given `environment = None()`.
> * Adds a warning inside `process::subprocess` if `LIBPROCESS_PORT` conflicts 
> with the current process's `LIBPROCESS_PORT`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 44ca6d0869f3dbcfda1ac01d0d6b79dc20c4267c 
> 
> Diff: https://reviews.apache.org/r/43271/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Tests are run in the next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Alex Rukletsov]

Folks,

Sorry for not communicating it properly, I should have discarded the patch
actually. As Ben already mentioned, we had agreed to rework it and maybe
split into multiple patches for clarity. However, I haven't published them
yet.

Best,
Alex
On 23 Feb 2016 18:35, "Bernd Mathiske"  wrote:

>
>
> > On Feb. 23, 2016, 9:33 a.m., Ben Mahler wrote:
> > > Apologies, did you check with Alex prior to committing? We talked
> about this change recently but we didn't publish the comments, sorry that
> it wasn't clear! I reverted it for now, since we didn't really like the
> empty map constants as class members. We also needed to look into whether
> using empty initializer lists inline worked instead of this change.
>
> Alex is on vacation and I found this lingering. Since it looks a lot like
> an improvement, I went ahead after reading it all and testing it. No
> problem, we can progress further from either version.
>
>
> - Bernd
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/#review120338
> ---
>
>
> On Jan. 28, 2016, 5:12 a.m., Alexander Rukletsov wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/41950/
> > ---
> >
> > (Updated Jan. 28, 2016, 5:12 a.m.)
> >
> >
> > Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and
> Joris Van Remoortere.
> >
> >
> > Repository: mesos
> >
> >
> > Description
> > ---
> >
> > Changes made:
> > - empty resource map promoted to a const class field;
> > - removed variable numeric suffixes where appropriate;
> > - added const where appropriate.
> >
> >
> > Diffs
> > -
> >
> >   src/tests/hierarchical_allocator_tests.cpp
> f18e6eb10572b0f5b8bbff338384d9406f6ad62b
> >
> > Diff: https://reviews.apache.org/r/41950/diff/
> >
> >
> > Testing
> > ---
> >
> > On Mac OS 10.10.4:
> >
> > `make check`
> >
> > `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> >
> >
> > Thanks,
> >
> > Alexander Rukletsov
> >
> >
>
>

Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

[Benjamin Bannier]

> On Feb. 25, 2016, 12:59 p.m., Guangya Liu wrote:
> > docs/monitoring.md, line 905
> > 
> >
> > s/framework_filters/framework_offer_filters
> > 
> > and ditto for the following
> > 
> > There are both offer filter and inverse offer filter in allocator, it 
> > is better to clarify that this is offer filter.
> > 
> > BTW: Do u have any plan to add inverse offer filter metrics?

Renamed.

Also filled MESOS-4775 for inverse offer gauges.


- Benjamin


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


On Feb. 25, 2016, 4:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> ---
> 
> (Updated Feb. 25, 2016, 4:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
> https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for number of offer filters per framework.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43883/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44004: Add 'name' field into NetworkInfo.

[Avinash sridharan]

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




include/mesos/mesos.proto (line 1576)


Should we be more elaborate in our description?
Maybe:
 This will be used by the container run-time to determine the network that 
this container joins. For e.g., for `MesosContainerizer` this would represent 
the CNI network to which the container will be attached.



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


Ditto.


- Avinash sridharan


On Feb. 25, 2016, 3:15 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44004/
> ---
> 
> (Updated Feb. 25, 2016, 3:15 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-4758
> https://issues.apache.org/jira/browse/MESOS-4758
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'name' field into NetworkInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 33f6b0838360b61db70a247e5d6dfb16af15aa06 
>   include/mesos/v1/mesos.proto 1b0e709e76f3f6b44ab0434c649c064e8866c8a1 
> 
> Diff: https://reviews.apache.org/r/44004/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 44005: CMake: StoutTestsConfigure-removed FindApr & FindSvn, moved to master.

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: StoutTestsConfigure-removed FindApr & FindSvn, moved to master.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
a27cb98fa45cbd135ebfeca65e215fb3ff054739 

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


Testing
---

Tested on Ubuntu along with:
https://reviews.apache.org/r/44007/
https://reviews.apache.org/r/44008/


Thanks,

Diana Arroyo

Review Request 44008: CMake: Top CMakeLists-add master to cmake module path.

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: Top CMakeLists-add master to cmake module path.


Diffs
-

  CMakeLists.txt 7f83dc84997d3b824d1f63012894bd9fc5284053 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Review Request 44007: CMake: MesosConfigure-add include of master configure.

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: MesosConfigure-add include of master configure.


Diffs
-

  cmake/MesosConfigure.cmake 9a4fdb57e1281d9ec421e639819de5786c11744a 

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


Testing
---

Tested on Ubuntu along with:
https://reviews.apache.org/r/44008/


Thanks,

Diana Arroyo

Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 4:27 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Rename framework_filters to framework_offer_filters.


Summary (updated)
-

Added allocator metrics for number of offer filters per framework.


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


Repository: mesos


Description (updated)
---

Added allocator metrics for number of offer filters per framework.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

> On Feb. 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 229-251
> > 
> >
> > What about as this:
> > 
> > Try Docker::validateVersion(const Version& minVersion) const
> > {
> >   if (dockerVersion < minVersion) {
> > string msg = "Insufficient version '" + stringify(version.get()) +
> >  "' of Docker. Please upgrade to >=' " +
> >  stringify(minVersion) + "'";
> > return Error(msg);
> >   }
> > 
> >   return Nothing();
> > }

As we have to use dockerVersion as string, we may expect a parsing error. So 
that error check is necessary.


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

> On Feb. 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 144-172
> > 
> >
> > why update this function, this can also get the docker version, why do 
> > you want to update the docker version to string?

As I mentioned, it is feasible to update docker version as string. If we use 
Version type instead, we can't use "=" assignment operator without modifying 
version.hpp.


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

> On Feb. 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.hpp, line 269
> > 
> >
> > What about
> > 
> > `Version dockerVersion`

That is not working. First of all , we have to initialize dockerVersion here as 
there is no Version() constructor, so we have to do like Version dockerVersion 
= Version(0,0,0). Again, afterwards we can't assign any value to it using "=" 
operator as there is no "=" overloaded in version.hpp. I started as you are 
guiding, but ended up writing current implementation.


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Review Request 44004: Add 'name' field into NetworkInfo.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Add 'name' field into NetworkInfo.


Diffs
-

  include/mesos/mesos.proto 33f6b0838360b61db70a247e5d6dfb16af15aa06 
  include/mesos/v1/mesos.proto 1b0e709e76f3f6b44ab0434c649c064e8866c8a1 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

> On Feb. 25, 2016, 12:08 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 123-127
> > 
> >
> > What about update here as:
> > 
> > 
> >   // Get docker version.
> >   Future version = this->version();
> > 
> >   if (!version.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
> > return Error("Timed out getting docker version");
> >   }
> > 
> >   if (version.isFailed()) {
> > return Error("Failed to get docker version: " + version.failure());
> >   }
> >   
> >   dockerVersion = version.get();

As I mentioned , there are some testcases, which does not create a docker 
instance using Docker::create. So, we can't save dockerVersion inside create()


- Abhishek


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


On Feb. 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Review Request 44003: CMake: Add CMakeLists for master executable build.

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: Add CMakeLists for master executable build.


Diffs
-

  src/master/CMakeLists.txt PRE-CREATION 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Review Request 44001: CMake: Add MasterConfigure for master executable build.

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: Add MasterConfigure for master executable build.


Diffs
-

  src/master/cmake/MasterConfigure.cmake PRE-CREATION 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Guangya Liu]

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




src/docker/docker.hpp (line 269)


What about

`Version dockerVersion`



src/docker/docker.cpp (lines 123 - 127)


What about update here as:

  // Get docker version.
  Future version = this->version();

  if (!version.await(DOCKER_VERSION_WAIT_TIMEOUT)) {
return Error("Timed out getting docker version");
  }

  if (version.isFailed()) {
return Error("Failed to get docker version: " + version.failure());
  }
  
  dockerVersion = version.get();



src/docker/docker.cpp (lines 144 - 172)


why update this function, this can also get the docker version, why do you 
want to update the docker version to string?



src/docker/docker.cpp (lines 229 - 248)


What about as this:

Try Docker::validateVersion(const Version& minVersion) const
{
  if (dockerVersion < minVersion) {
string msg = "Insufficient version '" + stringify(version.get()) +
 "' of Docker. Please upgrade to >=' " +
 stringify(minVersion) + "'";
return Error(msg);
  }

  return Nothing();
}


- Guangya Liu


On 二月 24, 2016, 2:10 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 24, 2016, 2:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
>   src/slave/containerizer/docker.cpp 0303208083f1ebd9f9df51178fd91ee4c763f61c 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43883: Added allocator metrics for number of filters per framework.

[Guangya Liu]

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




docs/monitoring.md (line 905)


s/framework_filters/framework_offer_filters

and ditto for the following

There are both offer filter and inverse offer filter in allocator, it is 
better to clarify that this is offer filter.

BTW: Do u have any plan to add inverse offer filter metrics?



src/master/allocator/mesos/hierarchical.hpp (line 402)


s/filters/offer filters



src/master/allocator/mesos/hierarchical.cpp (line 265)


move the comments to the below line


- Guangya Liu


On 二月 25, 2016, 11:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> ---
> 
> (Updated 二月 25, 2016, 11:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
> https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for number of filters per framework.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43883/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43881: Added allocator metric for the number of allocations to a framework.

[Benjamin Bannier]

> On Feb. 25, 2016, 12:11 a.m., Alexander Rojas wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1204
> > 
> >
> > Even though I really like the anonymous namespace, we don't use it for 
> > some reasons (I think the main one is that symbols are still exported, even 
> > though with mangled names).
> > 
> > All in all, there is only one place in non automatically generated code 
> > which uses them (and I wonder how it managed).
> > 
> > All this to say, I guess this function should be static.
> > 
> > Moreover, does this need to be a free function? I much rather have a 
> > private method.

I introduced a type here so only an anon namespace would have prevented public 
linkage. I removed the anon namespace and documented the type now.


- Benjamin


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


On Feb. 25, 2016, 12:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43881/
> ---
> 
> (Updated Feb. 25, 2016, 12:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4719
> https://issues.apache.org/jira/browse/MESOS-4719
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metric for the number of allocations to a framework.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43881/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43882: Added allocation metrics for allocation time.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 12:37 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added allocation metrics for allocation time.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43881: Added allocator metric for the number of allocations to a framework.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 12:37 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Avoided anon namespaces and added a comment.


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


Repository: mesos


Description
---

Added allocator metric for the number of allocations to a framework.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43963: Fixed flakiness in DockerContainerizerTest.ROOT_DOCKER_Logs.

[Bernd Mathiske]

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




src/tests/containerizer/docker_containerizer_tests.cpp (line 1855)


Why the extra image? This test seems to run just fine with the other 
changes and simply "alpine".

What is the significance of "Expect" here?


- Bernd Mathiske


On Feb. 24, 2016, 9:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43963/
> ---
> 
> (Updated Feb. 24, 2016, 9:43 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Timothy Chen.
> 
> 
> Bugs: MESOS-4676
> https://issues.apache.org/jira/browse/MESOS-4676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the `unbuffer` utility in front of each `echo` in the test.  Since 
> Docker appears to handle simultaneous stdout/stderr in a non-robust fashion, 
> this mitigates the amount of overlap the two streams will have in the test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a299c9e0744b5657984e5bb0afbe4874a266ddb6 
> 
> Diff: https://reviews.apache.org/r/43963/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> On Centos7: sudo bin/mesos-tests.sh --gtest_filter="\*ROOT_DOCKER_Logs" 
> --gtest_repeat=1200 --gtest_break_on_failure
> 
> ---
> 
> CI results: 
> sudo make check
> 
>  | OSX | CentOS 7 | CentOS 6 | Debian 8 | Ubuntu 15.10 | Ubuntu 14 | 
> Ubuntu 12 |
>  Non-SSL |  :) |:)| *|:)|   :) |:) |  
>:)|
> With-SSL |  :) |:)|&*| &|^ |:) |  
>:)|
> 
>  :) = Passed.
> 
> Known flaky tests:
>   * = DockerContainerizerTest.ROOT_DOCKER_LaunchWithPersistentVolumes
>   & = MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery
>   ^ = LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 43880: Added allocator metrics for total and allocated scalar resources.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 12:01 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Added documentation, fixed stray `FIXME`s and ws, and stayed away from `const` 
locals.


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


Repository: mesos


Description
---

Added allocator metrics for total and allocated scalar resources.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43881: Added allocator metric for the number of allocations to a framework.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 12:01 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Added documentation and ws.


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


Repository: mesos


Description
---

Added allocator metric for the number of allocations to a framework.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43882: Added allocation metrics for allocation time.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 12:01 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Added documentation and fixed ws.


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


Repository: mesos


Description
---

Added allocation metrics for allocation time.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43879: Added allocator metrics for number of allocations made.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 12:01 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Added user documentation and reused read-out code for endpoint.


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


Repository: mesos


Description
---

Added allocator metrics for number of allocations made.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43883: Added allocator metrics for number of filters per framework.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 12:01 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Added documentation, fixed ws, and stayed away from `const` locals.


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


Repository: mesos


Description
---

Added allocator metrics for number of filters per framework.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43884: Added allocator metrics for used quotas.

[Benjamin Bannier]

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

(Updated Feb. 25, 2016, 12:01 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Added documentation, fixed ws, and stayed away from C++ range-based `for` loops.


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


Repository: mesos


Description
---

Added allocator metrics for used quotas.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 43961: Added some additional synchronization in ROOT_CGROUPS_Pids_and_Tids.

[Bernd Mathiske]

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


Fix it, then Ship it!





src/tests/containerizer/isolator_tests.cpp (line 813)


"finished exec'ing" reads like "finished executing", i.e. the process has 
terminated. But that's not what is meant here! Only after I realized this, the 
sequence of events here became clear to me. Suggestion:

// This observation ensures that the "cat" process 
// has completed its part of the exec() procedure 
// and is now executing normally."


- Bernd Mathiske


On Feb. 24, 2016, 9:59 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43961/
> ---
> 
> (Updated Feb. 24, 2016, 9:59 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Chi Zhang, Artem Harutyunyan, and 
> Jian Qiu.
> 
> 
> Bugs: MESOS-4677
> https://issues.apache.org/jira/browse/MESOS-4677
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a set of pipes to mitigate a race between `exec`ing + updating cgroup 
> tasks and the test reading said cgroup tasks.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 653b037c489072f43e53dec01a811a9249dcd660 
> 
> Diff: https://reviews.apache.org/r/43961/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Centos7: sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_Pids_and_Tids" 
> --gtest_repeat=5000 --gtest_break_on_failure
> 
> ---
> 
> CI results:
> 
> sudo make check
> 
>  | OSX | CentOS 7 | CentOS 6 | Debian 8 | Ubuntu 15.10 | Ubuntu 14 | 
> Ubuntu 12 |
>  Non-SSL |  :) |@_@   | *|X_X   |   :) |:) |  
>:)|
> With-SSL |  :) |@_@   | *| &|   :) |:) |  
>:)|
> 
>  :) = Passed.
> X_X = Configuration error with docker.
> @_@ = Ran out of disk space in some tests: 
> LinuxFilesystemIsolatorTest.ROOT_SandboxEnvironmentVariable, 
> LinuxFilesystemIsolatorTest.ROOT_MultipleContainers, 
> MesosContainerizerLaunchTest.ROOT_ChangeRootfs, 
> LinuxFilesystemIsolatorTest.ROOT_ImageInVolumeWithRootFilesystem
> 
>   * = DockerContainerizerTest.ROOT_DOCKER_LaunchWithPersistentVolumes (Known 
> issue)
>   % = DockerContainerizerTest.ROOT_DOCKER_Logs
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 43999: Use relative path to create libraries symbolic link.

[Zhiwei Chen]

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

Review request for mesos.


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


Repository: mesos


Description
---

Use relative path to create libraries symbolic link.


Diffs
-

  src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5 

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


Testing
---


Thanks,

Zhiwei Chen

Re: Review Request 43799: Removed race condition from libevent based poll implementation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43799]

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

- Mesos ReviewBot


On Feb. 24, 2016, 9:30 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43799/
> ---
> 
> (Updated Feb. 24, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Bugs: MESOS-3271 and MESOS-4711
> https://issues.apache.org/jira/browse/MESOS-3271
> https://issues.apache.org/jira/browse/MESOS-4711
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Under certains circumstances, the future returned by poll is discarded right
> after the event is triggered, this causes the event callback to be called
> before the discard callback which results in an abort signal being raised
> by the libevent library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_poll.cpp 
> 461624ca003e97be5ea9cf956d86fc294e6f1bc1 
> 
> Diff: https://reviews.apache.org/r/43799/diff/
> 
> 
> Testing
> ---
> 
> ```bash
> # On CentOS 6.7 running in virtualbox
> ../configure --enable-ssl --enable-libevent
> make -j4 check
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" 
> --gtest_repeat=1000
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 43997: CMake: Move FindApr and FindSvn to master.

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: Move FindApr and FindSvn to master.


Diffs
-

  src/master/cmake/FindApr.cmake PRE-CREATION 
  src/master/cmake/FindSvn.cmake PRE-CREATION 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Re: Review Request 43461: Used foreach loop to iterate std list/set.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Feb. 25, 2016, 4:11 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43461/
> ---
> 
> (Updated Feb. 25, 2016, 4:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used foreach loop to iterate std list/set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 77655f22ad4ce6e81fd46bcdebbf2a82786e8f49 
> 
> Diff: https://reviews.apache.org/r/43461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 43879: Added allocator metrics for number of allocations made.

[Benjamin Bannier]

- Benjamin


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


On Feb. 24, 2016, 9:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> ---
> 
> (Updated Feb. 24, 2016, 9:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for number of allocations made.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43879/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43889: CMAKE: Add leveldb library to 3rdparty external builds.

[Diana Arroyo]

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

(Updated Feb. 25, 2016, 8:22 a.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, Joseph Wu, and Till Toenshoff.


Changes
---

Corrected LEVELDB_LIB definition.


Summary (updated)
-

CMAKE: Add leveldb library to 3rdparty external builds.


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


Repository: mesos


Description (updated)
---

CMAKE: Add leveldb library to 3rdparty external builds.


Diffs (updated)
-

  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
0c80fb8d799ea1252492cd98ac0780f1228aadcd 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
3a2e0999722007475c023ade75719093e35cfc80 
  3rdparty/libprocess/cmake/macros/External.cmake 
e3901b67048f1c028216ae8323ee1c318a46f3cc 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo

Review Request 43995: CMake: Creating MACROS to get time and date.

[Diana Arroyo]

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

Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

CMake: Creating MACROS to get time and date.


Diffs
-

  cmake/CompilationConfigure.cmake ab503b23f054ebc9a3877a3eca27b1b4190aa51b 

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


Testing
---

Tested on Ubuntu.


Thanks,

Diana Arroyo