Re: Review Request 48530: Moved working groups from wiki to page.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48530]

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

- Mesos ReviewBot


On June 10, 2016, 1:53 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48530/
> ---
> 
> (Updated June 10, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5591
> https://issues.apache.org/jira/browse/MESOS-5591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/working_groups.yaml PRE-CREATION 
>   site/source/working-groups.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48530/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/working-groups/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48518: Moved release plan from wiki to page.

[Jie Yu]

> On June 9, 2016, 10:59 p.m., Tomasz Janiszewski wrote:
> > I have some concerns with this patch.
> > 
> > 1. Where it should be placed?
> > 2. Where is should be linked?
> > 3. It duplicates data from Downloads, maybe it could be merged into one 
> > page.
> 
> Jie Yu wrote:
> Tomasz, thanks for doing this.
> 
> This is slightly different from Downloads, because it might also contains 
> information about a future release (e.g., what will be the major feature 
> expected for a future release).
> 
> I agree with you that there's some duplication right now. Let's create a 
> ticket to track the merge of these two pages. Once that's resolved, we can 
> use the same link from the home page? An alternative is have a link in the 
> download page to point to this page. But I like the idea of merging the 
> Download page with this!
> 
> haosdent huang wrote:
> Thanks @janisz's worderful patch as well. For merge these two pages, I 
> create a ticket to trace it in 
> https://issues.apache.org/jira/browse/MESOS-5594

Thanks @haosdent!


- Jie


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


On June 9, 2016, 10:55 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48518/
> ---
> 
> (Updated June 9, 2016, 10:55 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5590
> https://issues.apache.org/jira/browse/MESOS-5590
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/release_plan.yaml PRE-CREATION 
>   site/source/release-plan.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48518/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/release-plan/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

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

[Anand Mazumdar]

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



Mostly comments around:

- Similar to comments from @haosdent abstract the common parts in a method 
`_setQuota`.
- Remove some additional tests.


src/internal/devolve.hpp (line 48)


Can you move this after L59 with a newline before and after?

It would be good to group all similar `*Request` messages together 
thereafter.

Also, we won't be needing this overload once MESOS-5593 is resolved since 
we would then directly devolve to `master::Call`. Can you also add a TODO to 
remove this once MESOS-5593 is resolved.



src/internal/evolve.hpp (lines 59 - 61)


We won't be needing them. See my comments on the tests.



src/master/quota_handler.cpp (line 278)


Can we abstract out the code after this in a function `_setQuota` that both 
 can use?

```cpp
Future Master::QuotaHandler::_setQuota(
const QuotaRequest& request,
const Option& principal) const
{
 ...
}
```

Makes sense?



src/tests/api_tests.cpp (lines 217 - 218)


Why do we need this comment here?



src/tests/api_tests.cpp (line 223)


Can you move this down to L241 where it is used?



src/tests/api_tests.cpp (lines 225 - 226)


Move this comment before L235 where you use `FORCE` and then just do 
`set_force(true)`



src/tests/api_tests.cpp (line 227)


Kill this variable



src/tests/api_tests.cpp (line 229)


Kill this comment



src/tests/api_tests.cpp (lines 254 - 410)


Kill these. The existing Quota handler workflow already tests this. We 
don't need to exercise those tests again.


- Anand Mazumdar


On June 6, 2016, 9:55 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48268/
> ---
> 
> (Updated June 6, 2016, 9:55 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5509
> https://issues.apache.org/jira/browse/MESOS-5509
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented SET_QUOTA Call in v1 master API. This call
> does not return v1::Response message, instead it returns
> http::Response.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/quota/quota.hpp PRE-CREATION 
>   src/Makefile.am c0be66ab28c452e217fe7c7ead8b1b3c283908cc 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 824c6e5adcebc83d1ec742c9bd036a8f24c9a343 
>   src/master/master.hpp 846edf37d13b44093832ca3d184426b403174b35 
>   src/master/quota_handler.cpp 06cf0d3f24844ec9e884237db3f33145ff406e80 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48268/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo GTEST_FILTER="*MasterAPITest.SetQuota*" make -j4 check
> 
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from ContentType/MasterAPITest
> [ RUN  ] ContentType/MasterAPITest.SetQuota/0
> [   OK ] ContentType/MasterAPITest.SetQuota/0 (129 ms)
> [ RUN  ] ContentType/MasterAPITest.SetQuota/1
> [   OK ] ContentType/MasterAPITest.SetQuota/1 (98 ms)
> [--] 2 tests from ContentType/MasterAPITest (227 ms total)
> 
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (236 ms total)
> [  PASSED  ] 2 tests.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48438]

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

- Mesos ReviewBot


On June 10, 2016, 1:16 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48438/
> ---
> 
> (Updated June 10, 2016, 1:16 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: mesos-5491
> https://issues.apache.org/jira/browse/mesos-5491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement the basic getAgents method for v1 operator master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48438/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 48518: Moved release plan from wiki to page.

[haosdent huang]

> On June 9, 2016, 10:59 p.m., Tomasz Janiszewski wrote:
> > I have some concerns with this patch.
> > 
> > 1. Where it should be placed?
> > 2. Where is should be linked?
> > 3. It duplicates data from Downloads, maybe it could be merged into one 
> > page.
> 
> Jie Yu wrote:
> Tomasz, thanks for doing this.
> 
> This is slightly different from Downloads, because it might also contains 
> information about a future release (e.g., what will be the major feature 
> expected for a future release).
> 
> I agree with you that there's some duplication right now. Let's create a 
> ticket to track the merge of these two pages. Once that's resolved, we can 
> use the same link from the home page? An alternative is have a link in the 
> download page to point to this page. But I like the idea of merging the 
> Download page with this!

Thanks @janisz's worderful patch as well. For merge these two pages, I create a 
ticket to trace it in https://issues.apache.org/jira/browse/MESOS-5594


- haosdent


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


On June 9, 2016, 10:55 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48518/
> ---
> 
> (Updated June 9, 2016, 10:55 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5590
> https://issues.apache.org/jira/browse/MESOS-5590
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/release_plan.yaml PRE-CREATION 
>   site/source/release-plan.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48518/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/release-plan/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > src/common/values.cpp, lines 654-657
> > 
> >
> > Can you pull this cleanup into a separate patch? Note that you don't 
> > need the variable:
> > 
> > ```
> >   foreach (const string& token, sstrings::tokenize(temp, "{},\n")) {
> > set->add_item(token);
> >   }
> > ```

Sorry for un-based code, this's handled in r/45813.


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 48527: Add NetworkInfo.labels to CNI Network before passing to CNI plugin.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On June 10, 2016, 12:16 a.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48527/
> ---
> 
> (Updated June 10, 2016, 12:16 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add NetworkInfo.labels to CNI Network before passing to CNI plugin.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 09890cedf2e7a1846bd1cb250e117be1680a1b80 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 106ff35320f37b2e75ed60381de1406459e6d515 
> 
> Diff: https://reviews.apache.org/r/48527/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Re: Review Request 48524: Moved flag validation after check for required flags.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48524]

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

- Mesos ReviewBot


On June 9, 2016, 11:54 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48524/
> ---
> 
> (Updated June 9, 2016, 11:54 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved flag validation after check for required flags.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 9271f722e5281d7be5e165021c2c48d3b3453a8e 
> 
> Diff: https://reviews.apache.org/r/48524/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 48530: Moved working groups from wiki to page.

[haosdent huang]

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



Hi, @janisz your patch looks quite nice! Wonderful jobs!


site/data/working_groups.yaml (line 1)


Is it would be more helpful if have a comment about how to add a new group 
in work_groups.yaml?



site/source/working-groups.html.erb (line 4)


Add one blank line below to keep consistent with other erb template.


- haosdent huang


On June 10, 2016, 1:53 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48530/
> ---
> 
> (Updated June 10, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5591
> https://issues.apache.org/jira/browse/MESOS-5591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/working_groups.yaml PRE-CREATION 
>   site/source/working-groups.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48530/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/working-groups/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Review Request 48530: Moved working groups from wiki to page.

[Tomasz Janiszewski]

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

Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Change was discussed at
http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html


Diffs
-

  site/data/working_groups.yaml PRE-CREATION 
  site/source/working-groups.html.erb PRE-CREATION 

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


Testing
---

Page could be viewed at 
http://janisz.github.io/mesos/site/publish/working-groups/


Thanks,

Tomasz Janiszewski

Re: Review Request 45905: Added metrics to the balloon framework.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46407, 48299, 45604, 46411, 48303, 45905]

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

- Mesos ReviewBot


On June 9, 2016, 11:55 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45905/
> ---
> 
> (Updated June 9, 2016, 11:55 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds metrics to gauge the health of the framework.  This includes:
> 
> * uptime_secs = How long the framework has been running.
> * registered = If the framework is registered.
> * tasks_finished = Number of tasks finished (successfully).
> * tasks_oomed = Number of tasks that were OOM killed.
> * allowed_terminations = Number of terminal status updates which
>   are acceptable due to infrastructure reasons.
> * abnormal_terminations = Number of terminal status updates which 
>   were not `TASK_FINISHED` or `TASK_FAILED` due to OOM.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 
> 
> Diff: https://reviews.apache.org/r/45905/diff/
> 
> 
> Testing
> ---
> 
> ```
> make check
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
> 
> # Also launched two instances on a cluster.
> # This one OOM's:
> ./balloon-framework --master=zk://localhost:2181/mesos --checkpoint 
> --balloon_limit=256MB --task_memory=128MB 
> --executor_uri="https://s3.amazonaws.com/url/to/balloon-executor; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./balloon-executor"
> 
> # This one does not OOM:
> ./balloon-framework --master=zk://localhost:2181/mesos --checkpoint 
> --balloon_limit=256MB --task_memory=256MB 
> --executor_uri="https://s3.amazonaws.com/url/to/balloon-executor; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./balloon-executor"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 48511: Renamed `responseContentType` to `responseType`.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On June 9, 2016, 9:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48511/
> ---
> 
> (Updated June 9, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Minor: The type of the variable is already `ContentType`. The
> name `responseType` has sufficient information that this
> refers to the content type of the response.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48511/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48510: Passed `ContentType` enum by value instead of const ref.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On June 10, 2016, midnight, Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48510/
> ---
> 
> (Updated June 10, 2016, midnight)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48510/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 37989: Enhanced log message when launching mesos docker executor.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 10, 2016, 12:47 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37989/
> ---
> 
> (Updated June 10, 2016, 12:47 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5348
> https://issues.apache.org/jira/browse/MESOS-5348
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced log message when launching mesos docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp bf384303f29b0c5c3d890b27519362ee41efd6b3 
>   src/slave/containerizer/docker.cpp 5ee039dd18869bdfeb7c57bea781d72d83336c0c 
> 
> Diff: https://reviews.apache.org/r/37989/diff/
> 
> 
> Testing
> ---
> 
> I0511 11:26:18.176844 29178 docker.cpp:1236] Launching 
> 'mesos-docker-executor' with flags 
> '--container="mesos-7f581417-e295-474f-b038-3900ee0479dc-S1.b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --docker="docker" --docker_socket="/var/run/docker.sock" --help="false" 
> --initialize_driver_logging="true" 
> --launcher_dir="/root/src/mesos/m1/mesos/build/src" --logbufsecs="0" 
> --logging_level="INFO" --mapped_directory="/mnt/mesos/sandbox" 
> --quiet="false" 
> --sandbox_directory="/tmp/mesos/slaves/7f581417-e295-474f-b038-3900ee0479dc-S1/frameworks/7f581417-e295-474f-b038-3900ee0479dc-0001/executors/test_mesos/runs/b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --stop_timeout="0ns"'
> 
> Start agent without VLOG_v=1, once the docker container is started, check 
> sandbox log:
> root@mesos002:/tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/latest#
>  cat stderr
> I0526 22:25:59.805173 11422 logging.cpp:195] Logging to STDERR
> I0526 22:25:59.808446 11422 process.cpp:1057] libprocess is initialized on 
> 192.168.56.12:36164 with 8 worker threads
> I0526 22:25:59.809834 11422 exec.cpp:150] Version: 0.29.0
> I0526 22:25:59.813784 11445 exec.cpp:200] Executor started at: 
> executor(1)@192.168.56.12:36164 with pid 11422
> I0526 22:25:59.818483 11446 exec.cpp:225] Executor registered on agent 
> fb930730-03f9-4027-96cb-81d612e8e35f-S0
> I0526 22:25:59.819725 11446 exec.cpp:237] Executor::registered took 272279ns
> I0526 22:25:59.820055 11446 exec.cpp:312] Executor asked to run task 'test'
> I0526 22:25:59.820132 11446 exec.cpp:321] Executor::launchTask took 55859ns
> I0526 22:25:59.820344 11446 docker.cpp:677] Running docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 -e 
> MESOS_SANDBOX=/mnt/mesos/sandbox -e 
> MESOS_CONTAINER_NAME=mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  -v 
> /tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/7accfa4d-909b-4e46-bdf7-6bcdf505f6d6:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  ubuntu:14.04 -c sleep 10
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45604: Updated balloon framework code with flags and better resource math.

[Greg Mann]

> On June 7, 2016, 9:19 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 73-76
> > 
> >
> > see above. don't need this.
> 
> Joseph Wu wrote:
> Note: This is fixed, but currently does not print the correct validation 
> message.
> 
> When `--task_memory_usage_limit` is not specified, the error is `Please 
> use a --task_memory_usage_limit greater than 64 MB` rather than `Flag 
> 'task_memory_usage_limit' is required, but it was not provided`.
> 
> Chatted with Greg offline, and he'll fix the flag validation code 
> separately:
> 
> https://github.com/apache/mesos/blob/56ebc7eb0b04c7c79656ad74c347b0a2e4debc99/3rdparty/stout/include/stout/flags/flags.hpp#L1005-L1009

Thanks for pointing this out, Joseph! Review for the related flag change is 
here: https://reviews.apache.org/r/48524/


- Greg


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


On June 9, 2016, 11:48 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45604/
> ---
> 
> (Updated June 9, 2016, 11:48 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `balloon-framework` enough options to run
> outside of the build environment:
> 
> * Adds an option for restricting the number of resources per task
>   (otherwise, it will eat up an entire node).
> * Adds an option for persisting the framework and launching one task
>   after another.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 
>   src/tests/balloon_framework_test.sh 
> a242f6cb9ca1850e5fef90e0938f41044bdaddbf 
> 
> Diff: https://reviews.apache.org/r/45604/diff/
> 
> 
> Testing
> ---
> 
> ```
> make check 
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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

[zhou xing]

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

(Updated 六月 10, 2016, 1:16 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

rebase the code to use http response for operator apis


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


Repository: mesos


Description
---

Implement the basic getAgents method for v1 operator master API.


Diffs (updated)
-

  include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
  include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

[Jiang Yan Xu]

> On June 9, 2016, 5:10 p.m., Jiang Yan Xu wrote:
> > I feel good about the overall structure of this change with this revision. 
> > I hope I have captured most things so if we do another pass after it should 
> > mostly be relatively minor readability/reusablity/documentation/cleanups 
> > etc. which is hard to do in one go due to the length. 
> > 
> > ## About v1 resources files
> > In retropsect I feel like it probably works the better if it's in a 
> > separate patch done **after the v0 files are shipped** to avoid having them 
> > go through all intermediate revisions. i.e., you can update v1 files 
> > according to the final version of the v0 files.
> > 
> > I have mostly skipped v1 files in this review because the comments would 
> > have been duplicated. Since we are still iterating on this, would you mind 
> > pulling them out from this review?
> > 
> > ## Validation
> > If we currently only allow persistent volumes to be shared, should we 
> > validate this here?
> > 
> > ## Additional test ideas
> > - Resources A + A - A == A with shared resources in it. 
> > - Resources B + A - A == B with shared resources in A (empty resources are 
> > discarded)
> > - Resources r1 = A + A, r2 = A + A + A, r1 - r2 is empty (invalid entries 
> > are discarded)
> > - Resources r1 + A == r1 (A is an invalid shared resource) (invalid entries 
> > are discarded)
> > - Shared resources that slightly differ are not grouped together.
> > - Shared resources stream output.
> > - Resources contains() with shared resources.
> > - Any existing resources tests interesting when shared resources are 
> > blended in?

About validation: sorry I didn't realize it's in a later review /r/45960/ but 
if you just look at this review one might wonder if you should also test shared 
cpus or other fungible resources. Anyways, it's fine if we don't since it's not 
a valid use case for now.


- Jiang Yan


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


On May 22, 2016, 12:08 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45959/
> ---
> 
> (Updated May 22, 2016, 12:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-4892
> https://issues.apache.org/jira/browse/MESOS-4892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A new class Resource_ is added that allows 'Resources' to group
> identical shared resource objects together into a single 'Resource_'
> object and tracked by its shared count. Non-shared resource objects
> are not grouped.
> 
> For resource addition and subtraction, the shared count is adjusted for
> shared resources as follows:
> a) Addition: If shared resource is absent from original, then the
>resource is added initialized with a consumer count of 1. Otherwise,
>the share count for the shared resource is incremented.
> b) Subtraction: If shared resource's share count is already 1, then
>the shared resource is removed from the original. Otherwise, its
>consumer count is decremented.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
>   include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
>   src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
>   src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 
>   src/tests/mesos.hpp 79bf1ff16412ce2a510a9b75ab1ac91c1c182653 
>   src/tests/resources_tests.cpp dc12bd8f1e2da6972bc8aed598811c55d664036e 
>   src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 
> 
> Diff: https://reviews.apache.org/r/45959/diff/
> 
> 
> Testing
> ---
> 
> New tests added to demonstrate arithmetic operations for shared resources 
> with consumer counts.
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 48455: Removed unused import from sorter.cpp.

[Guangya Liu]

> On 六月 9, 2016, 9:18 p.m., Benjamin Mahler wrote:
> > src/master/allocator/sorter/drf/sorter.cpp, lines 17-18
> > 
> >
> > Rather than just removing this one, would you mind doing an audit of 
> > the headers? We're missing a lot of includes here.

@Ben, how did you fix this? Seems the commit message below does not align with 
this patch.


- Guangya


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


On 六月 9, 2016, 1:30 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48455/
> ---
> 
> (Updated 六月 9, 2016, 1:30 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused import from sorter.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 65d473a5da0d846214c930c14d333040b2085b13 
> 
> Diff: https://reviews.apache.org/r/48455/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45960: Added interfaces to handle and track shareable resources.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/common/resources.cpp (line 756)


Two space indentation.



src/common/resources.cpp (line 760)


Two space indentation.



src/v1/resources.cpp (line 759)


Two space indentation.



src/v1/resources.cpp (line 763)


Two space indentation.


- Jiang Yan Xu


On May 22, 2016, 12:09 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45960/
> ---
> 
> (Updated May 22, 2016, 12:09 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added interfaces to handle and track shareable resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
>   include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
>   src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
>   src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 
> 
> Diff: https://reviews.apache.org/r/45960/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 37989: Enhanced log message when launching mesos docker executor.

[Guangya Liu]

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

(Updated 六月 10, 2016, 12:47 a.m.)


Review request for mesos, Gilbert Song, haosdent huang, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Enhanced log message when launching mesos docker executor.


Diffs (updated)
-

  src/docker/docker.cpp bf384303f29b0c5c3d890b27519362ee41efd6b3 
  src/slave/containerizer/docker.cpp 5ee039dd18869bdfeb7c57bea781d72d83336c0c 

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


Testing (updated)
---

I0511 11:26:18.176844 29178 docker.cpp:1236] Launching 'mesos-docker-executor' 
with flags 
'--container="mesos-7f581417-e295-474f-b038-3900ee0479dc-S1.b19cbf40-d69f-4507-9019-dbe61b9eff6b"
 --docker="docker" --docker_socket="/var/run/docker.sock" --help="false" 
--initialize_driver_logging="true" 
--launcher_dir="/root/src/mesos/m1/mesos/build/src" --logbufsecs="0" 
--logging_level="INFO" --mapped_directory="/mnt/mesos/sandbox" --quiet="false" 
--sandbox_directory="/tmp/mesos/slaves/7f581417-e295-474f-b038-3900ee0479dc-S1/frameworks/7f581417-e295-474f-b038-3900ee0479dc-0001/executors/test_mesos/runs/b19cbf40-d69f-4507-9019-dbe61b9eff6b"
 --stop_timeout="0ns"'

Start agent without VLOG_v=1, once the docker container is started, check 
sandbox log:
root@mesos002:/tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/latest#
 cat stderr
I0526 22:25:59.805173 11422 logging.cpp:195] Logging to STDERR
I0526 22:25:59.808446 11422 process.cpp:1057] libprocess is initialized on 
192.168.56.12:36164 with 8 worker threads
I0526 22:25:59.809834 11422 exec.cpp:150] Version: 0.29.0
I0526 22:25:59.813784 11445 exec.cpp:200] Executor started at: 
executor(1)@192.168.56.12:36164 with pid 11422
I0526 22:25:59.818483 11446 exec.cpp:225] Executor registered on agent 
fb930730-03f9-4027-96cb-81d612e8e35f-S0
I0526 22:25:59.819725 11446 exec.cpp:237] Executor::registered took 272279ns
I0526 22:25:59.820055 11446 exec.cpp:312] Executor asked to run task 'test'
I0526 22:25:59.820132 11446 exec.cpp:321] Executor::launchTask took 55859ns
I0526 22:25:59.820344 11446 docker.cpp:677] Running docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 -e 
MESOS_SANDBOX=/mnt/mesos/sandbox -e 
MESOS_CONTAINER_NAME=mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
 -v 
/tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/7accfa4d-909b-4e46-bdf7-6bcdf505f6d6:/mnt/mesos/sandbox
 --net host --entrypoint /bin/sh --name 
mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
 ubuntu:14.04 -c sleep 10


Thanks,

Guangya Liu

Re: Review Request 37989: Enhanced log message when launching mesos docker executor.

[Guangya Liu]

> On 六月 9, 2016, 5:44 p.m., Jie Yu wrote:
> > Can we first just log(info) docker run? Looking at the ticket that folks 
> > filed, looks like this is the one that's been asked the most?

+1 to this, seems end user cared more for the arguments of `docker run`, will 
post a patch soon.


- Guangya


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


On 五月 31, 2016, 12:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37989/
> ---
> 
> (Updated 五月 31, 2016, 12:09 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5348
> https://issues.apache.org/jira/browse/MESOS-5348
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced log message when launching mesos docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 19cf424dfd5748ae66a7023840aa2b0652e8f2c0 
>   src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
> 
> Diff: https://reviews.apache.org/r/37989/diff/
> 
> 
> Testing
> ---
> 
> I0511 11:26:18.176844 29178 docker.cpp:1236] Launching 
> 'mesos-docker-executor' with flags 
> '--container="mesos-7f581417-e295-474f-b038-3900ee0479dc-S1.b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --docker="docker" --docker_socket="/var/run/docker.sock" --help="false" 
> --initialize_driver_logging="true" 
> --launcher_dir="/root/src/mesos/m1/mesos/build/src" --logbufsecs="0" 
> --logging_level="INFO" --mapped_directory="/mnt/mesos/sandbox" 
> --quiet="false" 
> --sandbox_directory="/tmp/mesos/slaves/7f581417-e295-474f-b038-3900ee0479dc-S1/frameworks/7f581417-e295-474f-b038-3900ee0479dc-0001/executors/test_mesos/runs/b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --stop_timeout="0ns"'
> 
> Start agent without VLOG_v=1, once the docker container is started, check 
> sandbox log:
> root@mesos002:/tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/latest#
>  cat stderr
> I0526 22:25:59.805173 11422 logging.cpp:195] Logging to STDERR
> I0526 22:25:59.808446 11422 process.cpp:1057] libprocess is initialized on 
> 192.168.56.12:36164 with 8 worker threads
> I0526 22:25:59.809834 11422 exec.cpp:150] Version: 0.29.0
> I0526 22:25:59.813784 11445 exec.cpp:200] Executor started at: 
> executor(1)@192.168.56.12:36164 with pid 11422
> I0526 22:25:59.818483 11446 exec.cpp:225] Executor registered on agent 
> fb930730-03f9-4027-96cb-81d612e8e35f-S0
> I0526 22:25:59.819725 11446 exec.cpp:237] Executor::registered took 272279ns
> I0526 22:25:59.820055 11446 exec.cpp:312] Executor asked to run task 'test'
> I0526 22:25:59.820132 11446 exec.cpp:321] Executor::launchTask took 55859ns
> I0526 22:25:59.820344 11446 docker.cpp:677] Running docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 -e 
> MESOS_SANDBOX=/mnt/mesos/sandbox -e 
> MESOS_CONTAINER_NAME=mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  -v 
> /tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/7accfa4d-909b-4e46-bdf7-6bcdf505f6d6:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  ubuntu:14.04 -c sleep 10
> I0526 22:25:59.822684 11446 docker.cpp:824] Running docker -H 
> unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
> WARNING: Your kernel does not support swap limit capabilities, memory limited 
> without swap.
> I0526 22:25:59.924623 11445 docker.cpp:871] Retrying inspect with non-zero 
> status code. cmd: 'docker -H unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6',
>  interval: 500ms
> I0526 22:26:00.425278 11449 docker.cpp:824] Running docker -H 
> unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
> I0526 22:26:00.544518 11443 exec.cpp:535] Executor sending status update 
> TASK_RUNNING (UUID: b686d7d1-110f-4f42-a392-b9ed9a854d31) for task test of 
> framework fb930730-03f9-4027-96cb-81d612e8e35f-
> I0526 22:26:00.549468 11442 exec.cpp:358] Executor received status update 
> acknowledgement b686d7d1-110f-4f42-a392-b9ed9a854d31 for task test of 
> framework fb930730-03f9-4027-96cb-81d612e8e35f-
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48450: Add unit test for master /tasks endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48400, 48401, 48449, 48046, 48450]

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

- Mesos ReviewBot


On June 9, 2016, 11:13 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48450/
> ---
> 
> (Updated June 9, 2016, 11:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add unit test for master /tasks endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 0d64e353f0682017953b524a5f5246d76f52fa2e 
> 
> Diff: https://reviews.apache.org/r/48450/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04 & OSX
> 
> I didn't find unit test for this endpoint, therefore adding it.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Review Request 48527: Add NetworkInfo.labels to CNI Network before passing to CNI plugin.

[Dan Osborne]

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

Review request for mesos.


Repository: mesos


Description
---

Add NetworkInfo.labels to CNI Network before passing to CNI plugin.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
09890cedf2e7a1846bd1cb250e117be1680a1b80 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
106ff35320f37b2e75ed60381de1406459e6d515 

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


Testing
---


Thanks,

Dan Osborne

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

[Jiang Yan Xu]

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



I feel good about the overall structure of this change with this revision. I 
hope I have captured most things so if we do another pass after it should 
mostly be relatively minor readability/reusablity/documentation/cleanups etc. 
which is hard to do in one go due to the length. 

## About v1 resources files
In retropsect I feel like it probably works the better if it's in a separate 
patch done **after the v0 files are shipped** to avoid having them go through 
all intermediate revisions. i.e., you can update v1 files according to the 
final version of the v0 files.

I have mostly skipped v1 files in this review because the comments would have 
been duplicated. Since we are still iterating on this, would you mind pulling 
them out from this review?

## Validation
If we currently only allow persistent volumes to be shared, should we validate 
this here?

## Additional test ideas
- Resources A + A - A == A with shared resources in it. 
- Resources B + A - A == B with shared resources in A (empty resources are 
discarded)
- Resources r1 = A + A, r2 = A + A + A, r1 - r2 is empty (invalid entries are 
discarded)
- Resources r1 + A == r1 (A is an invalid shared resource) (invalid entries are 
discarded)
- Shared resources that slightly differ are not grouped together.
- Shared resources stream output.
- Resources contains() with shared resources.
- Any existing resources tests interesting when shared resources are blended in?


include/mesos/resources.hpp (line 64)


We have a private section below already. Put `Resource_` definition there?



include/mesos/resources.hpp (line 76)


Instead of stating what the line does literally, add a bit of explanation:

// Setting the counter to 1 to indicate "one copy" of the shared resource.



include/mesos/resources.hpp (lines 86 - 87)


There's a pattern worth explaining so we can put `contains` together with 
the operators and have a common comment block.

```
The Resource_ arithmetic operators and equality/contains comparison require 
the wrapped protobuf `resource` to have the same sharedness. 

For shared resources, their protobuf `resource` fields need to be the same 
(operators and comparisons apply to the counters). Otherwise the result is as 
though the arithmetic operators are not applied or 'false' for 
equality/contains comparison.

For non-shared resources the counters are none and the semantics of the 
Resource_ operators/comparison are the same as Resource's. See comments above 
the Resource operators.
```

Then I feel the comments for the operators and contains in `Resources_` are 
not necessary anymore. I feel either the comment is too trivial or the comments 
are too duplicated if on individual operators/methods. What do you think?



include/mesos/resources.hpp (line 116)


s/The share count for the protobuf Resource object./The counter for 
grouping shared 'resource' objects, None if 'resource' is non-shared./



include/mesos/resources.hpp (line 121)


Since we friend the outter class. Make data members private?



include/mesos/resources.hpp (lines 125 - 126)


One blank line here.



include/mesos/resources.hpp (lines 283 - 285)


Can we avoid explaining sharedCount or share count in the public method 
because it's an internal detail?

```
Count the Resource objects that match the specified value.

NOTE:
- For a non-shared resource the count can be at most 1 because all 
non-shared Resource objects in Resources are unique.
- For a shared resource the count can be greater than 1.
```



include/mesos/resources.hpp (lines 449 - 450)


Doesn't `Resource_` already friend the << operator?



include/mesos/resources.hpp (lines 451 - 452)


Why this `friend`? This operator is already defined and we are not changing 
it right?



src/common/resources.cpp (line 826)


How about "Invalid shared resource: count < 0"?



src/common/resources.cpp (line 843)


So if (this, that) = (cpus:5<3>, cpus:4<2>), should `this` contain `that`?
It probably shouldn't?

```
// The sharedness needs to match for two Resource_ 
// objects to be contained 

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

[Qian Zhang]

> On June 1, 2016, 11:09 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 265
> > 
> >
> > I think you need to handle `cgroups/devices/gpus/nvidia` which also 
> > starts with `cgroups` :-)
> 
> haosdent huang wrote:
> Yes, when I post the first version of this. The gpu have not been 
> submitted, I think it's time to add it now. :)
> 
> Jie Yu wrote:
> I think for GPU, the plan is to rename it to gpu/nvidia.
> 
> haosdent huang wrote:
> Does this mean we not need to consider gpu isolators when we implement 
> this?

In https://reviews.apache.org/r/48478 , I see `cgroups/devices/gpus/nvidia` 
isolator has been divided into 2 isolators: `gpu/nvidia` and `cgroups/devices`, 
so I think we still need to handle `cgruops/devices` which starts with 
`cgroups`.


- Qian


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


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

Re: Review Request 48458: Corrected comment for QuotaProvidesGuarantee.

[Guangya Liu]

> On 六月 9, 2016, 11:52 p.m., Benjamin Mahler wrote:
> > Ship It!
> 
> Benjamin Mahler wrote:
> We should clarify that QuotaProvidesGuarantee is a test in the summary of 
> this review because it will become the commit message.

Thanks Ben, got it.


- Guangya


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


On 六月 9, 2016, 2:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48458/
> ---
> 
> (Updated 六月 9, 2016, 2:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected comment for QuotaProvidesGuarantee.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e4579cb7978c966f639c0d2c196884e522b372c2 
> 
> Diff: https://reviews.apache.org/r/48458/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48524: Moved flag validation after check for required flags.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On June 9, 2016, 4:54 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48524/
> ---
> 
> (Updated June 9, 2016, 4:54 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved flag validation after check for required flags.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 9271f722e5281d7be5e165021c2c48d3b3453a8e 
> 
> Diff: https://reviews.apache.org/r/48524/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 45905: Added metrics to the balloon framework.

[Joseph Wu]

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

(Updated June 9, 2016, 4:55 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Renamed `allowed_terminations` to `launch_failures`.  Also touched up a related 
comment.


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


Repository: mesos


Description
---

Adds metrics to gauge the health of the framework.  This includes:

* uptime_secs = How long the framework has been running.
* registered = If the framework is registered.
* tasks_finished = Number of tasks finished (successfully).
* tasks_oomed = Number of tasks that were OOM killed.
* allowed_terminations = Number of terminal status updates which
  are acceptable due to infrastructure reasons.
* abnormal_terminations = Number of terminal status updates which 
  were not `TASK_FINISHED` or `TASK_FAILED` due to OOM.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 

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


Testing
---

```
make check

sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"

# Also launched two instances on a cluster.
# This one OOM's:
./balloon-framework --master=zk://localhost:2181/mesos --checkpoint 
--balloon_limit=256MB --task_memory=128MB 
--executor_uri="https://s3.amazonaws.com/url/to/balloon-executor; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./balloon-executor"

# This one does not OOM:
./balloon-framework --master=zk://localhost:2181/mesos --checkpoint 
--balloon_limit=256MB --task_memory=256MB 
--executor_uri="https://s3.amazonaws.com/url/to/balloon-executor; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./balloon-executor"
```


Thanks,

Joseph Wu

Review Request 48524: Moved flag validation after check for required flags.

[Greg Mann]

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

Review request for mesos, Joseph Wu and Vinod Kone.


Repository: mesos


Description
---

Moved flag validation after check for required flags.


Diffs
-

  3rdparty/stout/include/stout/flags/flags.hpp 
9271f722e5281d7be5e165021c2c48d3b3453a8e 

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


Testing
---

`make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 48458: Corrected comment for QuotaProvidesGuarantee.

[Benjamin Mahler]

> On June 9, 2016, 11:52 p.m., Benjamin Mahler wrote:
> > Ship It!

We should clarify that QuotaProvidesGuarantee is a test in the summary of this 
review because it will become the commit message.


- Benjamin


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


On June 9, 2016, 2:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48458/
> ---
> 
> (Updated June 9, 2016, 2:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected comment for QuotaProvidesGuarantee.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e4579cb7978c966f639c0d2c196884e522b372c2 
> 
> Diff: https://reviews.apache.org/r/48458/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48303: Split the balloon scheduler into a Process to support metrics.

[Joseph Wu]

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

(Updated June 9, 2016, 4:53 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Adds a `BalloonSchedulerProcess` which holds a subset of the `Scheduler`
class's methods.  The `BalloonScheduler` dispatches to the process
when for non-logging business logic.  This also opens up the balloon 
framework for libprocess metrics.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 48458: Corrected comment for QuotaProvidesGuarantee.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 9, 2016, 2:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48458/
> ---
> 
> (Updated June 9, 2016, 2:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected comment for QuotaProvidesGuarantee.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e4579cb7978c966f639c0d2c196884e522b372c2 
> 
> Diff: https://reviews.apache.org/r/48458/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45604: Updated balloon framework code with flags and better resource math.

[Joseph Wu]

> On June 7, 2016, 2:19 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 73-76
> > 
> >
> > see above. don't need this.

Note: This is fixed, but currently does not print the correct validation 
message.

When `--task_memory_usage_limit` is not specified, the error is `Please use a 
--task_memory_usage_limit greater than 64 MB` rather than `Flag 
'task_memory_usage_limit' is required, but it was not provided`.

Chatted with Greg offline, and he'll fix the flag validation code separately:
https://github.com/apache/mesos/blob/56ebc7eb0b04c7c79656ad74c347b0a2e4debc99/3rdparty/stout/include/stout/flags/flags.hpp#L1005-L1009


- Joseph


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


On June 9, 2016, 4:48 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45604/
> ---
> 
> (Updated June 9, 2016, 4:48 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `balloon-framework` enough options to run
> outside of the build environment:
> 
> * Adds an option for restricting the number of resources per task
>   (otherwise, it will eat up an entire node).
> * Adds an option for persisting the framework and launching one task
>   after another.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 
>   src/tests/balloon_framework_test.sh 
> a242f6cb9ca1850e5fef90e0938f41044bdaddbf 
> 
> Diff: https://reviews.apache.org/r/45604/diff/
> 
> 
> Testing
> ---
> 
> ```
> make check 
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45604: Updated balloon framework code with flags and better resource math.

[Joseph Wu]

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

(Updated June 9, 2016, 4:48 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Change some required flags to use the no-default -> required semantics.


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


Repository: mesos


Description
---

This gives the example `balloon-framework` enough options to run
outside of the build environment:

* Adds an option for restricting the number of resources per task
  (otherwise, it will eat up an entire node).
* Adds an option for persisting the framework and launching one task
  after another.
* Adds filters for declined offers.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 
  src/tests/balloon_framework_test.sh a242f6cb9ca1850e5fef90e0938f41044bdaddbf 

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


Testing
---

```
make check 

sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
```


Thanks,

Joseph Wu

Re: Review Request 48046: Implement v1::master::Call::GET_TASKS.

[Jay Guo]

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

(Updated June 9, 2016, 11:17 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Implement v1::master::Call::GET_TASKS.


Diffs (updated)
-

  include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

make check on ubuntu 14.04


Thanks,

Jay Guo

Re: Review Request 48400: Extract task collection logic to helper function.

[Jay Guo]

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

(Updated June 9, 2016, 11:13 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Fix indentation.


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


Repository: mesos


Description
---

Add a helper function _tasks() to collect tasks. This is
to be used by both tasks() and getTasks() in operator API.


Diffs (updated)
-

  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---

make check on ubuntu 14.04


Thanks,

Jay Guo

Re: Review Request 48299: Updated balloon framework spacing and logging.

[Joseph Wu]

> On June 7, 2016, 2:11 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 52-53
> > 
> >
> > IIRC for constructors we use the format on the left.

I think both are acceptable, but the code-base prefers this style.  i.e.
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/containerizer.cpp#L369-L384


- Joseph


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


On June 9, 2016, 4:09 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48299/
> ---
> 
> (Updated June 9, 2016, 4:09 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes some whitespace for the balloon framework and
> changes most logging messages to use glog.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 
> 
> Diff: https://reviews.apache.org/r/48299/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 48299: Updated balloon framework spacing and logging.

[Joseph Wu]

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

(Updated June 9, 2016, 4:09 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Tweaked two log messages.


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


Repository: mesos


Description
---

Changes some whitespace for the balloon framework and
changes most logging messages to use glog.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 46407: Updated balloon executor.

[Joseph Wu]

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

(Updated June 9, 2016, 4:09 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Comment tweak + move TASK_RUNNING status update into the thread.


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


Repository: mesos


Description
---

Fixes the logic in the example `balloon-executor` to work in cases not
exercised by the `ROOT_CGROUPS_BalloonFramework` test.

* The "task" logic was moved into a separate thread.  This fixes the 
  case where the balloon executor does not exceed the memory limit.
  (i.e. by Unblocking the driver's thread while sending status updates.)
* Changed log messages to use glog.
* Added logic to prevent multiple task launches with the same executor.


Diffs (updated)
-

  src/examples/balloon_executor.cpp 2a79c353e96930d25495732d15b83c82247974bb 

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


Testing
---

See next review in this chain.


Thanks,

Joseph Wu

Re: Review Request 48518: Moved release plan from wiki to page.

[Jie Yu]

> On June 9, 2016, 10:59 p.m., Tomasz Janiszewski wrote:
> > I have some concerns with this patch.
> > 
> > 1. Where it should be placed?
> > 2. Where is should be linked?
> > 3. It duplicates data from Downloads, maybe it could be merged into one 
> > page.

Tomasz, thanks for doing this.

This is slightly different from Downloads, because it might also contains 
information about a future release (e.g., what will be the major feature 
expected for a future release).

I agree with you that there's some duplication right now. Let's create a ticket 
to track the merge of these two pages. Once that's resolved, we can use the 
same link from the home page? An alternative is have a link in the download 
page to point to this page. But I like the idea of merging the Download page 
with this!


- Jie


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


On June 9, 2016, 10:55 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48518/
> ---
> 
> (Updated June 9, 2016, 10:55 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5590
> https://issues.apache.org/jira/browse/MESOS-5590
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/release_plan.yaml PRE-CREATION 
>   site/source/release-plan.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48518/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/release-plan/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48046: Implement v1::master::Call::GET_TASKS.

[Vinod Kone]

> On June 9, 2016, 9:26 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, line 171
> > 
> >
> > What does this comment mean? I didn't follow.
> > 
> > Also, comments should start with a capital letter and end in a period.
> 
> Jay Guo wrote:
> This line of code is intended to invoke `set_has_get_tasks()` (private 
> method), which is to toggle `GET_TASKS` bit.
> ```
> inline void Call::set_has_get_tasks() {
>   _has_bits_[0] |= 0x0020u;
> }
> ```
> However this line looks a bit weird, so I left the comment there. If you 
> know any other way to toggle the bit, let me know.

I see. Just kill the comment. I think it is clear that you creating get_tasks() 
object.


- Vinod


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


On June 9, 2016, 12:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48046/
> ---
> 
> (Updated June 9, 2016, 12:49 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement v1::master::Call::GET_TASKS.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48046/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48518: Moved release plan from wiki to page.

[Tomasz Janiszewski]

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



I have some concerns with this patch.

1. Where it should be placed?
2. Where is should be linked?
3. It duplicates data from Downloads, maybe it could be merged into one page.

- Tomasz Janiszewski


On June 9, 2016, 10:55 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48518/
> ---
> 
> (Updated June 9, 2016, 10:55 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5590
> https://issues.apache.org/jira/browse/MESOS-5590
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/release_plan.yaml PRE-CREATION 
>   site/source/release-plan.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48518/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/release-plan/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48518: Moved release plan from wiki to page.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 9, 2016, 10:55 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48518/
> ---
> 
> (Updated June 9, 2016, 10:55 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5590
> https://issues.apache.org/jira/browse/MESOS-5590
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/release_plan.yaml PRE-CREATION 
>   site/source/release-plan.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48518/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/release-plan/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48286: Implemented STOP_MAINTENANCE Call in v1 master API.

[Vinod Kone]

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


Ship it!




No tests for this and the previous review?

- Vinod Kone


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48286/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5507
> https://issues.apache.org/jira/browse/MESOS-5507
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented STOP_MAINTENANCE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48286/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 48518: Moved release plan from wiki to page.

[Tomasz Janiszewski]

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

Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Change was discussed at
http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html


Diffs
-

  site/data/release_plan.yaml PRE-CREATION 
  site/source/release-plan.html.erb PRE-CREATION 

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


Testing
---

Page could be viewed at http://janisz.github.io/mesos/site/publish/release-plan/


Thanks,

Tomasz Janiszewski

Re: Review Request 48285: Implemented START_MAINTENANCE Call in v1 master API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48285/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5506
> https://issues.apache.org/jira/browse/MESOS-5506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented START_MAINTENANCE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48285/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48260: Added test case `MasterAPITest.GetMaintenanceStatus`.

[Vinod Kone]

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




src/tests/api_tests.cpp (lines 323 - 337)


same comments as previous review.


- Vinod Kone


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48260/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5503
> https://issues.apache.org/jira/browse/MESOS-5503
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case `MasterAPITest.GetMaintenanceStatus`.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48260/diff/
> 
> 
> Testing
> ---
> 
> Test in CentOS 7.2
> 
> ```
> ./bin/mesos-tests.sh --gtest_filter="*MasterAPITest*"
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48259: Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.

[Vinod Kone]

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




src/tests/api_tests.cpp (line 73)


make this private? 

also move this below public functions.



src/tests/api_tests.cpp (line 113)


this returns `Nothing` and not `Accepted`.

I actually think this helper is a bit confusing than useful. Just calling 
http::post() directly in the tests that don't expect a v1::Response is probably 
better?



src/tests/api_tests.cpp (line 261)


s/schedule/v1Schedule/



src/tests/api_tests.cpp (lines 264 - 268)


lets just have an evolve function that evolves unversioned schedule to 
v1::maintenance::schedule?



src/tests/api_tests.cpp (lines 295 - 299)


lets have and use devolve() here.


- Vinod Kone


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48259/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5504
> https://issues.apache.org/jira/browse/MESOS-5504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48046: Implement v1::master::Call::GET_TASKS.

[Jay Guo]

> On June 9, 2016, 9:26 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, line 171
> > 
> >
> > What does this comment mean? I didn't follow.
> > 
> > Also, comments should start with a capital letter and end in a period.

This line of code is intended to invoke `set_has_get_tasks()` (private method), 
which is to toggle `GET_TASKS` bit.
```
inline void Call::set_has_get_tasks() {
  _has_bits_[0] |= 0x0020u;
}
```
However this line looks a bit weird, so I left the comment there. If you know 
any other way to toggle the bit, let me know.


- Jay


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


On June 9, 2016, 12:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48046/
> ---
> 
> (Updated June 9, 2016, 12:49 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement v1::master::Call::GET_TASKS.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48046/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48428: Updated Docker::run to return exit status of container.

[Benjamin Mahler]

> On June 9, 2016, 7:31 p.m., Joseph Wu wrote:
> > src/docker/docker.cpp, line 705
> > 
> >
> > Is there any reason why this `.onDiscard` isn't chained onto the 
> > `return s->status()` below instead of the `s->status()` above?

Chatted with joseph offline, the main reason to call it out separate from the 
return type is for readability. If this was a .then that affected the result it 
would make more sense to have it in the return statement, but since it's about 
setting up discard handling, it seems more readable to have it separate from 
what we return to the caller.


> On June 9, 2016, 7:31 p.m., Joseph Wu wrote:
> > src/slave/containerizer/docker.cpp, lines 1246-1249
> > 
> >
> > Since the `run`'s failure is now propagated manually below, is it still 
> > necessary to associate the `inspect` future within a callback?
> > 
> > i.e. rather than 
> > ```
> > promise->associate(inspect);
> > ```

Chatted with joseph offline, association binds the promise's future to another 
future, at which point the promise can no longer be explicitly transitioned. We 
should document this in the promise API :)


- Benjamin


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


On June 9, 2016, 3:57 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48428/
> ---
> 
> (Updated June 9, 2016, 3:57 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4279
> https://issues.apache.org/jira/browse/MESOS-4279
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, Docker::run would return a Future which
> requires the caller to perform an inspect to obtain the exit
> status of the container. This is difficult to get right, and
> so this patch leverages the fact that 'docker run' already
> returns the exit status from the container termination to
> simplify the calling code. Namely, in the docker executor we
> need to use the exit status to determine how to send a
> terminal status update (see the command executor code).
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9ffc18496718701fac8182506a8c36e21e9c319 
>   src/docker/docker.cpp ffbd7ef56d02dfe550a60851251e6645d2ca5925 
>   src/docker/executor.cpp 1b3a7795b1db83394d4b884c1041c341f88a7df1 
>   src/slave/containerizer/docker.cpp 63efbe6f45958d44d60fe4a7fea816f5fb0457b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 5591a6784afd10b4c7535f90c2e6745fa74c318b 
>   src/tests/containerizer/docker_tests.cpp 
> 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f 
>   src/tests/mesos.hpp 6f2888023c957f0af4f7374c98e406816a8423e3 
> 
> Diff: https://reviews.apache.org/r/48428/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 48429: Fixed broken docker logging redirection in the docker executor.

[Benjamin Mahler]

> On June 9, 2016, 7:43 p.m., Joseph Wu wrote:
> > src/docker/executor.cpp, lines 307-314
> > 
> >
> > If the `docker inspect` times out, is it possible that we still try to 
> > `docker stop` on a container docker does not know about?
> > 
> > And is it possible to leak a container if:
> > 1) `docker run ...` starts.
> > 2) `docker inspect` does not complete.
> > 3) Kill is issued.
> > 4) `docker inspect` times out.
> > 5) `_killTask` sends a `TASK_KILLED` and `docker stop`.
> > 6) `docker stop` does not kill the container because it hasn't started 
> > yet.

Chatted with joseph, this case is not possible because we'll only discard the 
inspect when the container has terminated (in reap). When we try to stop the 
container we do not discard the 'inspect' due to danger of leaving a container 
running.


- Benjamin


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


On June 9, 2016, 4:04 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48429/
> ---
> 
> (Updated June 9, 2016, 4:04 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4279 and MESOS-5195
> https://issues.apache.org/jira/browse/MESOS-4279
> https://issues.apache.org/jira/browse/MESOS-5195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, the docker executor will SIGKILL the 'docker run'
> subprocess immediately upon receiving a kill task request.
> This is problematic in that the 'docker run' subprocess is
> responsible for streaming the stdout and stderr from the
> container. When we kill the 'docker run' subprocess, we no
> longer stream the output.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 1b3a7795b1db83394d4b884c1041c341f88a7df1 
> 
> Diff: https://reviews.apache.org/r/48429/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 48511: Renamed `responseContentType` to `responseType`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48510, 48511]

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

- Mesos ReviewBot


On June 9, 2016, 9:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48511/
> ---
> 
> (Updated June 9, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Minor: The type of the variable is already `ContentType`. The
> name `responseType` has sufficient information that this
> refers to the content type of the response.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48511/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48084/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5503
> https://issues.apache.org/jira/browse/MESOS-5503
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::master::Call::GET_MAINTENANCE_STATUS.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48084/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48257: Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.

[Vinod Kone]

> On June 9, 2016, 10 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 2874
> > 
> >
> > to be consistent with how we did others, lets have this function return 
> > JSON::Object, i.e., JSON::Protobuf(schedule)
> > 
> > that way the evolve overload can still take JSON::Object.

looks like you are doing the same thing in the next review as well. i guess 
that's a pattern we could use as well. dropping this.


- Vinod


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


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48257/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5504
> https://issues.apache.org/jira/browse/MESOS-5504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48257/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48257: Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48257/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5504
> https://issues.apache.org/jira/browse/MESOS-5504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48257/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48257: Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.

[Vinod Kone]

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




src/master/http.cpp (line 2874)


to be consistent with how we did others, lets have this function return 
JSON::Object, i.e., JSON::Protobuf(schedule)

that way the evolve overload can still take JSON::Object.


- Vinod Kone


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48257/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5504
> https://issues.apache.org/jira/browse/MESOS-5504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48257/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48116/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5505
> https://issues.apache.org/jira/browse/MESOS-5505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48116/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[Anand Mazumdar]

> On June 9, 2016, 9:14 p.m., Joseph Wu wrote:
> > src/master/http.cpp, lines 3286-3289
> > 
> >
> > I probably missed a discussion elsewhere...
> > 
> > Aren't all (master) operator API's supposed to return 
> > `v1::master::Response`?

We recently changed them. Here is the dev@ discussion thread: 
https://mail-archives.apache.org/mod_mbox/mesos-dev/201606.mbox/%3CCAFt%3DROPU%2B22%3D0JUnuWcSFdc3KU_5i5n44DJa_v1pr6QwTyfqxA%40mail.gmail.com%3E


- Anand


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


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48084/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5503
> https://issues.apache.org/jira/browse/MESOS-5503
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::master::Call::GET_MAINTENANCE_STATUS.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48084/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 48511: Renamed `responseContentType` to `responseType`.

[Anand Mazumdar]

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

Review request for mesos, haosdent huang and Vinod Kone.


Repository: mesos


Description
---

Minor: The type of the variable is already `ContentType`. The
name `responseType` has sufficient information that this
refers to the content type of the response.


Diffs
-

  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 

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


Testing
---


Thanks,

Anand Mazumdar

Review Request 48510: Passed `ContentType` enum by value instead of const ref.

[Anand Mazumdar]

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

Review request for mesos, haosdent huang and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 

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


Testing
---


Thanks,

Anand Mazumdar

Re: Review Request 48450: Add unit test for master /tasks endpoint.

[Vinod Kone]

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




src/tests/master_tests.cpp (lines 2495 - 2518)


same comments apply as in the previous review.



src/tests/master_tests.cpp (line 2558)


this is awesome. thanks for going the extra mile to write this test!!


- Vinod Kone


On June 9, 2016, midnight, Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48450/
> ---
> 
> (Updated June 9, 2016, midnight)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add unit test for master /tasks endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 34be015aa314a7574e9065efb7b1bb8e1570c5b7 
> 
> Diff: https://reviews.apache.org/r/48450/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04 & OSX
> 
> I didn't find unit test for this endpoint, therefore adding it.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48046: Implement v1::master::Call::GET_TASKS.

[Vinod Kone]

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




src/master/http.cpp (line 645)


you want to rebase this on top of haosdent's recent changes that I 
committed.



src/master/http.cpp (lines 1412 - 1415)


uncomment this after you rebase because `getTasks` will return 
http::Response() now.



src/tests/api_tests.cpp (line 164)


s/presents/is present/



src/tests/api_tests.cpp (line 171)


What does this comment mean? I didn't follow.

Also, comments should start with a capital letter and end in a period.



src/tests/api_tests.cpp (line 217)


give it a name, say "test"



src/tests/api_tests.cpp (lines 229 - 233)


do we really need this expectation? i think scheduler receiving the update 
should be enough to gurantee that master has already updated the state of the 
task?



src/tests/api_tests.cpp (line 246)


see above. kill this?



src/tests/api_tests.cpp (line 249)


ditto. see above.



src/tests/api_tests.cpp (line 262)


align this with first argument.

```
ASSERT_EQ(arg1,
  arg2);
```


- Vinod Kone


On June 9, 2016, 12:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48046/
> ---
> 
> (Updated June 9, 2016, 12:49 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement v1::master::Call::GET_TASKS.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48046/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48455: Removed unused import from sorter.cpp.

[Benjamin Mahler]

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




src/master/allocator/sorter/drf/sorter.cpp 


Rather than just removing this one, would you mind doing an audit of the 
headers? We're missing a lot of includes here.


- Benjamin Mahler


On June 9, 2016, 1:30 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48455/
> ---
> 
> (Updated June 9, 2016, 1:30 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused import from sorter.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 65d473a5da0d846214c930c14d333040b2085b13 
> 
> Diff: https://reviews.apache.org/r/48455/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[Joseph Wu]

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



Content LGTM, but the return types are inconsistent with other operator API 
return types.


src/master/http.cpp (lines 3275 - 3278)


I probably missed a discussion elsewhere...

Aren't all (master) operator API's supposed to return 
`v1::master::Response`?


- Joseph Wu


On June 9, 2016, 10:49 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48084/
> ---
> 
> (Updated June 9, 2016, 10:49 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5503
> https://issues.apache.org/jira/browse/MESOS-5503
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::master::Call::GET_MAINTENANCE_STATUS.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48084/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48478: Created a `cgroups/devices` isolator.

[Benjamin Mahler]

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


Ship it!




Looks good, I made some minor tweaks and added the new files to the cmake build.

We should have some tests for this new isolator, I'll leave MESOS-5582 open for 
the addition of the tests.

- Benjamin Mahler


On June 9, 2016, 7:34 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48478/
> ---
> 
> (Updated June 9, 2016, 7:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5582
> https://issues.apache.org/jira/browse/MESOS-5582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, all the logic for the `cgroups/devices` isolator was
> bundled into the Nvidia GPU Isolator. Now we have abstracted it out
> into it's own component and removed the redundant logic from the
> Nvidia GPU Isolator. Because of the new guaranteed ordering between
> isolators, we can be sure that the dependency order between the
> `cgroups/devices` and `gpu/nvidia` isolators is met.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 29525c960e8fb2448260efdd774fd8fc1d68047b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> c7b9744463cf8e1921dcb5e2b7dec7d4e2c0e45f 
>   src/slave/containerizer/mesos/isolators/cgroups/devices.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/devices.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> f290e3c1d1114698039a505e972793dc325c7100 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> 7849e518af448d9557ca4d6de4ccaba8bc572992 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> b1e64b2fc5518463cd14e119f89e3cd298286052 
> 
> Diff: https://reviews.apache.org/r/48478/diff/
> 
> 
> Testing
> ---
> 
> make -j check
> GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48477: Added ability to express dependencies between isolators.

[Benjamin Mahler]

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


Ship it!




Very nice!

I made some slight adjustments, let me know if you see any issues!

```
diff --git a/src/slave/containerizer/mesos/containerizer.cpp 
b/src/slave/containerizer/mesos/containerizer.cpp
index 0e2a9c8..b151121 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -257,13 +257,17 @@ Try MesosContainerizer::create(
   }

   // Create isolators for the MesosContainerizer.
-  // The order of elements in this vector is important. It
-  // defines the relative dependencies between each isolator.
-  // Isolators that appear later in the vector depend on
-  // isolators that appear earlier. Specifically, this means that
-  // the `create()` and `prepare()` calls for each isolator are
-  // serialized in the order in which they appear, while the
-  // `cleanup()` call is serialized in reverse order.
+  //
+  // The order of elements in the creation vecot is used to express
+  // ordering dependencies between isolators. Currently, the `create`
+  // and `prepare` calls for each isolator are run serially in the
+  // order in which they appear in this list, while the `cleanup` call
+  // is serialized in reverse order. The current dependencies are:
+  //
+  //   (1) The filesystem isolator must be the first isolator so
+  //   that the runtime isolators have a consistent view of
+  //   the prepared filesystem (e.g. volume mounts).
+
   const vector(const Flags&)>>>
 creators = {
 // Filesystem isolators.
@@ -315,33 +319,34 @@ Try MesosContainerizer::create(

   vector isolators;

-  vector types_ = strings::tokenize(flags_.isolation, ",");
-  set types = set(types_.begin(), types_.end());
+  const vector tokens = strings::tokenize(flags_.isolation, ",");
+  const set isolations(tokens.begin(), tokens.end());

-  foreach (const auto creator, creators) {
-if (types.count(creator.first) > 0) {
+  if (isolations.size() != tokens.size()) {
+return Error("Duplicate entries found in --isolation flag"
+ " '" + stringify(tokens) + "'");
+  }
+
+  foreach (const auto& creator, creators) {
+if (isolations.count(creator.first) > 0) {
   Try isolator = creator.second(flags_);
   if (isolator.isError()) {
-return Error("Could not create isolator '" +
- creator.first + "': " + isolator.error());
+return Error("Failed to create isolator '" + creator.first + "': " +
+ isolator.error());
   }
-
-  isolators.push_back(Owned(isolator.get()));
-  types.erase(creator.first);
+  isolators.emplace_back(isolator.get());
 } else if (ModuleManager::contains(creator.first)) {
   Try isolator = ModuleManager::create(creator.first);
   if (isolator.isError()) {
-return Error("Could not create isolator '" +
- creator.first + "': " + isolator.error());
+return Error("Could not create isolator '" + creator.first + "': " +
+ isolator.error());
   }
-
-  isolators.push_back(Owned(isolator.get()));
-  types.erase(creator.first);
+  isolators.emplace_back(isolator.get());
 }
   }

-  if (types.size() != 0) {
-return Error("Unknown or unsupported isolators '" + stringify(types) + 
"'");
+  if (isolations.empty()) {
+return Error("Unknown --isolation entries '" + stringify(isolations) + 
"'");
   }

   return new MesosContainerizer(
```

- Benjamin Mahler


On June 9, 2016, 7:31 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48477/
> ---
> 
> (Updated June 9, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5581
> https://issues.apache.org/jira/browse/MESOS-5581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some isolators depend on other isolators. However, we previously did
> not have a generic method of expressing these dependencies. We special
> cased the `filesystem/*` isolators to make sure that dependencies on
> them were satisfied, but no other dependencies could be expressed.
> 
> Now we use a vector to represent the pairing of isolator name to
> isolator creator function, and the relative dependencies between each
> isolator is implicit in the ordering of this vector. Previously, a
> hashmap was used to hold this pairing, but this was inadequate because
> hashmaps are inherently unordered. The new implementation using a
> vector ensures everything 

Re: Review Request 48257: Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.

[Joseph Wu]

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




src/master/http.cpp (line 662)


Here it would make sense to use the `.then(serializer)`.



src/master/http.cpp (line 2916)


This should return `Future` and use the `serializer` 
to do the conversion to `Future`.


- Joseph Wu


On June 9, 2016, 10:49 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48257/
> ---
> 
> (Updated June 9, 2016, 10:49 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5504
> https://issues.apache.org/jira/browse/MESOS-5504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48257/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47664: Implemented image user support in docker runtime isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 101)


I suggest we return a Failure if container user is specified since we don't 
support it right now.


- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47664/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented image user support in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> 272636fcf12c865b31a11364f23d7aee9c6225db 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> eb2790d44ee08c81d31fbe28e7a8ffe88edba870 
> 
> Diff: https://reviews.apache.org/r/47664/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47662: Added image user to command executor.

[Jie Yu]

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



Since we do not support container user yet, can we punt on this patch? In 
containerizer, if a 'user' is specified for container, we should return an 
Error instead?

- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47662/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added image user to command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp e4c3b75b647b54ffecedfdfa34fb3925f686c0c7 
> 
> Diff: https://reviews.apache.org/r/47662/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[zhou xing]

> On 六月 9, 2016, 6:12 p.m., haosdent huang wrote:
> > Looks great to me. XD

Thanks hasdent for the review! hope some day can meet you in person:)

I will update the patch later


- zhou


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


On 六月 8, 2016, 6:37 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48438/
> ---
> 
> (Updated 六月 8, 2016, 6:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: mesos-5491
> https://issues.apache.org/jira/browse/mesos-5491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement the basic getAgents method for v1 operator master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48438/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

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

[Anand Mazumdar]

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



Thanks for taking this on. Looks pretty good minus a small bug that can lead to 
a crash (see review comments).


src/scheduler/constants.hpp (line 28)


I won't call it a backoff since it's just a _delay_ i.e. a wait time before 
the scheduler library tries to establish a connection with the master.

How about: `DEFAULT_CONNECTION_DELAY_MAX`



src/scheduler/flags.hpp (lines 35 - 36)


How about:

```cpp
The maximum amount of time to wait before trying to initiate a connection 
with the master. The library waits for a random amount of time between [0, b], 
where `b = connection_delay_max` before initiating a (re-)connection attempt 
with the master.
```



src/scheduler/flags.hpp (line 40)


s/connection_backoff_max/connection_delay_max



src/scheduler/scheduler.cpp (line 73)


Let's reorder these as per comments from @haosdent.



src/scheduler/scheduler.cpp (line 196)


Why do we need this variable? Can't we directly use 
`flags.connection_delay_max`?



src/scheduler/scheduler.cpp (lines 475 - 476)


How about:

```
// Wait for a random duration between 0 and `flags.connection_delay_max` 
before (re-)connecting with the master.
```



src/scheduler/scheduler.cpp (line 480)


How about:

```cpp
VLOG(1) << "Waiting for " << delay << " before initiating a (re-)connection 
attempt with the master";
```



src/scheduler/scheduler.cpp (line 481)


hmmm, this can crash the library in its current form due to a failed 
invariant check:

Consider this scenario:
- A new master is detected. You introduce a random amount of delay in 
connecting with this master.
- The new master fails over and a new master is detected before the `delay` 
can be invoked.
- The `delay` for the second master is lesser and the library connects with 
the master.
- The original `delay` now fires and we fail this `CHECK` invariant on: 
https://github.com/apache/mesos/blob/master/src/scheduler/scheduler.cpp#L320

Here is how to get around this:

Let's initialize `connectionId` here and not in `connect()` on L474.

The signature of `connect()` would then change to something like:
`void connect(const UUID& _connectionId)`

This is how a modified `connect` would look like:

```cpp
void connect(const UUID& _connectionId)
{
  // It is possible that a new master was detected while we were waiting to 
establish a connection with the old master.
  if (connectionId != _connectionId) {
VLOG(1) << "Ignoring connection attempt from stale connection";
return;
  }
   // existing content of `connect()` thereafter.
```

Makes sense?



src/scheduler/scheduler.cpp (line 739)


Kill this.


- Anand Mazumdar


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

Re: Review Request 48381: Updated CHANGELOG for 'work_dir' flag changes.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [48381, 48380]

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

Error:
2016-06-09 19:45:38 URL:https://reviews.apache.org/r/48381/diff/raw/ [640/640] 
-> "48381.patch" [1]
error: patch failed: CHANGELOG:153
error: CHANGELOG: patch does not apply

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

- Mesos ReviewBot


On June 9, 2016, 6:16 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48381/
> ---
> 
> (Updated June 9, 2016, 6:16 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CHANGELOG for 'work_dir' flag changes.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 
> 
> Diff: https://reviews.apache.org/r/48381/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 48429: Fixed broken docker logging redirection in the docker executor.

[Joseph Wu]

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




src/docker/executor.cpp (lines 307 - 314)


If the `docker inspect` times out, is it possible that we still try to 
`docker stop` on a container docker does not know about?

And is it possible to leak a container if:
1) `docker run ...` starts.
2) `docker inspect` does not complete.
3) Kill is issued.
4) `docker inspect` times out.
5) `_killTask` sends a `TASK_KILLED` and `docker stop`.
6) `docker stop` does not kill the container because it hasn't started yet.


- Joseph Wu


On June 8, 2016, 9:04 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48429/
> ---
> 
> (Updated June 8, 2016, 9:04 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4279 and MESOS-5195
> https://issues.apache.org/jira/browse/MESOS-4279
> https://issues.apache.org/jira/browse/MESOS-5195
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, the docker executor will SIGKILL the 'docker run'
> subprocess immediately upon receiving a kill task request.
> This is problematic in that the 'docker run' subprocess is
> responsible for streaming the stdout and stderr from the
> container. When we kill the 'docker run' subprocess, we no
> longer stream the output.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 1b3a7795b1db83394d4b884c1041c341f88a7df1 
> 
> Diff: https://reviews.apache.org/r/48429/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 48504: Updated libprocess to support GET_METRICS.

[Abhishek Dasgupta]

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

(Updated June 9, 2016, 7:37 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description (updated)
---

[WIP] Updated libprocess to support GET_METRICS.


Diffs
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

Abhishek Dasgupta

Review Request 48504: Updated libprocess to support GET_METRICS.

[Abhishek Dasgupta]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Updated libprocess to support GET_METRICS.


Diffs
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

Abhishek Dasgupta

Re: Review Request 48428: Updated Docker::run to return exit status of container.

[Joseph Wu]

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


Fix it, then Ship it!




LGTM


src/docker/docker.cpp (line 699)


Is there any reason why this `.onDiscard` isn't chained onto the `return 
s->status()` below instead of the `s->status()` above?



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


Typo: s/propgage/propagate/



src/slave/containerizer/docker.cpp (lines 1246 - 1248)


Since the `run`'s failure is now propagated manually below, is it still 
necessary to associate the `inspect` future within a callback?

i.e. rather than 
```
promise->associate(inspect);
```


- Joseph Wu


On June 8, 2016, 8:57 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48428/
> ---
> 
> (Updated June 8, 2016, 8:57 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4279
> https://issues.apache.org/jira/browse/MESOS-4279
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, Docker::run would return a Future which
> requires the caller to perform an inspect to obtain the exit
> status of the container. This is difficult to get right, and
> so this patch leverages the fact that 'docker run' already
> returns the exit status from the container termination to
> simplify the calling code. Namely, in the docker executor we
> need to use the exit status to determine how to send a
> terminal status update (see the command executor code).
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9ffc18496718701fac8182506a8c36e21e9c319 
>   src/docker/docker.cpp ffbd7ef56d02dfe550a60851251e6645d2ca5925 
>   src/docker/executor.cpp 1b3a7795b1db83394d4b884c1041c341f88a7df1 
>   src/slave/containerizer/docker.cpp 63efbe6f45958d44d60fe4a7fea816f5fb0457b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 5591a6784afd10b4c7535f90c2e6745fa74c318b 
>   src/tests/containerizer/docker_tests.cpp 
> 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f 
>   src/tests/mesos.hpp 6f2888023c957f0af4f7374c98e406816a8423e3 
> 
> Diff: https://reviews.apache.org/r/48428/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 47663: Added image user to 'ContainerLaunchInfo'.

[Jie Yu]

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




include/mesos/slave/isolator.proto (line 133)


See my comment in the following patch. Let's do:
```
// The user name or UID in the container.
optional string user = 7;

// The group name or GID in the container.
optional string group = 8;

// TODO: Add repeated supplementary groups.
```

No need 'image' prefix because it's pretty clear this is for the container.


- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47663/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added image user to 'ContainerLaunchInfo'.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 
> 
> Diff: https://reviews.apache.org/r/47663/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47664: Implemented image user support in docker runtime isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 397)


This can be in any of the following form:

https://github.com/docker/docker/blob/master/image/spec/v1.md#container-runconfig-field-descriptions

Are we going to support all of them? I guess we need to introduce a 
protobuf type for encapsulating the user and group info. You should take a look 
at OCI and Appc spec too:
https://github.com/opencontainers/runtime-spec/blob/master/config.md#user
https://github.com/appc/spec/blob/master/spec/aci.md

Maybe for now, we can add 'user' and 'group' in ContainerLaunchInfo, and 
document that they can either be numerical or non numerical. There are 
different meanings.

Also, add a TODO there, we probably need to handle suplementary groups as 
well in the future.


- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47664/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented image user support in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> 272636fcf12c865b31a11364f23d7aee9c6225db 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> eb2790d44ee08c81d31fbe28e7a8ffe88edba870 
> 
> Diff: https://reviews.apache.org/r/47664/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48291: Use varint comparator in replica log.

[Bing Li]

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



make check passed.
I guess there are already test cases that cover this change.

- Bing Li


On June 6, 2016, 7:54 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48291/
> ---
> 
> (Updated June 6, 2016, 7:54 p.m.)
> 
> 
> Review request for mesos, Bing Li, Benjamin Mahler, Zhiwei Chen, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-970
> https://issues.apache.org/jira/browse/MESOS-970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since bug discussed at
> https://groups.google.com/forum/#!topic/leveldb/F-rDkWiQm6c
> was fixed. After upgrading leveldb to 1.18 we could replace
> default byte-wise comparator with varint comparator.
> 
> 
> Diffs
> -
> 
>   src/log/leveldb.cpp f389d74b123574665c611b46cb52e3dc7042b331 
> 
> Diff: https://reviews.apache.org/r/48291/diff/
> 
> 
> Testing
> ---
> 
> `make check dist` on Ubuntu x64
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48286: Implemented STOP_MAINTENANCE Call in v1 master API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48115, 48116, 48257, 48084, 48259, 48260, 48285, 48286]

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

- Mesos ReviewBot


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48286/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5507
> https://issues.apache.org/jira/browse/MESOS-5507
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented STOP_MAINTENANCE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48286/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48046: Implement v1::master::Call::GET_TASKS.

[Jay Guo]

> On June 9, 2016, 6:20 p.m., haosdent huang wrote:
> > Hi, @guoger. Now the return type of RPC handlers become 
> > process::http::Response, may you rebase your patch?

Good job! working on it!


- Jay


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


On June 9, 2016, 12:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48046/
> ---
> 
> (Updated June 9, 2016, 12:49 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement v1::master::Call::GET_TASKS.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48046/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48380: Updated CHANGELOG for libprocess HTTP authorization.

[Greg Mann]

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

(Updated June 9, 2016, 6:33 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Updated CHANGELOG for libprocess HTTP authorization.


Diffs (updated)
-

  CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 48380: Updated CHANGELOG for libprocess HTTP authorization.

[Greg Mann]

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

(Updated June 9, 2016, 6:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Addressed comments.


Repository: mesos


Description
---

Updated CHANGELOG for libprocess HTTP authorization.


Diffs (updated)
-

  CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 48046: Implement v1::master::Call::GET_TASKS.

[haosdent huang]

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



Hi, @guoger. Now the return type of RPC handlers become 
process::http::Response, may you rebase your patch?

- haosdent huang


On June 9, 2016, 12:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48046/
> ---
> 
> (Updated June 9, 2016, 12:49 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement v1::master::Call::GET_TASKS.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48046/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48381: Updated CHANGELOG for 'work_dir' flag changes.

[Greg Mann]

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

(Updated June 9, 2016, 6:16 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Addressed comment.


Repository: mesos


Description
---

Updated CHANGELOG for 'work_dir' flag changes.


Diffs (updated)
-

  CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 

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


Testing
---


Thanks,

Greg Mann

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

[haosdent huang]

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



Looks great to me. XD


include/mesos/v1/mesos.proto (lines 1974 - 1976)


Should be required? According to

```
static void json(JSON::ObjectWriter* writer, const Summary& summary)
{
  const Slave& slave = summary;

  writer->field("id", slave.id.value());
  writer->field("pid", string(slave.pid));
  writer->field("hostname", slave.info.hostname());
  writer->field("registered_time", slave.registeredTime.secs());

  if (slave.reregisteredTime.isSome()) {
writer->field("reregistered_time", slave.reregisteredTime.get().secs());
  }
```



include/mesos/v1/mesos.proto (lines 1983 - 1984)


Same questions about optional here.


- haosdent huang


On June 8, 2016, 6:37 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48438/
> ---
> 
> (Updated June 8, 2016, 6:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: mesos-5491
> https://issues.apache.org/jira/browse/mesos-5491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement the basic getAgents method for v1 operator master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48438/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 48499: Moved design docs from wiki to page.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48499]

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

- Mesos ReviewBot


On June 9, 2016, 4:50 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48499/
> ---
> 
> (Updated June 9, 2016, 4:50 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5586
> https://issues.apache.org/jira/browse/MESOS-5586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   docs/design-docs.md PRE-CREATION 
>   docs/home.md 16a77da437584995496d199d58340efa5882f19d 
> 
> Diff: https://reviews.apache.org/r/48499/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/documentation/latest/design-docs/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

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

[haosdent huang]

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



Hi, @dongdong. Now the return type of RPC handlers become 
`process::http::Response`, may you rebase your patch?

- haosdent huang


On June 8, 2016, 6:37 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48438/
> ---
> 
> (Updated June 8, 2016, 6:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: mesos-5491
> https://issues.apache.org/jira/browse/mesos-5491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement the basic getAgents method for v1 operator master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp c255195b69f55f6429beed6c18a4a31b38528840 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48438/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 48259: Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.

[haosdent huang]

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

(Updated June 9, 2016, 5:49 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Add @kaysoky in reviewers.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48286: Implemented STOP_MAINTENANCE Call in v1 master API.

[haosdent huang]

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

(Updated June 9, 2016, 5:49 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Add @kaysoky in reviewers.


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


Repository: mesos


Description
---

Implemented STOP_MAINTENANCE Call in v1 master API.


Diffs
-

  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48259: Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.

[haosdent huang]

> On June 6, 2016, 10:21 p.m., Joseph Wu wrote:
> > src/tests/api_tests.cpp, lines 266-269
> > 
> >
> > Why don't you evolve/devolve here?

@kaysoky, thanks a lot for your review. Because we define

```
v1::master::Response evolve(const maintenance::Schedule& schedule)
```

in evolve.cpp, could not define it and return `v1::maintenance::ClusterStatus` 
again.


It is not nice here, do you have any suggestions to avoid this? How about 
rename `v1::master::Response evolve(const maintenance::Schedule& schedule)` to 
`v1::master::Response evolveToResponse(const maintenance::Schedule& schedule)`. 
Or to create a help routine in api_tests.cpp?


- haosdent


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


On June 9, 2016, 5:32 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48259/
> ---
> 
> (Updated June 9, 2016, 5:32 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5504
> https://issues.apache.org/jira/browse/MESOS-5504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37989: Enhanced log message when launching mesos docker executor.

[Jie Yu]

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



Can we first just log(info) docker run? Looking at the ticket that folks filed, 
looks like this is the one that's been asked the most?

- Jie Yu


On May 31, 2016, 12:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37989/
> ---
> 
> (Updated May 31, 2016, 12:09 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5348
> https://issues.apache.org/jira/browse/MESOS-5348
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced log message when launching mesos docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 19cf424dfd5748ae66a7023840aa2b0652e8f2c0 
>   src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
> 
> Diff: https://reviews.apache.org/r/37989/diff/
> 
> 
> Testing
> ---
> 
> I0511 11:26:18.176844 29178 docker.cpp:1236] Launching 
> 'mesos-docker-executor' with flags 
> '--container="mesos-7f581417-e295-474f-b038-3900ee0479dc-S1.b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --docker="docker" --docker_socket="/var/run/docker.sock" --help="false" 
> --initialize_driver_logging="true" 
> --launcher_dir="/root/src/mesos/m1/mesos/build/src" --logbufsecs="0" 
> --logging_level="INFO" --mapped_directory="/mnt/mesos/sandbox" 
> --quiet="false" 
> --sandbox_directory="/tmp/mesos/slaves/7f581417-e295-474f-b038-3900ee0479dc-S1/frameworks/7f581417-e295-474f-b038-3900ee0479dc-0001/executors/test_mesos/runs/b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --stop_timeout="0ns"'
> 
> Start agent without VLOG_v=1, once the docker container is started, check 
> sandbox log:
> root@mesos002:/tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/latest#
>  cat stderr
> I0526 22:25:59.805173 11422 logging.cpp:195] Logging to STDERR
> I0526 22:25:59.808446 11422 process.cpp:1057] libprocess is initialized on 
> 192.168.56.12:36164 with 8 worker threads
> I0526 22:25:59.809834 11422 exec.cpp:150] Version: 0.29.0
> I0526 22:25:59.813784 11445 exec.cpp:200] Executor started at: 
> executor(1)@192.168.56.12:36164 with pid 11422
> I0526 22:25:59.818483 11446 exec.cpp:225] Executor registered on agent 
> fb930730-03f9-4027-96cb-81d612e8e35f-S0
> I0526 22:25:59.819725 11446 exec.cpp:237] Executor::registered took 272279ns
> I0526 22:25:59.820055 11446 exec.cpp:312] Executor asked to run task 'test'
> I0526 22:25:59.820132 11446 exec.cpp:321] Executor::launchTask took 55859ns
> I0526 22:25:59.820344 11446 docker.cpp:677] Running docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 -e 
> MESOS_SANDBOX=/mnt/mesos/sandbox -e 
> MESOS_CONTAINER_NAME=mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  -v 
> /tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/7accfa4d-909b-4e46-bdf7-6bcdf505f6d6:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  ubuntu:14.04 -c sleep 10
> I0526 22:25:59.822684 11446 docker.cpp:824] Running docker -H 
> unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
> WARNING: Your kernel does not support swap limit capabilities, memory limited 
> without swap.
> I0526 22:25:59.924623 11445 docker.cpp:871] Retrying inspect with non-zero 
> status code. cmd: 'docker -H unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6',
>  interval: 500ms
> I0526 22:26:00.425278 11449 docker.cpp:824] Running docker -H 
> unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
> I0526 22:26:00.544518 11443 exec.cpp:535] Executor sending status update 
> TASK_RUNNING (UUID: b686d7d1-110f-4f42-a392-b9ed9a854d31) for task test of 
> framework fb930730-03f9-4027-96cb-81d612e8e35f-
> I0526 22:26:00.549468 11442 exec.cpp:358] Executor received status update 
> acknowledgement b686d7d1-110f-4f42-a392-b9ed9a854d31 for task test of 
> framework fb930730-03f9-4027-96cb-81d612e8e35f-
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[haosdent huang]

> On June 6, 2016, 9:06 p.m., Joseph Wu wrote:
> > src/master/http.cpp, line 678
> > 
> >
> > Depending on what the final return type ends up being (for 
> > `_updateMaintenanceSchedule`), is this missing a `.then(serializer);` ?

Not need, because we return a empty `OK` result.


- haosdent


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


On June 9, 2016, 5:31 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48116/
> ---
> 
> (Updated June 9, 2016, 5:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5505
> https://issues.apache.org/jira/browse/MESOS-5505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48116/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[haosdent huang]

> On June 6, 2016, 4:30 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 2773
> > 
> >
> > this should return `v1::master::Response`. see below comment.
> 
> Vinod Kone wrote:
> sorry this should return Future

After we change all RPC handlers return type to `Response`, I think could use 
`Response` here now.


> On June 6, 2016, 4:30 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 2951
> > 
> >
> > why want this function to return `v1::master::Response` instead of 
> > `http::Response` to be consistent with how we did for others. since this 
> > response doesn't have a body, this could be an empty `v1::master::Response`.
> 
> haosdent huang wrote:
> Hi, @vinodkone, thank you very much for your review. According to the 
> design document, seems we need return `Accepted` here?
> 
> ```
> Example #2: Call with parameters but no return.
> 
> TEARDOWN_FRAMEWORK HTTP Request (JSON):
> POST /api/v1/  HTTP/1.1
> 
> Host: masterhost:5050
> Content-Type: application/json
> 
> {
>   “type”  : “TEARDOWN_FRAMEWORK”,
> 
>  “teardown_network” : {
> 
> “framework_id” : “1242352-1235235-1235235-235235”
>   
>   }
> }
> 
> TEARDOWN_FRAMEWORK HTTP Response:
> HTTP/1.1 202 Accepted
> ```
> 
> Vinod Kone wrote:
> teardown happens asynchronously, so we send 202. afaict, updating of 
> maintenance schedule is done before we return a response? if yes, that should 
> be a 200? not sure why the original maintenance handler returned 202 in the 
> first place. @kaysoky?
> 
> haosdent huang wrote:
> Oh, got it. So I should return 200 for `UPDATE_MAINTENANCE_SCHEDULE`. 
> Other question is we want to validate `schedule` with `master->machines`.
> 
> ```
>   // Validate that the schedule only transitions machines between
>   // `UP` and `DRAINING` modes.
>   Try isValid = maintenance::validation::schedule(
>   schedule,
>   master->machines);
> ```
> 
> If change
> 
> ```
> Future Master::Http::updateMaintenanceSchedule(
> const v1::master::Call& call,
> const Option& principal) const
> ```
> 
> to 
> 
> ```
> Future Master::Http::updateMaintenanceSchedule(
> const v1::master::Call& call,
> const Option& principal) const
> ```
> 
> seems could not return `BadRequest` in this case. Are there some ways we 
> could avoid this?
> 
> Joseph Wu wrote:
> The original handler returning a 202 was one of the many inconsistencies 
> of operator endpoints, prior to the Operator API :)

Oh, I change to return `OK` now.


- haosdent


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


On June 9, 2016, 5:31 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48116/
> ---
> 
> (Updated June 9, 2016, 5:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5505
> https://issues.apache.org/jira/browse/MESOS-5505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48116/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

[Anindya Sinha]

> On June 8, 2016, 1:28 p.m., Neil Conway wrote:
> > Overall seems like a reasonable approach.
> > 
> > One thing that isn't clear to me: what is the advantage of updating the 
> > checkpoint to reflect any partial work that was done before exiting? It 
> > seems that adds a bunch of complexity and room for error. Why not only 
> > update the checkpoint if all changes were made successfully?
> 
> Anindya Sinha wrote:
> We would need to maintain what was actually successful in any case since 
> in a DESTROY, a failed rmdir does not lead to the agent exiting. So, if we 
> were to do it at one place, we would still need to keep account of the 
> successful operations so as to not update the checkpoint based on a failed 
> rmdir as an example (and hence can be a partial update).
> 
> Since we are keeping track of result of the operations anyway, I think it 
> is a good idea to update before exiting (only place we do that when CREATE 
> fails and the agent exits) so that the subsequent handling of 
> CheckpointResources does not need to redo such operations when the agent 
> reregisters.
> 
> Neil Conway wrote:
> On reflection, I wonder whether we should be handling `CREATE` errors 
> differently from `DESTROY` errors. In both cases, the user has asked the 
> agent to do something it wasn't able to do. A failed `DESTROY` has the 
> addditional concern that we might have destroyed some but not all of the data 
> on the volume.
> 
> Do you think handling `CREATE` vs. `DESTROY` errors differently is a good 
> idea?

Good point. Here is what I think are the use cases:
Say we have checkpointed resources (CR) as {V1,V2} where V1, V2 are 2 
persistent volumes. So, CR(master) = {V1,V2}, and CR(agent) = {V1,V2}.
If we now receive a DESTROY(V2): CR(master) = {V1} [since master's view is not 
dependent on what happened on the agent]. Suppose that fails on the agent, so 
CR(agent) = {V1,V2} [since we do not update checkpoint resources on agent on 
failure in DESTROY, which results in inconsistency between master and agent at 
this point of time].

Case 1 (current implementation): Agent does not restart on failure in DESTROY. 
Hence, CR(agent) = {V1,V2}. When the next CheckpointResources is received, ie. 
on a subsequent CREATE/DESTROY/RESERVE/UNRESERVE on a different resource, 
DESTROY(V2) will be reattempted and if that is successful, we will in sync 
between agent and master. However if the next CheckpointResources is due to a 
CREATE(V2) [that can happen since V2 is available as a resource based on offer 
from master], that would be a no-op on agent since agent does not treat it as a 
new resource based on the checkpoint since at this point CR(master) = {V1,V2}, 
and CR(agent) = {V1,V2}, which would be a problem.

Case 2 (if we exit agent on failure): The agent restarts which triggers a 
CheckpointResources from master->agent on ReregisterSlave. That would force a 
reattempt of DESTROY(V2) since current view is CR(master) = {V1} and CR(agent) 
= {V1,V2} which will reattempt to bring the checkpointed resources back in sync 
between master and agent.

So, I think it might be a better option to exit the agent on failure in DESTROY 
as well. However, I think we should still update the checkpoint based on the 
status of successful operations (other CREATE/DESTROY) on failure (when agent 
exits) so as to avoid these operations to be repeated in a subsequent 
CheckpointResources message. Does that sound reasonable to you?

Note: I think this use case probably is a good example to consider 
StatusUpdates (or something similar) for operations on reserved resources, viz. 
CREATE/DESTROY/RESERVE/UNRESERVE to ensure master and agent are in sync to 
ensure guaranteed view of offers (to frameworks) for reserved resources.


- Anindya


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


On June 9, 2016, 12:22 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48313/
> ---
> 
> (Updated June 9, 2016, 12:22 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5448
> https://issues.apache.org/jira/browse/MESOS-5448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> o Checkpoints on the agent are updated only after successful handling
>   of persistent volume creation and deletion to maintain consistency.
> o If volume creation or deletion fails, checkpoint is updated up until
>   that point, and the agent exits.
> o This ensures that after a agent restart, checkpoints are in sync
>   between the master and the agent after the reregistration workflow.
> 
> 
> Diffs
> -
> 
>   

Re: Review Request 48286: Implemented STOP_MAINTENANCE Call in v1 master API.

[haosdent huang]

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

(Updated June 9, 2016, 5:33 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented STOP_MAINTENANCE Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48260: Added test case `MasterAPITest.GetMaintenanceStatus`.

[haosdent huang]

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

(Updated June 9, 2016, 5:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.GetMaintenanceStatus`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

Test in CentOS 7.2

```
./bin/mesos-tests.sh --gtest_filter="*MasterAPITest*"
```


Thanks,

haosdent huang

Re: Review Request 48285: Implemented START_MAINTENANCE Call in v1 master API.

[haosdent huang]

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

(Updated June 9, 2016, 5:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented START_MAINTENANCE Call in v1 master API.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48259: Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.

[haosdent huang]

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

(Updated June 9, 2016, 5:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[haosdent huang]

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

(Updated June 9, 2016, 5:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::master::Call::GET_MAINTENANCE_STATUS.


Diffs (updated)
-

  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang