Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[Vinod Kone]

> On June 13, 2016, 10:07 p.m., Vinod Kone wrote:
> > src/master/http.cpp, lines 596-632
> > 
> >
> > s/mesos:://
> 
> Anand Mazumdar wrote:
> I guess @haosdent had to add them due to there being an already existing 
> `master` namespace in our code. The compiler would have barfed otherwise due 
> to it not being able to find a `Call` class in the existing `master` 
> namespace. Hence, it _had_ to be fully qualified.
> 
> Since, we _only_ use the unversioned protobuf internally in our code, we 
> should just do `using mesos::master::Call` and then just use `Call::TYPE` for 
> referring to it. Sound good?
> 
> Vinod Kone wrote:
> I see. Qualified mesos::master::Call seems fine then. `using 
> mesos::master::Call`  might be weird because we also have scheduler::Call in 
> this file.
> 
> haosdent huang wrote:
> Oh, yes, as @anandmazumdar said, we need add mesos:: otherwise compiler 
> return errors. @vinodkone Could I drop these issues?

yup!


- Vinod


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


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

[Yanyan Hu]

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

(Updated June 15, 2016, 3:39 a.m.)


Review request for mesos, Guangya Liu and Joris Van Remoortere.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make check


Thanks,

Yanyan Hu

Re: Review Request 48593: Refactor Ranges Subtraction.

[Yanyan Hu]

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

(Updated June 15, 2016, 3:36 a.m.)


Review request for mesos, Guangya Liu and Joris Van Remoortere.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs (updated)
-

  3rdparty/Makefile.am 485ed550c37b092763dc5d616a8b69cdff5a3acb 
  3rdparty/libprocess/src/pid.cpp f9313cde006dd067be265343eed60412ad6b0b95 
  3rdparty/stout/include/stout/flags/flags.hpp 
dd9362772d1fbd32638fc7e70126fd49d4a03c68 
  CHANGELOG 0df0d4ccd76379ede6492e595999711e5462f79b 
  docs/agent-recovery.md 6d1b402ae5a3e26832d4fa665ca3666d4c096bc2 
  docs/allocation-module.md 6cb1392aff9eeb5cb4a8f14c91df50f29d25948c 
  docs/authorization.md dcf2160424771c513579063911cc14792f464821 
  docs/documentation-guide.md fd9f5eeecb70381ae80d70ae74866278587a9078 
  docs/executor-http-api.md 17ba6f6a0ecbd271e6b1739ffaf92fba83c0fac1 
  docs/external-containerizer.md eece9e73d47c13cc50074ef77235147f09cd380a 
  docs/high-availability.md e467a0e1d592fa1aa8d8746f59cb753f28fc30e2 
  docs/markdown-style-guide.md 667d6dfbcf6c0864085cdfb19f4e53fb49a49c44 
  docs/mesos-containerizer.md 8dcf1af819741118604af876ad612737b8ff7b32 
  docs/monitoring.md 3cfdaf5de149c5a1a7b3d64ba2870887a092ffc6 
  docs/network-monitoring.md ee999ebbb61c8bb1f38599a0391885cdadff989f 
  docs/operational-guide.md ad858e359bd94b2c5eec397ecbad3f15e0066f5e 
  docs/persistent-volume.md a9775b187637bd6d2d90ab03613b6bb6dc1454bf 
  docs/reservation.md 6408646cd42408514566e323beff548b69b6af41 
  docs/scheduler-http-api.md 4be961214c63e4e3b25c5c350b2c4f0e66863817 
  docs/upgrades.md afc76f7a0e74042bd095b925f8119bfa5daa489c 
  docs/versioning.md da874143b0faed47e388d61aa6a3580711053aea 
  include/mesos/docker/spec.hpp 92367cf942c7ee4c23b1265b659f01d427212f62 
  include/mesos/docker/spec.proto 734395cbde81bc4ff99f1310fc4b913beeb5b11f 
  site/source/blog/2016-06-13-mesos-0-28-2-released.md 
64733af9efd3d0c1d05229582f31fca15158c13c 
  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/docker/spec.cpp 2711578626dd1847f73cbf7bd3771f36e6755a99 
  src/examples/balloon_executor.cpp 7343ee72c1fc8d6e58527809ffb74fc5dd09ee0e 
  src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 
  src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 
  src/master/http.cpp db625f0d656f207a89fcc14b18ae2fc31d30e673 
  src/master/master.hpp a0944ddccd3a4b33458cd2489bb5fcdbbdc55720 
  src/master/master.cpp 1971e9bf3875ceb117130b84533dc37873ca60df 
  src/slave/containerizer/docker.cpp bd4f5930684208d532f4ad1721aae353d309c0a7 
  src/slave/containerizer/mesos/containerizer.cpp 
915e4cdf4fed345515bffbf7cc5147d161640ede 
  src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
78ba4207c78fe51000e1fef487a57a1ab272d43a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
9014fc80e326e585a4f85eb0104de87156195f6d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
ad56cfad823fcd8244e9d9f7bb8b801228c1f06e 
  src/slave/http.cpp 93c23a0b31193b17acd09337337530e984803b48 
  src/slave/slave.hpp ef171188afb7169e7f386547af9fa334a65374af 
  src/slave/slave.cpp 0af04d6fe53f92e03905fb7b3bec72b09d5e8e57 
  src/tests/balloon_framework_test.sh e0b0b04c9e86614cd3696f34d1de2b9f981da840 
  src/tests/containerizer/docker_spec_tests.cpp 
1c6473aa9b9c2e56d32d10dc88d9bc66f563d91e 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
8d24583fef5a1f99763ece26ba55d3b389011a48 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
  src/uri/fetchers/docker.hpp 6cb57be97d9494ea59ce9759e3d52e37b19d6c43 
  src/uri/fetchers/docker.cpp 211be6fdf416621c467faa5592c9e52a49b450ca 

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


Testing
---

make check


Thanks,

Yanyan Hu

Re: Review Request 48587: Added unversioned protos for agent API.

[haosdent huang]

> On June 13, 2016, 8:18 p.m., Anand Mazumdar wrote:
> > Can we name the unversioned proto file as just `agent.proto`? It would be 
> > good to not introduce the `Slave` terminology going forward :-)
> 
> Vinod Kone wrote:
> also the package name, class name, namespace etc.

Awesome! Let me rename.


- haosdent


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


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48587/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unversioned protos for agent API.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/slave.hpp PRE-CREATION 
>   include/mesos/slave/slave.proto PRE-CREATION 
>   src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
> 
> Diff: https://reviews.apache.org/r/48587/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[haosdent huang]

> On June 13, 2016, 10:07 p.m., Vinod Kone wrote:
> > src/master/http.cpp, lines 596-632
> > 
> >
> > s/mesos:://
> 
> Anand Mazumdar wrote:
> I guess @haosdent had to add them due to there being an already existing 
> `master` namespace in our code. The compiler would have barfed otherwise due 
> to it not being able to find a `Call` class in the existing `master` 
> namespace. Hence, it _had_ to be fully qualified.
> 
> Since, we _only_ use the unversioned protobuf internally in our code, we 
> should just do `using mesos::master::Call` and then just use `Call::TYPE` for 
> referring to it. Sound good?
> 
> Vinod Kone wrote:
> I see. Qualified mesos::master::Call seems fine then. `using 
> mesos::master::Call`  might be weird because we also have scheduler::Call in 
> this file.

Oh, yes, as @anandmazumdar said, we need add mesos:: otherwise compiler return 
errors. @vinodkone Could I drop these issues?


- haosdent


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


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48206: Fix example in configuration docs.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On June 3, 2016, 2:31 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48206/
> ---
> 
> (Updated June 3, 2016, 2:31 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previous example configuration of `default_container_info`
> leads to following problem:
> > Failed to launch container: Relative host path '/./.private/tmp'
> > cannot contain relative components; Container destroyed while
> > preparing isolators
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 7612d39d88dc6c0229b5def9697a97ab387f6ef1 
>   src/slave/flags.cpp 7b0e347772646c177c1e18334633a71bc6cedb85 
> 
> Diff: https://reviews.apache.org/r/48206/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

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

[Jie Yu]

> On June 10, 2016, 4:25 p.m., Jie Yu wrote:
> > In fact, for working groups, I would prefer adding that to the docs so that 
> > people can find/read it on github. It is also constantly changing, updating 
> > it in docs sounds easier than updating the website. What do you think?
> 
> Tomasz Janiszewski wrote:
> So this will end the same as [powered by 
> Mesos](http://mesos.apache.org/documentation/latest/powered-by-mesos/) which 
> has noting to do with latest documentation but only with community. I think 
> real problem is site updating. Using docs for anything that change often is 
> not a solution.
> 
> haosdent huang wrote:
> In my opinion, if the target of document are users, I prefer to host it 
> in project website. If the target of document are devlopers(framework 
> developers, contributors, committer and etc), I perfer to host it in docs.
> 
> Jie Yu wrote:
> +1 on what haosdent said.
> 
> I also looked at k8s
> https://github.com/kubernetes/kubernetes/tree/master/docs
> 
> They also put design docs, design proposals, roadmap (community related) 
> stuff in docs.

Ping, @tomasz, are you still working on that?


- Jie


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


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 48369: Fixed comment in Nvidia GPU device isolator.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 11, 2016, 3:06 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48369/
> ---
> 
> (Updated June 11, 2016, 3:06 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed comment in Nvidia GPU device isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
> d7557a0c338e8c0e51461b2326600c03f89c2e8b 
> 
> Diff: https://reviews.apache.org/r/48369/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48368: Changed major/minor device types for Nvidia GPUs to `unsigned int`.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 11, 2016, 3:06 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48368/
> ---
> 
> (Updated June 11, 2016, 3:06 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5554
> https://issues.apache.org/jira/browse/MESOS-5554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the GPU struct specified the type of its `major` and
> `minor` fields as `dev_t`, which is actually a concatenation of both
> the major and minor device numbers accessible through the `major()`
> and `minor()` macros. These macros return an `unsigned int` when
> handed a `dev_t`, so it makes sense for these fields to be of that
> type instead.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
> 181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
>   src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
> d7557a0c338e8c0e51461b2326600c03f89c2e8b 
> 
> Diff: https://reviews.apache.org/r/48368/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Jie Yu]

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



I did a bunch of grammar fixes for you. Please take a look at the final 
committed doc.

- Jie Yu


On June 14, 2016, 9:19 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> ---
> 
> (Updated June 14, 2016, 9:19 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
> https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> ---
> 
> You can review the document here: 
> https://github.com/jay-lau/mesos/blob/master/docs/docker-volume.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47893: Changed initialization order of `Anonymous` modules in Master.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47893, 47892]

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

Error:
2016-06-15 00:14:47 URL:https://reviews.apache.org/r/47892/diff/raw/ 
[5494/5494] -> "47892.patch" [1]
error: patch failed: src/slave/main.cpp:230
error: src/slave/main.cpp: patch does not apply

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

- Mesos ReviewBot


On June 14, 2016, 9:45 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47893/
> ---
> 
> (Updated June 14, 2016, 9:45 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Bugs: MESOS-5456
> https://issues.apache.org/jira/browse/MESOS-5456
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed initialization order of `Anonymous` modules in Master.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp ce6291f2595163a578abac515cb8250b066cbc52 
> 
> Diff: https://reviews.apache.org/r/47893/diff/
> 
> 
> Testing
> ---
> 
> make check 
> and sudo build/bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 48669: Modified os user test for user switching.

[Gilbert Song]

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

(Updated June 14, 2016, 4:29 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


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


Repository: mesos


Description
---

This test verifies 'setuid', 'setgid', 'getgrouplist' and
'setgroups' work properly on both linux and os x.


Diffs (updated)
-

  3rdparty/stout/tests/os_tests.cpp af1a76c3d950cf9062a985c6040d92bd8be14cb8 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47668: Obtained uid/gids before changing filesystem root.

[Gilbert Song]

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

(Updated June 14, 2016, 4:29 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


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


Repository: mesos


Description
---

Obtained uid/gids before changing filesystem root.


Diffs (updated)
-

  src/launcher/executor.cpp 346aa30c50a21636ced5dfb33942d66feb4eb9c8 
  src/slave/containerizer/mesos/launch.cpp 
e6c83a358148d281dc5f9c170eda6e092bd40a32 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47667: Added stout functions to get and set supplementary groud ids.

[Gilbert Song]

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

(Updated June 14, 2016, 4:28 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


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


Repository: mesos


Description
---

Added stout functions to get and set supplementary groud ids.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/su.hpp 
0a3f23918be9f4d0b044cd9c45001114bc36fce5 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47666: Added stout functions to set uid and gid.

[Gilbert Song]

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

(Updated June 14, 2016, 4:28 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


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


Repository: mesos


Description
---

Added stout functions to set uid and gid.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/su.hpp 
0a3f23918be9f4d0b044cd9c45001114bc36fce5 

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


Testing
---

make check


Thanks,

Gilbert Song

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

[Gilbert Song]

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

(Updated June 14, 2016, 4:28 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


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


Repository: mesos


Description
---

Added image user to 'ContainerLaunchInfo'.


Diffs (updated)
-

  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.

[Gilbert Song]

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

(Updated June 14, 2016, 4:28 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


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


Repository: mesos


Description
---

Implemented image user support in docker runtime isolator.


Diffs (updated)
-

  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

Review Request 48711: Fixed isolator proto message bracket.

[Gilbert Song]

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

Review request for mesos, Guangya Liu and Jie Yu.


Repository: mesos


Description
---

Fixed isolator proto message bracket.


Diffs
-

  include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47661: Improved the mesos containerizer windows related logic.

[Gilbert Song]

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

(Updated June 14, 2016, 4:27 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


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


Repository: mesos


Description
---

Improved the mesos containerizer windows related logic.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
915e4cdf4fed345515bffbf7cc5147d161640ede 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47660: Fixed agent switch user redundant logic in 'getExecutorInfo'.

[Gilbert Song]

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

(Updated June 14, 2016, 4:27 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


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


Repository: mesos


Description
---

Since the 'user' in 'FrameworkInfo' is a required field,
so 'fromeworkInfo.has_user()' is always true. Currently
in our semantics, if the operator specifies the framework
user as an empty string, mesos will automagically set it
as the current host user. So the slave will always obtain
the user from 'FrameworkInfo', and 'switch_user' defaults
to be true from agent flag. We would prefer not to change
this semantics because a long deprecation cycle is needed.
So in this patch, we just fix the redundant check in slave
'getExecutorInfo' logic.


Diffs (updated)
-

  src/slave/slave.cpp 0af04d6fe53f92e03905fb7b3bec72b09d5e8e57 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48669: Modified os user test for user switching.

[Gilbert Song]

> On June 13, 2016, 6:07 p.m., James Peach wrote:
> > 3rdparty/stout/tests/os_tests.cpp, line 685
> > 
> >
> > Since you need root, consider making this a separate test using the 
> > "ROOT_" filter.

stout test does not support filtering for now. That is the reason we check root 
in this way. You can find other example in os_tests.cpp


> On June 13, 2016, 6:07 p.m., James Peach wrote:
> > 3rdparty/stout/tests/os_tests.cpp, line 689
> > 
> >
> > In general, this kind of stuff is hard to validate in unit tests. 
> > Ideally you would find a user account with > 16 groups, then create a 
> > resource that only allows access to the high groups and verify that you 
> > have access when you impersonate that use. That's pretty involved, so just 
> > mentioning it for the future :)

Good to know that! thanks, James!


- Gilbert


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


On June 13, 2016, 4:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48669/
> ---
> 
> (Updated June 13, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies 'setuid', 'setgid', 'getgrouplist' and
> 'setgroups' work properly on both linux and os x.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os_tests.cpp af1a76c3d950cf9062a985c6040d92bd8be14cb8 
> 
> Diff: https://reviews.apache.org/r/48669/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47667: Added stout functions to get and set supplementary groud ids.

[Gilbert Song]

> On June 13, 2016, 6:15 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/posix/su.hpp, line 177
> > 
> >
> > Bear in mind that ``SC_NGROUPS_MAX`` is not meaningful on Darwin other 
> > than it is the number of groups returned by ``getgroups(2)``. I don't 
> > remembee whether ``getgrouplist(3)`` gives you the fill list, but the 
> > manpage sounds promising.
> > 
> > AFAICT this code will always truncate the list at 16.
> 
> Gilbert Song wrote:
> Yeah, currently the `getgrouplist` always truncate at 16. 
> 
> I am following:
> 
> https://github.com/practicalswift/osx/blob/67cfd3a087d292368550619774c9a692cabe91a5/src/samba/samba/source/lib/system_smbd.c#L120
> 
> https://github.com/practicalswift/osx/blob/67cfd3a087d292368550619774c9a692cabe91a5/src/samba/samba/source/lib/system_smbd.c#L204~#L210
> 
> https://github.com/practicalswift/osx/blob/67cfd3a087d292368550619774c9a692cabe91a5/src/samba/samba/source/lib/system.c#L886~#L894
> 
> From above reference, seems like the max_grp always comes from either 
> `_SC_NGROUPS_MAX ` or `NGROUPS_MAX`. I am not familiar with Darwin kernel. Do 
> you mean I should determine the `ngroups` in `getgrouplist` by calling 
> `getgroups(2)` and find the size of groups returned? Could you point me to 
> some links if I misunderstand anything?
> 
> Thanks!

I would leave a TODO and comments for now. Thanks, James!


- Gilbert


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


On June 13, 2016, 4:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47667/
> ---
> 
> (Updated June 13, 2016, 4:50 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stout functions to get and set supplementary groud ids.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/su.hpp 
> 0a3f23918be9f4d0b044cd9c45001114bc36fce5 
> 
> Diff: https://reviews.apache.org/r/47667/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Gilbert Song]

> On June 14, 2016, 8:47 a.m., Guangya Liu wrote:
> > include/mesos/slave/isolator.proto, lines 131-139
> > 
> >
> > I saw that from 
> > https://github.com/docker/docker/blob/master/image/spec/v1.md#container-runconfig-field-descriptions,
> >  all of those info belong to `user`.
> > 
> > Does it make sense to put `user` to a message proto?
> > 
> > message UserInfo {
> >   // The user name or UID in the container. If 'user' is numerical,
> >   // it is an UID. Otherwise, it is an username.
> >   optional string user = 7;
> > 
> >   // The group name or GID in the container. If 'group' is numerical,
> >   // it is a GID. Otherwise, it is a groupname.
> >   optional string group = 8;
> > 
> >   // TODO: Add repeated supplementary groups.
> > }
> > 
> > message ContainerLaunchInfo {
> >   ...
> >   optional UserInfo user = 7;
> > }

This is not only for docker, but also for appc etc. Need to decide after user 
ns staff. Let's leave a TODO and not create that for now. Thanks:)


- Gilbert


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


On June 13, 2016, 4:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47663/
> ---
> 
> (Updated June 13, 2016, 4:47 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 47660: Fixed agent switch user redundant logic in 'getExecutorInfo'.

[Gilbert Song]

> On June 14, 2016, 8:19 a.m., Guangya Liu wrote:
> > Gilbert, I think it is better to file a JIRA to trace this patch chain, 
> > comments?

There is one, MESOS-4757. I am attaching it. Thanks, Guangya!


- Gilbert


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


On June 13, 2016, 4:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47660/
> ---
> 
> (Updated June 13, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'user' in 'FrameworkInfo' is a required field,
> so 'fromeworkInfo.has_user()' is always true. Currently
> in our semantics, if the operator specifies the framework
> user as an empty string, mesos will automagically set it
> as the current host user. So the slave will always obtain
> the user from 'FrameworkInfo', and 'switch_user' defaults
> to be true from agent flag. We would prefer not to change
> this semantics because a long deprecation cycle is needed.
> So in this patch, we just fix the redundant check in slave
> 'getExecutorInfo' logic.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d635dd2c6f6fce5a9eeefc5dcdf84e00cdc833b6 
> 
> Diff: https://reviews.apache.org/r/47660/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48602: Implemented v1::master::Call::GET_METRICS.

[Vinod Kone]

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




src/internal/evolve.cpp (line 551)


this might change after comments in the previous review.


- Vinod Kone


On June 12, 2016, 6:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48602/
> ---
> 
> (Updated June 12, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::master::Call::GET_METRICS.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48602/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48603: Implemented v1::agent::Call::GET_METRICS.

[Vinod Kone]

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



same comments as previous review.

- Vinod Kone


On June 12, 2016, 6:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48603/
> ---
> 
> (Updated June 12, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::agent::Call::GET_METRICS.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48603/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48602: Implemented v1::master::Call::GET_METRICS.

[Vinod Kone]

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




src/tests/api_tests.cpp (line 217)


s/respMetrics/metrics/


- Vinod Kone


On June 12, 2016, 6:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48602/
> ---
> 
> (Updated June 12, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::master::Call::GET_METRICS.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48602/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

[Vinod Kone]

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




3rdparty/libprocess/src/metrics/metrics.cpp (lines 251 - 279)


This induces a performance regression to /metrics/snapshot REST endpoint!

See https://reviews.apache.org/r/48046/ for inspiration on how to refactor 
methods that use JSON::ObjectWriter.


- Vinod Kone


On June 12, 2016, 6:03 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48601/
> ---
> 
> (Updated June 12, 2016, 6:03 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics information via `process::metrics::snapshot`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 25e8fcaeb9c71eaa43096165745e79c8198c139a 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> be03406c58439b7ab7c1ba12c19dd9f30aff109e 
> 
> Diff: https://reviews.apache.org/r/48601/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45961: Support sharing of resources through reference counting of resources.

[Jiang Yan Xu]

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



A high level comment is that when we are making significant changes to the 
allocator we have to measure the performance implication both when clusters 
don't use this feature and when clusters do in benchmarks.


src/common/resources.cpp (line 1225)


A blank line after `}`.



src/common/resources.cpp (line 1266)


A blank line after `}`.



src/common/resources.cpp (line 1773)


Make sure we add this to the tests.

So now the shared persistent volume could look like `disk(alice, hdfs-p, 
{foo: bar, foo})[hadoop:/hdfs:/data:rw]:1<6>`

Would `` be a little redundant given that we have `<6>`?



src/master/allocator/mesos/hierarchical.cpp (lines 898 - 901)


Can't quite understand this comment as to providing an answer for the use 
of `.nonShared()` on `resources`?



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


"the difference in non-shared resources between total and allocated plus 
all shared resources on the slave"



src/master/allocator/mesos/hierarchical.cpp (lines 1283 - 1284)


To clarify "we always have one copy of any shared resource", how about:

```
Since shared resources are offerable even when they are in use, in stage 1 
of each allocation cycle we make one copy of the shared resources available 
regardless of the past allocations.
```



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


If a framework explicitly turns down the shared resources we should not 
send them back again right?



src/master/allocator/mesos/hierarchical.cpp (lines 1426 - 1428)


Thinking about this, we should discuss what the best behavior is for now:

In the current design two offers from the same agent can already be created 
due to the two stage allocation algorithm. However the same resource are never 
sent in two offers.

Here we have two options for shared resources for stage 2:

1. Replenish 'available' with a (potential) 2nd copy of the same shared 
resources, or
2. Don't add the 2nd copy if it's already in an offer in this cycle but DO 
add another copy if it's not in an offer.

Eventually we may be sending the same shared resources to all eligible 
frameworks anyways but for now it feels that 2) is more in line with the 
current behavior: one resource in one offer in one offer cycle.

Thoughts?



src/master/allocator/sorter/drf/sorter.hpp (line 157)


Here also add:

```
NOTE: Sharedness info is also stripped out when resource identities are 
omitted because shareness inherently refers to the identities of resources and 
not quantities.
```



src/master/allocator/sorter/drf/sorter.hpp (lines 164 - 165)


At the end of the 1st sentences, add `(for non-shared resources only)`.



src/master/allocator/sorter/drf/sorter.hpp (lines 167 - 169)


So far there is not a place that lays out the algorithm coherently about 
how DRF works with shared resources. We can describe it in detail in 
`calculateShare` and refer to it here.

Here we can emphasize on how we structure the data members to make the 
algorithm work:

```
We do not aggregate shared resources here since the client's "normalized 
allocation" of a shared resource can only be derived based on the its state in 
`total_.sharedResources` when `calculateShare()` is run. See `calculateShare()` 
for comments on the algorithm.
```



src/master/allocator/sorter/drf/sorter.hpp (line 171)


Is `nonSharedScalarQuantities` a better name?



src/master/allocator/sorter/drf/sorter.hpp (lines 177 - 180)






src/master/allocator/sorter/drf/sorter.hpp (line 181)


I feel that we can find a better place/name for this field. 
`shareResources` doesn't say if it's the allocation or the total pool.

I feel like it should be in `total_` as it's reflects a state of the total 
pool. If we leave it here then perhaps give it a clearer name? What do you 
think?



src/master/allocator/sorter/drf/sorter.cpp (lines 206 - 207)

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48704, 48705]

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 14, 2016, 7:18 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48705/
> ---
> 
> (Updated June 14, 2016, 7:18 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented LIST_FILES Call in v1 agent API.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 93c23a0b31193b17acd09337337530e984803b48 
>   src/slave/slave.hpp ef171188afb7169e7f386547af9fa334a65374af 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48705/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 16.04:
> sudo GTEST_FILTER="*AgentAPITest.ListFiles*" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47893: Changed initialization order of `Anonymous` modules in Master.

[Avinash sridharan]

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

(Updated June 14, 2016, 9:45 p.m.)


Review request for mesos, Jie Yu and Kapil Arya.


Changes
---

Fixed comments.


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


Repository: mesos


Description
---

Changed initialization order of `Anonymous` modules in Master.


Diffs (updated)
-

  src/master/main.cpp ce6291f2595163a578abac515cb8250b066cbc52 

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


Testing
---

make check 
and sudo build/bin/mesos-tests.sh


Thanks,

Avinash sridharan

Re: Review Request 47892: Re-ordered initialization for Agent `Anonymous` modules.

[Avinash sridharan]

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

(Updated June 14, 2016, 9:45 p.m.)


Review request for mesos, Jie Yu and Kapil Arya.


Changes
---

Fixed comments.


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


Repository: mesos


Description
---

Re-ordered initialization for Agent `Anonymous` modules.


Diffs (updated)
-

  src/slave/main.cpp fddb05c2d2756f823a4fa393373a9ebc0f14895c 

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


Testing
---

make check
sudo ./build/bin/mesos-tests.sh


Thanks,

Avinash sridharan

Re: Review Request 48600: Used `DurationInfo` as `timeout` type in `GetMetrics` operation APIs.

[Vinod Kone]

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




include/mesos/master/master.proto (line 88)


Can you add some comments here about this call and its parameters. Please 
don't forget to do it for all calls in this review chain.


- Vinod Kone


On June 12, 2016, 6:03 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48600/
> ---
> 
> (Updated June 12, 2016, 6:03 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used `DurationInfo` as `timeout` type in `GetMetrics` operation APIs.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto PRE-CREATION 
>   include/mesos/slave/slave.proto PRE-CREATION 
>   include/mesos/v1/agent.proto b9b1680afec4e73c31a9e0aeb74153d129c54ee6 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
> 
> Diff: https://reviews.apache.org/r/48600/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48597: Implemented v1::agent::Call::SET_LOGGING_LEVEL.

[Vinod Kone]

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



Same comments as previous review.

- Vinod Kone


On June 12, 2016, 6:02 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48597/
> ---
> 
> (Updated June 12, 2016, 6:02 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5513
> https://issues.apache.org/jira/browse/MESOS-5513
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::agent::Call::SET_LOGGING_LEVEL.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48597/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48596: Implemented v1::master::Call::SET_LOGGING_LEVEL.

[Vinod Kone]

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




src/tests/api_tests.cpp (line 101)


no response doesn't necessarily mean OK(), it could be Accepted() as well. 
that's why i suggested earlier to not have this helper at all but rather inline 
it.



src/tests/api_tests.cpp (line 225)


s/first,/first;/



src/tests/api_tests.cpp (line 245)


Delete this comment.



src/tests/api_tests.cpp (line 250)


Put this comment above Clock::pause()



src/tests/api_tests.cpp (line 254)


s/revert/reverted/


- Vinod Kone


On June 12, 2016, 6:02 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48596/
> ---
> 
> (Updated June 12, 2016, 6:02 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5486
> https://issues.apache.org/jira/browse/MESOS-5486
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::master::Call::SET_LOGGING_LEVEL.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48596/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48363: Moved 'isolators/cgroups/devices/gpus' to 'isolators/gpu'.

[Benjamin Mahler]

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


Ship it!





src/slave/containerizer/mesos/containerizer.cpp (lines 59 - 63)


Looks like this should be alongside isolator includes?


- Benjamin Mahler


On June 11, 2016, 3:04 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48363/
> ---
> 
> (Updated June 11, 2016, 3:04 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5551
> https://issues.apache.org/jira/browse/MESOS-5551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved 'isolators/cgroups/devices/gpus' to 'isolators/gpu'.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 92c9898fb41ca0ffbda05e53b595b05c96f4f596 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> 181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> d7557a0c338e8c0e51461b2326600c03f89c2e8b 
> 
> Diff: https://reviews.apache.org/r/48363/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48370: Fixed method of populating device entries for `/dev/nvidia-uvm`, etc.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp (line 155)


Looks like we should include what we were trying to do:

```
return Error("Failed to obtain device ID for '/dev/nvidiactl': " +
 device.error());
```

Ditto below.


- Benjamin Mahler


On June 11, 2016, 3:06 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48370/
> ---
> 
> (Updated June 11, 2016, 3:06 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5556
> https://issues.apache.org/jira/browse/MESOS-5556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the major/minor numbers of `/dev/nvidiactl` and
> `/dev/nvidia-uvm` were hard-coded. This actually caused problems for
> `/dev/nvidia-uvm` because its major number is part of the
> "Experimental" device range on Linux.
> 
> Because this range is experimental, there is no guarantee which device
> number will be assigned to it on a given machine. We actually
> encountered this problem in the wild, prompting this change.
> 
> We now use `os:stat::rdev()` to extract the major/minor numbers
> programatically.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
> 181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
>   src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
> d7557a0c338e8c0e51461b2326600c03f89c2e8b 
> 
> Diff: https://reviews.apache.org/r/48370/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48694: Optimized `UUID::fromString` and `UUID::toString` in stout.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48692, 48693, 48694]

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 14, 2016, 2:56 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48694/
> ---
> 
> (Updated June 14, 2016, 2:56 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than using `std::istringstream` and `std::ostringstream`,
> instead use the facilities provided by Boost's UUID for input
> and output respectively. This improves a simple benchmark that
> uses `fromString` and `toString` by ~7x.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/uuid.hpp 
> cde3bdbbdcc428410c83a7724389056f7aaf63a1 
> 
> Diff: https://reviews.apache.org/r/48694/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmark is here: 
> https://gist.github.com/anonymous/138d0333656871bddc2dfaa61fbf033e -- happy 
> to submit it as an RR as well (with cleanup), but I didn't think it was 
> warranted.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 48578: Fixed bug with double destruction of cgroups devices subsystem.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 11, 2016, 3:03 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48578/
> ---
> 
> (Updated June 11, 2016, 3:03 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5582
> https://issues.apache.org/jira/browse/MESOS-5582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In a previous commit, the `cgroups/devices` isolator was abstracted
> out of the Nvidia GPU isolator. However, the code for destroying the
> devices susbsystem wasn't removed from the GPU isolator, causing it to
> "double destroy" the subsystem upon cleanup. This commit removed this
> redundant code.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> 181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> d7557a0c338e8c0e51461b2326600c03f89c2e8b 
> 
> Diff: https://reviews.apache.org/r/48578/diff/
> 
> 
> Testing
> ---
> 
> make -j
> GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48704: Updated FilesProcess to support List_Files Call in Operator API v1.

[Abhishek Dasgupta]

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

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


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


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


Repository: mesos


Description
---

Updated FilesProcess to support List_Files Call in Operator API v1.


Diffs
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
  src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 

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


Testing (updated)
---

Ubuntu 16.04:

sudo GTEST_FILTER="FilesTest.BrowseTest" make -j4 check


Thanks,

Abhishek Dasgupta

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

[Abhishek Dasgupta]

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

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


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


Repository: mesos


Description
---

Implemented LIST_FILES Call in v1 agent API.


Diffs
-

  src/slave/http.cpp 93c23a0b31193b17acd09337337530e984803b48 
  src/slave/slave.hpp ef171188afb7169e7f386547af9fa334a65374af 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

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


Thanks,

Abhishek Dasgupta

Review Request 48704: Updated FilesProcess to support List_Files Call in Operator API v1.

[Abhishek Dasgupta]

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

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


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


Repository: mesos


Description
---

Updated FilesProcess to support List_Files Call in Operator API v1.


Diffs
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
  src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 

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


Testing
---

Ubuntu 16.04:

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


Thanks,

Abhishek Dasgupta

Re: Review Request 48497: Added documentation on starting to use acls.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48495, 48496, 48497]

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 14, 2016, 1:20 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48497/
> ---
> 
> (Updated June 14, 2016, 1:20 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-5583
> https://issues.apache.org/jira/browse/MESOS-5583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A common problem for a users starting to use
> acls is that once they set `permissive = false`  and
> not add acls allowing common operations (e.g.,
> register_framework) their Mesos cluster don't
> behave as expected. This patch adds some
> documentation and a sample acls template to
> help users to avoid this problem.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md dcf2160424771c513579063911cc14792f464821 
>   support/acls_template.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48497/diff/
> 
> 
> Testing
> ---
> 
> viewed via website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 47667: Added stout functions to get and set supplementary groud ids.

[Gilbert Song]

> On June 13, 2016, 6:15 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/posix/su.hpp, line 177
> > 
> >
> > Bear in mind that ``SC_NGROUPS_MAX`` is not meaningful on Darwin other 
> > than it is the number of groups returned by ``getgroups(2)``. I don't 
> > remembee whether ``getgrouplist(3)`` gives you the fill list, but the 
> > manpage sounds promising.
> > 
> > AFAICT this code will always truncate the list at 16.

Yeah, currently the `getgrouplist` always truncate at 16. 

I am following:
https://github.com/practicalswift/osx/blob/67cfd3a087d292368550619774c9a692cabe91a5/src/samba/samba/source/lib/system_smbd.c#L120
https://github.com/practicalswift/osx/blob/67cfd3a087d292368550619774c9a692cabe91a5/src/samba/samba/source/lib/system_smbd.c#L204~#L210
https://github.com/practicalswift/osx/blob/67cfd3a087d292368550619774c9a692cabe91a5/src/samba/samba/source/lib/system.c#L886~#L894

>From above reference, seems like the max_grp always comes from either 
>`_SC_NGROUPS_MAX ` or `NGROUPS_MAX`. I am not familiar with Darwin kernel. Do 
>you mean I should determine the `ngroups` in `getgrouplist` by calling 
>`getgroups(2)` and find the size of groups returned? Could you point me to 
>some links if I misunderstand anything?

Thanks!


- Gilbert


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


On June 13, 2016, 4:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47667/
> ---
> 
> (Updated June 13, 2016, 4:50 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stout functions to get and set supplementary groud ids.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/su.hpp 
> 0a3f23918be9f4d0b044cd9c45001114bc36fce5 
> 
> Diff: https://reviews.apache.org/r/47667/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48595: Exposed the logging process `PID` in libprocess.

[Vinod Kone]

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/logging.hpp (line 42)


s/setLevel/set_level/

stout and libprocess use snake case.


- Vinod Kone


On June 12, 2016, 6:02 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48595/
> ---
> 
> (Updated June 12, 2016, 6:02 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5486
> https://issues.apache.org/jira/browse/MESOS-5486
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed the logging process `PID` in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/logging.hpp 
> 651cb3736705124bec9e4366c0e7adc5d5b488f0 
>   3rdparty/libprocess/include/process/process.hpp 
> 646059e2740a32dd9c483dfa56c71dc46727862c 
>   3rdparty/libprocess/src/logging.cpp 
> 222449b904268a47868be374bf8202aafe6516fe 
>   3rdparty/libprocess/src/process.cpp 
> b4cdba6b0cd4f8f373f37118cd2e9d4955f2425a 
> 
> Diff: https://reviews.apache.org/r/48595/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48687: Enhanced log message if the absolute path does not exist.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48687]

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 14, 2016, 12:34 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48687/
> ---
> 
> (Updated June 14, 2016, 12:34 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5611
> https://issues.apache.org/jira/browse/MESOS-5611
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced log message if the absolute path does not exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 8cebfebbd9bf3f616f3cd305889f3372aca5e3f3 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 35213ce8f9d0e81fdd2da15c7a2cee1c779f3555 
> 
> Diff: https://reviews.apache.org/r/48687/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> root@mesos002:~/src/mesos/m2/mesos/build# ./src/mesos-execute 
> --master=192.168.56.12:5050 --command="sleep 10" --name=test 
> --volumes=/root/test/volume4.json
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W0614 09:21:25.100939 28209 parse.hpp:115] Specifying an absolute filename to 
> read a command line option out of without using 'file:// is deprecated and 
> will be removed in a future release. Simply adding 'file://' to the beginning 
> of the path should eliminate this warning.
> I0614 09:21:25.104532 28209 scheduler.cpp:187] Version: 1.0.0
> I0614 09:21:25.106834 28231 scheduler.cpp:471] New master detected at 
> master@192.168.56.12:5050
> Subscribed with ID '57cda92a-b703-435a-94f0-28e88746f8b5-'
> Submitted task 'test' to agent '57cda92a-b703-435a-94f0-28e88746f8b5-S0'
> Received status update TASK_FAILED for task 'test'
>   message: 'Failed to launch container: Absolute container path '/tmp/abc' 
> does not exist; Container destroyed while preparing isolators'
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47972: Updated rmdir to optionally continue deletion on error.

[Megha Sharma]

> On June 8, 2016, 5:22 a.m., Jiang Yan Xu wrote:
> > 3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 120
> > 
> >
> > This looks to be over 80 chars too, let's fix all of them by running 
> > mesos-style.py.

Actually most of the lines over 80 characters including this one didn't get 
flagged when I ran mesos-style.py


- Megha


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


On June 8, 2016, 8:42 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47972/
> ---
> 
> (Updated June 8, 2016, 8:42 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joris Van Remoortere, Joseph Wu, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5196
> https://issues.apache.org/jira/browse/MESOS-5196
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updates made to rmdir to prevent early exit in the event of error.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> a6d3b578762d5c570542b0497578c330820b821a 
>   3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 772b86dd111d28aefbeeba5f829ffa377fd12efb 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> a11bfc9f9e6cbb05f3e9ce0ea48297b8f88fe53f 
> 
> Diff: https://reviews.apache.org/r/47972/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> Tested with option --gtest_also_run_disabled_tests.
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 46762: Enabled volume support for mesos-execute.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48680, 46762]

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 14, 2016, 12:19 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46762/
> ---
> 
> (Updated June 14, 2016, 12:19 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5265
> https://issues.apache.org/jira/browse/MESOS-5265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled volume support for mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 9b8f8a34fef4ac3225f0104a6a0acc1e43accd78 
> 
> Diff: https://reviews.apache.org/r/46762/diff/
> 
> 
> Testing
> ---
> 
> root@mesos002:~/src/mesos/m3/mesos/build# cat /root/test/volume4.json
>   [{
>   "container_path":"\/tmp\/abc1",
>   "mode":"RW",
>   "source":
> {
>   "docker_volume":
> {
>   "driver":"convoy",
>   "name":"dvd1"
> },
> "type":"DOCKER_VOLUME"
> }
> },
> {
>   "container_path":"\/tmp\/abc2",
>   "mode":"RW",
>   "source":
> {
>   "docker_volume":
> {
>   "driver":"convoy",
>   "driver_options":
> {"parameter":[
>   {
> "key":"iops",
> "value":"150"
>   }
> ]},
> "name":"dvd2"
>  },
>  "type":"DOCKER_VOLUME"
> }
> }]
> 
> root@mesos002:~/src/mesos/m3/mesos/build# src/mesos-execute 
> --master=192.168.56.12:5050 --name=test  --command="sleep 100" 
> --volumes=/root/test/volume4.json
> I0428 21:27:52.406168  3775 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID '2a69e042-4e51-4cd9-bcab-e303d6b4b1dd-0003'
> Submitted task 'test' to agent '2a69e042-4e51-4cd9-bcab-e303d6b4b1dd-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Anand Mazumdar]

> On June 14, 2016, 2:40 p.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, lines 484-490
> > 
> >
> > hmm .. Why do you need to load the flags again here? You already did so 
> > on line L154, no?
> 
> Jose Guilherme Vanz wrote:
> The flag variable is initializated in constructor stack, right? I can not 
> access it from other function. Or... I'm missing something else around how 
> stout and libprocess handle the process.

My bad, looks like I jumped the gun here. I was referring to having `Flags` as 
a member variable and then accessing it directly here instead of loading it 
again here. This is similar to other places in the code that deal with `Flags` 
like the driver, master, agent etc.


> On June 14, 2016, 2:40 p.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, line 320
> > 
> >
> > As per my review comment in an earlier version of the patch, let's have 
> > the signature of this method as:
> > 
> > ```cpp
> > void connect(const UUID& _connectionId);
> > ```
> > 
> > And you can invoke this as:
> > 
> > ```cpp
> > process::delay(delay, self(), ::connect, 
> > connectionId.get());
> > ```
> 
> Jose Guilherme Vanz wrote:
> Considering that Option's == operator already checks the content, uses 
> the UUID instead of Option is a standart or just a prefererence in the 
> way to do stuff? ;D

An `Option` can wrap any `T`. If a reader see the argument of a function as 
`Option arg`, it means that `arg` would either have a value of type `T` or 
would also have a `None()` in some scenarios. In this particular case though, 
`arg` would always be of type `UUID` and it can't ever be a `None`. This helps 
in readability and reasoning about code invariants/flow better. Makes sense?


- Anand


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


On June 13, 2016, 11:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 11:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/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 47663: Added image user to 'ContainerLaunchInfo'.

[Guangya Liu]

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




include/mesos/slave/isolator.proto (lines 30 - 31)


Not urs, but I think that we need to adjust the format of the proto as 

message  {

}

by putting `{` in same line as `message`.

Ditto as following. We can handle this in a separate patch.



include/mesos/slave/isolator.proto (lines 131 - 139)


I saw that from 
https://github.com/docker/docker/blob/master/image/spec/v1.md#container-runconfig-field-descriptions,
 all of those info belong to `user`.

Does it make sense to put `user` to a message proto?

message UserInfo {
  // The user name or UID in the container. If 'user' is numerical,
  // it is an UID. Otherwise, it is an username.
  optional string user = 7;

  // The group name or GID in the container. If 'group' is numerical,
  // it is a GID. Otherwise, it is a groupname.
  optional string group = 8;

  // TODO: Add repeated supplementary groups.
}

message ContainerLaunchInfo {
  ...
  optional UserInfo user = 7;
}


- Guangya Liu


On 六月 13, 2016, 11:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47663/
> ---
> 
> (Updated 六月 13, 2016, 11:47 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 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 14, 2016, 11:40 a.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, line 320
> > 
> >
> > As per my review comment in an earlier version of the patch, let's have 
> > the signature of this method as:
> > 
> > ```cpp
> > void connect(const UUID& _connectionId);
> > ```
> > 
> > And you can invoke this as:
> > 
> > ```cpp
> > process::delay(delay, self(), ::connect, 
> > connectionId.get());
> > ```

Considering that Option's == operator already checks the content, uses the UUID 
instead of Option is a standart or just a prefererence in the way to do 
stuff? ;D


> On June 14, 2016, 11:40 a.m., Anand Mazumdar wrote:
> > src/scheduler/scheduler.cpp, lines 484-490
> > 
> >
> > hmm .. Why do you need to load the flags again here? You already did so 
> > on line L154, no?

The flag variable is initializated in constructor stack, right? I can not 
access it from other function. Or... I'm missing something else around how 
stout and libprocess handle the process.


- Jose Guilherme


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


On June 13, 2016, 8:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 8:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/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 47660: Fixed agent switch user redundant logic in 'getExecutorInfo'.

[Guangya Liu]

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



Gilbert, I think it is better to file a JIRA to trace this patch chain, 
comments?

- Guangya Liu


On 六月 13, 2016, 11:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47660/
> ---
> 
> (Updated 六月 13, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'user' in 'FrameworkInfo' is a required field,
> so 'fromeworkInfo.has_user()' is always true. Currently
> in our semantics, if the operator specifies the framework
> user as an empty string, mesos will automagically set it
> as the current host user. So the slave will always obtain
> the user from 'FrameworkInfo', and 'switch_user' defaults
> to be true from agent flag. We would prefer not to change
> this semantics because a long deprecation cycle is needed.
> So in this patch, we just fix the redundant check in slave
> 'getExecutorInfo' logic.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d635dd2c6f6fce5a9eeefc5dcdf84e00cdc833b6 
> 
> Diff: https://reviews.apache.org/r/47660/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 48693: Added tests for `UUID::fromString`.

[Neil Conway]

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

Added tests for `UUID::fromString`.


Diffs
-

  3rdparty/stout/tests/uuid_tests.cpp 50295ad25843a3938a5a58f5ba7f082f8ffeba45 

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


Testing
---

make check


Thanks,

Neil Conway

Review Request 48692: Cleaned up header includes in stout/uuid.

[Neil Conway]

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

Cleaned up header includes in stout/uuid.


Diffs
-

  3rdparty/stout/include/stout/uuid.hpp 
cde3bdbbdcc428410c83a7724389056f7aaf63a1 

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


Testing
---

make check


Thanks,

Neil Conway

Review Request 48694: Optimized `UUID::fromString` and `UUID::toString` in stout.

[Neil Conway]

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

Rather than using `std::istringstream` and `std::ostringstream`,
instead use the facilities provided by Boost's UUID for input
and output respectively. This improves a simple benchmark that
uses `fromString` and `toString` by ~7x.


Diffs
-

  3rdparty/stout/include/stout/uuid.hpp 
cde3bdbbdcc428410c83a7724389056f7aaf63a1 

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


Testing
---

make check

Benchmark is here: 
https://gist.github.com/anonymous/138d0333656871bddc2dfaa61fbf033e -- happy to 
submit it as an RR as well (with cleanup), but I didn't think it was warranted.


Thanks,

Neil Conway

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/#review137507
---



This is looking much better. 

Can you include both `flags.hpp/flags.cpp` in `src/Makefile.am`? This would fix 
the ReviewBot build failures.


src/scheduler/constants.hpp (line 27)


s/connection/connecting with the master



src/scheduler/scheduler.cpp (line 319)


As per my review comment in an earlier version of the patch, let's have the 
signature of this method as:

```cpp
void connect(const UUID& _connectionId);
```

And you can invoke this as:

```cpp
process::delay(delay, self(), ::connect, connectionId.get());
```



src/scheduler/scheduler.cpp (line 477)


Bring this assignment down to L488 i.e. just before `Duration delay` where 
it is used.



src/scheduler/scheduler.cpp (lines 481 - 487)


hmm .. Why do you need to load the flags again here? You already did so on 
line L154, no?



src/scheduler/scheduler.cpp (line 496)


Nit: new line before

We typically have a new line after a statement spanning multiple lines for 
readability.


- Anand Mazumdar


On June 13, 2016, 11:20 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 13, 2016, 11:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5359
> https://issues.apache.org/jira/browse/MESOS-5359
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/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 48320: Update C++ style checker to prevent NULL usage.

[Tomasz Janiszewski]

> On June 14, 2016, 2:36 p.m., Benjamin Bannier wrote:
> > support/cpplint.py, line 1
> > 
> >
> > Not in this file, but do we need to update `cpplint.patch` as well? We 
> > seem to have stopped doing that at some point.

Good question. I didn't do that becouse I saw it's out of sync. Maybe it could 
be deleted but present a  way of generating it with` git diff`?


- Tomasz


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


On June 7, 2016, 7:47 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48320/
> ---
> 
> (Updated June 7, 2016, 7:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3243
> https://issues.apache.org/jira/browse/MESOS-3243
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update C++ style checker to prevent NULL usage.
> 
> 
> Diffs
> -
> 
>   support/cpplint.py 96e7fbcfe84067becbac89de909a2c6bdf99a60b 
>   support/mesos-style.py 35ef7867c45afa02f0c74b86149f2e0489fe2d15 
> 
> Diff: https://reviews.apache.org/r/48320/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48684: Revised documentation for task reconciliation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48684]

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 14, 2016, 12:03 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48684/
> ---
> 
> (Updated June 14, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In particular, add a note that the `TaskStatus` messages
> produced by reconciliation can be identified by looking at
> the `reason` field, and that these messages won't have
> other fields like `labels` and `data` set. Also, add
> recommendations for the how to do reconciliation and edit
> some of the previous text for tone and brevity.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 2605f68c983403d4fe0cbb5d76b31a9244681d43 
>   docs/reconciliation.md ce8435587e93e8e07dc9b0637fad7f900fbe65bb 
> 
> Diff: https://reviews.apache.org/r/48684/diff/
> 
> 
> Testing
> ---
> 
> previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 48320: Update C++ style checker to prevent NULL usage.

[Benjamin Bannier]

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


Fix it, then Ship it!





support/cpplint.py (line 1)


Not in this file, but do we need to update `cpplint.patch` as well? We seem 
to have stopped doing that at some point.



support/cpplint.py (line 33)


@mpark: Do we need to update this here (e.g., for legal reasons)? I see you 
were the first one to stop doing that.



support/cpplint.py (line 161)


Is it worth introducing an extra category for this? -- I think it might fit 
well into `build/`, e.g., as `build/nullptr`.



support/mesos-style.py (line 48)


`s~c++11/null~build~c++/nullptr~`


- Benjamin Bannier


On June 7, 2016, 9:47 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48320/
> ---
> 
> (Updated June 7, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3243
> https://issues.apache.org/jira/browse/MESOS-3243
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update C++ style checker to prevent NULL usage.
> 
> 
> Diffs
> -
> 
>   support/cpplint.py 96e7fbcfe84067becbac89de909a2c6bdf99a60b 
>   support/mesos-style.py 35ef7867c45afa02f0c74b86149f2e0489fe2d15 
> 
> Diff: https://reviews.apache.org/r/48320/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48496: Fixed misleading acls example in authorization.md.

[Joerg Schad]

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

(Updated June 14, 2016, 1:21 p.m.)


Review request for mesos and Adam B.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Previously we we explicity diallowing access to all principals which
was already done by `permissive = false`.


Diffs (updated)
-

  docs/authorization.md dcf2160424771c513579063911cc14792f464821 

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


Testing
---

viewed via website container.


Thanks,

Joerg Schad

Re: Review Request 48495: Added missing 'get_weights' actions to authorization.md.

[Joerg Schad]

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

(Updated June 14, 2016, 1:21 p.m.)


Review request for mesos and Adam B.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Additionally fixed typo(s). Note that 'get_endpoints' which
is also missing here will be added with 46887.


Diffs (updated)
-

  docs/authorization.md dcf2160424771c513579063911cc14792f464821 

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


Testing
---

viewed via docker website container.


Thanks,

Joerg Schad

Re: Review Request 48497: Added documentation on starting to use acls.

[Joerg Schad]

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

(Updated June 14, 2016, 1:20 p.m.)


Review request for mesos, Adam B and Neil Conway.


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


Repository: mesos


Description
---

A common problem for a users starting to use
acls is that once they set `permissive = false`  and
not add acls allowing common operations (e.g.,
register_framework) their Mesos cluster don't
behave as expected. This patch adds some
documentation and a sample acls template to
help users to avoid this problem.


Diffs (updated)
-

  docs/authorization.md dcf2160424771c513579063911cc14792f464821 
  support/acls_template.json PRE-CREATION 

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


Testing
---

viewed via website container.


Thanks,

Joerg Schad

Re: Review Request 46887: Added the HTTP GET authorization action to the documentation.

[Joerg Schad]

> On June 14, 2016, 1:13 p.m., Joerg Schad wrote:
> > docs/authorization.md, line 140
> > 
> >
> > Could we
> > a) check that the HTTP HELP text is correct for all these endpoints 
> > (i.e, mentions AUTHORIZATION)
> > b) mention here that the HTTP endpoint HELP contains information about 
> > this?
> > 
> > s/supports/covers (as it does not allow individual endpoints, it is a 
> > all or nothing decision)

Forget the seconds part i.e, covers


- Joerg


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


On June 14, 2016, 11:46 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46887/
> ---
> 
> (Updated June 14, 2016, 11:46 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Joerg Schad, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the HTTP GET authorization action to the documentation.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0df0d4ccd76379ede6492e595999711e5462f79b 
>   docs/authorization.md dcf2160424771c513579063911cc14792f464821 
>   docs/upgrades.md afc76f7a0e74042bd095b925f8119bfa5daa489c 
> 
> Diff: https://reviews.apache.org/r/46887/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 48497: Added documentation on starting to use acls.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [48497, 48496, 48495]

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

Error:
2016-06-14 13:14:24 URL:https://reviews.apache.org/r/48495/diff/raw/ 
[2065/2065] -> "48495.patch" [1]
error: patch failed: docs/authorization.md:127
error: docs/authorization.md: patch does not apply

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

- Mesos ReviewBot


On June 14, 2016, 11:54 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48497/
> ---
> 
> (Updated June 14, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-5583
> https://issues.apache.org/jira/browse/MESOS-5583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A common problem for a users starting to use
> acls is that once they set `permissive = false`  and
> not add acls allowing common operations (e.g.,
> register_framework) their Mesos cluster don't
> behave as expected. This patch adds some
> documentation and a sample acls template to
> help users to avoid this problem.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 7af7cd2f14dc6b324c9681bad70228972d5299f0 
>   support/acls_template.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48497/diff/
> 
> 
> Testing
> ---
> 
> viewed via website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 46887: Added the HTTP GET authorization action to the documentation.

[Joerg Schad]

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




docs/authorization.md (line 140)


Could we
a) check that the HTTP HELP text is correct for all these endpoints (i.e, 
mentions AUTHORIZATION)
b) mention here that the HTTP endpoint HELP contains information about this?

s/supports/covers (as it does not allow individual endpoints, it is a all 
or nothing decision)



docs/authorization.md (line 601)


.. endpoints (due to the `permissive = false`).



docs/upgrades.md (line 75)


can you double check that label is still available (I believe I saw/wrote 
another patch using that label).



docs/upgrades.md (line 217)


enables required authorization for the following endpoint.



docs/upgrades.md (line 224)


Add something along the lines: Be careful when using this together with 
ACLs and permissive = false.


- Joerg Schad


On June 14, 2016, 11:46 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46887/
> ---
> 
> (Updated June 14, 2016, 11:46 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Joerg Schad, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the HTTP GET authorization action to the documentation.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0df0d4ccd76379ede6492e595999711e5462f79b 
>   docs/authorization.md dcf2160424771c513579063911cc14792f464821 
>   docs/upgrades.md afc76f7a0e74042bd095b925f8119bfa5daa489c 
> 
> Diff: https://reviews.apache.org/r/46887/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46887: Added the HTTP GET authorization action to the documentation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46887]

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 14, 2016, 11:46 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46887/
> ---
> 
> (Updated June 14, 2016, 11:46 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Joerg Schad, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the HTTP GET authorization action to the documentation.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0df0d4ccd76379ede6492e595999711e5462f79b 
>   docs/authorization.md dcf2160424771c513579063911cc14792f464821 
>   docs/upgrades.md afc76f7a0e74042bd095b925f8119bfa5daa489c 
> 
> Diff: https://reviews.apache.org/r/46887/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 48687: Enhanced log message if the absolute path does not exist.

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Enhanced log message if the absolute path does not exist.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
8cebfebbd9bf3f616f3cd305889f3372aca5e3f3 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
35213ce8f9d0e81fdd2da15c7a2cee1c779f3555 

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


Testing
---

make
make check

root@mesos002:~/src/mesos/m2/mesos/build# ./src/mesos-execute 
--master=192.168.56.12:5050 --command="sleep 10" --name=test 
--volumes=/root/test/volume4.json
WARNING: Logging before InitGoogleLogging() is written to STDERR
W0614 09:21:25.100939 28209 parse.hpp:115] Specifying an absolute filename to 
read a command line option out of without using 'file:// is deprecated and will 
be removed in a future release. Simply adding 'file://' to the beginning of the 
path should eliminate this warning.
I0614 09:21:25.104532 28209 scheduler.cpp:187] Version: 1.0.0
I0614 09:21:25.106834 28231 scheduler.cpp:471] New master detected at 
master@192.168.56.12:5050
Subscribed with ID '57cda92a-b703-435a-94f0-28e88746f8b5-'
Submitted task 'test' to agent '57cda92a-b703-435a-94f0-28e88746f8b5-S0'
Received status update TASK_FAILED for task 'test'
  message: 'Failed to launch container: Absolute container path '/tmp/abc' does 
not exist; Container destroyed while preparing isolators'


Thanks,

Guangya Liu

Review Request 48680: Stout: Added parse() for JSON::Array.

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Stout: Added parse() for JSON::Array.


Diffs
-

  3rdparty/stout/include/stout/flags/parse.hpp 
ef365e4d714a2c25d358a702722c5f1216869382 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46762: Enabled volume support for mesos-execute.

[Guangya Liu]

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

(Updated 六月 14, 2016, 12:19 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Enabled volume support for mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp 9b8f8a34fef4ac3225f0104a6a0acc1e43accd78 

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


Testing
---

root@mesos002:~/src/mesos/m3/mesos/build# cat /root/test/volume4.json
  [{
  "container_path":"\/tmp\/abc1",
  "mode":"RW",
  "source":
{
  "docker_volume":
{
  "driver":"convoy",
  "name":"dvd1"
},
"type":"DOCKER_VOLUME"
}
},
{
  "container_path":"\/tmp\/abc2",
  "mode":"RW",
  "source":
{
  "docker_volume":
{
  "driver":"convoy",
  "driver_options":
{"parameter":[
  {
"key":"iops",
"value":"150"
  }
]},
"name":"dvd2"
 },
 "type":"DOCKER_VOLUME"
}
}]

root@mesos002:~/src/mesos/m3/mesos/build# src/mesos-execute 
--master=192.168.56.12:5050 --name=test  --command="sleep 100" 
--volumes=/root/test/volume4.json
I0428 21:27:52.406168  3775 scheduler.cpp:177] Version: 0.29.0
Subscribed with ID '2a69e042-4e51-4cd9-bcab-e303d6b4b1dd-0003'
Submitted task 'test' to agent '2a69e042-4e51-4cd9-bcab-e303d6b4b1dd-S0'
Received status update TASK_RUNNING for task 'test'
  source: SOURCE_EXECUTOR


Thanks,

Guangya Liu

Review Request 48684: Revised documentation for task reconciliation.

[Neil Conway]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

In particular, add a note that the `TaskStatus` messages
produced by reconciliation can be identified by looking at
the `reason` field, and that these messages won't have
other fields like `labels` and `data` set. Also, add
recommendations for the how to do reconciliation and edit
some of the previous text for tone and brevity.


Diffs
-

  docs/high-availability-framework-guide.md 
2605f68c983403d4fe0cbb5d76b31a9244681d43 
  docs/reconciliation.md ce8435587e93e8e07dc9b0637fad7f900fbe65bb 

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


Testing
---

previewed in site-docker.


Thanks,

Neil Conway

Re: Review Request 48497: Added documentation on starting to use acls.

[Neil Conway]

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




docs/authorization.md (line 100)


"non-matching" again :)


- Neil Conway


On June 14, 2016, 11:54 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48497/
> ---
> 
> (Updated June 14, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-5583
> https://issues.apache.org/jira/browse/MESOS-5583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A common problem for a users starting to use
> acls is that once they set `permissive = false`  and
> not add acls allowing common operations (e.g.,
> register_framework) their Mesos cluster don't
> behave as expected. This patch adds some
> documentation and a sample acls template to
> help users to avoid this problem.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 7af7cd2f14dc6b324c9681bad70228972d5299f0 
>   support/acls_template.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48497/diff/
> 
> 
> Testing
> ---
> 
> viewed via website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48497: Added documentation on starting to use acls.

[Joerg Schad]

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

(Updated June 14, 2016, 11:54 a.m.)


Review request for mesos, Adam B and Neil Conway.


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


Repository: mesos


Description (updated)
---

A common problem for a users starting to use
acls is that once they set `permissive = false`  and
not add acls allowing common operations (e.g.,
register_framework) their Mesos cluster don't
behave as expected. This patch adds some
documentation and a sample acls template to
help users to avoid this problem.


Diffs (updated)
-

  docs/authorization.md 7af7cd2f14dc6b324c9681bad70228972d5299f0 
  support/acls_template.json PRE-CREATION 

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


Testing
---

viewed via website container.


Thanks,

Joerg Schad

Re: Review Request 48497: Added documentation on starting to use acls.

[Neil Conway]

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



Typo in summary ("permissive").


docs/authorization.md (line 100)


"non-matching"



docs/authorization.md (line 104)


"`register_frameworks`"

"ACLs"



docs/authorization.md (line 114)


If we're going to provide an example file, it would be great if this was 
linkable from the documentation; `support/` is not typically installed by Mesos 
packages. We could link to Github I suppose, or else have a `docs/examples` 
subdirectory?


- Neil Conway


On June 14, 2016, 11:44 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48497/
> ---
> 
> (Updated June 14, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-5583
> https://issues.apache.org/jira/browse/MESOS-5583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A common problem for a users starting to use
> acls is that once they set `permisse = false`  and
> not add acls allowing common operations (e.g.,
> register_framework) their Mesos cluster don't
> behave as expected. This patch adds some
> documentation and a sample acls template to
> help users to avoid this problem.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 7af7cd2f14dc6b324c9681bad70228972d5299f0 
>   support/acls_template.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48497/diff/
> 
> 
> Testing
> ---
> 
> viewed via website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 46887: Added the HTTP GET authorization action to the documentation.

[Jan Schlicht]

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

(Updated June 14, 2016, 1:46 p.m.)


Review request for mesos, Alexander Rojas, Joerg Schad, and Till Toenshoff.


Changes
---

Added additional documentation.


Repository: mesos


Description
---

Added the HTTP GET authorization action to the documentation.


Diffs (updated)
-

  CHANGELOG 0df0d4ccd76379ede6492e595999711e5462f79b 
  docs/authorization.md dcf2160424771c513579063911cc14792f464821 
  docs/upgrades.md afc76f7a0e74042bd095b925f8119bfa5daa489c 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45377, 36440]

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 14, 2016, 9:49 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36440/
> ---
> 
> (Updated June 14, 2016, 9:49 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5341
> https://issues.apache.org/jira/browse/MESOS-5341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled docker volume support for DockerContainerizer.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/docker/docker.cpp a6dcbe788abb8802baa2c70ec2fced1da75c7210 
> 
> Diff: https://reviews.apache.org/r/36440/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 1) Start convoy
> 2) Create a volume named as c1
> root@kvm-002605:~# convoy list
> {
> "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
> "UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Name": "c1",
> "Driver": "devicemapper",
> "MountPoint": "",
> "CreatedTime": "Mon May 09 09:38:52 +0800 2016",
> "DriverInfo": {
> "DevID": "1",
> "Device": 
> "/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Driver": "devicemapper",
> "MountPoint": "",
> "Size": "107374182400"
> },
> "Snapshots": {}
> }
> }
> 3) Update  mesos-execte to set up volume
> index 4711e80..6c712a2 100644
> --- a/src/cli/execute.cpp
> +++ b/src/cli/execute.cpp
> @@ -72,6 +72,8 @@ using mesos::v1::TaskID;
>  using mesos::v1::TaskInfo;
>  using mesos::v1::TaskState;
>  using mesos::v1::TaskStatus;
> +using mesos::v1::Volume;
> +using mesos::v1::Parameters;
>  
>  using mesos::v1::scheduler::Call;
>  using mesos::v1::scheduler::Event;
> @@ -581,6 +583,18 @@ private:
>  
>containerInfo.set_type(ContainerInfo::DOCKER);
>containerInfo.mutable_docker()->set_image(dockerImage.get());
> +  //containerInfo.mutable_docker()->set_volume_driver("convoy");
> +
> +  Volume* volume1 = containerInfo.add_volumes();
> +  //volume1->set_host_path("c1");
> +  volume1->set_container_path("/tmp");
> +  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
> +  volume1->mutable_source()->mutable_docker_volume()->set_driver(
> +  "convoy");
> +  volume1->mutable_source()->mutable_docker_volume()->set_name(
> +  "c1");
> +  volume1->set_mode(Volume::RW);
> +  cout << "Add Voume 1" << endl;
>  
>if (networks.isSome() && !networks->empty()) {
>  vector tokens = strings::tokenize(networks.get(), ",");
> 4) Start master
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master
> 5) Start agent
> GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
> --isolation=filesystem/linux,docker/runtime --containerizers=docker 
> --work_dir=/tmp/mesos --image_providers=docker
> 5) Start mesos-execute
> src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
> --docker_image=ubuntu:14.04 --containerizer=docker
> I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
> I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
> master@10.0.0.65:5050
> Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
> Add Voume 1
> Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_LOST for task 'test'
> 
> 6) Inspect container
> docker inspect 
> ...
> {
> "Name": "c1",
> "Source": 
> "/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Destination": "/tmp",
> "Driver": "convoy",
> "Mode": "rw",
> "RW": true,
> "Propagation": "rprivate"
> }
> ...
> 7) Check sandbox executer log
> I0509 13:14:56.313102  8361 logging.cpp:195] Logging to STDERR
> I0509 13:14:56.315385  8361 process.cpp:999] libprocess is initialized on 
> 10.0.0.65:52853 with 16 worker threads
> I0509 13:14:56.316789  8361 exec.cpp:150] Version: 0.29.0
> I0509 13:14:56.321717  8387 exec.cpp:200] Executor started at: 
> executor(1)@10.0.0.65:52853 with pid 

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Guangya Liu]

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

(Updated 六月 14, 2016, 9:49 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Enabled docker volume support for DockerContainerizer.


Diffs (updated)
-

  include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
  include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
  src/docker/docker.cpp a6dcbe788abb8802baa2c70ec2fced1da75c7210 

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


Testing (updated)
---

make
make check

1) Start convoy
2) Create a volume named as c1
root@kvm-002605:~# convoy list
{
"f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
"UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Name": "c1",
"Driver": "devicemapper",
"MountPoint": "",
"CreatedTime": "Mon May 09 09:38:52 +0800 2016",
"DriverInfo": {
"DevID": "1",
"Device": 
"/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Driver": "devicemapper",
"MountPoint": "",
"Size": "107374182400"
},
"Snapshots": {}
}
}
3) Update  mesos-execte to set up volume
index 4711e80..6c712a2 100644
--- a/src/cli/execute.cpp
+++ b/src/cli/execute.cpp
@@ -72,6 +72,8 @@ using mesos::v1::TaskID;
 using mesos::v1::TaskInfo;
 using mesos::v1::TaskState;
 using mesos::v1::TaskStatus;
+using mesos::v1::Volume;
+using mesos::v1::Parameters;
 
 using mesos::v1::scheduler::Call;
 using mesos::v1::scheduler::Event;
@@ -581,6 +583,18 @@ private:
 
   containerInfo.set_type(ContainerInfo::DOCKER);
   containerInfo.mutable_docker()->set_image(dockerImage.get());
+  //containerInfo.mutable_docker()->set_volume_driver("convoy");
+
+  Volume* volume1 = containerInfo.add_volumes();
+  //volume1->set_host_path("c1");
+  volume1->set_container_path("/tmp");
+  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
+  volume1->mutable_source()->mutable_docker_volume()->set_driver(
+  "convoy");
+  volume1->mutable_source()->mutable_docker_volume()->set_name(
+  "c1");
+  volume1->set_mode(Volume::RW);
+  cout << "Add Voume 1" << endl;
 
   if (networks.isSome() && !networks->empty()) {
 vector tokens = strings::tokenize(networks.get(), ",");
4) Start master
./bin/mesos-master.sh --work_dir=/tmp/mesos/master
5) Start agent
GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
--isolation=filesystem/linux,docker/runtime --containerizers=docker 
--work_dir=/tmp/mesos --image_providers=docker
5) Start mesos-execute
src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
--docker_image=ubuntu:14.04 --containerizer=docker
I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
master@10.0.0.65:5050
Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
Add Voume 1
Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
Received status update TASK_RUNNING for task 'test'
  source: SOURCE_EXECUTOR
Received status update TASK_LOST for task 'test'

6) Inspect container
docker inspect 
...
{
"Name": "c1",
"Source": 
"/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Destination": "/tmp",
"Driver": "convoy",
"Mode": "rw",
"RW": true,
"Propagation": "rprivate"
}
...
7) Check sandbox executer log
I0509 13:14:56.313102  8361 logging.cpp:195] Logging to STDERR
I0509 13:14:56.315385  8361 process.cpp:999] libprocess is initialized on 
10.0.0.65:52853 with 16 worker threads
I0509 13:14:56.316789  8361 exec.cpp:150] Version: 0.29.0
I0509 13:14:56.321717  8387 exec.cpp:200] Executor started at: 
executor(1)@10.0.0.65:52853 with pid 8361
I0509 13:14:56.325158  8388 exec.cpp:225] Executor registered on agent 
5454a953-83d0-42a4-b7fe-0825a6667f8c-S0
I0509 13:14:56.326740  8388 exec.cpp:237] Executor::registered took 325332ns
I0509 13:14:56.327030  8388 exec.cpp:312] Executor asked to run task 'test'
I0509 13:14:56.327163  8388 exec.cpp:321] Executor::launchTask took 98803ns
I0509 13:14:56.327437  8386 docker.cpp:707] 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-5454a953-83d0-42a4-b7fe-0825a6667f8c-S0.d2fb7a57-d364-41b8-a671-30b95c5cebb4
 --volume-driver=convoy -v c1:/tmp:rw -v 

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Guangya Liu]

> On 六月 14, 2016, 8:42 a.m., Gilbert Song wrote:
> > src/docker/docker.cpp, line 548
> > 
> >
> > This will segfault if no driver specified, right?
> > 
> > I roughly remember docker supports volume by using default driver (not 
> > necessary to specify).

Good catch! I will remove this log message as all of the information was 
already covered by 
https://github.com/apache/mesos/blob/master/src/docker/docker.cpp#L676 after we 
fix https://reviews.apache.org/r/37989/


- Guangya


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


On 六月 13, 2016, 4:36 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36440/
> ---
> 
> (Updated 六月 13, 2016, 4:36 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5341
> https://issues.apache.org/jira/browse/MESOS-5341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled docker volume support for DockerContainerizer.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/docker/docker.cpp a6dcbe788abb8802baa2c70ec2fced1da75c7210 
> 
> Diff: https://reviews.apache.org/r/36440/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 1) Start convoy
> 2) Create a volume named as c1
> root@kvm-002605:~# convoy list
> {
> "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
> "UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Name": "c1",
> "Driver": "devicemapper",
> "MountPoint": "",
> "CreatedTime": "Mon May 09 09:38:52 +0800 2016",
> "DriverInfo": {
> "DevID": "1",
> "Device": 
> "/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Driver": "devicemapper",
> "MountPoint": "",
> "Size": "107374182400"
> },
> "Snapshots": {}
> }
> }
> 3) Update  mesos-execte to set up volume
> index 4711e80..6c712a2 100644
> --- a/src/cli/execute.cpp
> +++ b/src/cli/execute.cpp
> @@ -72,6 +72,8 @@ using mesos::v1::TaskID;
>  using mesos::v1::TaskInfo;
>  using mesos::v1::TaskState;
>  using mesos::v1::TaskStatus;
> +using mesos::v1::Volume;
> +using mesos::v1::Parameters;
>  
>  using mesos::v1::scheduler::Call;
>  using mesos::v1::scheduler::Event;
> @@ -581,6 +583,18 @@ private:
>  
>containerInfo.set_type(ContainerInfo::DOCKER);
>containerInfo.mutable_docker()->set_image(dockerImage.get());
> +  //containerInfo.mutable_docker()->set_volume_driver("convoy");
> +
> +  Volume* volume1 = containerInfo.add_volumes();
> +  //volume1->set_host_path("c1");
> +  volume1->set_container_path("/tmp");
> +  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
> +  volume1->mutable_source()->mutable_docker_volume()->set_driver(
> +  "convoy");
> +  volume1->mutable_source()->mutable_docker_volume()->set_name(
> +  "c1");
> +  volume1->set_mode(Volume::RW);
> +  cout << "Add Voume 1" << endl;
>  
>if (networks.isSome() && !networks->empty()) {
>  vector tokens = strings::tokenize(networks.get(), ",");
> 4) Start master
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master
> 5) Start agent
> GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
> --isolation=filesystem/linux,docker/runtime --containerizers=docker 
> --work_dir=/tmp/mesos --image_providers=docker
> 5) Start mesos-execute
> src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
> --docker_image=ubuntu:14.04 --containerizer=docker
> I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
> I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
> master@10.0.0.65:5050
> Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
> Add Voume 1
> Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_LOST for task 'test'
> 
> 6) Inspect container
> docker inspect 
> ...
> {
> "Name": "c1",
> "Source": 
> "/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Destination": "/tmp",
> "Driver": "convoy",
> "Mode": "rw",
> "RW": true,
> "Propagation": "rprivate"
> }
> ...
> 7) Check sandbox executer log
> I0509 13:14:56.313102  

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47511]

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

Error:
2016-06-14 09:37:21 URL:https://reviews.apache.org/r/47511/diff/raw/ 
[11711/11711] -> "47511.patch" [1]
error: missing binary patch data for 'docs/images/docker-volume-isolator.png'
error: binary patch does not apply to 'docs/images/docker-volume-isolator.png'
error: docs/images/docker-volume-isolator.png: patch does not apply

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

- Mesos ReviewBot


On June 14, 2016, 9:19 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> ---
> 
> (Updated June 14, 2016, 9:19 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
> https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> ---
> 
> You can review the document here: 
> https://github.com/jay-lau/mesos/blob/master/docs/docker-volume.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Guangya Liu]

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

(Updated 六月 14, 2016, 9:19 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added documentation for `docker/volume` isolator.


Diffs (updated)
-

  docs/docker-volume.md PRE-CREATION 
  docs/images/docker-volume-isolator.png PRE-CREATION 

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


Testing
---

You can review the document here: 
https://github.com/jay-lau/mesos/blob/master/docs/docker-volume.md


Thanks,

Guangya Liu

Re: Review Request 44500: Implemented passing image and user env var to command task.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44498, 44499, 44500]

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 14, 2016, 8:04 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44500/
> ---
> 
> (Updated June 14, 2016, 8:04 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing image and user env var to command task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> eb2790d44ee08c81d31fbe28e7a8ffe88edba870 
> 
> Diff: https://reviews.apache.org/r/44500/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Neil Conway]

> 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?
> 
> Anindya Sinha wrote:
> 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.
> 
> Neil Conway wrote:
> Thanks for the writeup! Makes sense.
> 
> If we cause the agent to exit on DESTROY failure, if we only re-apply 
> DESTROYs at `ReregisterSlave`, it seems to me there is still a window in 
> which another `CREATE` can be applied at the master. That would mean we 
> wouldn't reapply the `DESTROY`, which would be bad.
> 
> My concern about updating the checkpointed resources to reflect *partial* 
> results is that (a) it seems unnecessary, (b) it means the checkpointed 
> resources don't reflect either the *original* or the *desired* state of the 
> agent, which seems problematic.
> 
> What about the following: when an agent gets a 
> `CheckpointedResourcesMessage` that differs from its current state, it first 
> writes the desired resource state to a new checkpoint file, 
> `checkpoint.target`. Then it tries to apply the delta between 
> `checkpoint.target` and the current checkpoint. If the on-disk state at the 
> agent is updated successfully to match `checkpoint.target`, we rename 
> 

Re: Review Request 48315: Restart slave if a volume is CREATED on a non-empty path.

[Neil Conway]

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




src/slave/slave.cpp (line 2540)


I think this justifies a unit test. What do you think?


- Neil Conway


On June 13, 2016, 9:03 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48315/
> ---
> 
> (Updated June 13, 2016, 9:03 p.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
> ---
> 
> Root of a MOUNT disk is not deleted on volume DELETE. When we do a
> CREATE on a persistent volume and the root directory exists (which
> can happen for MOUNT disks), we allow the operation only if the
> contents within the root directory is empty. If not, we do not update
> the checkpoint with this volume and exit, so as to cleanup when the
> slave restarts and handles the CheckpointResourcesMessage.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 0ccc56f153024ff45d2fd1f25c79c7bb6ed7120e 
> 
> Diff: https://reviews.apache.org/r/48315/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Gilbert Song]

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




src/docker/docker.cpp (line 548)


This will segfault if no driver specified, right?

I roughly remember docker supports volume by using default driver (not 
necessary to specify).


- Gilbert Song


On June 12, 2016, 9:36 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36440/
> ---
> 
> (Updated June 12, 2016, 9:36 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5341
> https://issues.apache.org/jira/browse/MESOS-5341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled docker volume support for DockerContainerizer.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/docker/docker.cpp a6dcbe788abb8802baa2c70ec2fced1da75c7210 
> 
> Diff: https://reviews.apache.org/r/36440/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 1) Start convoy
> 2) Create a volume named as c1
> root@kvm-002605:~# convoy list
> {
> "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
> "UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Name": "c1",
> "Driver": "devicemapper",
> "MountPoint": "",
> "CreatedTime": "Mon May 09 09:38:52 +0800 2016",
> "DriverInfo": {
> "DevID": "1",
> "Device": 
> "/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Driver": "devicemapper",
> "MountPoint": "",
> "Size": "107374182400"
> },
> "Snapshots": {}
> }
> }
> 3) Update  mesos-execte to set up volume
> index 4711e80..6c712a2 100644
> --- a/src/cli/execute.cpp
> +++ b/src/cli/execute.cpp
> @@ -72,6 +72,8 @@ using mesos::v1::TaskID;
>  using mesos::v1::TaskInfo;
>  using mesos::v1::TaskState;
>  using mesos::v1::TaskStatus;
> +using mesos::v1::Volume;
> +using mesos::v1::Parameters;
>  
>  using mesos::v1::scheduler::Call;
>  using mesos::v1::scheduler::Event;
> @@ -581,6 +583,18 @@ private:
>  
>containerInfo.set_type(ContainerInfo::DOCKER);
>containerInfo.mutable_docker()->set_image(dockerImage.get());
> +  //containerInfo.mutable_docker()->set_volume_driver("convoy");
> +
> +  Volume* volume1 = containerInfo.add_volumes();
> +  //volume1->set_host_path("c1");
> +  volume1->set_container_path("/tmp");
> +  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
> +  volume1->mutable_source()->mutable_docker_volume()->set_driver(
> +  "convoy");
> +  volume1->mutable_source()->mutable_docker_volume()->set_name(
> +  "c1");
> +  volume1->set_mode(Volume::RW);
> +  cout << "Add Voume 1" << endl;
>  
>if (networks.isSome() && !networks->empty()) {
>  vector tokens = strings::tokenize(networks.get(), ",");
> 4) Start master
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master
> 5) Start agent
> GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
> --isolation=filesystem/linux,docker/runtime --containerizers=docker 
> --work_dir=/tmp/mesos --image_providers=docker
> 5) Start mesos-execute
> src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
> --docker_image=ubuntu:14.04 --containerizer=docker
> I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
> I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
> master@10.0.0.65:5050
> Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
> Add Voume 1
> Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_LOST for task 'test'
> 
> 6) Inspect container
> docker inspect 
> ...
> {
> "Name": "c1",
> "Source": 
> "/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Destination": "/tmp",
> "Driver": "convoy",
> "Mode": "rw",
> "RW": true,
> "Propagation": "rprivate"
> }
> ...
> 7) Check sandbox executer log
> I0509 13:14:56.313102  8361 logging.cpp:195] Logging to STDERR
> I0509 13:14:56.315385  8361 process.cpp:999] libprocess is initialized on 
> 10.0.0.65:52853 with 16 worker threads
> I0509 13:14:56.316789  8361 exec.cpp:150] Version: 0.29.0
> I0509 13:14:56.321717  8387 exec.cpp:200] Executor started at: 
> 

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Gilbert Song]

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


Fix it, then Ship it!




This is great! Thanks, Guangya!


docs/docker-volume.md (lines 28 - 31)


The integration of external storage in Mesos is an attractive feature, 
since it supports stateful services in mesos containers by using external 
volumes.



docs/docker-volume.md (line 35)


"size-limited"?



docs/docker-volume.md (line 37)


s/is/becomes/g



docs/docker-volume.md (line 88)


remove "to be"



docs/docker-volume.md (line 89)


generates



docs/docker-volume.md (line 95)


Replace "The cleanup module" by "The isolator cleanup"?



docs/docker-volume.md (line 159)


should we kill this line? (I would suggest for here in doc).



docs/docker-volume.md (line 165)


ditto.



docs/docker-volume.md (line 167)


ditto.



docs/docker-volume.md (line 172)


ditto.



docs/docker-volume.md (lines 306 - 311)


It is highly not recommended to use the same Docker volume by 
DockerContainerizer and MesosContainerizer simultaneously, because
MesosContainerizer has its own reference counter to decide when to unmount 
the
Docker volume mount point. Otherwise, it would be problematic if a Docker 
volume is
unmounted by MesosContainerizer but the DockerContainerizer is still using 
it.


- Gilbert Song


On June 12, 2016, 8:18 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> ---
> 
> (Updated June 12, 2016, 8:18 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
> https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> ---
> 
> You can review the document here: 
> https://github.com/jay-lau/mesos/blob/master/docs/docker-volume.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44500: Implemented passing image and user env var to command task.

[Gilbert Song]

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

(Updated June 14, 2016, 1:04 a.m.)


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


Repository: mesos


Description
---

Implemented passing image and user env var to command task.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
eb2790d44ee08c81d31fbe28e7a8ffe88edba870 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 44499: Added task_environment flag/used execvpe for command executor.

[Gilbert Song]

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

(Updated June 14, 2016, 1:04 a.m.)


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


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


Repository: mesos


Description
---

Added task_environment flag/used execvpe for command executor.


Diffs (updated)
-

  src/launcher/executor.cpp ffebb3a496be6a20396f8f539372a1e7527c9b6d 

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


Testing
---

make check


Thanks,

Gilbert Song

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

[Gilbert Song]

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

(Updated June 14, 2016, 1:03 a.m.)


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


Repository: mesos


Description
---

Forbid the executor to inherit from slave environment.


Diffs (updated)
-

  docs/configuration.md 7612d39d88dc6c0229b5def9697a97ab387f6ef1 
  src/slave/containerizer/containerizer.hpp 
ff78b4d0fd4a3b862f6019fc757c16b7367cd3cf 
  src/slave/containerizer/containerizer.cpp 
faa0c789dda8a6f36fdb6217b0bae270b6b8f2e2 
  src/slave/containerizer/docker.hpp 311dca23ac17fb533866aba1de2b81d750bbe6df 
  src/slave/containerizer/docker.cpp 63efbe6f45958d44d60fe4a7fea816f5fb0457b2 
  src/slave/flags.cpp 7b0e347772646c177c1e18334633a71bc6cedb85 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48675: Added TODO to support Docker 'whiteout' deletions on Windows.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [48675, 48002, 48004, 48003, 47842, 47603, 47602, 47821, 
47577, 47576, 47536, 47472, 47469, 47943, 47470, 47942, 47874, 47873, 47442, 
47489, 47474, 47486, 47473]

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

Error:
2016-06-14 07:44:24 URL:https://reviews.apache.org/r/48003/diff/raw/ 
[9944/9944] -> "48003.patch" [1]
error: patch failed: src/Makefile.am:187
error: src/Makefile.am: patch does not apply

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

- Mesos ReviewBot


On June 14, 2016, 7:17 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48675/
> ---
> 
> (Updated June 14, 2016, 7:17 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-5610
> https://issues.apache.org/jira/browse/MESOS-5610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added TODO to support Docker 'whiteout' deletions on Windows.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 9e803f6ae55f09f7e871c60531acce16ebd5e8fe 
> 
> Diff: https://reviews.apache.org/r/48675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 48675: Added TODO to support Docker 'whiteout' deletions on Windows.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


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


Repository: mesos


Description
---

Added TODO to support Docker 'whiteout' deletions on Windows.


Diffs
-

  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
9e803f6ae55f09f7e871c60531acce16ebd5e8fe 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 48593: Refactor Ranges Subtraction.

[Yanyan Hu]

> On June 13, 2016, 12:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, lines 409-420
> > 
> >
> > Thanks for digging into this issue further.
> > Please do read my response on your e-mail thread for further context.
> > 
> > This patch looks like it fixes a particular bottleneck by converting to 
> > intervalset inline.
> > I understand why you did this, as my e-mail outlines that changing the 
> > stored data-type is more complicated.
> > 
> > I can see how this improves performance; however, is the resource 
> > validation of ranges also not a problem?
> > 
> > Your benchmarks are specific to this operation. Have you tried this 
> > patch your example 10K task / 20 node cluster to identify if the validation 
> > is still a bottleneck?
> > 
> > If we run all the benchmarks in the test suite, are all of them 
> > positively impacted? Or are some negatively?
> > 
> > If this is purely a win, then we can consider this approach as a 
> > temporary fix assuming the following:
> > - Simple patch (yes! :-D )
> > - A JIRA to follow up with changing the stored data-type so we don't 
> > treat this as a permanent solution

Hi, Joris, thank you so much for your comment! Understand your consideration 
and totally agree that refactoring the resource definition should be the 
permanent solution. I have made a test by running 10K tasks in 20 nodes and 
didn't found performance bottleneck caused by resource validation. Mesos 
allocator now works very efficiently in that scale. And I will make further 
tests to evalute the impact on other test cases to see whether there is new 
performance regression induced.

BTW, is there any approach to analyze the benchmark result(time consumption) 
automatically or I need to compare the difference manually?


- Yanyan


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


On June 13, 2016, 12:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 13, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>