Re: Review Request 38910: Added `-v` flag to `docker rm`.

[Greg Mann]

> On Oct. 1, 2015, 4:31 a.m., Jojy Varghese wrote:
> > src/docker/docker.cpp, line 681
> > 
> >
> > wondering this behavior should be defaulted or not. We might be 
> > overloading stop with more than what it should be doing isnt it? Do we 
> > always want to force remove the volumes when docker stops?
> 
> Greg Mann wrote:
> My & Tim's original thought was that because persistent volumes aren't 
> explicitly implemented for the Docker containerizer currently, we could 
> safely make `-v` the default behavior. However, after considering a bit more, 
> I'm thinking that existing user workflows might establish a Docker volume on 
> an agent with the intention of returning to it later with a subsequent task, 
> in which case defaulting to `-v` would break this workflow, so making this an 
> optional argument may be a good idea. I'll have to think about what is the 
> best way to pass this option to the agent, any thoughts on that matter would 
> be appreciated!
> 
> Greg Mann wrote:
> After further review, I do think that we should make the default behavior 
> include the `-v` flag. By default, Docker will create directories in 
> `/var/lib/docker/aufs/`, `/var/lib/docker/vfs`, etc. depending on the 
> filesystem used. These directories contain information related to the Docker 
> image itself, and they will not be deleted if the `-v` flag isn't used. Thus, 
> if an agent runs long enough and runs enough new Docker images, it can fill 
> up the disk with these default volumes.
> 
> While it's possible that a user might hack together a "persistent volume" 
> using Docker volumes as they're currently implemented, we should discourage 
> this in favor of utilizing Mesos-monitored persistent volumes. Always using 
> `docker rm -v` allows us to properly clean the agent, leaving it in a good 
> state after the containerized task has completed.
> 
> It's also worth noting that `docker rm -v` will NOT remove volumes where 
> a host path has been explicitly mounted into the container (i.e. using 
> `docker run -v /host/path:/container/path mycontainer`). So, in fact, some 
> volumes will be unaffected by this change. The primary purpose of this patch 
> is to clean the agent by removing the default volumes created in 
> `/var/lib/docker/`.
> 
> Guangya Liu wrote:
> Greg, so if I was using -v /host/path:/container/path when create a 
> container, I will always need to remove the files explictly on the docker 
> server when the container stopped?

Guangya, yes you have to remove these volumes manually. Even if the directory 
/host/path does not exist prior to running the container, Docker does not 
remove volumes mounted in that way, and I don't believe Mesos removes such 
volumes either. This is perhaps another feature that we should add into the 
Docker containerizer: when attaching a volume of the form `-v 
/host/path:/container/path`, Mesos could check to see if that directory exists 
already; if it doesn't, then it could be removed along with the container after 
the task is complete.


- Greg


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


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38910: Added `-v` flag to `docker rm`.

[Timothy Chen]

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



src/docker/docker.cpp (line 681)


Mounts doesn't work that way where you can remove the directory in the 
container.
IMO this is not something we are looking to support since we don't have a 
strong use case for this.

+1 for what Greg said and we should make -v the default.


- Timothy Chen


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 39007: libprocess: Used thread-safe replacement for strerror.

[Benjamin Bannier]

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Summary (updated)
-

libprocess: Used thread-safe replacement for strerror.


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


Repository: mesos


Description (updated)
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  3rdparty/libprocess/src/io.cpp 26686e1a96484e3f09d41a7292f38b7579ce9c48 
  3rdparty/libprocess/src/poll_socket.cpp 
28ed102972a9d8f88048aea4046ed837b6a25b35 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/profiler.cpp 0c515568880aa6b7a65cfe2955eb7132bdfb3baf 
  3rdparty/libprocess/src/subprocess.cpp 
a457cbe35ad33531c49f74550cd570cf28758f5d 

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


Testing (updated)
---

make check


Thanks,

Benjamin Bannier

Review Request 39006: stout: Used thread-safe replacement for strerror.

[Benjamin Bannier]

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Summary (updated)
-

stout: Used thread-safe replacement for strerror.


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


Repository: mesos


Description (updated)
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
7eb51e8771e95f57548fc35753e75c6d56cd97cd 
  3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
e740d5bc0f0cc5cf8e99b2064c1e39c08282da67 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 

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


Testing (updated)
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 39037: Allow description empty in help information.

[haosdent huang]

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

(Updated Oct. 7, 2015, 3:04 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Update according @bmahler's review


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


Repository: mesos


Description
---

Allow description empty in help information.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
e7dc670648e8abd2fef7be79835f0b71e3e91258 
  3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 39060: Create master detector per url & not per framework

[Guangya Liu]

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



src/sched/sched.cpp (line 139)


Suggest: Increment Master Detector reference count to xxx for url xxx



src/sched/sched.cpp (line 156)


Suggest: Decrement Master Detector reference count to xx for url xxx



src/sched/sched.cpp (line 160)


Suggest: Delete detector xxx for url xxx



src/sched/sched.cpp (lines 187 - 189)


Why not finish this in L121?



src/sched/sched.cpp (lines 1804 - 1805)


string message = "Failed to find master detector for '" + url;

The url  can be moved to upper line.


- Guangya Liu


On 十月 6, 2015, 7:54 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39060/
> ---
> 
> (Updated 十月 6, 2015, 7:54 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3595
> https://issues.apache.org/jira/browse/MESOS-3595
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the number of framework created exceeds the lib process
> threads then during master failover the zookeeper updates can
> cause deadlock.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
> 
> Diff: https://reviews.apache.org/r/39060/diff/
> 
> 
> Testing
> ---
> 
> make check successful 
> Created 100 framework instances on a 24 CPU machine. Master failover detected 
> by the framework process and continue to work as expected.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Guangya Liu]

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



src/common/resources.cpp (line 354)


What does this mean? In my understanding, the resource falg should support 
both string and json format, why remove old resource flag format?


- Guangya Liu


On 十月 6, 2015, 11:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated 十月 6, 2015, 11:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38570: Change documentation image links to absolute paths.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38570]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 4:45 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Oct. 6, 2015, 4:45 p.m.)
> 
> 
> Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3183
> https://issues.apache.org/jira/browse/MESOS-3183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
>   docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
>   docs/networking-for-mesos-managed-containers.md 
> 33568a8233523ff3805f962371c94346e608208b 
>   docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Patched `Rakefile` to include images in the website (See JIRA for the patch).
> Then rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38919: Validation of Docker Image Manifests

[Gilbert Song]

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

(Updated Oct. 6, 2015, 12:08 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Validation of Docker Image Manifests


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (Ubuntu14.04 + clang++-3.6)


Thanks,

Gilbert Song

Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

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

Review request for mesos.


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


Repository: mesos


Description
---

Check failed due to double comparison : MESOS-3552.


Diffs
-

  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 

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


Testing
---

Added unit test.
make check successful.


Thanks,

Mandeep Chadha

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Neil Conway]

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



src/common/resources.cpp (line 870)


This change should also be applied to Resources::apply() in 
src/v1/resources.cpp



src/common/resources.cpp (line 871)


Capitalize "check"; also, maybe include a brief explanation of why we might 
see some error in the floating point calculation.



src/common/resources.cpp (line 872)


Should be "constexpr", maybe "static" too.



src/tests/reservation_tests.cpp (line 175)


I think the comment here should explain why using a double value is 
relevant.



src/tests/reservation_tests.cpp (line 245)


Whitespace/capitalization.


- Neil Conway


On Oct. 6, 2015, 5:59 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38527]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 4:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38527/
> ---
> 
> (Updated Oct. 6, 2015, 4:47 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-3470
> https://issues.apache.org/jira/browse/MESOS-3470
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix UserCgroupIsolatorTest failed on CentOS 6.6.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 237f3f27722b01ff92d0dcbaba7910613542a1a7 
>   src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 
>   src/tests/mesos.cpp ab2d85b091d121113931e4190a5b496901dcd7a5 
> 
> Diff: https://reviews.apache.org/r/38527/diff/
> 
> 
> Testing
> ---
> 
> # In CentOS 6.6
> sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 39037: Allow description empty in help information.

[Ben Mahler]

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



3rdparty/libprocess/include/process/help.hpp (line 51)


Please use an Option :)


- Ben Mahler


On Oct. 6, 2015, 4:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> ---
> 
> (Updated Oct. 6, 2015, 4:15 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow description empty in help information.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> e7dc670648e8abd2fef7be79835f0b71e3e91258 
>   3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7 
> 
> Diff: https://reviews.apache.org/r/39037/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37024: Exposes mesos version information in components.

[Ben Mahler]

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


Would be great to add a unit test for VersionProcess as a follow up, not in 
this patch.


src/version/version.hpp (lines 40 - 41)


Looks like this should be static?



src/version/version.cpp (lines 56 - 58)


Couple of things:

(1) Can you use a 2 space indent instead of 4?

(2) Can you add the git fields here as well?

```
"{",
"  \"build_user\":\"username\",",
"  \"build_time\":1443894750,",
"  \"build_date\":\"2015-10-04 01:52:30\",",
"  \"git_branch\":\"branch\", // Optional",
"  \"git_sha\":\"2199a599d4e57cce0c9209660e488f530156e07b\", // Optional",
"  \"git_tag\":\"0.26.0-rc1\", // Optional",
"  \"version\":\"0.26.0\"",
"}",
```

(3) Ideally we could just make VersionProcess::version static and then just 
pretty print the json object directly here instead of hard coding an example. 
However, we don't currently have pretty printing, so feel free to just leave 
this example here and have a TODO for generating the example automatically.


- Ben Mahler


On Oct. 6, 2015, 2:02 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37024/
> ---
> 
> (Updated Oct. 6, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1841
> https://issues.apache.org/jira/browse/MESOS-1841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an endpoint exposes Apache Mesos build informations and version 
> information.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
>   src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
>   src/slave/main.cpp 364dc7fc7ab2e3cef01aea7267dafa014b60e2b9 
>   src/version/version.hpp PRE-CREATION 
>   src/version/version.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37024/diff/
> 
> 
> Testing
> ---
> 
> Manual test result:
> 
> ```
> $ curl http://localhost:5050/version 2>/dev/null|jq .
> 
> {
>   "version": "0.24.0",
>   "build_user": "haosdent",
>   "build_time": 1439702338,
>   "build_date": "2015-08-16 13:18:58"
> }
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37993: Add explanatory comments for Allocator interface

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37993]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 12:26 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> ---
> 
> (Updated Oct. 6, 2015, 12:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
> https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[haosdent huang]

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



src/slave/slave.cpp (line 2420)


Should use return here directly instead of break?



src/slave/slave.cpp (line 2545)


When execute is in unexpected state, we keep http connection open and not 
close it?



src/slave/slave.cpp (line 4244)


I just suggest if it possible to change it like this pattern.

```
if (executor->pid.isSome() && executor->pid.get()) {
// Normal executor
} else if (executor->pid.isNone()) {
// Http executor
} else {
// Other cases
}
```

My concern is `else` maybe have more exception cases in the future, assume 
the only case in `else` is http executor seems not always match.


- haosdent huang


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 37024: Exposes mesos version information in components.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37024]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 2:02 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37024/
> ---
> 
> (Updated Oct. 6, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1841
> https://issues.apache.org/jira/browse/MESOS-1841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an endpoint exposes Apache Mesos build informations and version 
> information.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
>   src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
>   src/slave/main.cpp 364dc7fc7ab2e3cef01aea7267dafa014b60e2b9 
>   src/version/version.hpp PRE-CREATION 
>   src/version/version.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37024/diff/
> 
> 
> Testing
> ---
> 
> Manual test result:
> 
> ```
> $ curl http://localhost:5050/version 2>/dev/null|jq .
> 
> {
>   "version": "0.24.0",
>   "build_user": "haosdent",
>   "build_time": 1439702338,
>   "build_date": "2015-08-16 13:18:58"
> }
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mei Wan]

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

(Updated Oct. 6, 2015, 6:36 a.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.


Changes
---

I've corrected everything! Will start writing tests. Sorry this took me so long.


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


Repository: mesos


Description
---

Implemented the overlay filesystem backend by layering the images as a 
read-only filesystem.


Diffs (updated)
-

  src/Makefile.am e698927 
  src/slave/containerizer/provisioner/backend.cpp b5d9670 
  src/slave/containerizer/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/backends/overlay.cpp PRE-CREATION 

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


Testing
---

I haven't done any official testing. When I was working off Ian's branch, I 
tested it manually and the provisioning works.


Thanks,

Mei Wan

Re: Review Request 39018: Added JSON parsing for Resources.

[haosdent huang]

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



src/common/resources.cpp (line 362)


Is it would more clear to split three functions, one is for parse old form, 
one is for parse json object from and third one is for parse json array form.


- haosdent huang


On Oct. 5, 2015, 9:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 5, 2015, 9:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[haosdent huang]

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



src/slave/slave.cpp (line 2479)


Seems this line is incomplete.


- haosdent huang


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38779: Use new HTTP status code check in scheduler.

[Gilbert Song]

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

Ship it!


Ship It!

- Gilbert Song


On Oct. 6, 2015, 1:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38779/
> ---
> 
> (Updated Oct. 6, 2015, 1:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use new HTTP status code check in scheduler.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/38779/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38919: Validation of Docker Image Manifests

[Timothy Chen]

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



src/slave/containerizer/provisioner/docker/spec.cpp (line 62)


Align this next to (



src/slave/containerizer/provisioner/docker/spec.cpp (line 75)


I don't think we need to check SHA length, or signature length


- Timothy Chen


On Oct. 6, 2015, 7:08 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38919/
> ---
> 
> (Updated Oct. 6, 2015, 7:08 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-3099
> https://issues.apache.org/jira/browse/MESOS-3099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validation of Docker Image Manifests
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38919/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 39060: Create master detector per url & not per framework

[Mandeep Chadha]

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

Review request for mesos.


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


Repository: mesos


Description
---

If the number of framework created exceeds the lib process
threads then during master failover the zookeeper updates can
cause deadlock.


Diffs
-

  src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 

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


Testing
---

make check successful 
Created 100 framework instances on a 24 CPU machine. Master failover detected 
by the framework process and continue to work as expected.


Thanks,

Mandeep Chadha

Re: Review Request 38779: Use new HTTP status code check in scheduler.

[Timothy Chen]

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

(Updated Oct. 6, 2015, 8:27 p.m.)


Review request for mesos, Anand Mazumdar and Ben Mahler.


Repository: mesos


Description
---

Use new HTTP status code check in scheduler.


Diffs (updated)
-

  src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 39013: RegistryClient refactor: Fixed comments style.

[Ben Mahler]

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

Ship it!


Thanks for pulling away the dependencies, I'll commit this shortly without the 
const change.


src/slave/containerizer/provisioner/docker/registry_client.hpp (line 53)


Ah, please avoid slipping in other changes like this, looks like this 
belongs in your layer id patch.


- Ben Mahler


On Oct. 6, 2015, 2:30 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39013/
> ---
> 
> (Updated Oct. 6, 2015, 2:30 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegistryClient refactor: Fixed comments style.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
> 
> Diff: https://reviews.apache.org/r/39013/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

[Ben Mahler]

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


I would have liked to ship this but it looks like the fsLayers rename related 
changes need adjustement. Can you please pull that out into a separate patch 
please?

Let's have this patch focus only on the renaming of ManifestResponse.


src/slave/containerizer/provisioner/docker/registry_client.hpp (line 67)


Please let's avoid conflating independent changes in these patches, as it 
makes it much easier to go through review when we're doing one thing at a time.

We should pull out naming cleanup into a seperate patch, it looks like 
there is a lot of naming cleanup we need to do on these files outside of just 
fsLayers. Also, how about s/fsLayers/layers/?



src/tests/containerizer/provisioner_docker_tests.cpp (line 67)


Can you do a sweep to remove all of the namespace aliases? If you want to 
pull them out of the RegistryClient let's use another patch.



src/tests/containerizer/provisioner_docker_tests.cpp (line 429)


In general please try to use succict, non-redundant names:

s/manifestResponseFuture/manifest

It's clear here that this is a future from the type and now that we've 
removed response from the type we should remove it from the name as well.


- Ben Mahler


On Oct. 5, 2015, 8:57 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39014/
> ---
> 
> (Updated Oct. 5, 2015, 8:57 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegistryClient refactor: renamed ManifestResponse as per review comments.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/39014/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Oct. 6, 2015, 5:59 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Timothy Chen]

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

(Updated Oct. 6, 2015, 8:27 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

Allow HTTP response codes to be checked with code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/decoder.hpp 53b8079968a0145651bad9d11aa4be2b504de57a 
  3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
  3rdparty/libprocess/src/tests/http_tests.cpp 
38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
  3rdparty/libprocess/src/tests/process_tests.cpp 
ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 

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


Testing
---

make


Thanks,

Timothy Chen

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

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

(Updated Oct. 6, 2015, 9:33 p.m.)


Review request for mesos and Neil Conway.


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


Repository: mesos


Description
---

Check failed due to double comparison : MESOS-3552.


Diffs (updated)
-

  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

Added unit test.
make check successful.


Thanks,

Mandeep Chadha

Re: Review Request 38919: Validation of Docker Image Manifests

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38901, 38919]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 7:08 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38919/
> ---
> 
> (Updated Oct. 6, 2015, 7:08 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-3099
> https://issues.apache.org/jira/browse/MESOS-3099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validation of Docker Image Manifests
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38919/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Anand Mazumdar]

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/v1/resources.cpp, lines 880-886
> > 
> >
> > Not yours, but too bad we need to duplicate the logic here. I am now 
> > sure what will be the long term plan here. If we have 10 versions, do we 
> > need to copy the code 10 times... That's not good :(
> 
> Mandeep Chadha wrote:
> I agree !! I already missed src/v1/resources.cpp in my first review 
> request :) 
> Thanks Neil.

Jie, this is a tech-debt that should not hopefully exist across 10 version 
iterations :)

On a more serious note, this is the related JIRA :  
https://issues.apache.org/jira/browse/MESOS-3404 
We intend to work on it as part of the HTTP API epic.


- Anand


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


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 37996: Added property manager

[Ben Mahler]

> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote:
> > Ship It!

I don't think we should introduce this into stout in its current form. I 
realize you're planning to use this for authentication stuff, but looking at 
this on its own, it seems like a confusing abstraction. Why would we couple the 
notion of a Tree with the semantics around properties and property inheritance?


- Ben


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


On Oct. 6, 2015, 8:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 6, 2015, 8:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38570: Change documentation image links to absolute paths.

[Ben Mahler]

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



docs/external-containerizer.md (line 92)


How will this work when we add multiple versions of documents to the 
website? Now this hardcodes "latest" as the version?


- Ben Mahler


On Oct. 6, 2015, 4:45 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Oct. 6, 2015, 4:45 p.m.)
> 
> 
> Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3183
> https://issues.apache.org/jira/browse/MESOS-3183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
>   docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
>   docs/networking-for-mesos-managed-containers.md 
> 33568a8233523ff3805f962371c94346e608208b 
>   docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Patched `Rakefile` to include images in the website (See JIRA for the patch).
> Then rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Neil Conway]

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

Ship it!


Ship It!


src/tests/reservation_tests.cpp (line 173)


"a Reserved"


- Neil Conway


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Jie Yu]

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



src/common/resources.cpp (line 876)


Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to

https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros

To choose a default epsilon, you probably want to check the above link. 
Gtest uses "Units in the Last Place (ULPs)" as the default.



src/v1/resources.cpp (lines 876 - 882)


Not yours, but too bad we need to duplicate the logic here. I am now sure 
what will be the long term plan here. If we have 10 versions, do we need to 
copy the code 10 times... That's not good :(


- Jie Yu


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

> On Oct. 6, 2015, 6:34 p.m., Neil Conway wrote:
> > src/common/resources.cpp, line 874
> > 
> >
> > This change should also be applied to Resources::apply() in 
> > src/v1/resources.cpp

Thanks Neil.


- Mandeep


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


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/common/resources.cpp, line 880
> > 
> >
> > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to
> > 
> > https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros
> > 
> > To choose a default epsilon, you probably want to check the above link. 
> > Gtest uses "Units in the Last Place (ULPs)" as the default.
> 
> Mandeep Chadha wrote:
> CHECK_DOUBLE_EQ will also fail. 
> 
> F0930 18:15:38.169140 26984 resources.cpp:874] Check failed: 
> (result.cpus().get()) >= (cpus().get())-0.001L (24 vs. 24) *** 
> Check failure stack trace: *** F0930 18:15:38.169322 26991 resources.cpp:874] 
> Check failed: (result.cpus().get()) >= (cpus().get())-0.001L (24 
> vs. 24) *** Check failure stack trace: ***
> 
> CHECK_NEAR() is the right Macro to use. 
> 
> 
> But we also need to check (isNone() and isNone()) equality and hence the 
> present implementation. I think the long term fix is to wrap double into 
> Double and have the opeartor== to the right thing.
> 
> Jie Yu wrote:
> How do you implemente CHECK_DOUBLE_EQ? You copied from gtest code?

you can just call it:

CHECK_DOUBLE_EQ(result.cpus.get(), cpus.get());

src/common/resources.cpp ( includes the glog/logging.h and all the Macros are 
available).

#include 


- Mandeep


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


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39060: Create master detector per url & not per framework

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Oct. 6, 2015, 7:54 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39060/
> ---
> 
> (Updated Oct. 6, 2015, 7:54 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3595
> https://issues.apache.org/jira/browse/MESOS-3595
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the number of framework created exceeds the lib process
> threads then during master failover the zookeeper updates can
> cause deadlock.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
> 
> Diff: https://reviews.apache.org/r/39060/diff/
> 
> 
> Testing
> ---
> 
> make check successful 
> Created 100 framework instances on a 24 CPU machine. Master failover detected 
> by the framework process and continue to work as expected.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

[Ben Mahler]

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



3rdparty/libprocess/include/process/http.hpp (lines 146 - 188)


What is the value of this being an enum? We can't store the enum anywhere 
so every time we want to use these we have to do explicit casting, like you 
have in Response below.

How about making this a class called 'Status' which holds each of these as 
a static member variable?

That way we can still do:

```
Status::OK
```

And no explicit casting is necessary.



3rdparty/libprocess/include/process/http.hpp (lines 414 - 417)


Rather than adding this to try to cope with the casting from the enum, 
let's avoid the enum per my suggestion above and remove this helper.



3rdparty/libprocess/src/decoder.hpp (line 439)


newline after this?



3rdparty/libprocess/src/decoder.hpp (line 441)


Can you put this above the if, or just remove it?



3rdparty/libprocess/src/http.cpp (line 76)


We avoid static non-POD types due to destruction issues. Can you put this 
on the heap?

Per my suggestion above, perhaps we can have `Status::string(uint16_t 
code)` as a static function on the `Status` class that provides the string 
version of the status.

This should clean up the tests as well.


- Ben Mahler


On Oct. 6, 2015, 8:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Oct. 6, 2015, 8:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/decoder.hpp 
> 53b8079968a0145651bad9d11aa4be2b504de57a 
>   3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

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

(Updated Oct. 6, 2015, 10:07 p.m.)


Review request for mesos and Neil Conway.


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


Repository: mesos


Description
---

Check failed due to double comparison : MESOS-3552.


Diffs (updated)
-

  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

Added unit test.
make check successful.


Thanks,

Mandeep Chadha

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Jie Yu]

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/common/resources.cpp, line 880
> > 
> >
> > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to
> > 
> > https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros
> > 
> > To choose a default epsilon, you probably want to check the above link. 
> > Gtest uses "Units in the Last Place (ULPs)" as the default.
> 
> Mandeep Chadha wrote:
> CHECK_DOUBLE_EQ will also fail. 
> 
> F0930 18:15:38.169140 26984 resources.cpp:874] Check failed: 
> (result.cpus().get()) >= (cpus().get())-0.001L (24 vs. 24) *** 
> Check failure stack trace: *** F0930 18:15:38.169322 26991 resources.cpp:874] 
> Check failed: (result.cpus().get()) >= (cpus().get())-0.001L (24 
> vs. 24) *** Check failure stack trace: ***
> 
> CHECK_NEAR() is the right Macro to use. 
> 
> 
> But we also need to check (isNone() and isNone()) equality and hence the 
> present implementation. I think the long term fix is to wrap double into 
> Double and have the opeartor== to the right thing.

How do you implemente CHECK_DOUBLE_EQ? You copied from gtest code?


- Jie


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


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39042: Updated comment referencing deprecated and removed /master/shutdown endpoint.

[Joerg Schad]

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

(Updated Oct. 6, 2015, 8:18 a.m.)


Review request for mesos and Bernd Mathiske.


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


Repository: mesos


Description (updated)
---

/master/shutdown endpoint has been fully removed with 36732.


Diffs
-

  src/master/master.hpp 4bb65f0b6b77ea7324b0dee943602cfdb0f6a11c 

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


Testing
---


Thanks,

Joerg Schad

Review Request 39042: Updated comment referencing deprecated and removed /master/shutdown endpoint.

[Joerg Schad]

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

Review request for mesos and Bernd Mathiske.


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


Repository: mesos


Description
---

Updated comment referencing deprecated and removed /master/shutdown endpoint.


Diffs
-

  src/master/master.hpp 4bb65f0b6b77ea7324b0dee943602cfdb0f6a11c 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 37996: Added property manager

[Alexander Rojas]

> On Oct. 6, 2015, 1:14 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp, lines 
> > 30-52
> > 
> >
> > Hm.. what will this abstraction be used for? I have a hard time 
> > understanding what this is abstracting, is this general enough to belong in 
> > stout?

To your first question, while performing HTTP Authentication, it will be used 
to check the authentication realm of the endpoint. I also though that it will 
allow the firewall rules "disable_endpoints" to be able to cover children 
endpoints and not only the full path given.

Which leads to the second question, the use cases are all built in libprocess, 
but given that this is a header only, template class, it makes sense to be in 
stout. 

And finally yes, I can think on multiple cases in which this abstraction is 
general enough, think of setting ownership of a directory, using this 
abstraction allows you to set ownership to one folder, and all its children 
automatically inherit the ownership unless explicitly stated otherwise. I 
already mention the one of setting authentication realms for endpoints or other 
rules that are passed to children. You can also set permissions in directories 
or files without having to create a node for each, etc.


- Alexander


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


On Oct. 5, 2015, 6:02 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 5, 2015, 6:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38579: Refactored registry client

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39013, 38443, 38579]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 2:33 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38579/
> ---
> 
> (Updated Oct. 6, 2015, 2:33 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Cleanup and broke big methods into smaller chunks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38579/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38919: Validation of Docker Image Manifests

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38901, 38919]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 10:35 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38919/
> ---
> 
> (Updated Oct. 6, 2015, 10:35 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-3099
> https://issues.apache.org/jira/browse/MESOS-3099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validation of Docker Image Manifests
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38919/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 37996: Added property manager

[Till Toenshoff]

> On Oct. 7, 2015, 1:42 a.m., Till Toenshoff wrote:
> > Like most other reviewers, I must confess that my first reaction when 
> > seeing this tree application where mixed and triggered doubts if we really 
> > needed to introduce it. However, I found Alexander's given points very 
> > convincing and I also see no alternatives by now.

Ow, lets please rephrase the subject of this RR towards "Added InheritanceTree."


- Till


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


On Oct. 6, 2015, 8:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 6, 2015, 8:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Klaus Ma]

> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote:
> > One high level suggestion.
> > 
> > After looking at our http code, I realized that we use the same pattern 
> > again and again, for example:
> > ```
> > JSON::Array array;
> > array.values.reserve(status.network_infos().size()); // MESOS-2353.
> > foreach (const NetworkInfo& info, status.network_infos()) {
> >   array.values.push_back(model(info));
> > }
> > object.values["network_infos"] = std::move(array);
> > ```
> > We cannot use newly added `JSON::protobuf()` here, because a different way 
> > for rendering JSON from protobuf is used. Without digging deep inside, I 
> > know three ways how we create a `JSON` out of a proto in our codebase:
> > - wrap in `JSON::Protobuf()` for individual messages;
> > - wrap in one of the `model()` family functions;
> > - pass as it is for built-in types.
> > 
> > The proposed conversion function covers one of the possible ways. How about 
> > add one more convertion? Something like:
> > ```
> > template 
> > Array protobuf(const google::protobuf::RepeatedPtrField& repeated,
> > const lambda::function(const T&)>& converter)
> > {
> >   static_assert(std::is_convertible::value,
> > "T must be a google::protobuf::Message");
> >   JSON::Array array;
> >   array.values.reserve(repeated.size());
> >   foreach (const T& elem, repeated) {
> > array.values.push_back(converter(elem));
> >   }
> >   
> >   return array;
> > }
> > ```
> > 
> > Then the snippet above could be rewritten as:
> > ```
> > object.values["network_infos"] = 
> > std::move(JSON::protobuf(status.network_infos(), [](const NetworkInfo& 
> > info) { return model(info); });
> > ```
> > 
> > A further improvement would be to accept any iterable collection, not only 
> > `RepeatedPtrField<>`, for example `hashset`.
> > 
> > What do you think?
> 
> Klaus Ma wrote:
> Awesome! I've also try similar proposal, but failed when `function` 
> converting with `template`; your suggestion using lambda is great!
> For the `hashset`, I'd suggest to address it when we have such case in 
> our code :).
> 
> I'll also address your comments above.
> 
> Alexander Rukletsov wrote:
> We do have such cases in our codebase ; ). Here are a few as an example:
> - https://github.com/apache/mesos/blob/master/src/master/http.cpp#L217
> - https://github.com/apache/mesos/blob/master/src/master/http.cpp#L229
> 
> Michael Park wrote:
> I like the idea of taking a projection function as an argument, but let's 
> do it as a separate ticket to keep the scope of this review narrow.
> 
> Klaus Ma wrote:
> I'll log a new ticket to trace projection fuction part.
> 
> Klaus Ma wrote:
> MESOS-3580 is logged to trace this requirement, @alex-mesos, would you 
> shepherd it?
> 
> Alexander Rukletsov wrote:
> Great, thanks! I'm not a Mesos committer, hence I cannot shepherd, but I 
> would love to review and help out with that! Maybe @MPark will agree to 
> shepherd?
> 
> Michael Park wrote:
> @klaus1982: Thanks for filing MESOS-3580. I think we'll probably hold off 
> working on that ticket to see if it'll still be useful after the work around 
> speeding up JSON parsing/streaming.

OK, got it for MESOS-3580.


- Klaus


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


On Oct. 4, 2015, 11:29 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Oct. 4, 2015, 11:29 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37468: Removed allocation types to mesos::master namespace

[Jose Guilherme Vanz]

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

(Updated Oct. 7, 2015, 2:04 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

The allocation-related types was moved to mesos::master namespace.

MESOS-2516


Diffs
-

  docs/allocation-module.md 4ba5ba8c27cccebf0e5fc5064fdc1d6af05d38f0 
  include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
  include/mesos/module/allocator.hpp 376eb4860322582f911d7a07253b79b1c9aa9292 
  src/examples/test_allocator_module.cpp 
9679fd9f9a998834bc5329efa07d46a0403ab3f6 
  src/local/local.hpp 6bb25f16982d7be067b5f1307d5eca7745820e81 
  src/local/local.cpp 4d98bf23705027f3ba0cbb571289f21b288fe7db 
  src/master/allocator/allocator.cpp 42214048e83cb50bdb669d79fa49401c1716de87 
  src/master/allocator/mesos/allocator.hpp 
aa55755a9c3250579e9366bdbc17a2449e95d659 
  src/master/allocator/mesos/hierarchical.hpp 
e278139f856888d6c6f538f7c0f664087e97f629 
  src/master/allocator/sorter/drf/sorter.hpp 
f66ade06c6a5b4bf816839477cec2d18036c7b1a 
  src/master/allocator/sorter/drf/sorter.cpp 
bfc273493419fe46a4d907f4f7fa282cff71b800 
  src/master/allocator/sorter/sorter.hpp 
536a7ad9a2d661bc8aa352d2e0ae41115b1e8a04 
  src/master/main.cpp e024a2e4beeb58438accd07e1b87ea239fb1d88b 
  src/master/master.hpp 1e12f650796acafcee83cf9bd0b85f57d359333c 
  src/master/master.cpp c5e6c6f3304060d4c92d52851951f10bc432500e 
  src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
  src/tests/fault_tolerance_tests.cpp c63599ad36d883d25a23f5bab1929c4e2dad4960 
  src/tests/hierarchical_allocator_tests.cpp 
9748ca0b3fee25dcec51c64d8ba84dbd4aaf 
  src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 
  src/tests/master_authorization_tests.cpp 
f3f0cc81ab958e5b9a2bc458f5c38b3e12202514 
  src/tests/master_tests.cpp a4703afc331244a9e959c48652342f31ef787288 
  src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
  src/tests/mesos.cpp 9bff4a66604a67d9add5a05247548e0162f3cda7 
  src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c 
  src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 
  src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b 
  src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519 
  src/tests/scheduler_driver_tests.cpp 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 
  src/tests/scheduler_tests.cpp 7842f27673cc0fc11864916dda4ca4bbb66d9dbd 
  src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc 
  src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 37024: Exposes mesos version information in components.

[haosdent huang]

> On Oct. 6, 2015, 7:23 p.m., Ben Mahler wrote:
> > Would be great to add a unit test for VersionProcess as a follow up, not in 
> > this patch.

Thank you very much, let me add it later.


- haosdent


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


On Oct. 7, 2015, 2:30 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37024/
> ---
> 
> (Updated Oct. 7, 2015, 2:30 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1841
> https://issues.apache.org/jira/browse/MESOS-1841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an endpoint exposes Apache Mesos build informations and version 
> information.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
>   src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
>   src/slave/main.cpp 854ade491c2bad0bb041cd94968215f9fd8fa4ae 
>   src/version/version.hpp PRE-CREATION 
>   src/version/version.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37024/diff/
> 
> 
> Testing
> ---
> 
> Manual test result:
> 
> ```
> $ curl http://localhost:5050/version 2>/dev/null|jq .
> 
> {
>   "version": "0.24.0",
>   "build_user": "haosdent",
>   "build_time": 1439702338,
>   "build_date": "2015-08-16 13:18:58"
> }
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37024: Exposes mesos version information in components.

[haosdent huang]

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

(Updated Oct. 7, 2015, 2:30 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Update according @bmahler's review


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


Repository: mesos


Description
---

Add an endpoint exposes Apache Mesos build informations and version information.


Diffs (updated)
-

  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
  src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
  src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
  src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
  src/slave/main.cpp 854ade491c2bad0bb041cd94968215f9fd8fa4ae 
  src/version/version.hpp PRE-CREATION 
  src/version/version.cpp PRE-CREATION 

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


Testing
---

Manual test result:

```
$ curl http://localhost:5050/version 2>/dev/null|jq .

{
  "version": "0.24.0",
  "build_user": "haosdent",
  "build_time": 1439702338,
  "build_date": "2015-08-16 13:18:58"
}
```


Thanks,

haosdent huang

Re: Review Request 37024: Exposes mesos version information in components.

[haosdent huang]

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

(Updated Oct. 7, 2015, 2:47 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Add an endpoint exposes Apache Mesos build informations and version information.


Diffs (updated)
-

  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
  src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
  src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
  src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
  src/slave/main.cpp 854ade491c2bad0bb041cd94968215f9fd8fa4ae 
  src/version/version.hpp PRE-CREATION 
  src/version/version.cpp PRE-CREATION 

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


Testing
---

Manual test result:

```
$ curl http://localhost:5050/version 2>/dev/null|jq .

{
  "version": "0.24.0",
  "build_user": "haosdent",
  "build_time": 1439702338,
  "build_date": "2015-08-16 13:18:58"
}
```


Thanks,

haosdent huang

Re: Review Request 39018: Added JSON parsing for Resources.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39018]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 11:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 6, 2015, 11:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37024: Exposes mesos version information in components.

[haosdent huang]

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

(Updated Oct. 7, 2015, 2:56 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Update test description


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


Repository: mesos


Description
---

Add an endpoint exposes Apache Mesos build informations and version information.


Diffs
-

  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
  src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
  src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
  src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
  src/slave/main.cpp 854ade491c2bad0bb041cd94968215f9fd8fa4ae 
  src/version/version.hpp PRE-CREATION 
  src/version/version.cpp PRE-CREATION 

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


Testing (updated)
---

Manual test result:

```
$ curl http://localhost:5050/version 2>/dev/null|jq .
{
  "version": "0.26.0",
  "git_sha": "4566f05e1c4c82d4d5e1c6ac563ea0fb362324e3",
  "git_branch": "refs/heads/MESOS-1841",
  "build_user": "haosdent",
  "build_time": 1444185166,
  "build_date": "2015-10-07 10:32:46"
}
```


Thanks,

haosdent huang

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

[Jojy Varghese]

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

(Updated Oct. 7, 2015, 12:16 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

separating out other changes.


Repository: mesos


Description
---

RegistryClient refactor: renamed ManifestResponse as per review comments.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 39068: RegistryClient refactor: Renamed fsLayerInfoList

[Jojy Varghese]

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

Review request for mesos.


Repository: mesos


Description
---

RegistryClient refactor: Renamed fsLayerInfoList


Diffs
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38941: Moved structs outside RegistryClient

[Jojy Varghese]

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

(Updated Oct. 7, 2015, 12:19 a.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased after reordering patches.


Repository: mesos


Description
---

Moved:
  - ManifestResponse
  - FilesystemLayerInfo


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 37996: Added property manager

[Till Toenshoff]

> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote:
> > Ship It!
> 
> Ben Mahler wrote:
> I don't think we should introduce this into stout in its current form. I 
> realize you're planning to use this for authentication stuff, but looking at 
> this on its own, it seems like a confusing abstraction. Why would we couple 
> the notion of a Tree with the semantics around properties and property 
> inheritance?

This code is being used in libprocess. So the options are libprocess or stout 
for introducing it. I believe it would be a better fit for stout than for 
libprocess as it is a data structure implementation that has no threading or 
process specifics. Given that it already is in a reusable state, I think that 
we should go as proposed by this RR.


- Till


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


On Oct. 6, 2015, 8:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 6, 2015, 8:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38910: Added `-v` flag to `docker rm`.

[Greg Mann]

> On Oct. 1, 2015, 4:31 a.m., Jojy Varghese wrote:
> > src/docker/docker.cpp, line 681
> > 
> >
> > wondering this behavior should be defaulted or not. We might be 
> > overloading stop with more than what it should be doing isnt it? Do we 
> > always want to force remove the volumes when docker stops?
> 
> Greg Mann wrote:
> My & Tim's original thought was that because persistent volumes aren't 
> explicitly implemented for the Docker containerizer currently, we could 
> safely make `-v` the default behavior. However, after considering a bit more, 
> I'm thinking that existing user workflows might establish a Docker volume on 
> an agent with the intention of returning to it later with a subsequent task, 
> in which case defaulting to `-v` would break this workflow, so making this an 
> optional argument may be a good idea. I'll have to think about what is the 
> best way to pass this option to the agent, any thoughts on that matter would 
> be appreciated!

After further review, I do think that we should make the default behavior 
include the `-v` flag. By default, Docker will create directories in 
`/var/lib/docker/aufs/`, `/var/lib/docker/vfs`, etc. depending on the 
filesystem used. These directories contain information related to the Docker 
image itself, and they will not be deleted if the `-v` flag isn't used. Thus, 
if an agent runs long enough and runs enough new Docker images, it can fill up 
the disk with these default volumes.

While it's possible that a user might hack together a "persistent volume" using 
Docker volumes as they're currently implemented, we should discourage this in 
favor of utilizing Mesos-monitored persistent volumes. Always using `docker rm 
-v` allows us to properly clean the agent, leaving it in a good state after the 
containerized task has completed.

It's also worth noting that `docker rm -v` will NOT remove volumes where a host 
path has been explicitly mounted into the container (i.e. using `docker run -v 
/host/path:/container/path mycontainer`). So, in fact, some volumes will be 
unaffected by this change. The primary purpose of this patch is to clean the 
agent by removing the default volumes created in `/var/lib/docker/`.


- Greg


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


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39056]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39068: RegistryClient refactor: Renamed fsLayerInfoList

[Jojy Varghese]

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

(Updated Oct. 7, 2015, 12:39 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

fixed braced initializer


Repository: mesos


Description (updated)
---

RegistryClient refactor: renamed fsLayerInfoList


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Klaus Ma]

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



src/common/resources.cpp (line 875)


This fix is ok for this ticket; but how to handle other part about cpu()? 
Here's some question from me:
1. if `epsilon` is 0.01 here, does it mean the min cpu is 0.01?
2. is this the only code that need `epsilon`? It seems not

@jieyu/@mcypark, show we start a EPIC to include all precision related 
ticket? so, we can 
1. unify the min cpu/men/disk to user
2. unify the operator/code within allocator
3. unify the precision between backend and UI
4. clarify the requirement to cpu/mem, e.g. whether accept empty cpu/mem

Any comments?


- Klaus Ma


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39015: RegistryClient refactor: expanded abbreviated names.

[Jojy Varghese]

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

(Updated Oct. 7, 2015, 12:41 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

RegistryClient refactor: expanded abbreviated names.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39017: RegistryClient refactor: encapsulated Manifest

[Jojy Varghese]

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

(Updated Oct. 7, 2015, 12:41 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

Wrapped Manifest with ManifestResponse. getManifest returns the encapsulated 
ManifestResponse.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38873: Added helper functions for evolving old style executor messages to V1 Executor Events

[Ben Mahler]

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

Ship it!


Will get this committed shortly.


src/internal/evolve.hpp (lines 100 - 101)


Why did you wrap?

```
>>> len('v1::executor::Event evolve(const 
StatusUpdateAcknowledgementMessage& message);')
78
```



src/internal/evolve.cpp (lines 304 - 305)


Why did you wrap?

```
>>> len('  kill->mutable_task_id()->CopyFrom(evolve(message.task_id()));')
63
```



src/internal/evolve.cpp (lines 317 - 318)


Ditto about wrapping here and elsewhere


- Ben Mahler


On Oct. 5, 2015, 11:09 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38873/
> ---
> 
> (Updated Oct. 5, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3480
> https://issues.apache.org/jira/browse/MESOS-3480
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change implements helper functions for evolving old style Executor 
> messages in `src/messages/messages.proto` to V1 Executor Events defined in 
> `include/mesos/v1/executor/executor.proto`. This is needed for MESOS-3480
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 9babac3ccbfb2bf9a3989a3ae20cf96e5f3a2903 
>   src/internal/evolve.cpp 625706e089984b32d8298a2eacf2f8af2bca931e 
> 
> Diff: https://reviews.apache.org/r/38873/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 39016: RegistryClient refactor: refactored lambdas

[Jojy Varghese]

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

(Updated Oct. 7, 2015, 12:41 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

RegistryClient refactor: refactored lambdas as per review comments.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

Make check.


Thanks,

Jojy Varghese

Re: Review Request 38941: Moved structs outside RegistryClient

[Jojy Varghese]

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

(Updated Oct. 7, 2015, 12:40 a.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

Moved:
  - ManifestResponse
  - FilesystemLayerInfo


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39053: RegistryClient refactor: priv method const'ness

[Jojy Varghese]

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

(Updated Oct. 7, 2015, 12:42 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

RegistryClient refactor: priv method const'ness


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

[Benjamin Mahler]

This patch still depends on https://reviews.apache.org/r/38579/, are you
planning to remove the dependency? I can't ship these smaller changes since
they depend on the large refactor and your layer id patch:
https://reviews.apache.org/r/38443/

On Tue, Oct 6, 2015 at 5:16 PM, Jojy Varghese  wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39014/
> Review request for mesos and Ben Mahler.
> By Jojy Varghese.
>
> *Updated Oct. 7, 2015, 12:16 a.m.*
> Changes
>
> separating out other changes.
>
> *Repository: * mesos
> Description
>
> RegistryClient refactor: renamed ManifestResponse as per review comments.
>
> Testing
>
> make check.
>
> Diffs (updated)
>
>- src/slave/containerizer/provisioner/docker/registry_client.hpp
>(fdb68b675582f603ffb3e96f31c1c9405e238567)
>- src/slave/containerizer/provisioner/docker/registry_client.cpp
>(4931ae8869a697b1e9d8d4cbc0a871e7cd506285)
>- src/tests/containerizer/provisioner_docker_tests.cpp
>(d895eb9d0723e52cff8b21ef2deeaef1911d019c)
>
> View Diff 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Guangya Liu]

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



src/common/resources.cpp (line 875)


The meos is now using 0.01 as the MIN_CPUS


- Guangya Liu


On 十月 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated 十月 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Guangya Liu]

> On 十月 7, 2015, 1:03 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 879
> > 
> >
> > The meos is now using 0.01 as the MIN_CPUS

As the mesos is using 0.01 as the MIN_CPUS, I think it is OK using 0.01 as the 
epsilon


- Guangya


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


On 十月 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated 十月 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 38910: Added `-v` flag to `docker rm`.

[Guangya Liu]

> On 十月 1, 2015, 4:31 a.m., Jojy Varghese wrote:
> > src/docker/docker.cpp, line 681
> > 
> >
> > wondering this behavior should be defaulted or not. We might be 
> > overloading stop with more than what it should be doing isnt it? Do we 
> > always want to force remove the volumes when docker stops?
> 
> Greg Mann wrote:
> My & Tim's original thought was that because persistent volumes aren't 
> explicitly implemented for the Docker containerizer currently, we could 
> safely make `-v` the default behavior. However, after considering a bit more, 
> I'm thinking that existing user workflows might establish a Docker volume on 
> an agent with the intention of returning to it later with a subsequent task, 
> in which case defaulting to `-v` would break this workflow, so making this an 
> optional argument may be a good idea. I'll have to think about what is the 
> best way to pass this option to the agent, any thoughts on that matter would 
> be appreciated!
> 
> Greg Mann wrote:
> After further review, I do think that we should make the default behavior 
> include the `-v` flag. By default, Docker will create directories in 
> `/var/lib/docker/aufs/`, `/var/lib/docker/vfs`, etc. depending on the 
> filesystem used. These directories contain information related to the Docker 
> image itself, and they will not be deleted if the `-v` flag isn't used. Thus, 
> if an agent runs long enough and runs enough new Docker images, it can fill 
> up the disk with these default volumes.
> 
> While it's possible that a user might hack together a "persistent volume" 
> using Docker volumes as they're currently implemented, we should discourage 
> this in favor of utilizing Mesos-monitored persistent volumes. Always using 
> `docker rm -v` allows us to properly clean the agent, leaving it in a good 
> state after the containerized task has completed.
> 
> It's also worth noting that `docker rm -v` will NOT remove volumes where 
> a host path has been explicitly mounted into the container (i.e. using 
> `docker run -v /host/path:/container/path mycontainer`). So, in fact, some 
> volumes will be unaffected by this change. The primary purpose of this patch 
> is to clean the agent by removing the default volumes created in 
> `/var/lib/docker/`.

Greg, so if I was using -v /host/path:/container/path when create a container, 
I will always need to remove the files explictly on the docker server when the 
container stopped?


- Guangya


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


On 九月 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated 九月 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37996: Added property manager

[Till Toenshoff]

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

Ship it!


Like most other reviewers, I must confess that my first reaction when seeing 
this tree application where mixed and triggered doubts if we really needed to 
introduce it. However, I found Alexander's given points very convincing and I 
also see no alternatives by now.


3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp (lines 197 
- 198)


The empty brackets should go up according to our style.
```
InheritanceTree::InheritanceTree() : root_(std::make_shared()) {}
```



3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp (lines 202 
- 203)


See above.



3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp (line 65)


s/trailered/trailed/



3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp (line 101)


s/of /of a/
s/introduce/introduced/ 
s/return/returns/


- Till Toenshoff


On Oct. 6, 2015, 8:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 6, 2015, 8:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38919: Validation of Docker Image Manifests

[Gilbert Song]

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

(Updated Oct. 6, 2015, 3:35 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Validation of Docker Image Manifests


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (Ubuntu14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38779: Use new HTTP status code check in scheduler.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38416, 38779]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 8:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38779/
> ---
> 
> (Updated Oct. 6, 2015, 8:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use new HTTP status code check in scheduler.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/38779/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Greg Mann]

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

(Updated Oct. 6, 2015, 11:02 p.m.)


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


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

This includes code changes necessary for JSON parsing of Resources. 
Documentation changes will be posted soon in another review.


Diffs (updated)
-

  include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
  include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 39018: Added JSON parsing for Resources.

[Greg Mann]

> On Oct. 6, 2015, 7:09 a.m., haosdent huang wrote:
> > src/common/resources.cpp, line 362
> > 
> >
> > Is it would more clear to split three functions, one is for parse old 
> > form, one is for parse json object from and third one is for parse json 
> > array form.

Good idea, thanks haosdent!


- Greg


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


On Oct. 6, 2015, 11:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 6, 2015, 11:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38570: Change documentation image links to absolute paths.

[Joseph Wu]

> On Oct. 6, 2015, 2:29 p.m., Ben Mahler wrote:
> > docs/external-containerizer.md, line 92
> > 
> >
> > How will this work when we add multiple versions of documents to the 
> > website? Now this hardcodes "latest" as the version?

I'm not sure.

>From what I've seen of the website generation code, we currently wipe out 
>older versions of the docs during generation.  We'd first have to change the 
>site's Rakefile (and probably the release process) to keep older versions.

After that, we'd still have to deal with the problem of relative URLs and 
trailing slashes:
http://stackoverflow.com/questions/5457885/relative-urls-and-trailing-slashes

Perhaps we could change the markdown parser to replace image links with a 
versioned absolute path.  (I'm not sure what that would entail.)

OR

We could change all the image links to relative links (i.e. 
`images/ec_launch_seqdiag.png`) and remove trailing slashes from all 
documentation links.  (Maybe add redirects from trailing slash pages to 
non-trailing slash pages.)


What do you think?


- Joseph


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


On Oct. 6, 2015, 9:45 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Oct. 6, 2015, 9:45 a.m.)
> 
> 
> Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3183
> https://issues.apache.org/jira/browse/MESOS-3183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
>   docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
>   docs/networking-for-mesos-managed-containers.md 
> 33568a8233523ff3805f962371c94346e608208b 
>   docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Patched `Rakefile` to include images in the website (See JIRA for the patch).
> Then rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

[Jojy Varghese]

> On Oct. 6, 2015, 7:57 p.m., Ben Mahler wrote:
> > src/slave/containerizer/provisioner/docker/registry_client.hpp, line 67
> > 
> >
> > Please let's avoid conflating independent changes in these patches, as 
> > it makes it much easier to go through review when we're doing one thing at 
> > a time.
> > 
> > We should pull out naming cleanup into a seperate patch, it looks like 
> > there is a lot of naming cleanup we need to do on these files outside of 
> > just fsLayers. Also, how about s/fsLayers/layers/?

I would like to keep fsLayers as it reflects the language of manifest.


> On Oct. 6, 2015, 7:57 p.m., Ben Mahler wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 67
> > 
> >
> > Can you do a sweep to remove all of the namespace aliases? If you want 
> > to pull them out of the RegistryClient let's use another patch.

Its already removed in a later patch in the series 
(https://reviews.apache.org/r/38941)


> On Oct. 6, 2015, 7:57 p.m., Ben Mahler wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 429
> > 
> >
> > In general please try to use succict, non-redundant names:
> > 
> > s/manifestResponseFuture/manifest
> > 
> > It's clear here that this is a future from the type and now that we've 
> > removed response from the type we should remove it from the name as well.

The idea was to make it obvious 50 lines down from initialization. I can change 
it and create another patch for replacing all xxxFuture with xxx for variable 
names.


- Jojy


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


On Oct. 5, 2015, 8:57 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39014/
> ---
> 
> (Updated Oct. 5, 2015, 8:57 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegistryClient refactor: renamed ManifestResponse as per review comments.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/39014/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38337: Extract gz file in fetcher.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Oct. 5, 2015, 2:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38337/
> ---
> 
> (Updated Oct. 5, 2015, 2:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, and Timothy Chen.
> 
> 
> Bugs: MESOS-3407
> https://issues.apache.org/jira/browse/MESOS-3407
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract gz file in fetcher.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0f1533a0d7dc453e143a15e988d04ca6e55446ff 
>   src/tests/fetcher_tests.cpp 8d13352d0d3f8fb80581e7913c9416b543cfd009 
> 
> Diff: https://reviews.apache.org/r/38337/diff/
> 
> 
> Testing
> ---
> 
> sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="FetcherTest.ExtractGzipFile" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 36180: Avoid multi writers write to same file in PortMappingIsolatorTests.

[haosdent huang]

> On Oct. 6, 2015, 3:20 a.m., Cong Wang wrote:
> > src/tests/containerizer/port_mapping_tests.cpp, line 270
> > 
> >
> > traffic_invalid_via_loopback is not good either, because there is no 
> > traffic from an invalid port

how about `traffice_empty_via_loopback` and `traffic_empty_via_public`


- haosdent


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


On Oct. 6, 2015, 3:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36180/
> ---
> 
> (Updated Oct. 6, 2015, 3:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Ian Downes, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2765
> https://issues.apache.org/jira/browse/MESOS-2765
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid multi writers write to same file in PortMappingIsolatorTests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> feca2043503436ac9abac6017ae9059b3fcbed21 
> 
> Diff: https://reviews.apache.org/r/36180/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 39019: Windows: Added dirent compat code for non-Unix systems.

[Joseph Wu]

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



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 41 - 43)


Is this necessary?  (Do the calls to `FindNextFile`, `FindClose`, or 
`FindFirstFile` require C or something?)



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 51 - 53)


I'm not really sure how necessary this comment is.  (And the two identical 
notes below.)

Presumably, we already know this because:
1) This is Stout.
2) This file is in the `internal` folder.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 87)


Seems like this will give a negative array access if you set `path = "";`.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 165 - 166)


Opening curly brace should be moved up.


- Joseph Wu


On Oct. 5, 2015, 3:12 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39019/
> ---
> 
> (Updated Oct. 5, 2015, 3:12 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dirent compat code for non-Unix systems.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 50e35f43d87c69a83a9e7d039d1881404ea8be38 
> 
> Diff: https://reviews.apache.org/r/39019/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

[Cong Wang]

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



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


When strerror_r() returns EINVAL, you return an empty string?


- Cong Wang


On Oct. 6, 2015, 3:07 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39005/
> ---
> 
> (Updated Oct. 6, 2015, 3:07 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a thread-safe wrapper around strerror_r which has semantics similar 
> to strerror. We plan to use this at call sites currently relying on strerror.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 
> 12ba1ca861114e60f8276c0ee91c543abcfc2519 
>   3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 
> 9e7605c53e6636e7fea32e4f69fbaff9100a979f 
> 
> Diff: https://reviews.apache.org/r/39005/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

[Cong Wang]

> On Oct. 6, 2015, 5:09 p.m., Cong Wang wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp, line 37
> > 
> >
> > When strerror_r() returns EINVAL, you return an empty string?

Also, you should force the XSI-compliant one rather than the GNU one which 
returns a pointer.


- Cong


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


On Oct. 6, 2015, 3:07 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39005/
> ---
> 
> (Updated Oct. 6, 2015, 3:07 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a thread-safe wrapper around strerror_r which has semantics similar 
> to strerror. We plan to use this at call sites currently relying on strerror.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 
> 12ba1ca861114e60f8276c0ee91c543abcfc2519 
>   3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 
> 9e7605c53e6636e7fea32e4f69fbaff9100a979f 
> 
> Diff: https://reviews.apache.org/r/39005/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[haosdent huang]

> On Oct. 6, 2015, 1:04 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2389
> > 
> >
> > I think just print a number is not easy to understand when troubleshoot 
> > error happens.
> 
> Anand Mazumdar wrote:
> Can you elaborate a bit more on this ?

never mind, I just now found other places also just print `state`.


- haosdent


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


On Oct. 6, 2015, 4:39 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Oct. 6, 2015, 4:39 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 39043: Added support for HTTP Authentication in Mesos.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37714, 37996, 37997, 37998, 37999, 38000, 38094, 38627, 
38950, 39043]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 2:56 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> ---
> 
> (Updated Oct. 6, 2015, 2:56 p.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 1. Adds a flag to load an HTTP Authenticator module from the flags.
> 2. If provided, uses the credentials file to initialize the default HTTP 
> Authenticator.
> 3. Updates the existing endpoints which implement their own basic HTTP 
> authenticator with the libprocess one.
> 4. Updates one test which expected the wrong results since now credentials 
> are checked before the body of the request.
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp 51d477444c4d1a2c9dd2f32164ebffbb1fd5c8c2 
>   src/master/constants.cpp 2b66b27783f18930010ee912e9977ea1647eba09 
>   src/master/flags.hpp 5fd5d502697b2edc22ae98a5a8e361bf62bf8bb6 
>   src/master/flags.cpp 0285ce70cefca09e81ef7137968d024e297fec87 
>   src/master/http.cpp 4b9f9ed005a4af2897171659d15168955cc60660 
>   src/master/master.hpp 9d957519bb0f717526af9b2717dc870fae93c20f 
>   src/master/master.cpp 6bee4f351c3fd0fb72f64bbc863968e4786b318b 
>   src/tests/teardown_tests.cpp 2eeead74b877e282660c4009910c42d93ef3ff46 
> 
> Diff: https://reviews.apache.org/r/39043/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

[haosdent huang]

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

(Updated Oct. 6, 2015, 4:47 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.


Changes
---

Update according @wangcong's review.


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


Repository: mesos


Description
---

Fix UserCgroupIsolatorTest failed on CentOS 6.6.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
237f3f27722b01ff92d0dcbaba7910613542a1a7 
  src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 
  src/tests/mesos.cpp ab2d85b091d121113931e4190a5b496901dcd7a5 

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


Testing
---

# In CentOS 6.6
sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose


Thanks,

haosdent huang

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[Anand Mazumdar]

> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2420
> > 
> >
> > Should use return here directly instead of break?

What is the advantage of one over the other here ? I was being consistent with 
`(re-)registerExecutor(...)` functions in the same file.


> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 4244
> > 
> >
> > I just suggest if it possible to change it like this pattern.
> > 
> > ```
> > if (executor->pid.isSome() && executor->pid.get()) {
> > // Normal executor
> > } else if (executor->pid.isNone()) {
> > // Http executor
> > } else {
> > // Other cases
> > }
> > ```
> > 
> > My concern is `else` maybe have more exception cases in the future, 
> > assume the only case in `else` is http executor seems not always match.

Good suggestion. Fixed.


> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2545
> > 
> >
> > When execute is in unexpected state, we keep http connection open and 
> > not close it?

We are not leaking any descriptors. The OS would reclaim these after the crash 
on this line.


- Anand


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


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[Anand Mazumdar]

> On Oct. 6, 2015, 1:04 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2389
> > 
> >
> > I think just print a number is not easy to understand when troubleshoot 
> > error happens.

Can you elaborate a bit more on this ?


- Anand


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


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

[Gilbert Song]

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

(Updated Oct. 6, 2015, 10:29 a.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs (updated)
-

  src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[Anand Mazumdar]

> On Oct. 6, 2015, 6:20 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2479
> > 
> >
> > Seems this line is incomplete.

It was intentionally left this way to avoid warning as error issues. This would 
be fixed when `MESOS-3476` is resolved. (Read comments before)


- Anand


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


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>