Re: Review Request 41932: Updated "teardown_framework" requests in the authorizer.

[Guangya Liu]

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

(Updated 一月 14, 2016, 7:53 a.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description (updated)
---

Updated "teardown_framework" requests in the authorizer.


Diffs (updated)
-

  src/authorizer/local/authorizer.cpp c1db9c2131ea8fbf835278203a240f108c6372c5 
  src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 41930: Added "TeardownFramework" to ACL protobuf.

[Guangya Liu]

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

(Updated 一月 14, 2016, 7:51 a.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description (updated)
---

Added "TeardownFramework" to ACL protobuf.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
84727e66dd14be9a25705ab1141e92eee72fae50 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 41930: Added "TeardownFramework" to ACL protobuf.

[Guangya Liu]

> On 一月 11, 2016, 6:44 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, lines 147-148
> > 
> >
> > What is the expected behavior if both are set? Error? Union?

Yes, Union.


> On 一月 11, 2016, 6:44 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, line 148
> > 
> >
> > This is never used? Or is that in a subsequent patch?

subsequent patch.


> On 一月 11, 2016, 6:44 a.m., Adam B wrote:
> > src/authorizer/local/authorizer.cpp, line 77
> > 
> >
> > Did you mean to iterate through `shutdown_frameworks()` or 
> > `teardown_frameworks()`? Or both?

iterate both in the dependency patch.


- Guangya


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


On 一月 6, 2016, 2:35 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41930/
> ---
> 
> (Updated 一月 6, 2016, 2:35 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4154
> https://issues.apache.org/jira/browse/MESOS-4154
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch included the following:
> 1) Renamed "ShutdownFramework" to "TeardownFramework".
> 2) Added teardown_frameworks ACL to protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> f61613948b7b5c5c2118f1782a0c5f8ff7e8e057 
>   include/mesos/authorizer/authorizer.proto 
> 7b729e19484d92be48bbde4dff2c833a4109936e 
>   src/authorizer/local/authorizer.hpp 
> 1563c11709c2612350354690b50ceb33d2720f98 
>   src/authorizer/local/authorizer.cpp 
> 1d135fb6906c7050a788cbac9ca2d8164ff064ef 
>   src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5 
>   src/tests/master_tests.cpp 865fa4a71f4bae2a218cd2c4e10873222d1ea3c4 
>   src/tests/mesos.hpp 49a4c48e6887e6f0921d96c359746e39be10e222 
>   src/tests/mesos.cpp 082e57bc73fad02de77e16e4b34451e6c0903038 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
>   src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 
> 
> Diff: https://reviews.apache.org/r/41930/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41595: Updated `Master::Http::state` to use `jsonify` in mesos.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41593]

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

Error:
 2016-01-14 07:41:12 URL:https://reviews.apache.org/r/41593/diff/raw/ 
[40207/40207] -> "41593.patch" [1]
error: patch failed: 3rdparty/libprocess/3rdparty/stout/include/Makefile.am:42
error: 3rdparty/libprocess/3rdparty/stout/include/Makefile.am: patch does not 
apply

- Mesos ReviewBot


On Jan. 6, 2016, 2:48 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41595/
> ---
> 
> (Updated Jan. 6, 2016, 2:48 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 4f4cbf6e53588b72204f9628dea5696c71eb66a5 
>   src/common/http.cpp 7165551321bedb8a4d711a64d0d6d8fd15215424 
>   src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5 
> 
> Diff: https://reviews.apache.org/r/41595/diff/
> 
> 
> Testing
> ---
> 
> # Some preliminery numbers
> 
> These numbers are from my Ubuntu VM on my Mac OS X with @vinodkone's 
> benchmark test: [r40844](https://reviews.apache.org/r/40844/).
> 
> ### Before
> 
> ```
> [==] Running 36 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 36 tests from SlaveAndFrameworkCount/MasterState_BENCHMARK_Test
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/0
> Added 1000 slaves and 1 frameworks
> Received state.json response in 683.138216ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/0 (1436 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/1
> Added 1000 slaves and 50 frameworks
> Received state.json response in 550.636939ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/1 (1474 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/2
> Added 1000 slaves and 100 frameworks
> Received state.json response in 632.835236ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/2 (1491 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/3
> Added 1000 slaves and 200 frameworks
> Received state.json response in 584.035771ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/3 (1431 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/4
> Added 1000 slaves and 500 frameworks
> Received state.json response in 688.404348ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/4 (1586 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/5
> Added 1000 slaves and 1000 frameworks
> Received state.json response in 666.713683ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/5 (1590 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/6
> Added 5000 slaves and 1 frameworks
> Received state.json response in 3.916201532secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/6 (7852 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/7
> Added 5000 slaves and 50 frameworks
> Received state.json response in 3.362618796secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/7 (8315 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/8
> Added 5000 slaves and 100 frameworks
> Received state.json response in 3.126815189secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/8 (7153 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/9
> Added 5000 slaves and 200 frameworks
> Received state.json response in 3.079956539secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/9 (7534 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/10
> Added 5000 slaves and 500 frameworks
> Received state.json response in 3.222014521secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/10 (8129 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/11
> Added 5000 slaves and 1000 frameworks
> Received state.json response in 3.286657158secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/11 (8133 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/12
> Added 1 slaves and 1 frameworks
> Received state.json response in 7.332592151secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/12 
> (15639 ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_B

Re: Review Request 42288: Docker container REST API /monitor/statistics.json output have no timestamp field

[Guangya Liu]

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



src/slave/containerizer/docker.cpp (line 1367)


What about update the comments same as mesos docker containizer? The 
current comments seems does not fit in here well.

// Set the timestamp now we have all statistics.


- Guangya Liu


On 一月 14, 2016, 7:09 a.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42288/
> ---
> 
> (Updated 一月 14, 2016, 7:09 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4328
> https://issues.apache.org/jira/browse/MESOS-4328
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Docker container REST API /monitor/statistics.json output have no timestamp 
> field
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp da19975 
> 
> Diff: https://reviews.apache.org/r/42288/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 41963: Logger Module: Implement ContainerLogger recovery.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [42052, 41779, 41780, 41781, 41782, 41783, 41962, 41963]

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

- Mesos ReviewBot


On Jan. 12, 2016, 10:31 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41963/
> ---
> 
> (Updated Jan. 12, 2016, 10:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4150
> https://issues.apache.org/jira/browse/MESOS-4150
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a call to `ContainerLogger::recover` inside each containerizer's 
> `Containerizer::recover`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp da19975c1f1e59fab0a96ebd9aa815bba2a35ece 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
> 
> Diff: https://reviews.apache.org/r/41963/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX & Centos7)
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT*" (Centos7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 42288: Docker container REST API /monitor/statistics.json output have no timestamp field

[Andy Pang]

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

Review request for mesos, Jie Yu and Timothy Chen.


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


Repository: mesos


Description
---

Docker container REST API /monitor/statistics.json output have no timestamp 
field


Diffs
-

  src/slave/containerizer/docker.cpp da19975 

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


Testing
---

make check


Thanks,

Andy Pang

Re: Review Request 42194: Handle unreserve logic for dynamic reservation (3/3).

[Klaus Ma]

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


If there is no much different with `updateAllocation`, I'd suggest to merge 
this patch with `updateAllocation`.

- Klaus Ma


On Jan. 13, 2016, 9:10 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42194/
> ---
> 
> (Updated Jan. 13, 2016, 9:10 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle unreserve logic for dynamic reservation with allocation slack.
> 
> This patch is halding the case when using updateAvailable to unreserve
> some resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/42194/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42113: Handle unreserve logic for dynamic reservation (2/3).

[Klaus Ma]

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



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


`freeAllocationSlack` is unused in the following codes? Why it's here.



src/master/allocator/mesos/hierarchical.cpp (lines 707 - 710)


Should be

if (freeAllocationSlack.empty())
  break;

Resources freeResources = freeAllocationSlack.get(name);

if (freeResources.contains(unreservedResources)) {
  slaves[slaveId].total -= unreservedResources;
  freeAllocationSlack -= unreservedResources;
} else {
  slaves[slaveId].total -= freeResources;
  freeAllocationSlack -= freeResources;
}



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


There should be `freeAllocationSlack` which is not allocated for now.


- Klaus Ma


On Jan. 13, 2016, 9:01 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42113/
> ---
> 
> (Updated Jan. 13, 2016, 9:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle unreserve logic for dynamic reservation with allocation slack.
> 
> This patch is halding the case when using updateAllocation to unreserve
> some resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/42113/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42164: Allowed (un)reserve operations without a principal.

[Guangya Liu]

> On 一月 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > First thing here is that the `principal` field in `ReservationInfo` is 
> > still marked `required`. What's our plan for changing it to `optional`?
> > 
> > Second thing is that even if `principal.isNone()`, if is authorization 
> > is turned off we don't care, right? That is, if authorization is turned 
> > off, even frameworks without a principal can reserve any resources?
> 
> Greg Mann wrote:
> Regarding your first question: per our discussion, we do indeed need a 
> good plan for migrating to an `optional` principal within `ReservationInfo`. 
> It would seem that we have to choose between two less-than-ideal situations: 
> either we tolerate HTTP endpoints that don't work when authentication is 
> disabled, or we enforce that frameworks cannot register with `principal == 
> ""`, so that we can use the empty string to represent a null principal while 
> we migrate to an `optional` principal over a deprecation cycle.
> 
> To your second point: again per our discussion, semantically the 
> principal within `ReservationInfo` represents the principal (or lack thereof) 
> which reserved the resources, so we should enforce that it matches the 
> principal of the framework/operator, even if authorization is not enabled.
> 
> Guangya Liu wrote:
> If authorization is not enabled, then it is not a must to set principal 
> for a framework, so how can we enforce the match of principal?
> 
> Greg Mann wrote:
> We should enforce that the principal (or lack of a principal) used for 
> authorization is the same as the principal in the `ReservationInfo`. So if no 
> principal is provided (i.e., authorization disabled), then there should be no 
> principal in `ReservationInfo`. Since the principal in `ReservationInfo` is 
> currently a required field, we would need to use something like the empty 
> string `""` to represent the case of no principal. Ideally, the principal 
> field in `ReservationInfo` should be an optional field so that it can be 
> omitted when no principal is used. The plan is to change this field to 
> optional, but we're debating how and when it will be best to do so. Any 
> thoughts on how best to do this?

Compared with `HTTP endpoints that don't work when authentication is disabled`, 
I think that `enforce that frameworks cannot register with principal == ""` 
might be better as it will only impact some framework logic and more simple, 
but authorization require the whole cluster and all frameworks must enable 
authorization.


- Guangya


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


On 一月 13, 2016, 2:45 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> ---
> 
> (Updated 一月 13, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed (un)reserve operations without a principal.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp 
> fbf8fadbc04a7cbc60ee6091e0224339389b400f 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/42164/diff/
> 
> 
> Testing
> ---
> 
> Two master validation tests were removed which tested to make sure that 
> reserve and unreserve operations would not succeed without a principal.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42273: Modified scheduler library to move the queue contents before `async`.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [42273]

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

- Mesos ReviewBot


On Jan. 13, 2016, 11:06 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42273/
> ---
> 
> (Updated Jan. 13, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This trivial fix invokes `std::move` on the `queue` object to not perform a 
> copy before invoking the `async(..)` call.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp a17872b46ec600e0fae6c43247ccb63f5ee55ac0 
> 
> Diff: https://reviews.apache.org/r/42273/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42164: Allowed (un)reserve operations without a principal.

[Greg Mann]

> On Jan. 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > First thing here is that the `principal` field in `ReservationInfo` is 
> > still marked `required`. What's our plan for changing it to `optional`?
> > 
> > Second thing is that even if `principal.isNone()`, if is authorization 
> > is turned off we don't care, right? That is, if authorization is turned 
> > off, even frameworks without a principal can reserve any resources?
> 
> Greg Mann wrote:
> Regarding your first question: per our discussion, we do indeed need a 
> good plan for migrating to an `optional` principal within `ReservationInfo`. 
> It would seem that we have to choose between two less-than-ideal situations: 
> either we tolerate HTTP endpoints that don't work when authentication is 
> disabled, or we enforce that frameworks cannot register with `principal == 
> ""`, so that we can use the empty string to represent a null principal while 
> we migrate to an `optional` principal over a deprecation cycle.
> 
> To your second point: again per our discussion, semantically the 
> principal within `ReservationInfo` represents the principal (or lack thereof) 
> which reserved the resources, so we should enforce that it matches the 
> principal of the framework/operator, even if authorization is not enabled.
> 
> Guangya Liu wrote:
> If authorization is not enabled, then it is not a must to set principal 
> for a framework, so how can we enforce the match of principal?

We should enforce that the principal (or lack of a principal) used for 
authorization is the same as the principal in the `ReservationInfo`. So if no 
principal is provided (i.e., authorization disabled), then there should be no 
principal in `ReservationInfo`. Since the principal in `ReservationInfo` is 
currently a required field, we would need to use something like the empty 
string `""` to represent the case of no principal. Ideally, the principal field 
in `ReservationInfo` should be an optional field so that it can be omitted when 
no principal is used. The plan is to change this field to optional, but we're 
debating how and when it will be best to do so. Any thoughts on how best to do 
this?


- Greg


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


On Jan. 13, 2016, 2:45 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> ---
> 
> (Updated Jan. 13, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed (un)reserve operations without a principal.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp 
> fbf8fadbc04a7cbc60ee6091e0224339389b400f 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/42164/diff/
> 
> 
> Testing
> ---
> 
> Two master validation tests were removed which tested to make sure that 
> reserve and unreserve operations would not succeed without a principal.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 35711: Disallow special characters in role name.

[haosdent huang]

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

(Updated Jan. 14, 2016, 6 a.m.)


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


Changes
---

Rebase


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


Repository: mesos


Description
---

Disallow special characters in role name.


Diffs (updated)
-

  docs/roles.md af1adad7ec8122fd10f7de44848014b850416bcd 
  include/mesos/roles.hpp PRE-CREATION 
  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
  src/common/roles.cpp PRE-CREATION 
  src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 
  src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
  src/tests/role_tests.cpp 373ae267b85588fe491ab0a0ce8aa195f971aac3 

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


Testing
---

make -j8 check


Thanks,

haosdent huang

Re: Review Request 41791: Updated allocation slack for dynamic reserve (1/3).

[Guangya Liu]

> On 一月 14, 2016, 5:14 a.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 732-737
> > 
> >
> > Update the comments to list cases, for example:
> > 1. if new `reserved.stateless` > old, pala...
> > 2. if new ... <, pala...
> >   - if xxx
> >   - if xxx

It was handled in following patches


- Guangya


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


On 一月 13, 2016, 12:58 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated 一月 13, 2016, 12:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update allocation slack resources if reserve some
> new stateless reserved resources. For such case,
> the allocation slack resources will be increased.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41791: Updated allocation slack for dynamic reserve (1/3).

[Klaus Ma]

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



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


It should be `newStatelessReserved.contains(total)`; in `contains`, it'll 
return false if any resource did not contain.



src/master/allocator/mesos/hierarchical.cpp (lines 732 - 737)


Update the comments to list cases, for example:
1. if new `reserved.stateless` > old, pala...
2. if new ... <, pala...
  - if xxx
  - if xxx


- Klaus Ma


On Jan. 13, 2016, 8:58 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated Jan. 13, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update allocation slack resources if reserve some
> new stateless reserved resources. For such case,
> the allocation slack resources will be increased.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41617: Added a new category called whitespace/mesos-comments to capture missing, leading, white-space in comments

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41617]

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

Error:
 2016-01-14 05:12:09 URL:https://reviews.apache.org/r/41617/diff/raw/ 
[1614/1614] -> "41617.patch" [1]
No files to lint

Error: Commit message summary (the first line) must not exceed 72 characters.

- Mesos ReviewBot


On Jan. 14, 2016, 1:59 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41617/
> ---
> 
> (Updated Jan. 14, 2016, 1:59 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4231
> https://issues.apache.org/jira/browse/MESOS-4231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new category called whitespace/mesos-comments to capture missing, 
> leading, white-space in comments
> 
> 
> Diffs
> -
> 
>   support/cpplint.py 6890e27f92603b025e25e4db01decf351c33c9a1 
>   support/mesos-style.py 5ee17593153df7815426918a1da6e25cb1188bca 
> 
> Diff: https://reviews.apache.org/r/41617/diff/
> 
> 
> Testing
> ---
> 
> Ran the modified mesos-style.py on the existing code-base and got the 
> following errors:
> src/authentication/cram_md5/authenticatee.hpp:60:  Should have a space 
> between // and comment  [whitespace/comments] [4]
> src/authentication/cram_md5/authenticator.hpp:60:  Should have a space 
> between // and comment  [whitespace/comments] [4]
> src/authorizer/local/authorizer.hpp:84:  Should have a space between // and 
> comment  [whitespace/comments] [4]
> src/tests/containerizer/docker_containerizer_tests.cpp:978:  Should have a 
> space between // and comment  [whitespace/comments] [4]
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp:90:  At 
> least a single space is required between code and comments  
> [whitespace/comments] [2]
> 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp:127:  Should have a 
> space between // and comment  [whitespace/comments] [4]
>   Doesn't seem to give any false-positives. All the above errors do have a 
> missing leading white-space. 
>   
>   Tried a commit for the following patch:
>   diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
> index eb47b53..e579d20 100644
> --- a/src/tests/slave_tests.cpp
> +++ b/src/tests/slave_tests.cpp
> @@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)
> 
>AWAIT_READY(launchTask);
> 
> -  // Verify label key and value in slave state.json.
> +  //Verify label key and value in slave state.json.
>Future response =
>  process::http::get(slave.get(), "state.json");
> 
> And the git commit hook correctly failed it:
> $ git commit -m "test"
> Checking 1 files
> src/tests/slave_tests.cpp:2231:  Should have a space between // and comment  
> [whitespace/comments] [4]
> Total errors found: 1
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41595: Updated `Master::Http::state` to use `jsonify` in mesos.

[Alexander Rojas]

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


I was wondering what the results of the benchmark would be with an optimized 
build.

- Alexander Rojas


On Jan. 6, 2016, 3:48 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41595/
> ---
> 
> (Updated Jan. 6, 2016, 3:48 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 4f4cbf6e53588b72204f9628dea5696c71eb66a5 
>   src/common/http.cpp 7165551321bedb8a4d711a64d0d6d8fd15215424 
>   src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5 
> 
> Diff: https://reviews.apache.org/r/41595/diff/
> 
> 
> Testing
> ---
> 
> # Some preliminery numbers
> 
> These numbers are from my Ubuntu VM on my Mac OS X with @vinodkone's 
> benchmark test: [r40844](https://reviews.apache.org/r/40844/).
> 
> ### Before
> 
> ```
> [==] Running 36 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 36 tests from SlaveAndFrameworkCount/MasterState_BENCHMARK_Test
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/0
> Added 1000 slaves and 1 frameworks
> Received state.json response in 683.138216ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/0 (1436 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/1
> Added 1000 slaves and 50 frameworks
> Received state.json response in 550.636939ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/1 (1474 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/2
> Added 1000 slaves and 100 frameworks
> Received state.json response in 632.835236ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/2 (1491 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/3
> Added 1000 slaves and 200 frameworks
> Received state.json response in 584.035771ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/3 (1431 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/4
> Added 1000 slaves and 500 frameworks
> Received state.json response in 688.404348ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/4 (1586 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/5
> Added 1000 slaves and 1000 frameworks
> Received state.json response in 666.713683ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/5 (1590 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/6
> Added 5000 slaves and 1 frameworks
> Received state.json response in 3.916201532secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/6 (7852 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/7
> Added 5000 slaves and 50 frameworks
> Received state.json response in 3.362618796secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/7 (8315 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/8
> Added 5000 slaves and 100 frameworks
> Received state.json response in 3.126815189secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/8 (7153 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/9
> Added 5000 slaves and 200 frameworks
> Received state.json response in 3.079956539secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/9 (7534 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/10
> Added 5000 slaves and 500 frameworks
> Received state.json response in 3.222014521secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/10 (8129 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/11
> Added 5000 slaves and 1000 frameworks
> Received state.json response in 3.286657158secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/11 (8133 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/12
> Added 1 slaves and 1 frameworks
> Received state.json response in 7.332592151secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/12 
> (15639 ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/13
> Added 1 slaves and 50 frameworks
> Received state.json response in 6.998751033secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/13 
> (17175 ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/14
> Added 100

Re: Review Request 42172: Add documentation for logging and ContainerLogger.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 12, 2016, 7:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42172/
> ---
> 
> (Updated Jan. 12, 2016, 7:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4206
> https://issues.apache.org/jira/browse/MESOS-4206
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moves (and updates) logging flags from `configuration.md` into the new 
> `logging.md`.
> Adds documentation about logging in Master/Agent/Framework and the 
> `ContainerLogger`.
> 
> Allows the doc-linking pattern of `[link text](other_doc.md#Link-to-text)`.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md cbe7f5a338a0fc350c4b6c0e2f1f48bd0869ac34 
>   docs/home.md 344c5617dfb003453fa17974500d44a8f766cb16 
>   docs/logging.md PRE-CREATION 
>   docs/modules.md 1aad8e0958554c0219a287dffd6c6fb925edb025 
>   site/Rakefile 94833488c8e29f9b07e3cab3c8adce223f685679 
> 
> Diff: https://reviews.apache.org/r/42172/diff/
> 
> 
> Testing
> ---
> 
> Previewed on GitHub and via:
> ```
> docker build -t mesos/website support/site-docker
> docker run -it --rm -p 4567:4567 -v /path/to/mesos:/mesos mesos/website
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41783: Logger Module: Implement the rotating container logger module.

[Benjamin Hindman]

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



src/slave/container_loggers/rotating.hpp (line 103)


{ on newline please.



src/slave/container_loggers/rotating.cpp (lines 148 - 151)


Why not do this in the constructor and then no need for the `if 
(process.get() != NULL) {` everywhere?


- Benjamin Hindman


On Jan. 14, 2016, 2:12 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Jan. 14, 2016, 2:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a non-default ContainerLogger that constrains total log size by rotating 
> logs (i.e. renaming the head log file).
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/rotate.hpp PRE-CREATION 
>   src/slave/container_loggers/rotate.cpp PRE-CREATION 
>   src/slave/container_loggers/rotating.hpp PRE-CREATION 
>   src/slave/container_loggers/rotating.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41783/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41962: Logger Module: Add tests for module recovery after agent failover.

[Benjamin Hindman]

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



src/tests/container_logger_tests.cpp (lines 153 - 154)


Ugh, using something we passed in as `Owned` above? Have we still not 
cleaned up this pattern in the code base?


- Benjamin Hindman


On Jan. 12, 2016, 10:31 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41962/
> ---
> 
> (Updated Jan. 12, 2016, 10:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Artem Harutyunyan, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4150
> https://issues.apache.org/jira/browse/MESOS-4150
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds two heavily-mocked tests for the Mesos containerizer and Docker 
> containerizer.  Each checks that `ContainerLogger::recover` is called during 
> `Containerizer::recover`.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
> 
> Diff: https://reviews.apache.org/r/41962/diff/
> 
> 
> Testing
> ---
> 
> make (OSX & Centos7)
> 
> Tests are run in the next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41963: Logger Module: Implement ContainerLogger recovery.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 12, 2016, 10:31 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41963/
> ---
> 
> (Updated Jan. 12, 2016, 10:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4150
> https://issues.apache.org/jira/browse/MESOS-4150
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a call to `ContainerLogger::recover` inside each containerizer's 
> `Containerizer::recover`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp da19975c1f1e59fab0a96ebd9aa815bba2a35ece 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
> 
> Diff: https://reviews.apache.org/r/41963/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX & Centos7)
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT*" (Centos7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41782: Logger Module: Wire up the rotating container logger module and test.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 14, 2016, 2:12 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41782/
> ---
> 
> (Updated Jan. 14, 2016, 2:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Creates a new binary "mesos-rotate-logger" for use by the non-default 
> "Rotating" ContainerLogger module.
> Adds the `RotatingContainerLogger` to the test module configuration.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c 
>   src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475 
> 
> Diff: https://reviews.apache.org/r/41782/diff/
> 
> 
> Testing
> ---
> 
> Note: Some of the files added to the makefile are created in the next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41780: Changed ContainerLogger and Fetcher to not duplicate FDs.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 14, 2016, 2:12 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41780/
> ---
> 
> (Updated Jan. 14, 2016, 2:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem 
> Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change shifts the burden of FD lifecycle management away from 
> ContainerLogger module writers.
> This change also cleans up the Fetcher code slightly.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/container_logger.hpp 
> a2362070ead0afcef1e6c2ca784083ddb01ba51a 
>   src/slave/containerizer/fetcher.cpp 
> 4ac9149980cf7f013b318218a1b29369c5dcff17 
> 
> Diff: https://reviews.apache.org/r/41780/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41779: Extended Subprocess::FD to optionally not duplicate the given FD.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 14, 2016, 2:12 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41779/
> ---
> 
> (Updated Jan. 14, 2016, 2:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a subprocess is started with a FD, and the parent process does not need 
> the FD afterwards, the FD does not need to be duplicated.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> 8704cd0a8af92b848eb9ff7cffe8e5251c5dcfec 
>   3rdparty/libprocess/src/subprocess.cpp 
> 863523404b9ef1d2024c108f3e9e1c77c3916049 
> 
> Diff: https://reviews.apache.org/r/41779/diff/
> 
> 
> Testing
> ---
> 
> Tests are run in the next review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 42052: Refactored Subprocess::IO to improve readability.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 14, 2016, 1:55 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42052/
> ---
> 
> (Updated Jan. 14, 2016, 1:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced `int pipefd[2]` with the structs `InputFileDescriptors` and 
> `OutputFileDescriptors`.
> Replaced the `switch` statements inside `process::subprocess` with lambdas 
> defined in the static helpers (`PIPE`, `PATH`, `FD`).
> 
> Slight cleanup of Subprocess code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> 8704cd0a8af92b848eb9ff7cffe8e5251c5dcfec 
>   3rdparty/libprocess/src/subprocess.cpp 
> 863523404b9ef1d2024c108f3e9e1c77c3916049 
> 
> Diff: https://reviews.apache.org/r/42052/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41595: Updated `Master::Http::state` to use `jsonify` in mesos.

[Benjamin Hindman]

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

Ship it!



src/master/http.cpp (line 112)


s/Type_Name/Type::Name/ here?



src/master/http.cpp (line 116)


{ on newline please.



src/master/http.cpp (lines 135 - 146)


A basic comment that implies that a `Summary` representation is for the 
'/state-summary' endpoints and `Full` representation is for the '/state' 
endpoints (or maybe in the future for any "summary" endpoint) would be great. 
;-)



src/master/http.cpp (line 175)


I can't remember what the outcome of our converation about this was, but do 
you even need the `Full` representation? Could you just have a `void 
json(JSON::ObjectWriter* writer, const Slave& slave)` work without needing to 
explicitly use `Full`? If we need `Full` in addition to `Summary` a comment 
above over `Full` that explains that for others would be great!


- Benjamin Hindman


On Jan. 6, 2016, 2:48 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41595/
> ---
> 
> (Updated Jan. 6, 2016, 2:48 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 4f4cbf6e53588b72204f9628dea5696c71eb66a5 
>   src/common/http.cpp 7165551321bedb8a4d711a64d0d6d8fd15215424 
>   src/master/http.cpp d7afa2af989eebfc9039b3681f087ce570f601d5 
> 
> Diff: https://reviews.apache.org/r/41595/diff/
> 
> 
> Testing
> ---
> 
> # Some preliminery numbers
> 
> These numbers are from my Ubuntu VM on my Mac OS X with @vinodkone's 
> benchmark test: [r40844](https://reviews.apache.org/r/40844/).
> 
> ### Before
> 
> ```
> [==] Running 36 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 36 tests from SlaveAndFrameworkCount/MasterState_BENCHMARK_Test
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/0
> Added 1000 slaves and 1 frameworks
> Received state.json response in 683.138216ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/0 (1436 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/1
> Added 1000 slaves and 50 frameworks
> Received state.json response in 550.636939ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/1 (1474 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/2
> Added 1000 slaves and 100 frameworks
> Received state.json response in 632.835236ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/2 (1491 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/3
> Added 1000 slaves and 200 frameworks
> Received state.json response in 584.035771ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/3 (1431 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/4
> Added 1000 slaves and 500 frameworks
> Received state.json response in 688.404348ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/4 (1586 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/5
> Added 1000 slaves and 1000 frameworks
> Received state.json response in 666.713683ms
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/5 (1590 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/6
> Added 5000 slaves and 1 frameworks
> Received state.json response in 3.916201532secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/6 (7852 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/7
> Added 5000 slaves and 50 frameworks
> Received state.json response in 3.362618796secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/7 (8315 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/8
> Added 5000 slaves and 100 frameworks
> Received state.json response in 3.126815189secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/8 (7153 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/9
> Added 5000 slaves and 200 frameworks
> Received state.json response in 3.079956539secs
> [   OK ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/9 (7534 
> ms)
> [ RUN  ] SlaveAndFrameworkCount/MasterState_BENCHMARK_Test.State/10
> Added 5000 slaves and 500 frameworks
> Received state.json response

Re: Review Request 41946: Added a `Representation` abstraction to stout.

[Benjamin Hindman]

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

Ship it!


LGTM, but see issues on documentation and comments please.

This is a really nice addition MPark, thank you! Are you also going to follow 
up and replace our current `RFC*` implemenetations too?


3rdparty/libprocess/3rdparty/stout/include/stout/representation.hpp (line 36)


Can you document why you need this please? What happens if you don't add 
this?



3rdparty/libprocess/3rdparty/stout/include/stout/representation.hpp (line 58)


{ on newline please.



3rdparty/libprocess/3rdparty/stout/include/stout/representation.hpp (line 68)


Why do you need this? Can you add a comment please?


- Benjamin Hindman


On Jan. 6, 2016, 10 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41946/
> ---
> 
> (Updated Jan. 6, 2016, 10 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/representation.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41946/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 41594: Added support for `jsonify` result to `OK` response in libprocess.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 5, 2016, 10:19 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41594/
> ---
> 
> (Updated Jan. 5, 2016, 10:19 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 084475ac4ef58c60da67a8d50ac031a5591eb335 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/http.cpp 06231d96c6c99cada0cd46d6ef1e3f64039215c2 
> 
> Diff: https://reviews.apache.org/r/41594/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 41593: Added `jsonify` function to stout.

[Benjamin Hindman]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/README.md (line 250)


{ on newline.



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 637)


Are you pulling this variable out to optimize away the calls to 
`field_count()`?



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 659)


Why do you need to pull this variable out?



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 699)


s/str/s/g please.



3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp (line 85)


{ on newline please (below too).



3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp (lines 148 - 150)


:-) !!


- Benjamin Hindman


On Jan. 13, 2016, 12:57 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41593/
> ---
> 
> (Updated Jan. 13, 2016, 12:57 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5ab7bc4966fe32eaddd573a4dbfd997f98b5d481 
>   3rdparty/libprocess/3rdparty/stout/README.md 
> a2a3dc6120b10c8fe82088b9f0ebecfa5642f945 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ec851dcb08d938defeb6066810011e003d594b29 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 017cef466a30e73af36d72332514a4dbb2beb6bc 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 62ad461eb228b688f1ceac16cfb003561ed5a806 
>   3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 34bb2523ebf3d6d390cbfe1623b89fae0053b712 
> 
> Diff: https://reviews.apache.org/r/41593/diff/
> 
> 
> Testing
> ---
> 
> Added tests to ``
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42274: Added common command utils file.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41958, 41959, 42156, 42157, 42274]

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

- Mesos ReviewBot


On Jan. 13, 2016, 11:13 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42274/
> ---
> 
> (Updated Jan. 13, 2016, 11:13 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This common file is a good place to add common command line utilities like 
> tar,
> digests(sha256, sha512, etc). Currently this functionality is spread in the 
> code
> base and this change would enable all those call sites to be replaced with a
> common code.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42274/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41462: Used SFINAE-friendly `result_of` in libprocess.

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On Jan. 6, 2016, 2:28 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41462/
> ---
> 
> (Updated Jan. 6, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Alex Clemmer, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4220
> https://issues.apache.org/jira/browse/MESOS-4220
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used the SFINAE `result_of` implemented in 
> [r41461](https://reviews.apache.org/r/41461/).
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/async.hpp 
> b80b60a7ceb5e87eed8cdab17144ae9617485a04 
>   3rdparty/libprocess/include/process/future.hpp 
> bcb5668565298825056f1b48d48efe12d2e56e7c 
> 
> Diff: https://reviews.apache.org/r/41462/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 41461: Added SFINAE-friendly `result_of` to stout.

[Benjamin Hindman]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp (line 35)


s/fail/Fail/ to be consistent with `Prefer` and `LessPrefer`?


- Benjamin Hindman


On Jan. 8, 2016, 10:07 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41461/
> ---
> 
> (Updated Jan. 8, 2016, 10:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Alex Clemmer, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4220
> https://issues.apache.org/jira/browse/MESOS-4220
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> VS 2015 won't support C++14 `std::result_of` SFINAE until Update 2, so 
> `result_of` must be replaced with `decltype(invoke)`.
> 
> Here, we implement SFINAE `result_of` in `stout`.
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41461/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 41460: Used `std::is_bind_expression` to SFINAE correctly.

[Benjamin Hindman]

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

Ship it!


This LGTM; IIUC we're going to rely on the lexical substitution of the template 
parameters and that we're going to just do `typename =` instead of `typename G 
=`?

- Benjamin Hindman


On Jan. 6, 2016, 2:27 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41460/
> ---
> 
> (Updated Jan. 6, 2016, 2:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Alex Clemmer, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4228
> https://issues.apache.org/jira/browse/MESOS-4228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Standard (C++11 through 17) does not require `std::bind`'s function call 
> operator to SFINAE, and VS 2015's doesn't. `std::is_bind_expression` can be 
> used to manually reroute bind expressions to the 1-arg overload, where 
> (conveniently) the argument will be ignored if necessary.
> 
> Follow-up from [r40114](https://reviews.apache.org/r/40114/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> bcb5668565298825056f1b48d48efe12d2e56e7c 
> 
> Diff: https://reviews.apache.org/r/41460/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OS X, compiled on Windows.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 41772: Added helper function to flatten resources.

[Klaus Ma]

> On Jan. 7, 2016, 8:31 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 894-904
> > 
> >
> > This seems more appropriate as an optional parameter for 
> > `Resources::flatten`, just like `Option`.
> 
> Guangya Liu wrote:
> My original thinking was adding an optional parameter to flatten(), the 
> reason that I did not do that is because every time I want to set 
> ALLOCATION_SLACK or USAGE_SLACK for a resource, I need to put all paramters, 
> it is a long sentence, so I just add this new helper.
> 
> The caller need to call {flatten("*", None(), 
> Resource::RevocableInfo::ALLOCATION_SLACK);} if want to set a resources as 
> allocation slack.
> 
> Joseph Wu wrote:
> It's fine to have somewhat long expressions.  I think it's preferable 
> over having too many helpers.
> 
> Guangya Liu wrote:
> @Joseph, seems we need to reach an agreement for this ;-) Can you please 
> take a look at the following patches in the patch chain, you may notice that 
> other patches highly depend on those helper functions.
> 
> So what about the following:
> 1) Rename flattenReserved() to reserved_() which will return all reserved 
> resources with different roles in a flatten mode.
> 2) Remove flattenSlack() and merge it to flatten().
> 
> Comments?
> 
> Klaus Ma wrote:
> regarding 1), I'd like to add `allocationSlackable` which mean the 
> reosurces can be offered as `ALLOCATION_SLACK`, it's peer of 
> `allocationSlack`.
> 
> Guangya Liu wrote:
> `allcationSlackable` is more like a `bool` function, but here I want to 
> return a `flatten reserved` resources.

It's not about `bool` :). I'd like to avoid the backgroud of 
`ALLOCATION_SLACK-able` == `stateless.reserved`: the general point is 
`ALLOCATION_SLACK-able` resources can be lend to other Tenant framework, and 
those resource will be preempted when Lender framework launch tasks. That's 
major reason that I'd suggest to introduce helper fuction to avoid 
`stateless.reserved` in Master/Agent source code.

En, maybe this can be treat as new MVP named borrow/lend policy worked with 
Quota; it's different with Optimistic Offer Phase 2, the revocalbe resources 
will not send to multiple framework; the revocable resources will be shared 
between frameworks by ratio. @Joseph/Ben, any comments.


- Klaus


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


On Jan. 13, 2016, 8:16 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41772/
> ---
> 
> (Updated Jan. 13, 2016, 8:16 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4267
> https://issues.apache.org/jira/browse/MESOS-4267
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two new helper functions to flatten resources.
> 1) Flatten reserved resources.
> 2) Flatten allocation slack revocable resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41772/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="ResourcesOperationTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42164: Allowed (un)reserve operations without a principal.

[Guangya Liu]

> On 一月 13, 2016, 5:22 a.m., Michael Park wrote:
> > src/master/validation.cpp, line 703
> > 
> >
> > First thing here is that the `principal` field in `ReservationInfo` is 
> > still marked `required`. What's our plan for changing it to `optional`?
> > 
> > Second thing is that even if `principal.isNone()`, if is authorization 
> > is turned off we don't care, right? That is, if authorization is turned 
> > off, even frameworks without a principal can reserve any resources?
> 
> Greg Mann wrote:
> Regarding your first question: per our discussion, we do indeed need a 
> good plan for migrating to an `optional` principal within `ReservationInfo`. 
> It would seem that we have to choose between two less-than-ideal situations: 
> either we tolerate HTTP endpoints that don't work when authentication is 
> disabled, or we enforce that frameworks cannot register with `principal == 
> ""`, so that we can use the empty string to represent a null principal while 
> we migrate to an `optional` principal over a deprecation cycle.
> 
> To your second point: again per our discussion, semantically the 
> principal within `ReservationInfo` represents the principal (or lack thereof) 
> which reserved the resources, so we should enforce that it matches the 
> principal of the framework/operator, even if authorization is not enabled.

If authorization is not enabled, then it is not a must to set principal for a 
framework, so how can we enforce the match of principal?


- Guangya


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


On 一月 13, 2016, 2:45 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42164/
> ---
> 
> (Updated 一月 13, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
> https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed (un)reserve operations without a principal.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp 
> fbf8fadbc04a7cbc60ee6091e0224339389b400f 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/42164/diff/
> 
> 
> Testing
> ---
> 
> Two master validation tests were removed which tested to make sure that 
> reserve and unreserve operations would not succeed without a principal.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42273: Modified scheduler library to move the queue contents before `async`.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 一月 13, 2016, 11:06 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42273/
> ---
> 
> (Updated 一月 13, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This trivial fix invokes `std::move` on the `queue` object to not perform a 
> copy before invoking the `async(..)` call.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp a17872b46ec600e0fae6c43247ccb63f5ee55ac0 
> 
> Diff: https://reviews.apache.org/r/42273/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41772: Added helper function to flatten resources.

[Guangya Liu]

> On 一月 7, 2016, 12:31 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 894-904
> > 
> >
> > This seems more appropriate as an optional parameter for 
> > `Resources::flatten`, just like `Option`.
> 
> Guangya Liu wrote:
> My original thinking was adding an optional parameter to flatten(), the 
> reason that I did not do that is because every time I want to set 
> ALLOCATION_SLACK or USAGE_SLACK for a resource, I need to put all paramters, 
> it is a long sentence, so I just add this new helper.
> 
> The caller need to call {flatten("*", None(), 
> Resource::RevocableInfo::ALLOCATION_SLACK);} if want to set a resources as 
> allocation slack.
> 
> Joseph Wu wrote:
> It's fine to have somewhat long expressions.  I think it's preferable 
> over having too many helpers.
> 
> Guangya Liu wrote:
> @Joseph, seems we need to reach an agreement for this ;-) Can you please 
> take a look at the following patches in the patch chain, you may notice that 
> other patches highly depend on those helper functions.
> 
> So what about the following:
> 1) Rename flattenReserved() to reserved_() which will return all reserved 
> resources with different roles in a flatten mode.
> 2) Remove flattenSlack() and merge it to flatten().
> 
> Comments?
> 
> Klaus Ma wrote:
> regarding 1), I'd like to add `allocationSlackable` which mean the 
> reosurces can be offered as `ALLOCATION_SLACK`, it's peer of 
> `allocationSlack`.

`allcationSlackable` is more like a `bool` function, but here I want to return 
a `flatten reserved` resources.


- Guangya


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


On 一月 13, 2016, 12:16 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41772/
> ---
> 
> (Updated 一月 13, 2016, 12:16 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4267
> https://issues.apache.org/jira/browse/MESOS-4267
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two new helper functions to flatten resources.
> 1) Flatten reserved resources.
> 2) Flatten allocation slack revocable resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41772/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="ResourcesOperationTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42263: Added more structure for containerizer related subpages in home.md.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [42263]

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

- Mesos ReviewBot


On Jan. 13, 2016, 11:10 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42263/
> ---
> 
> (Updated Jan. 13, 2016, 11:10 p.m.)
> 
> 
> Review request for mesos, Joerg Schad, Jojy Varghese, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added more structure for containerizer related subpages in home.md.
> 
> 
> Diffs
> -
> 
>   docs/home.md 344c5617dfb003453fa17974500d44a8f766cb16 
> 
> Diff: https://reviews.apache.org/r/42263/diff/
> 
> 
> Testing
> ---
> 
> Viewed via Docker website generator.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 42212: Added unit test for framework/task history flags.

[Kevin Klues]

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



src/tests/master_tests.cpp (line 3989)


As bmahler pointed out offline, mesos doesn't typically use loop variables 
such as 'fid' and 'tid'. It prefers simple variable names such as {i, j} or 
more explicit variable names such as {frameworkIndex, taskIndex}.  I will 
update this throughout.



src/tests/master_tests.cpp (line 4104)


As bmahler pointed out offline, this comment is unnecessary. It doesn't add 
to the story being told by this code and implies that such a comment would be 
needed in all tests that called Shutdown().

What I meant to clarify was that we were shutting down the current master 
for this iteration of the loop -- which isn't necessarily clear and does 
deserve a comment. I plan to update the comment accordingly.



src/tests/master_tests.cpp (line 4106)


As bmahler pointed out offline, we should really be using 
Stop(master.get()) here instead of shutdown.

I originally called Shutdown() due to a comment in MesosTest::Shutdown() 
about properly unsetting the authenticators on the master upon shutdown. 
However, bmahler said this shouldn't be an issue for the current test and that 
Stop() is more appropriate in this loop with a single master per iteration.


- Kevin Klues


On Jan. 13, 2016, 11:01 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42212/
> ---
> 
> (Updated Jan. 13, 2016, 11:01 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3307
> https://issues.apache.org/jira/browse/MESOS-3307
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a unit test to verify that the the max_frameworks and
> max_tasks_per_frameworks flags for master work properly. Specifically,
> we test to verify that the proper amount of history is maintained for
> both 0 values to these flags as well as positive values <= to the total
> number frameworks and tasks per framework actually launched.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 
> 
> Diff: https://reviews.apache.org/r/42212/diff/
> 
> 
> Testing
> ---
> 
> This is a unit test.  I ran it on my mac and on ubuntu 14.04.
> 
> GTEST_FILTER="MasterTest.FrameworksTasksCompletedFlags" make check -j 7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42098: Added unit test-case for CgroupsNetClsIsolatorProcess.

[Avinash sridharan]

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

(Updated Jan. 14, 2016, 2:42 a.m.)


Review request for mesos, Jie Yu and Joseph Wu.


Repository: mesos


Description
---

Added unit test-case for CgroupsNetClsIsolatorProcess.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
91178b69ccbf5b6cbf64421e5602e6d554fc34ca 

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


Testing
---


Thanks,

Avinash sridharan

Re: Review Request 42212: Added unit test for framework/task history flags.

[Kevin Klues]

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



src/tests/master_tests.cpp (line 4090)


Talking to bmahler offline, we can now use -> to avoid calling .get() on 
{Option, Result, Try, Future, etc.}.  I plan to patch up all instance of this 
in this test.


- Kevin Klues


On Jan. 13, 2016, 11:01 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42212/
> ---
> 
> (Updated Jan. 13, 2016, 11:01 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3307
> https://issues.apache.org/jira/browse/MESOS-3307
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a unit test to verify that the the max_frameworks and
> max_tasks_per_frameworks flags for master work properly. Specifically,
> we test to verify that the proper amount of history is maintained for
> both 0 values to these flags as well as positive values <= to the total
> number frameworks and tasks per framework actually launched.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 
> 
> Diff: https://reviews.apache.org/r/42212/diff/
> 
> 
> Testing
> ---
> 
> This is a unit test.  I ran it on my mac and on ubuntu 14.04.
> 
> GTEST_FILTER="MasterTest.FrameworksTasksCompletedFlags" make check -j 7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42212: Added unit test for framework/task history flags.

[Kevin Klues]

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


Talking to bmahler offline, we decided to split the current test into two (more 
isolated) tests, which test the max_completed_frameworks and 
max_completed_tasks_per_framework flags separately.

- Kevin Klues


On Jan. 13, 2016, 11:01 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42212/
> ---
> 
> (Updated Jan. 13, 2016, 11:01 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3307
> https://issues.apache.org/jira/browse/MESOS-3307
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a unit test to verify that the the max_frameworks and
> max_tasks_per_frameworks flags for master work properly. Specifically,
> we test to verify that the proper amount of history is maintained for
> both 0 values to these flags as well as positive values <= to the total
> number frameworks and tasks per framework actually launched.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 
> 
> Diff: https://reviews.apache.org/r/42212/diff/
> 
> 
> Testing
> ---
> 
> This is a unit test.  I ran it on my mac and on ubuntu 14.04.
> 
> GTEST_FILTER="MasterTest.FrameworksTasksCompletedFlags" make check -j 7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 41783: Logger Module: Implement the rotating container logger module.

[Joseph Wu]

> On Jan. 13, 2016, 11:25 a.m., Benjamin Hindman wrote:
> >

Note: I'm still gathering feedback regarding the naming scheme of the rotated 
logs.


> On Jan. 13, 2016, 11:25 a.m., Benjamin Hindman wrote:
> > src/slave/container_loggers/rotate.hpp, line 54
> > 
> >
> > How about s/of log/of rotated log/ here?
> > 
> > Also, is 'head' better than 'current'? Just a suggestion, given that 
> > head sounds like "first", I thought by head you meant the oldest log file, 
> > not the newest.

"Current" doesn't fit very well in some comments, so I renamed to "Leading" 
instead.  (And renamed some variables.)


> On Jan. 13, 2016, 11:25 a.m., Benjamin Hindman wrote:
> > src/slave/container_loggers/rotate.cpp, lines 35-37
> > 
> >
> > These pulled out?

Yup, done in diff#4.


- Joseph


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


On Jan. 13, 2016, 6:12 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41783/
> ---
> 
> (Updated Jan. 13, 2016, 6:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a non-default ContainerLogger that constrains total log size by rotating 
> logs (i.e. renaming the head log file).
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/rotate.hpp PRE-CREATION 
>   src/slave/container_loggers/rotate.cpp PRE-CREATION 
>   src/slave/container_loggers/rotating.hpp PRE-CREATION 
>   src/slave/container_loggers/rotating.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41783/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41779: Extended Subprocess::FD to optionally not duplicate the given FD.

[Joseph Wu]

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

(Updated Jan. 13, 2016, 6:12 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Rebased on a patch that refactors `Subprocess::IO`.


Summary (updated)
-

Extended Subprocess::FD to optionally not duplicate the given FD.


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


Repository: mesos


Description
---

If a subprocess is started with a FD, and the parent process does not need the 
FD afterwards, the FD does not need to be duplicated.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
8704cd0a8af92b848eb9ff7cffe8e5251c5dcfec 
  3rdparty/libprocess/src/subprocess.cpp 
863523404b9ef1d2024c108f3e9e1c77c3916049 

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


Testing
---

Tests are run in the next review.


Thanks,

Joseph Wu

Re: Review Request 41783: Logger Module: Implement the rotating container logger module.

[Joseph Wu]

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

(Updated Jan. 13, 2016, 6:12 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

* Reworked some flag comments.  Moved validation code into flags.
* Removed `logger_log_filename` flag in favor of printing to the agent's stderr 
upon (critical) failure.
* Changed types of `max_..._size` flags to `Bytes`.
* Renamed `outgoing` and `head` language to `leading`.


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


Repository: mesos


Description
---

Adds a non-default ContainerLogger that constrains total log size by rotating 
logs (i.e. renaming the head log file).


Diffs (updated)
-

  src/slave/container_loggers/rotate.hpp PRE-CREATION 
  src/slave/container_loggers/rotate.cpp PRE-CREATION 
  src/slave/container_loggers/rotating.hpp PRE-CREATION 
  src/slave/container_loggers/rotating.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 41782: Logger Module: Wire up the rotating container logger module and test.

[Joseph Wu]

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

(Updated Jan. 13, 2016, 6:12 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Remove the `logger_log_filename` flag.  Change type of `max_stdout_size` flag.


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


Repository: mesos


Description
---

Creates a new binary "mesos-rotate-logger" for use by the non-default 
"Rotating" ContainerLogger module.
Adds the `RotatingContainerLogger` to the test module configuration.


Diffs (updated)
-

  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/tests/module.hpp 49a9203fbbef013a47367c1912c6b642e8b6bd5c 
  src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475 

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


Testing
---

Note: Some of the files added to the makefile are created in the next review.


Thanks,

Joseph Wu

Re: Review Request 41780: Changed ContainerLogger and Fetcher to not duplicate FDs.

[Joseph Wu]

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

(Updated Jan. 13, 2016, 6:12 p.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem 
Harutyunyan.


Changes
---

Tweaked a comment.  + Rebase.


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


Repository: mesos


Description
---

This change shifts the burden of FD lifecycle management away from 
ContainerLogger module writers.
This change also cleans up the Fetcher code slightly.


Diffs (updated)
-

  include/mesos/slave/container_logger.hpp 
a2362070ead0afcef1e6c2ca784083ddb01ba51a 
  src/slave/containerizer/fetcher.cpp 4ac9149980cf7f013b318218a1b29369c5dcff17 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 40940: Windows: Modified `launch.cpp`

[Daniel Pravat]

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

(Updated Jan. 14, 2016, 2:10 a.m.)


Review request for mesos, Alex Naparu, Alex Clemmer, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Windows: Modified `launch.cpp`


Diffs (updated)
-

  src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
  src/launcher/executor.cpp 356d311fdf97b2c4663c60e13ede7cdb71a264c7 
  src/slave/containerizer/mesos/launch.cpp 
0cfdc0ca1a7e8c6107f8ea46e7a4c6e52811d7ac 

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


Testing
---

OSX: make check
Windows: make.bat


Thanks,

Daniel Pravat

Re: Review Request 41491: Unified Container: Exposed docker/appc image manifest to mesos containerizer.

[Gilbert Song]

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

(Updated Jan. 13, 2016, 6:03 p.m.)


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


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


Repository: mesos


Description
---

Unified Container: Exposed docker/appc image manifest to mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
03425daaf015cf231920b7eda1d5424895a41286 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
b2e23d82ea255e1c91d7537a58f86632763d9a56 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
89d06e94bd2255bb4f2dad3fae75c1b3d789611d 
  src/slave/containerizer/mesos/provisioner/store.hpp 
aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
  src/tests/containerizer/provisioner_docker_tests.cpp 
8d6a06057c7600aeb1aca76d4dfadc45b6eae99d 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 42266: Updated post-reviews.py to strip review URL in RR summary.

[Till Toenshoff]

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

Ship it!


The whole RBT amending now now works like a charm - thanks Kevin!

- Till Toenshoff


On Jan. 14, 2016, 1:29 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42266/
> ---
> 
> (Updated Jan. 14, 2016, 1:29 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, when setting GUESS_FIELDS in .reviewboardrc to automatically
> update the summary message when posting a review, the summary would
> include the review URL in the summary message. This was due to the fact
> that GUESS_FILEDS simply copies the entirety of the commit message into
> the RR summary field on reviewboard. Including this line is both
> redundant, and, in some cases, has been found to appear (at least) twice
> in the summary message instead of just once.
> 
> This commit ensures that the commit message posted when GUESS_FIELDS is
> set will have the review URL stripped before posting.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 170be83aa6dca6e8175292169d78e8f7915f7e6e 
> 
> Diff: https://reviews.apache.org/r/42266/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 41617: Added a new category called whitespace/mesos-comments to capture missing, leading, white-space in comments

[Avinash sridharan]

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

(Updated Jan. 14, 2016, 1:59 a.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Added a new category called whitespace/mesos-comments to capture missing, 
leading, white-space in comments


Diffs (updated)
-

  support/cpplint.py 6890e27f92603b025e25e4db01decf351c33c9a1 
  support/mesos-style.py 5ee17593153df7815426918a1da6e25cb1188bca 

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


Testing (updated)
---

Ran the modified mesos-style.py on the existing code-base and got the following 
errors:
src/authentication/cram_md5/authenticatee.hpp:60:  Should have a space between 
// and comment  [whitespace/comments] [4]
src/authentication/cram_md5/authenticator.hpp:60:  Should have a space between 
// and comment  [whitespace/comments] [4]
src/authorizer/local/authorizer.hpp:84:  Should have a space between // and 
comment  [whitespace/comments] [4]
src/tests/containerizer/docker_containerizer_tests.cpp:978:  Should have a 
space between // and comment  [whitespace/comments] [4]
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp:90:  At 
least a single space is required between code and comments  
[whitespace/comments] [2]
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp:127:  Should have a 
space between // and comment  [whitespace/comments] [4]
  Doesn't seem to give any false-positives. All the above errors do have a 
missing leading white-space. 
  
  Tried a commit for the following patch:
  diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index eb47b53..e579d20 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)

   AWAIT_READY(launchTask);

-  // Verify label key and value in slave state.json.
+  //Verify label key and value in slave state.json.
   Future response =
 process::http::get(slave.get(), "state.json");

And the git commit hook correctly failed it:
$ git commit -m "test"
Checking 1 files
src/tests/slave_tests.cpp:2231:  Should have a space between // and comment  
[whitespace/comments] [4]
Total errors found: 1


Thanks,

Avinash sridharan

Re: Review Request 42052: Refactored Subprocess::IO to improve readability.

[Joseph Wu]

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

(Updated Jan. 13, 2016, 5:55 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Got rid of the abstract type, in favor of Lambdas!!


Summary (updated)
-

Refactored Subprocess::IO to improve readability.


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


Repository: mesos


Description (updated)
---

Replaced `int pipefd[2]` with the structs `InputFileDescriptors` and 
`OutputFileDescriptors`.
Replaced the `switch` statements inside `process::subprocess` with lambdas 
defined in the static helpers (`PIPE`, `PATH`, `FD`).

Slight cleanup of Subprocess code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
8704cd0a8af92b848eb9ff7cffe8e5251c5dcfec 
  3rdparty/libprocess/src/subprocess.cpp 
863523404b9ef1d2024c108f3e9e1c77c3916049 

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


Testing (updated)
---

make check


Thanks,

Joseph Wu

Review Request 42241: Speed up HookTest.VerifySlaveLaunchExecutorHook.

[Jian Qiu]

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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Add Clock::advance to fasten the speed of executor to be shutdown.


Diffs
-

  src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 

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


Testing
---

Before
HookTest.VerifySlaveLaunchExecutorHook (5061 ms)

After
HookTest.VerifySlaveLaunchExecutorHook (132 ms)


Thanks,

Jian Qiu

Re: Review Request 42282: Test commit 4.

[Till Toenshoff]

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

(Updated Jan. 14, 2016, 1:37 a.m.)


Review request for mesos and Kevin Klues.


Summary (updated)
-

Test commit 4.


Repository: mesos


Description (updated)
---

Yo test this! AGAIN! And now really!


Diffs (updated)
-

  configure.ac 40d60a63cdba41d06305f09141f4d14d6e229d95 

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


Testing
---


Thanks,

Till Toenshoff

Re: Review Request 35711: Disallow special characters in role name.

[Klaus Ma]

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

Ship it!


Ship It!

- Klaus Ma


On Jan. 14, 2016, 1:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35711/
> ---
> 
> (Updated Jan. 14, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2210
> https://issues.apache.org/jira/browse/MESOS-2210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disallow special characters in role name.
> 
> 
> Diffs
> -
> 
>   docs/roles.md af1adad7ec8122fd10f7de44848014b850416bcd 
>   include/mesos/roles.hpp PRE-CREATION 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/common/roles.cpp PRE-CREATION 
>   src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/tests/role_tests.cpp 373ae267b85588fe491ab0a0ce8aa195f971aac3 
> 
> Diff: https://reviews.apache.org/r/35711/diff/
> 
> 
> Testing
> ---
> 
> make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42266: Updated post-reviews.py to strip review URL in RR summary.

[Kevin Klues]

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

(Updated Jan. 14, 2016, 1:29 a.m.)


Review request for mesos, Artem Harutyunyan and Till Toenshoff.


Changes
---

Explicitly set "stripped_message = message[:pos]" in the block of code that 
creates the temporary commit with the review URL stripped.  Previously, this 
was hidden at the end of 'git commit' line so it was a bit confusing how this 
stripping was taking place.


Repository: mesos


Description
---

Previously, when setting GUESS_FIELDS in .reviewboardrc to automatically
update the summary message when posting a review, the summary would
include the review URL in the summary message. This was due to the fact
that GUESS_FILEDS simply copies the entirety of the commit message into
the RR summary field on reviewboard. Including this line is both
redundant, and, in some cases, has been found to appear (at least) twice
in the summary message instead of just once.

This commit ensures that the commit message posted when GUESS_FIELDS is
set will have the review URL stripped before posting.


Diffs (updated)
-

  support/post-reviews.py 170be83aa6dca6e8175292169d78e8f7915f7e6e 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 40115: Windows: Added support for `slave/gc.cpp`.

[Daniel Pravat]

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

(Updated Jan. 14, 2016, 1:15 a.m.)


Review request for Alex Naparu and Alex Clemmer.


Summary (updated)
-

Windows: Added support for `slave/gc.cpp`.


Repository: mesos


Description (updated)
---

Windows: Added support for `slave/gc.cpp`.


Diffs
-

  src/CMakeLists.txt 0d46043dd06007f2cba56626c82e8edd251cb8fb 
  src/slave/gc.cpp 7a8c69b4410df46ca8fd6ac009cc14e8fe5ff6d3 

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


Testing
---

Windows 10: make.bat
OSX: make check
Ubuntu: 15.1 make check


Thanks,

Daniel Pravat

Re: Review Request 40620: Windows: Added suppport for `slave/monitor.cpp`.

[Daniel Pravat]

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

(Updated Jan. 14, 2016, 1:15 a.m.)


Review request for Alex Naparu, Alex Clemmer, M Lawindi, and Yi Sun.


Summary (updated)
-

Windows: Added suppport for `slave/monitor.cpp`.


Repository: mesos


Description
---

Windows: Added suppport for `slave/monitor.cpp`.


Diffs
-

  src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 

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


Testing
---

OSX: make
Windows: make


Thanks,

Daniel Pravat

Re: Review Request 40620: Windows: Added suppport for `slave/monitor.cpp'.

[Daniel Pravat]

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

(Updated Jan. 14, 2016, 1:14 a.m.)


Review request for Alex Naparu, Alex Clemmer, M Lawindi, and Yi Sun.


Repository: mesos


Description (updated)
---

Windows: Added suppport for `slave/monitor.cpp`.


Diffs
-

  src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 

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


Testing
---

OSX: make
Windows: make


Thanks,

Daniel Pravat

Re: Review Request 40620: Windows: Added suppport for `slave/monitor.cpp'.

[Daniel Pravat]

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

(Updated Jan. 14, 2016, 1:13 a.m.)


Review request for Alex Naparu, Alex Clemmer, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Windows: Added suppport for `slave/monitor.cpp'.


Diffs (updated)
-

  src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 

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


Testing
---

OSX: make
Windows: make


Thanks,

Daniel Pravat

Re: Review Request 42035: Windows: Removed the `--switch_user` flag in Windows.

[Alex Clemmer]

> On Jan. 13, 2016, 6:21 p.m., Daniel Pravat wrote:
> > src/slave/flags.cpp, line 196
> > 
> >
> > There is a reason why this change is so extensive. This block should be 
> > replaced with a reasonable default for switch_user in Windows and it should 
> > be enough.

Per our Slack discussion, we've decided this isn't a blocking issue.

The question was about why we would not compile this flag for Windows 
specifically, and why it is acceptable here, but not elsewhere in the codebase. 
The answer is that we already conditionally don't compile other Linux-specific 
features, so there is precedent in this code for not compiling features that 
don't exist on Windows. The reason we _need_ to do this (or something like it) 
because the POSIX notion of `chown` simply doesn't exist on Windows. So, the 
result is this patch. Specifically, we `#ifdef` out the parts of the code that 
would result in `chown` being called, which is very much inline with other 
parts of the same function that are conditionally compiled only on certain 
types of Linux.


- Alex


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


On Jan. 7, 2016, 9:47 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42035/
> ---
> 
> (Updated Jan. 7, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Jie 
> Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4310
> https://issues.apache.org/jira/browse/MESOS-4310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Mesos agent provides a flag, `--[no-]switch_user`, which instructs
> the agent to run tasks as the user who submitted the task instead of the
> user that is running the agent process itself.
> 
> On POSIX, this requires the usual suspects (`su`, `chown`, and `chroot`,
> for example to do things like make sure the correct user owns the
> executor directory, and things like that.
> 
> On Windows, many of these operations have very different semantics. To
> use one example, to `chown` on Windows, you normally have to give
> *permission* for something to `chown` an object, and then that thing
> actually has to use that permission to `chown` it, on its own.
> 
> Clearly a semantic mismatch of this magnitude will require some work to
> formally bridge, and the result is that we will simply disable the
> functionality entirely on the Windows agent until we are prepared to go
> back and support it formally.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
> 
> Diff: https://reviews.apache.org/r/42035/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 42265: Fixed more tests that didn't set a shutdown expect for MockExecutor.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [42265]

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

Error:
 + : ubuntu:14.04
+ : gcc
+ : --verbose
+++ dirname ./support/docker_build.sh
++ cd ./support/..
++ pwd
+ MESOS_DIRECTORY=/home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ cd /home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ DOCKERFILE=Dockerfile
+ rm -f Dockerfile
+ case $OS in
+ append_dockerfile 'FROM ubuntu:14.04'
+ echo FROM ubuntu:14.04
+ append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
+ echo RUN rm -rf 
/var/lib/apt/lists/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
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_i18n_Translation-en
 /var/lib/apt/lists
 /us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_source_Sources 
/var/lib/apt/lists/us.archive.ubuntu.com

Re: Review Request 42035: Windows: Removed the `--switch_user` flag in Windows.

[Daniel Pravat]

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

Ship it!


Ship It!

- Daniel Pravat


On Jan. 7, 2016, 9:47 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42035/
> ---
> 
> (Updated Jan. 7, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Jie 
> Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4310
> https://issues.apache.org/jira/browse/MESOS-4310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Mesos agent provides a flag, `--[no-]switch_user`, which instructs
> the agent to run tasks as the user who submitted the task instead of the
> user that is running the agent process itself.
> 
> On POSIX, this requires the usual suspects (`su`, `chown`, and `chroot`,
> for example to do things like make sure the correct user owns the
> executor directory, and things like that.
> 
> On Windows, many of these operations have very different semantics. To
> use one example, to `chown` on Windows, you normally have to give
> *permission* for something to `chown` an object, and then that thing
> actually has to use that permission to `chown` it, on its own.
> 
> Clearly a semantic mismatch of this magnitude will require some work to
> formally bridge, and the result is that we will simply disable the
> functionality entirely on the Windows agent until we are prepared to go
> back and support it formally.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
> 
> Diff: https://reviews.apache.org/r/42035/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 42016: Windows: Apply patch.exe without elevation prompt

[M Lawindi]

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

(Updated Jan. 14, 2016, 12:43 a.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Alex Clemmer, M Lawindi, 
and Yi Sun.


Repository: mesos


Description (updated)
---

Windows:[2/2] Added zookeeper-3.4.5 to Mesos build


Diffs (updated)
-

  3rdparty/CMakeLists.txt ac5c25a8797a687e84384682975ab99fb3e30448 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
34e61ff90eca0ffdddb6b6b8e2f8e552691637fa 
  3rdparty/patch.exe.manifest PRE-CREATION 
  CMakeLists.txt 9b7044b7860fd64b854ac27b28a48d297dfdeae8 
  src/slave/cmake/SlaveConfigure.cmake cf378a27297474b2a9f338e0c832612370f7302a 

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


Testing
---


Thanks,

M Lawindi

Re: Review Request 42018: Windows: Updating ZK patch. Update to winconfig.h. Ignore deprecated warnings

[M Lawindi]

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

(Updated Jan. 14, 2016, 12:43 a.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Alex Clemmer, M Lawindi, 
and Yi Sun.


Repository: mesos


Description (updated)
---

Windows:[1/2] Updated zookeeper-3.4.5.patch to fix VS2015 build.


Diffs (updated)
-

  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing
---


Thanks,

M Lawindi

Re: Review Request 42238: Implemented the Docker URI fetcher plugin based on curl.

[Jojy Varghese]

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



src/uri/fetchers/docker.cpp (line 280)


Why not use Path here instead of string? Here and other places.



src/uri/fetchers/docker.cpp (line 375)


recursive mkdir?


- Jojy Varghese


On Jan. 13, 2016, 11:14 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42238/
> ---
> 
> (Updated Jan. 13, 2016, 11:14 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the Docker URI fetcher plugin based on curl.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 3595f915cb4b305659fa803e741333be5115aecf 
> 
> Diff: https://reviews.apache.org/r/42238/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

[Kevin Klues]

> On Jan. 14, 2016, 12:30 a.m., Till Toenshoff wrote:
> > bootstrap, line 16
> > 
> >
> > s/allows/allow/
> > 
> > fixing while committing...

sounds good


> On Jan. 14, 2016, 12:30 a.m., Till Toenshoff wrote:
> > bootstrap, line 23
> > 
> >
> > I'll adapt this function name to match the all lowercase pattern you 
> > started above while committing (here and in the invocation, at line 36).

sounds good


- Kevin


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


On Jan. 13, 2016, 6:41 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 13, 2016, 6:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

[Till Toenshoff]

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

Ship it!



bootstrap (line 16)


s/allows/allow/

fixing while committing...



bootstrap (line 23)


I'll adapt this function name to match the all lowercase pattern you 
started above while committing (here and in the invocation, at line 36).


- Till Toenshoff


On Jan. 13, 2016, 6:41 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 13, 2016, 6:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42238: Implemented the Docker URI fetcher plugin based on curl.

[Jojy Varghese]

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



src/uri/fetchers/docker.cpp (line 219)


Can be refactored to common function (and avoid duplicate code at L124).


- Jojy Varghese


On Jan. 13, 2016, 11:14 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42238/
> ---
> 
> (Updated Jan. 13, 2016, 11:14 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the Docker URI fetcher plugin based on curl.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 3595f915cb4b305659fa803e741333be5115aecf 
> 
> Diff: https://reviews.apache.org/r/42238/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 41820: Pulled out provisioner from linux filesystem isolator.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41814, 41999, 41815, 41816, 41817, 41819, 41820]

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

- Mesos ReviewBot


On Jan. 13, 2016, 9:19 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> ---
> 
> (Updated Jan. 13, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pulled out provisioner from linux filesystem isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 89d18624b08f2c953b9fa21663aeda694d4aac12 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> a29f9d184a0d1088577b3168a12bc8c06493a477 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 8af2cecdca947e55d8d39e26d2fc3d42212f36c3 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5bb85034c22caef64054c1629f6fd55d227e48b1 
> 
> Diff: https://reviews.apache.org/r/41820/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42278: Fixed volume paths for command tasks with image.

[Timothy Chen]

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

(Updated Jan. 14, 2016, 12:15 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.


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


Repository: mesos


Description
---

Fixed volume paths for command tasks with image.


Diffs
-

  src/launcher/executor.cpp 356d311fdf97b2c4663c60e13ede7cdb71a264c7 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
5bb85034c22caef64054c1629f6fd55d227e48b1 

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


Testing
---

make check


Thanks,

Timothy Chen

Review Request 42278: Fixed volume paths for command tasks with image.

[Timothy Chen]

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

Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.


Repository: mesos


Description
---

Fixed volume paths for command tasks with image.


Diffs
-

  src/launcher/executor.cpp 356d311fdf97b2c4663c60e13ede7cdb71a264c7 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
5bb85034c22caef64054c1629f6fd55d227e48b1 

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


Testing
---

make check


Thanks,

Timothy Chen

Review Request 42277: Add mount_all to linux fs.

[Timothy Chen]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Add mount_all to linux fs.


Diffs
-

  src/linux/fs.hpp 76c72930724b882d3b74edb3a91d5988fcbfc2ed 
  src/linux/fs.cpp bfcf97186cd1b0696a9537c4a332083def6b462e 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

[Michael Park]

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



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 29)


`s/(e.g.)//`



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 67)


This looks like it's > 80 chars long. Is there maybe a shorter linik? If 
not, let's just add `// NOLINT(whitespace/line_length)`.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (lines 95 - 96)


I don't quite understand why these are `std::wstring`. It seems like we do 
some `x / sizeof(WCHAR)` to make this work? Could you elaborate on this a 
little bit please?



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 104)


This looks like it should return a `Try`?



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 112)


I think we can remove this variable. The intent is clear from the function 
name.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 121)


Why do we take a copy of the `std::shared_ptr` here?



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 126)


I noticed the use of `WindowsError` below. Should this return 
`WindowsError` as well? Here and below.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (lines 158 - 162)


```
bool resolvedPathIsDirectory =
  ::_stat(absolutePath.c_str(), &s) >= 0 && S_ISDIR(s.st_mode);
```



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 200)


Similar to above, why does this function need to take a copy of the 
`std::shared_ptr`? It looks to me like this should simply take a `void*`, and 
we can actually return a `std::unique_ptr` from `getHandleNoFollow` 
instead.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (lines 227 - 228)


Why do we store this as a `std::shared_ptr`? Can we just store the data on 
the stack, and pass a pointer to it to `buildSymbolicLink`?



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (line 229)


It seems like we pass the address of this variable as a dummy out parameter 
below. Presumably this can't be declared `const`. It looks like would be that 
C-style cast of `(LPDWORD)&ignored` ignores the `const` and makes it so. This 
induces undefined behavior.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
 (lines 248 - 249)


`return buildSymbolicLink(reparsePointData);`



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp 
(line 45)


`s/filesysem/filesystem/`



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp 
(line 55)


Remove newline.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp 
(lines 76 - 78)


```
return getSymbolicLinkData(symlinkHandle.get());
```



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp (line 98)


Is this supposed to be `ErrnoError`? It looks like in the case above, 
`ErrnoError` was changed to `Error`. Do we not want `WindowsError`?



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp (line 106)


Is this `UNREACHABLE` necessary?


- Michael Park


On Jan. 12, 2016, 2:03 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39803/
> ---
> 
> (Updated Jan. 12, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> 

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

[Michael Hopcroft]

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

Ship it!


Ship It!

- Michael Hopcroft


On Jan. 12, 2016, 2:03 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39803/
> ---
> 
> (Updated Jan. 12, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented stout/os/stat.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
>  PRE-CREATION 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp 
> ffe074830ef90f492990bf695e686c885b9a155c 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 
> 5b38b9af654d7d1c574f0cc573083b66693ced1d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> d46e262e0fd1c2de36f3bf19d8bd693c23bf58cd 
> 
> Diff: https://reviews.apache.org/r/39803/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 42239: Added tests for the Docker URI fetcher plugin.

[Jie Yu]

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

(Updated Jan. 13, 2016, 11:29 p.m.)


Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and Timothy 
Chen.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added tests for the Docker URI fetcher plugin.

I left a TODO here because those tests depend on internet access. We should try 
to make the tests locally testable.


Diffs (updated)
-

  src/tests/uri_fetcher_tests.cpp ca1710cc852ebfd14dd180a245da67e6c98f148f 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 42266: Updated post-reviews.py to strip review URL in RR summary.

[Kevin Klues]

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

(Updated Jan. 13, 2016, 11:23 p.m.)


Review request for mesos, Artem Harutyunyan and Till Toenshoff.


Changes
---

Simplify so that this patch is less invasive to the previous revision than 
before. In general, this file could use a massive overhaul to extract key 
functionality into helper functions and call them from some primary flow of 
logic. It also shares functionality with apply-reviews, which could be 
abstracted out as well eventually.  For now, let's do the minimal possible to 
get the review URL stripped out of the commit message before posting back to 
reviewboard.


Repository: mesos


Description
---

Previously, when setting GUESS_FIELDS in .reviewboardrc to automatically
update the summary message when posting a review, the summary would
include the review URL in the summary message. This was due to the fact
that GUESS_FILEDS simply copies the entirety of the commit message into
the RR summary field on reviewboard. Including this line is both
redundant, and, in some cases, has been found to appear (at least) twice
in the summary message instead of just once.

This commit ensures that the commit message posted when GUESS_FIELDS is
set will have the review URL stripped before posting.


Diffs (updated)
-

  support/post-reviews.py 170be83aa6dca6e8175292169d78e8f7915f7e6e 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 42268: Fixed comment in ReservationTests.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Jan. 13, 2016, 10:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42268/
> ---
> 
> (Updated Jan. 13, 2016, 10:02 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed comment in ReservationTests.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 61d275c47fbb02baf22e4ad8f9b1580886da51d9 
> 
> Diff: https://reviews.apache.org/r/42268/diff/
> 
> 
> Testing
> ---
> 
> Corrected the text of a comment in 
> ReservationTest.GoodACLReserveThenUnreserve, which incorrectly described an 
> ACL.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42238: Implemented the Docker URI fetcher plugin based on curl.

[Jie Yu]

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

(Updated Jan. 13, 2016, 11:14 p.m.)


Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and Timothy 
Chen.


Changes
---

Supported http. Also, supported no authentication case.


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


Repository: mesos


Description
---

Implemented the Docker URI fetcher plugin based on curl.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp 3595f915cb4b305659fa803e741333be5115aecf 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 42274: Added common command utils file.

[Jojy Varghese]

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

(Updated Jan. 13, 2016, 11:13 p.m.)


Review request for mesos and Jie Yu.


Changes
---

fixed comments.


Repository: mesos


Description
---

This common file is a good place to add common command line utilities like tar,
digests(sha256, sha512, etc). Currently this functionality is spread in the code
base and this change would enable all those call sites to be replaced with a
common code.


Diffs (updated)
-

  src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/common/command_utils.hpp PRE-CREATION 
  src/common/command_utils.cpp PRE-CREATION 
  src/tests/common/command_utils_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 42275: Added utility functions to create docker URI.

[Jie Yu]

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

Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and Timothy 
Chen.


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


Repository: mesos


Description
---

Added utility functions to create docker URI.

See the usage in subsequent patches.


Diffs
-

  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/uri/schemes/docker.hpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 41806: Cleaned up assertions in test cases around JSON HTTP responses.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41806]

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

- Mesos ReviewBot


On Jan. 13, 2016, 7:19 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41806/
> ---
> 
> (Updated Jan. 13, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used `AWAIT_EXPECT_RESPONSE_HEADER_EQ()` to check the "Content-Type" of
> the response, rather than accessing the "headers" field directly, and
> used the symbol `APPLICATION_JSON` rather than a string literal. Also
> added "Content-Type" checks to a few places that had neglected to make
> them, and cleaned up some whitespace style.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 952aacb720715ad26f91fa08bc386b242b7e007b 
>   src/tests/fault_tolerance_tests.cpp 
> fd1c3c90101eed9ef9352511e0c72a463cca965c 
>   src/tests/master_tests.cpp 223b9d20a3a8a8194a3a6a605ec2394c37ab5957 
>   src/tests/metrics_tests.cpp f081fb9b68f25c6d6005f195c34253fba8abc146 
>   src/tests/monitor_tests.cpp a848d14ebb6cab79c06bcf55bd39f044b41a006e 
>   src/tests/repair_tests.cpp 63ec889c4954c2c60d3466952551aa25b3284ddf 
>   src/tests/scheduler_driver_tests.cpp 
> 1365d21ccad87923b862fb4942f1fd51630a62b7 
>   src/tests/scheduler_http_api_tests.cpp 
> 4d23a5a8368e0ed126469fa4a90a889b339ad004 
>   src/tests/slave_tests.cpp eb47b5319b16601b8600c4d53035b8337382070c 
>   src/tests/status_update_manager_tests.cpp 
> bd34b97a3559a5fea9a7a253a89e0ac3029f4a33 
>   src/tests/utils.cpp 877139e97249761658dce3b1058cdc2e2a52367b 
> 
> Diff: https://reviews.apache.org/r/41806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42262: Improved links at containerizer.md.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Jan. 13, 2016, 11:10 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42262/
> ---
> 
> (Updated Jan. 13, 2016, 11:10 p.m.)
> 
> 
> Review request for mesos, Joerg Schad, Jojy Varghese, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved links in containerizer.md.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md bf0b9279703f341c136d1d5b3999a9fef0f25911 
> 
> Diff: https://reviews.apache.org/r/42262/diff/
> 
> 
> Testing
> ---
> 
> Viewed via docker website generator.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 42274: Added common command utils file.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

This common file is a good place to add common command line utilities like tar,
digests(sha256, sha512, etc). Currently this functionality is spread in the code
base and this change would enable all those call sites to be replaced with a
common code.


Diffs
-

  src/CMakeLists.txt 39a23df3227a4f524ea0d408dc894fa5bbab7d10 
  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/common/command_utils.hpp PRE-CREATION 
  src/common/command_utils.cpp PRE-CREATION 
  src/tests/common/command_utils_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42263: Added more structure for containerizer related subpages in home.md.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Jan. 13, 2016, 8:38 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42263/
> ---
> 
> (Updated Jan. 13, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added more structure for containerizer related subpages in home.md.
> 
> 
> Diffs
> -
> 
>   docs/home.md 344c5617dfb003453fa17974500d44a8f766cb16 
> 
> Diff: https://reviews.apache.org/r/42263/diff/
> 
> 
> Testing
> ---
> 
> Viewed via Docker website generator.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 42238: Implemented the Docker URI fetcher plugin based on curl.

[Timothy Chen]

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



src/uri/fetchers/docker.cpp (line 75)


End with period.



src/uri/fetchers/docker.cpp (line 532)


Is it possible to have multiple images/manifests per directory? Assuming a 
task with multiple volumes and multiple images?


- Timothy Chen


On Jan. 13, 2016, 6:23 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42238/
> ---
> 
> (Updated Jan. 13, 2016, 6:23 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the Docker URI fetcher plugin based on curl.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 3595f915cb4b305659fa803e741333be5115aecf 
> 
> Diff: https://reviews.apache.org/r/42238/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 41959: Statically initializing fetcher plugins.

[Jojy Varghese]

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

(Updated Jan. 13, 2016, 11:08 p.m.)


Review request for mesos and Jie Yu.


Changes
---

rebased with master.


Repository: mesos


Description
---

Since fetcher plugins are not dynamically pluggable today, simplified the
factory method to use the statically intitalized plugins. This would be
useful when Fetchers are created multiple times.


Diffs (updated)
-

  src/uri/fetcher.cpp dfda732348fec3b686cf82b55ad94fda4829469b 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 42273: Modified scheduler library to move the queue contents before `async`.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This trivial fix invokes `std::move` on the `queue` object to not perform a 
copy before invoking the `async(..)` call.


Diffs
-

  src/scheduler/scheduler.cpp a17872b46ec600e0fae6c43247ccb63f5ee55ac0 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42238: Implemented the Docker URI fetcher plugin based on curl.

[Gilbert Song]

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



src/uri/fetchers/docker.cpp (lines 435 - 437)


Should this be possible? We may get an error when constructing URI protobuf 
as `path` is a required field.


- Gilbert Song


On Jan. 12, 2016, 10:23 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42238/
> ---
> 
> (Updated Jan. 12, 2016, 10:23 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, Jojy Varghese, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4296
> https://issues.apache.org/jira/browse/MESOS-4296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the Docker URI fetcher plugin based on curl.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 3595f915cb4b305659fa803e741333be5115aecf 
> 
> Diff: https://reviews.apache.org/r/42238/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 40939: Windows: Unified POSIX and Windows implementation of su

[M Lawindi]

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

Ship it!


Ship It!

- M Lawindi


On Dec. 23, 2015, 7:16 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40939/
> ---
> 
> (Updated Dec. 23, 2015, 7:16 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Alex Clemmer, and M Lawindi.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Unified POSIX and Windows implementation of `su`
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a25e2c1e5584e744c666bbc654eafbfc5f7b10e6 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 581ec5baac05bd8b702bfe000893e67fb04bde3b 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/su.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/su.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/su.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 42733d429814bf0512540188264830aeaabcabbe 
> 
> Diff: https://reviews.apache.org/r/40939/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: make.bat
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 41243: Updated how we find the .git directory in bootstrap.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41244, 41243]

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

- Mesos ReviewBot


On Jan. 13, 2016, 6:41 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41243/
> ---
> 
> (Updated Jan. 13, 2016, 6:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joseph Wu, 
> Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-4125
> https://issues.apache.org/jira/browse/MESOS-4125
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building from git, bootstrap will (among other things) install
> pre-commit and post-rewrite hooks into the .git/hooks directory of the
> mesos tree. However the current implementation always assumes that .git
> exists in the same directory as the bootstrap file. This may not always
> be true.
> 
> Most notably, it is not true if the mesos tree is included as a
> submodule inside another project. When included as a submodule, .git is
> no longer a directory, but rather a file whose text contains a pointer
> back to the actual location of the .git folder inside the containing
> project. To get at this directory, we need to run 'git rev-parse
> --git-common-dir' instead of simply assuming that the local .git is the
> proper directory.
> 
> Review: https://reviews.apache.org/r/41243
> 
> 
> Diffs
> -
> 
>   bootstrap ea71ff22e42bf5e224483508d91c33ddfd350f52 
> 
> Diff: https://reviews.apache.org/r/41243/diff/
> 
> 
> Testing
> ---
> 
> Tested on native Mac OS X El Capitan
> Tested on vagrant boxes for bento/centos-7.1, ubuntu/trusty64, and 
> https://github.com/tommy-muehle/puppet-vagrant-boxes/releases/download/1.0.0/centos-6.6-x86_64.box
> Also tested on docker image centos:7
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42216: Fixed gmock warnings in hook tests.

[Anand Mazumdar]

> On Jan. 13, 2016, 9:17 p.m., Neil Conway wrote:
> > Can you include the names of the tests in the commit message?

Done.


- Anand


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


On Jan. 13, 2016, 10:20 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42216/
> ---
> 
> (Updated Jan. 13, 2016, 10:20 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Neil Conway, and Timothy Chen.
> 
> 
> Bugs: MESOS-4348
> https://issues.apache.org/jira/browse/MESOS-4348
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We did not have an expectation on the `shutdown` method of the 
> `MockExecutor`. This led to the gmock warning being emitted in some runs. The 
> tests that are being fixed are:
> 
> - `HookTest.VerifyMasterLaunchTaskHook`
> - `HookTest.VerifySlaveRunTaskHook`
> - `HookTest.VerifySlaveTaskStatusDecorator`
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
> 
> Diff: https://reviews.apache.org/r/42216/diff/
> 
> 
> Testing
> ---
> 
> Loop the tests for 1000 times.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42216: Fixed gmock warnings in hook tests.

[Anand Mazumdar]

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

(Updated Jan. 13, 2016, 10:20 p.m.)


Review request for mesos, Joris Van Remoortere, Neil Conway, and Timothy Chen.


Changes
---

Modified description to include test names


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


Repository: mesos


Description (updated)
---

We did not have an expectation on the `shutdown` method of the `MockExecutor`. 
This led to the gmock warning being emitted in some runs. The tests that are 
being fixed are:

- `HookTest.VerifyMasterLaunchTaskHook`
- `HookTest.VerifySlaveRunTaskHook`
- `HookTest.VerifySlaveTaskStatusDecorator`


Diffs
-

  src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 

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


Testing
---

Loop the tests for 1000 times.


Thanks,

Anand Mazumdar

Re: Review Request 42269: Fixed broken links in mesos-provisioner.md.

[Joerg Schad]

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

(Updated Jan. 13, 2016, 10:09 p.m.)


Review request for mesos, Neil Conway and Timothy Chen.


Repository: mesos


Description
---

Fixed broken links in mesos-provisioner.md.


Diffs
-

  docs/mesos-provisioner.md f2777b21ce3b2278613b5915a6500c20f7db94e1 

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


Testing (updated)
---

Verified with docker based website generator.


Thanks,

Joerg Schad

Re: Review Request 42269: Fixed broken links in mesos-provisioner.md.

[Neil Conway]

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

Ship it!


Ship It!

- Neil Conway


On Jan. 13, 2016, 10:06 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42269/
> ---
> 
> (Updated Jan. 13, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed broken links in mesos-provisioner.md.
> 
> 
> Diffs
> -
> 
>   docs/mesos-provisioner.md f2777b21ce3b2278613b5915a6500c20f7db94e1 
> 
> Diff: https://reviews.apache.org/r/42269/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 42269: Fixed broken links in mesos-provisioner.md.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 13, 2016, 10:06 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42269/
> ---
> 
> (Updated Jan. 13, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed broken links in mesos-provisioner.md.
> 
> 
> Diffs
> -
> 
>   docs/mesos-provisioner.md f2777b21ce3b2278613b5915a6500c20f7db94e1 
> 
> Diff: https://reviews.apache.org/r/42269/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 42269: Fixed broken links in mesos-provisioner.md.

[Joerg Schad]

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

Review request for mesos, Neil Conway and Timothy Chen.


Repository: mesos


Description
---

Fixed broken links in mesos-provisioner.md.


Diffs
-

  docs/mesos-provisioner.md f2777b21ce3b2278613b5915a6500c20f7db94e1 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 42059: Updated ContainerLogger to use Subprocess::IO type.

[Joseph Wu]

> On Jan. 13, 2016, 11:11 a.m., Benjamin Hindman wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 829-832
> > 
> >
> > Subprocess::IO out = Subprocess::FD(STDOUT_FILENO);
> > Subprocess::IO err = Subprocess::FD(STDERR_FILENO);
> > 
> > if (!local) {
> >   out = subprocessInfo.out;
> >   err = subprocessInfo.err;
> > }

I remember why I didn't do this before.  Essentially, you would need to:

1) Change the field `process::Owned io;` to a public 
field.  (Making this public also breaks the ContainerLogger's restrictions on 
the types of `Subprocess::IO`.)
2) Add this to the containerizer:
```
Owned out(new Subprocess::FD(STDOUT_FILENO));
Owned err(new Subprocess::FD(STDERR_FILENO));
if (!local) {
  out.reset(subprocessInfo.out.io.release());
  err.reset(subprocessInfo.err.io.release());
}
```

---

Addressing your original point (that `SubprocessInfo` non-obviously redirects 
to STDOUT_FILENO and STDERR_FILENO by default), I think it would be neater to 
add a NOTE here instead.

---

Note: Trying to use a flavor of the ternary, i.e.
```
(local ? Subprocess::FD(STDOUT_FILENO) : subprocessInfo.out)
```

Gives the compiler error:
"error: allocating an object of abstract class type 'Subprocess::IO'"


- Joseph


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


On Jan. 13, 2016, 2:02 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42059/
> ---
> 
> (Updated Jan. 13, 2016, 2:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update ContainerLogger to use Subprocess::IO type
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/container_logger.hpp 
> a2362070ead0afcef1e6c2ca784083ddb01ba51a 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
> 
> Diff: https://reviews.apache.org/r/42059/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 42268: Fixed comment in ReservationTests.

[Greg Mann]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Fixed comment in ReservationTests.


Diffs
-

  src/tests/reservation_tests.cpp 61d275c47fbb02baf22e4ad8f9b1580886da51d9 

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


Testing
---

Corrected the text of a comment in ReservationTest.GoodACLReserveThenUnreserve, 
which incorrectly described an ACL.


Thanks,

Greg Mann

Re: Review Request 42059: Updated ContainerLogger to use Subprocess::IO type.

[Joseph Wu]

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

(Updated Jan. 13, 2016, 2:02 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Change `operator` to return a `const`.  Add comment in containerizer.


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


Repository: mesos


Description
---

Update ContainerLogger to use Subprocess::IO type


Diffs (updated)
-

  include/mesos/slave/container_logger.hpp 
a2362070ead0afcef1e6c2ca784083ddb01ba51a 
  src/slave/containerizer/mesos/containerizer.cpp 
f3c370aeb331beb6202fd30cd0278877da0b42e0 

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


Testing
---

make check


Thanks,

Joseph Wu