Re: Review Request 45214: Updated protobuf to support external storage.

[Gilbert Song]

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



Ditto to the v1. Thanks, Guangya!


include/mesos/mesos.proto (line 1563)


+1.



include/mesos/mesos.proto (line 1576)


Let's make it 0.30.



include/mesos/mesos.proto (line 1581)


ditto.



include/mesos/mesos.proto (lines 1597 - 1600)


Would we possibly have more fields in this message? If no, I guess we may 
want `repeated string driver_option = 3` only below.

BTW, if we want to keep this msg, could we clarify more detail in comment 
(e.g., add an example in \'\')?



include/mesos/mesos.proto (line 1605)


Newline above.



include/mesos/mesos.proto (line 1607)


ditto.


- Gilbert Song


On April 1, 2016, 10:41 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45214/
> ---
> 
> (Updated April 1, 2016, 10:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5012
> https://issues.apache.org/jira/browse/MESOS-5012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf to support external storage.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e1fc02e05df531e29601c6764a5a48ba2b18569f 
>   include/mesos/v1/mesos.proto 35789e051608ea7f1be3ba5b63eaa1fc4e501c84 
> 
> Diff: https://reviews.apache.org/r/45214/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45310: Removed unnecessary constructors in `Try`.

[Daniel Pravat]

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


Ship it!




Ship It!

- Daniel Pravat


On March 24, 2016, 8:22 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45310/
> ---
> 
> (Updated March 24, 2016, 8:22 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Correctly constrained the templated constructor.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
> c444c0118d39ee6a5da4618d7c62784464377280 
> 
> Diff: https://reviews.apache.org/r/45310/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45311: Added an additional template parameter to 'Try' for typeful error.

[Daniel Pravat]

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


Ship it!




Ship It!

- Daniel Pravat


On March 24, 2016, 8:23 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45311/
> ---
> 
> (Updated March 24, 2016, 8:23 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
> c444c0118d39ee6a5da4618d7c62784464377280 
> 
> Diff: https://reviews.apache.org/r/45311/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45313: Introduced `WindowsSocketError` and refactored out `WindowsErrorBase`.

[Daniel Pravat]

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


Ship it!




Ship It!

- Daniel Pravat


On March 28, 2016, 8:57 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45313/
> ---
> 
> (Updated March 28, 2016, 8:57 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced `WindowsSocketError` and refactored out `WindowsErrorBase`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
> 64102e1f31437d4271a1126e339fb2f33f0181b8 
> 
> Diff: https://reviews.apache.org/r/45313/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45314: Updated `network::connect` to use the typeful `Try` error state.

[Daniel Pravat]

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


Ship it!




Ship It!

- Daniel Pravat


On April 3, 2016, 9:34 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45314/
> ---
> 
> (Updated April 3, 2016, 9:34 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `network::connect` to use the typeful `Try` error state.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/network.hpp 
> 9976257d2d13316062bc95a22ab564ca0df165e5 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 6e6634b4b352e3723096521843546cf56ec6dd8b 
> 
> Diff: https://reviews.apache.org/r/45314/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 45668: Enable CMake build for Linux as an extra COMPILER option.

[Juan Larriba]

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

Review request for mesos, Alex Clemmer and Vinod Kone.


Repository: mesos


Description
---

Enable CMake build for Linux as an extra COMPILER option.


Diffs
-

  support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 

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


Testing
---

Built using docker_build.sh on both centos:7 and ubuntu:14.04


Thanks,

Juan Larriba

Re: Review Request 45668: Enable CMake build for Linux as an extra COMPILER option.

[Juan Larriba]

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

(Updated Abr. 4, 2016, 6:54 a.m.)


Review request for mesos, Alex Clemmer and Vinod Kone.


Repository: mesos


Description (updated)
---

[MESOS-5101] Enable CMake build for Linux as an extra COMPILER option.


Diffs
-

  support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 

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


Testing
---

Built using docker_build.sh on both centos:7 and ubuntu:14.04


Thanks,

Juan Larriba

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[Qian Zhang]

> On March 8, 2016, 8:08 a.m., Anand Mazumdar wrote:
> > include/mesos/v1/mesos.proto, lines 1796-1813
> > 
> >
> > hmmm .. Did you test if the health check workflow works?
> > 
> > IIUC, the `mesos-health-check` binary sends a `TaskHealthStatus` 
> > message back to the executor and that message is not of type 
> > `v1::TaskHealthStatus`. If we try to deserialize, it should fail at that 
> > point. 
> > 
> > For now, it seems to me that the best course of action is to 
> > preserve/keep using the unversioned health check binary/message. In future, 
> > we might want to either modify the existing `mesos-health-check` binary to 
> > emit `v1::TaskHealthStatus` messages in addition to the unversioned ones or 
> > create a new binary for versioned health checks. I would recommend filing a 
> > JIRA and a TODO in the code mentioning this. Makes sense?
> 
> Qian Zhang wrote:
> Thanks for the comment! I think `TaskHealthStatus` and `v1:: 
> TaskHealthStatus` have exactly same fields, so it should be OK to do 
> serialize/deserialize between them, right? Actually all the Call messages 
> sent by this HTTP command executor are v1, and agent is always trying to 
> receive non-v1 messages, I see there is no issues between them.
> 
> Anand Mazumdar wrote:
> Looks like there is some confusion here.
> 
> Regarding your comment:
> "Actually all the Call messages sent by this HTTP command executor are 
> v1, and agent is always trying to receive non-v1 messages"
> 
> This is _not_ how it works. The executor sends the `v1` protobuf and the 
> agent devolves them to an unversioned one before passing it on to the 
> internal code. 
> https://github.com/apache/mesos/blob/master/src/slave/http.cpp#L242
> 
> Also, I would be _really_ surprised if protobuf's allow you to mix and 
> match between different messages if the fields are the same. The descriptors 
> for both the messages are still not the same.
> 
> Does my original issue make more sense now?
> 
> Qian Zhang wrote:
> Yeah, I agree with you!
> > For now, it seems to me that the best course of action is to 
> preserve/keep using the unversioned health check binary/message.
> 
> I am afraid that we can not keep using the unversioned one in this HTTP 
> command executor, the reason is, in the unversioned `TaskHealthStatus`, the 
> field `task_id` is of type "mesos::TaskID" rather than "mesos::v1::TaskID", 
> but the rest of the this HTTP command executor codes use "mesos::v1::TaskID", 
> so there will be some compilation errors if we use the unversioned one, like:
> `error: no viable conversion from 'const mesos::TaskID' to 'const 
> mesos::v1::TaskID'`
> 
> Maybe now we should modify the existing `mesos-health-check` by 
> introducing a new string flag (e.g., `--executor_version`), its default value 
> is `unversioned`, but this HTTP command executor will set its value to `v1`, 
> so when `mesos-health-check` is launched, it will know which 
> `TaskHealthStatus` message should be sent. Please let me know your comment :-)
> 
> Anand Mazumdar wrote:
> Why can't you use the `evolve` function to convert the `mesos::TaskID` 
> received from `mesos-health-check` to `mesos::v1::TaskID` or am I missing 
> something?
> 
> Qian Zhang wrote:
> I am not sure if I get your point. Did you mean we call the `evolve` 
> function to do the conversion in HTTP command executor? Can you please 
> elaborate where we can call it in the code?
> 
> Anand Mazumdar wrote:
> Sorry, by bad. I should have included a code snippet.
> 
> ```cpp
> virtual void initialize()
> {
>   // A big TODO about why we are still handling the unversioned protobuf 
> message with a possible link to the JIRA.
>   install(
>   ::taskHealthUpdated,
>   ::task_id,
>   ::healthy,
>   ::kill_task);
> }
> ```
> 
> ```cpp
> void taskHealthUpdated(...)
> {
>   sendStatusUpdate(evolve(taskId), evolve(state), healthy, None());
>   
>   
> }
> ```
> 
> Does it make more sense now?

Yes, it does make sense now. But I think it is kind of temp solution, why not 
we modify `mesos-health-check` now to make it can send `v1:: TaskHealthStatus` 
message as I suggested above (I can file a JIRA and post a patch for it)? And 
then for this patch, we can use `v1:: TaskHealthStatus` so that all the 
protobuf messages in this HTTP command executor are v1.


- Qian


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


On April 4, 2016, 9:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically 

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[Anand Mazumdar]

> On March 8, 2016, 12:08 a.m., Anand Mazumdar wrote:
> > include/mesos/v1/mesos.proto, lines 1796-1813
> > 
> >
> > hmmm .. Did you test if the health check workflow works?
> > 
> > IIUC, the `mesos-health-check` binary sends a `TaskHealthStatus` 
> > message back to the executor and that message is not of type 
> > `v1::TaskHealthStatus`. If we try to deserialize, it should fail at that 
> > point. 
> > 
> > For now, it seems to me that the best course of action is to 
> > preserve/keep using the unversioned health check binary/message. In future, 
> > we might want to either modify the existing `mesos-health-check` binary to 
> > emit `v1::TaskHealthStatus` messages in addition to the unversioned ones or 
> > create a new binary for versioned health checks. I would recommend filing a 
> > JIRA and a TODO in the code mentioning this. Makes sense?
> 
> Qian Zhang wrote:
> Thanks for the comment! I think `TaskHealthStatus` and `v1:: 
> TaskHealthStatus` have exactly same fields, so it should be OK to do 
> serialize/deserialize between them, right? Actually all the Call messages 
> sent by this HTTP command executor are v1, and agent is always trying to 
> receive non-v1 messages, I see there is no issues between them.
> 
> Anand Mazumdar wrote:
> Looks like there is some confusion here.
> 
> Regarding your comment:
> "Actually all the Call messages sent by this HTTP command executor are 
> v1, and agent is always trying to receive non-v1 messages"
> 
> This is _not_ how it works. The executor sends the `v1` protobuf and the 
> agent devolves them to an unversioned one before passing it on to the 
> internal code. 
> https://github.com/apache/mesos/blob/master/src/slave/http.cpp#L242
> 
> Also, I would be _really_ surprised if protobuf's allow you to mix and 
> match between different messages if the fields are the same. The descriptors 
> for both the messages are still not the same.
> 
> Does my original issue make more sense now?
> 
> Qian Zhang wrote:
> Yeah, I agree with you!
> > For now, it seems to me that the best course of action is to 
> preserve/keep using the unversioned health check binary/message.
> 
> I am afraid that we can not keep using the unversioned one in this HTTP 
> command executor, the reason is, in the unversioned `TaskHealthStatus`, the 
> field `task_id` is of type "mesos::TaskID" rather than "mesos::v1::TaskID", 
> but the rest of the this HTTP command executor codes use "mesos::v1::TaskID", 
> so there will be some compilation errors if we use the unversioned one, like:
> `error: no viable conversion from 'const mesos::TaskID' to 'const 
> mesos::v1::TaskID'`
> 
> Maybe now we should modify the existing `mesos-health-check` by 
> introducing a new string flag (e.g., `--executor_version`), its default value 
> is `unversioned`, but this HTTP command executor will set its value to `v1`, 
> so when `mesos-health-check` is launched, it will know which 
> `TaskHealthStatus` message should be sent. Please let me know your comment :-)
> 
> Anand Mazumdar wrote:
> Why can't you use the `evolve` function to convert the `mesos::TaskID` 
> received from `mesos-health-check` to `mesos::v1::TaskID` or am I missing 
> something?
> 
> Qian Zhang wrote:
> I am not sure if I get your point. Did you mean we call the `evolve` 
> function to do the conversion in HTTP command executor? Can you please 
> elaborate where we can call it in the code?
> 
> Anand Mazumdar wrote:
> Sorry, by bad. I should have included a code snippet.
> 
> ```cpp
> virtual void initialize()
> {
>   // A big TODO about why we are still handling the unversioned protobuf 
> message with a possible link to the JIRA.
>   install(
>   ::taskHealthUpdated,
>   ::task_id,
>   ::healthy,
>   ::kill_task);
> }
> ```
> 
> ```cpp
> void taskHealthUpdated(...)
> {
>   sendStatusUpdate(evolve(taskId), evolve(state), healthy, None());
>   
>   
> }
> ```
> 
> Does it make more sense now?
> 
> Qian Zhang wrote:
> Yes, it does make sense now. But I think it is kind of temp solution, why 
> not we modify `mesos-health-check` now to make it can send `v1:: 
> TaskHealthStatus` message as I suggested above (I can file a JIRA and post a 
> patch for it)? And then for this patch, we can use `v1:: TaskHealthStatus` so 
> that all the protobuf messages in this HTTP command executor are v1.

FWIW, I don't quite know how we would like to tackle this in the future. The 
solution proposed by you is one possible route. Another possible solution can 
be to create a separate health check binary itself among others? Hence, I was 
proposing to tackle this as part of a separate JIRA issue later and not worry 
about it for now.

There are still 

Review Request 45670: Added tests for HTTP command executor.

[Qian Zhang]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Added tests for HTTP command executor.


Diffs
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/tests/http_command_executor_tests.cpp PRE-CREATION 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 45528: Upgrade http-parser to v2.6.2 [mesos].

[Zhiwei Chen]

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

(Updated April 4, 2016, 7:40 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


Summary (updated)
-

Upgrade http-parser to v2.6.2 [mesos].


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


Repository: mesos


Description
---

Upgrade http-parser to v2.6.2 [mesos].


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake 35d7e98e9de7dcebbba3b7cdf4d139c918b6f6de 
  LICENSE 1c0355c1462654e1c30546b448f503cd6365dc46 

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


Testing
---

../configure --enable-libevent --enable-ssl

sudo make dist check


Thanks,

Zhiwei Chen

Re: Review Request 45377: Updated prepare() logic for unified container.

[Guangya Liu]

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

(Updated 四月 4, 2016, 9:28 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Updated prepare() logic for unified container.


Diffs (updated)
-

  src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 
  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 

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


Testing
---

Updating the execute.cpp for test, will remove it once test finished.


Thanks,

Guangya Liu

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[Qian Zhang]

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

(Updated April 4, 2016, 5:11 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Updated http_command_executor.cpp to use v1 API.


Diffs (updated)
-

  src/launcher/http_command_executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 45623: Renamed the parameter 'override' to '_override'.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45623]

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

- Mesos ReviewBot


On April 3, 2016, 8:30 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45623/
> ---
> 
> (Updated April 3, 2016, 8:30 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed the parameter 'override' to '_override'.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 76773914adb021dda2324eb566db0c697a353526 
> 
> Diff: https://reviews.apache.org/r/45623/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44454: Checkpointed the external mount info for container.

[Guangya Liu]

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

(Updated 四月 4, 2016, 9:58 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Summary (updated)
-

Checkpointed the external mount info for container.


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


Repository: mesos


Description (updated)
---

Checkpointed the external mount info for container.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Review Request 45671: Updated checkpoint external mount info in cleanup().

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Updated checkpoint external mount info in cleanup().


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45527: Upgrade http-parser to v2.6.2 [libprocess].

[Zhiwei Chen]

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

(Updated April 4, 2016, 7:26 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

Upgrade http-parser to v2.6.2 [libprocess].


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch  
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz 
06528d5a456d6aa8dcee18159392d2ed4e5f3866 
  3rdparty/libprocess/3rdparty/http-parser-2.6.2.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
2616b2d3fbecc0f9f5ef8eea20a61c927da70814 

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


Testing
---

../configure --enable-libevent --enable-ssl

sudo make dist check


Thanks,

Zhiwei Chen

Re: Review Request 45670: Added tests for HTTP command executor.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44423, 44424, 44427, 45670]

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

- Mesos ReviewBot


On April 4, 2016, 9:12 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45670/
> ---
> 
> (Updated April 4, 2016, 9:12 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for HTTP command executor.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/tests/http_command_executor_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45670/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 45674: Implemented recover() for dvd isolator.

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Implemented recover() for dvd isolator.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45370: Implemented prepare() for dvd isolator.

[Guangya Liu]

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

(Updated 四月 4, 2016, 9:21 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Implemented prepare() for dvd isolator.


Diffs (updated)
-

  src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 
  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/dvd/spec.hpp PRE-CREATION 

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


Testing
---

The execute.cpp is mainly for test, will remove this file once test finished.


Thanks,

Guangya Liu

Re: Review Request 45527: Upgrade http-parser to v2.6.2 [libprocess].

[Zhiwei Chen]

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

(Updated April 4, 2016, 7:33 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

Upgrade http-parser to v2.6.2 [libprocess].


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch 2877597 
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz 
06528d5a456d6aa8dcee18159392d2ed4e5f3866 
  3rdparty/libprocess/3rdparty/http-parser-2.6.2.patch PRE-CREATION 
  3rdparty/libprocess/3rdparty/http-parser-2.6.2.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 2616b2d 

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


Testing
---

../configure --enable-libevent --enable-ssl

sudo make dist check


Thanks,

Zhiwei Chen

Re: Review Request 45033: Add a commit hook for checking non-printable characters (MESOS-4033).

[Benjamin Bannier]

> On April 4, 2016, 10:36 a.m., Benjamin Bannier wrote:
> > support/mesos-style.py, line 120
> > 
> >
> > It would be great if we could `+ 1` the `chars`  here like we already 
> > do for the `line_number` in order to keep this all in one place. We could 
> > also use this opportunity to go for less incidental list formatting, e.g.,
> > 
> > chars=', '.join([str(char + 1) for char in char_errors])  # gives 
> > e.g., "Line: 10, Chars: 1, 2, 3: 
> 
> Yong Tang wrote:
> Thanks Benjamin. In Ln 112 the "+ 1" of the Chars has already been 
> processed ("offset + 1").

Exactly! My point was that I wasn't sure that doing these operations on 
`line_numer` and `char_errors` in different operations helped clarity.


- Benjamin


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


On April 4, 2016, 4:31 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 4, 2016, 4:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an addition check in mesos-style.pl to check
> for non-printable characters. It scans .cpp, .hpp, .cc, .h
> files and reports an error if non-printable characters exist.
> 
> As part of this patch, two non-printable characters have been identified
> in versioning.md (one in Line 85 and another in Line 96) and are corrected
> accordingly.
> 
> Note: Scanning .md files is skipped based on feedback from reviews.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45668: Enable CMake build for Linux as an extra COMPILER option.

[Juan Larriba]

> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 19
> > 
> >
> > What is the reason for this change? Was it a bug before? (Feel free to 
> > drop if there is a good reason, it just seems unclear at this point why you 
> > changed it).

Yes, it is a bug, at least on Ubuntu and CentOS:

[root@jlarriba ~]# dirname "$0"
dirname: invalid option -- 'b'
Try 'dirname --help' for more information.

[root@jlarriba ~]# dirname -- "$0"
.


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 21
> > 
> >
> > Do we need these echo 1 etc for something? Or is it just for debugging?

It is indeed for debugging; It should not be there.


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 90
> > 
> >
> > again it feels weird to see cmake on the same level as gcc vs clang.
> > Shouldn't these decisions be orthogonal (i.e., using Cmake and gcc, 
> > cmake and clang, autotools and gcc, ...)

I don't know about this. Currently CMake always uses gcc to compile, I have not 
tried to use clang with CMake yet. However, making it orthogonal makes sense, I 
will try to build using cmake with clang and make it orthogonal.


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 91
> > 
> >
> > Why is this needed here?

Just to notice the build that cmake is being used, just in case. However, if we 
make the build orthogonal, this will dissapear.


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 1
> > 
> >
> > When testing did you test both cmake and the non-cmake way on both 
> > systems? If so could you make this explicit in testing done? 
> > THX!

I did not test the non-cmake way, but I will and modify the testing done field.


- Juan


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


On Abr. 4, 2016, 6:54 a.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated Abr. 4, 2016, 6:54 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5101] Enable CMake build for Linux as an extra COMPILER option.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>

Re: Review Request 45033: Add a commit hook for checking non-printable characters (MESOS-4033).

[Yong Tang]

> On April 4, 2016, 8:36 a.m., Benjamin Bannier wrote:
> > support/mesos-style.py, line 113
> > 
> >
> > Nit-pick: We do not have a Python style, but e.g., the Google Python 
> > style guide would set `if` right below `offset` (btw, this is also what 
> > e.g., `pylint` advocates by default).

Thanks Benjamin. The review request has been updated for the Google Python 
style change.


- Yong


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


On April 4, 2016, 2:31 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 4, 2016, 2:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an addition check in mesos-style.pl to check
> for non-printable characters. It scans .cpp, .hpp, .cc, .h
> files and reports an error if non-printable characters exist.
> 
> As part of this patch, two non-printable characters have been identified
> in versioning.md (one in Line 85 and another in Line 96) and are corrected
> accordingly.
> 
> Note: Scanning .md files is skipped based on feedback from reviews.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-printable characters (MESOS-4033).

[Yong Tang]

> On April 4, 2016, 8:36 a.m., Benjamin Bannier wrote:
> > support/mesos-style.py, line 120
> > 
> >
> > It would be great if we could `+ 1` the `chars`  here like we already 
> > do for the `line_number` in order to keep this all in one place. We could 
> > also use this opportunity to go for less incidental list formatting, e.g.,
> > 
> > chars=', '.join([str(char + 1) for char in char_errors])  # gives 
> > e.g., "Line: 10, Chars: 1, 2, 3: 

Thanks Benjamin. In Ln 112 the "+ 1" of the Chars has already been processed 
("offset + 1").


- Yong


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


On April 4, 2016, 2:31 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 4, 2016, 2:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an addition check in mesos-style.pl to check
> for non-printable characters. It scans .cpp, .hpp, .cc, .h
> files and reports an error if non-printable characters exist.
> 
> As part of this patch, two non-printable characters have been identified
> in versioning.md (one in Line 85 and another in Line 96) and are corrected
> accordingly.
> 
> Note: Scanning .md files is skipped based on feedback from reviews.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45659: Changed some non-ascii characters in libprocess to ascii equivalent.

[Yong Tang]

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

(Updated April 4, 2016, 2:34 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi Xiao.


Changes
---

Update the review request as https://reviews.apache.org/r/45033/ has changed.


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


Repository: mesos


Description
---

This is part of MESOS-4033: Add a commit hook for checking non-ascii characters.

Note: The commit hook requires separate commits for libprocess and mesos.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/windows/format.hpp 
a1bfc808eb1b1c372acb028019901f0ab3f15238 

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


Testing
---

make check


Thanks,

Yong Tang

Re: Review Request 45660: Changed some non-ascii characters to ascii equivalent.

[Yong Tang]

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

(Updated April 4, 2016, 2:35 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi Xiao.


Changes
---

Update the review request as https://reviews.apache.org/r/45033/ has changed.


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


Repository: mesos


Description
---

This is part of MESOS-4033: Add a commit hook for checking non-ascii characters.

Note: The commit hook requires separate commits for libprocess and mesos.


Diffs (updated)
-

  src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 
  src/uri/fetchers/curl.cpp cb50103775fb69e0696e0c389f2119f397a1c499 
  src/uri/fetchers/docker.cpp fbfc96c3f86cd2d2d585516f37be12834c2bb3a4 

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


Testing
---

make check


Thanks,

Yong Tang

Re: Review Request 45528: Upgrade http-parser to v2.6.2 [mesos].

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45527, 45528]

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

- Mesos ReviewBot


On April 4, 2016, 11:40 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45528/
> ---
> 
> (Updated April 4, 2016, 11:40 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5069
> https://issues.apache.org/jira/browse/MESOS-5069
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to v2.6.2 [mesos].
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 35d7e98e9de7dcebbba3b7cdf4d139c918b6f6de 
>   LICENSE 1c0355c1462654e1c30546b448f503cd6365dc46 
> 
> Diff: https://reviews.apache.org/r/45528/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-libevent --enable-ssl
> 
> sudo make dist check
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45668: Enable CMake build for Linux as an extra COMPILER option.

[Joerg Schad]

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




support/docker_build.sh (line 1)


When testing did you test both cmake and the non-cmake way on both systems? 
If so could you make this explicit in testing done? 
THX!



support/docker_build.sh (line 19)


What is the reason for this change? Was it a bug before? (Feel free to drop 
if there is a good reason, it just seems unclear at this point why you changed 
it).



support/docker_build.sh (line 21)


Do we need these echo 1 etc for something? Or is it just for debugging?



support/docker_build.sh (line 90)


again it feels weird to see cmake on the same level as gcc vs clang.
Shouldn't these decisions be orthogonal (i.e., using Cmake and gcc, cmake 
and clang, autotools and gcc, ...)



support/docker_build.sh (line 91)


Why is this needed here?


- Joerg Schad


On April 4, 2016, 6:54 a.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated April 4, 2016, 6:54 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5101] Enable CMake build for Linux as an extra COMPILER option.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>

Re: Review Request 45534: Added per-role and quota share metrics to the DRFSorter.

[Benjamin Bannier]

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

(Updated April 4, 2016, 2:32 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

Added per-role and quota share metrics to the DRFSorter.


Diffs (updated)
-

  docs/monitoring.md 76be32842e8b437fda40c8565a34ec4f8d8dfbcc 
  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/master/allocator/mesos/hierarchical.hpp 
e979fdf60da1409d1c2d08f0e9f03cef067506dd 
  src/master/allocator/sorter/drf/metrics.hpp PRE-CREATION 
  src/master/allocator/sorter/drf/metrics.cpp PRE-CREATION 
  src/master/allocator/sorter/drf/sorter.hpp 
f316bb5b8bfe93311ecac57198392e104b234b04 
  src/master/allocator/sorter/drf/sorter.cpp 
c14f9a45b9e2ea8d79bd8d2f092d313afa4cbac3 
  src/master/allocator/sorter/sorter.hpp 
e2338d5297e11a1ca4f6e5d72a4526aa4579610c 
  src/tests/hierarchical_allocator_tests.cpp 
8f78a204d296f94f515f21511710a35c33e27255 

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


Testing
---

make check (OS X, clang trunk, not optimized)

I did also benchmark the slowdown of the allocator due to this addition with 
the benchmark allocator https://reviews.apache.org/r/44853. There I saw that 
for an unoptimized build this patch adds up to 70 ms to the time needed to 
query the metrics endpoint (this was for the case of 5000 slaves and 1000 
frameworks), though one could expected that an optimized build might perform 
better. The numbers I got where

`#`slaves | #frameworks | old time [us] | new time [us] | slowdown
|-|---|---|-
   1000 |   1 | 38980 | 23847 |0.6
   1000 |  50 | 27834 | 42091 |1.5
   1000 | 100 | 40060 | 47571 |1.2
   1000 | 200 | 63132 | 75806 |1.2
   1000 | 500 |145170 |171929 |1.2
   1000 |1000 |427721 |473822 |1.1
   5000 |   1 | 23249 | 21426 |0.9
   5000 |  50 | 41032 | 36318 |0.9
   5000 | 100 | 43636 | 45210 |1.0
   5000 | 200 | 60204 | 65570 |1.1
   5000 | 500 |121509 |196894 |1.6
   5000 |1000 |449476 |496641 |1.1


Thanks,

Benjamin Bannier

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45214, 45217, 45265, 45270, 45360, 45373, 45370, 45375, 
45377, 45326, 44454, 45671, 45674]

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

- Mesos ReviewBot


On April 4, 2016, 11:05 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 4, 2016, 11:05 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44571: Added timeout for destroying Docker containers.

[Jan Schlicht]

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

(Updated April 4, 2016, 4:05 p.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


Changes
---

Changed order of continuations.


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


Repository: mesos


Description
---

Commands issued to the Docker daemon can hang, causing problems within Mesos.
For example a hanging 'docker stop' can result in an unresponsive executor,
causing the Mesos agent to issue an to run a 'docker stop' itself which might
result in an unresponsive agent (see MESOS-4673).
Adding a timeout can be used as a workaround.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 89d450e10a84f24ddd46d517e2b4b46ab02c4fda 
  src/slave/containerizer/docker.cpp 9314d1f9e0b6077fe7c48b860783ab21acc48be6 

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


Testing
---

sudo ./bin/mesos-tests.sh (to test if existing tests break due to the changed 
behavior)

Because docker must hang for both the Mesos agent as well as the 
`mesos-docker-executor`, it can't currently be tested as part of the Mesos 
integration tests. Here's how to test that the timeout works:
Run with Fedora 23 (Kernel 4.2.3, Docker 1.9.1)
# Start a master
./bin/mesos-master.sh --work_dir=/tmp/mesos &

# Start an agent
sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --containerizers=docker &

# Run a task using the docker containerizer
./src/mesos-execute --containerizer=docker --docker_image=alpine 
--master=127.0.0.1:5050 --name="sleep" --command="sleep 1000" &
# Note the pid of `mesos-execute` as well as the pid of the sleep task run by 
docker (eg 3323 and 3474)

# Have mesos run `docker inspect` to gather the pid of the docker task
curl -X GET localhost:5051/monitor/statistics

# Now overload docker by trying to run a lot of tasks in parallel
for i in `seq 1 100`; do sudo docker run --rm alpine sleep 60 & done

# Wait until the first of these docker tasks finish, `sudo docker ps` should be 
unresponsible now
# Kill the `mesos-execute` task (eg 3323)
kill 3323

# Watch the logs of the Mesos agent. At some point it will send a SIGKILL to 
the docker task (eg 3474)
# Make sure that the docker task is indeed termintad (using `ps fax` or the 
like)


Thanks,

Jan Schlicht

Re: Review Request 45033: Add a commit hook for checking non-printable characters (MESOS-4033).

[Yong Tang]

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

(Updated April 4, 2016, 2:31 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi Xiao.


Changes
---

Update the review request to conform to Google Python style (feedback from 
Benjamin)


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


Repository: mesos


Description
---

This patch adds an addition check in mesos-style.pl to check
for non-printable characters. It scans .cpp, .hpp, .cc, .h
files and reports an error if non-printable characters exist.

As part of this patch, two non-printable characters have been identified
in versioning.md (one in Line 85 and another in Line 96) and are corrected
accordingly.

Note: Scanning .md files is skipped based on feedback from reviews.

Note: This commit includes patches from Kevin Klues and haosdent.


Diffs (updated)
-

  docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
  support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 

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


Testing
---

Tested manually and found two non ascii characters in docs/versioning.md (fixed 
as part of this review request).


Thanks,

Yong Tang

Re: Review Request 45717: Added checks to verify that `rootDir` and `pluginDir` `isSome.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45717]

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

- Mesos ReviewBot


On April 4, 2016, 11:55 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45717/
> ---
> 
> (Updated April 4, 2016, 11:55 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5113
> https://issues.apache.org/jira/browse/MESOS-5113
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `rootDir` and `pluginDir` can be `None` in case the operator does not
> specify the network cni flags. In this case invoking `recover` on the
> isolator without checking the `rootDir` and `pluginDir` for `isSome`
> will cause the Agent to crash.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> af5f49007e0afe57006c9db940611e56eb75a621 
> 
> Diff: https://reviews.apache.org/r/45717/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Bought a slave and master and verified that the slave does not crash.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Klaus Ma]

> On April 4, 2016, 9:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 311-320
> > 
> >
> > Conceptutally, `Try` is not something we pass around like this. A 
> > function returning a `Try` is emulating a function that throws an 
> > exception. So if a `Try` results in an error, we need to handle it 
> > immediately, or propagate the error. We do not pass it along to a 
> > subsequent function.
> > 
> > There are other issues here as well. Even if we were to pass it around, 
> > it should have been passed by `const &`. What does `rc` stand for?
> > 
> > Functionally speaking, this says that if `rc` is an error, we log 
> > saying we failed to update the state of an agent. But `reserveResources` 
> > returns `Error` if the offer doesn't contain sufficient amount of 
> > resources. This has really little to do with "failing to update the state 
> > of an agent."

Thanks very much for your explaintation on `Try`; that's clear to me :).

BTW, `rc` means `return code`.


- Klaus


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


On March 22, 2016, 11:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 45707: Minor cleanups to command executor.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45707]

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

- Mesos ReviewBot


On April 4, 2016, 9:32 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45707/
> ---
> 
> (Updated April 4, 2016, 9:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Trivial cleanups: 
> - Not capture boolean arguments by reference.
> - Added newline after a statement.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 76773914adb021dda2324eb566db0c697a353526 
> 
> Diff: https://reviews.apache.org/r/45707/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45726: Fixed a bug in the `flags::parse` function and added a test case.

[Jie Yu]

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



Can you also add a test case for the empty string case? e.g., `--name=`.

Do you think this should an Error case or not?

- Jie Yu


On April 5, 2016, 2:57 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45726/
> ---
> 
> (Updated April 5, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5114
> https://issues.apache.org/jira/browse/MESOS-5114
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp 
> 6313b6b1f535822e1dee39f5780895a3691ff78c 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> e87bee24b8c57d15f3903ecd8d55e0e51e00f0ff 
> 
> Diff: https://reviews.apache.org/r/45726/diff/
> 
> 
> Testing
> ---
> 
> Added a new test case that fails before the changes to `parse.hpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

[Jiang Yan Xu]

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




src/slave/containerizer/fetcher.cpp (line 150)


What does this check against?

`Path(filename).basename()` never returns an Error().


- Jiang Yan Xu


On March 20, 2016, 8:14 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 20, 2016, 8:14 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.hpp 
> bbdce88da6e41dbb88681bc9d604b00923033b3d 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 45620: Adjusted the status semantics in CNI isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45620]

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

- Mesos ReviewBot


On April 2, 2016, 6:44 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45620/
> ---
> 
> (Updated April 2, 2016, 6:44 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Qian Zhang.
> 
> 
> Bugs: MESOS-4764
> https://issues.apache.org/jira/browse/MESOS-4764
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch also does a few minor cleanups.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> b1173d5adee6b4eb45dbed5e693f40ffb5275210 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> af5f49007e0afe57006c9db940611e56eb75a621 
> 
> Diff: https://reviews.apache.org/r/45620/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 45707: Minor cleanups to command executor.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On April 5, 2016, 5:32 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45707/
> ---
> 
> (Updated April 5, 2016, 5:32 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Trivial cleanups: 
> - Not capture boolean arguments by reference.
> - Added newline after a statement.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 76773914adb021dda2324eb566db0c697a353526 
> 
> Diff: https://reviews.apache.org/r/45707/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Klaus Ma]

> On April 4, 2016, 9:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 350-364
> > 
> >
> > We should be able to just use the ones in `src/tests/mesos.hpp`.

I'd like to de-couple example from Mesos internal code, e.g. `tests/mesos.hpp`. 
Ideally, the user can build example with installed Mesos binaries & headers.


- Klaus


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


On March 22, 2016, 11:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 45726: Fixed a bug in the `flags::parse` function and added a test case.

[Michael Park]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp 
6313b6b1f535822e1dee39f5780895a3691ff78c 
  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
e87bee24b8c57d15f3903ecd8d55e0e51e00f0ff 

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


Testing
---

Added a new test case that fails before the changes to `parse.hpp`.


Thanks,

Michael Park

Re: Review Request 45717: Added checks to verify that `rootDir` and `pluginDir` `isSome.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 325)


Do we need to check `pluginDir` here? I think in `recover()`, `pluginDir` 
will not be used, it should only be used in `attach` and `detach`. And in 
another hand, `rootDir` and `pluginDir` are always both set or both not set, it 
is not possible that we have one set but another not set, so I think checking 
one (`rootDir`) should be enough.


- Qian Zhang


On April 5, 2016, 7:55 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45717/
> ---
> 
> (Updated April 5, 2016, 7:55 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5113
> https://issues.apache.org/jira/browse/MESOS-5113
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `rootDir` and `pluginDir` can be `None` in case the operator does not
> specify the network cni flags. In this case invoking `recover` on the
> isolator without checking the `rootDir` and `pluginDir` for `isSome`
> will cause the Agent to crash.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> af5f49007e0afe57006c9db940611e56eb75a621 
> 
> Diff: https://reviews.apache.org/r/45717/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Bought a slave and master and verified that the slave does not crash.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 45217: Implemented docker volume driver isolator interface.

[Guangya Liu]

> On 四月 4, 2016, 11:01 p.m., Gilbert Song wrote:
> > src/Makefile.am, line 860
> > 
> >
> > Could you move it below runtime.cpp?

There are some files such as spec.proto, dvd_client.cpp, dvd_client.hpp, 
dvd.cpp, dvd.hpp etc, I want to put them into ta single folder to make it 
clear, comments?


- Guangya


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


On 四月 2, 2016, 5:45 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45217/
> ---
> 
> (Updated 四月 2, 2016, 5:45 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented docker volume driver isolator interface.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt ff225c0d51a32b03a1b5f2ba31718ec2305c7ced 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45217/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Klaus Ma]

> On April 4, 2016, 9:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 308
> > 
> >
> > `static const Resources TASK_RESOURCES;`

If we mark it `const`, we have to move the following code into constructor's 
init, is that OK?

```
Resources::parse(
"cpus:" + stringify(CPUS_PER_TASK) +
";mem:" + stringify(MEM_PER_TASK)).get();
```


- Klaus


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


On March 22, 2016, 11:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 45731: Introduced '--networks' flag to mesos-execute.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Introduced '--networks' flag to mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 

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


Testing (updated)
---

Ran the following command:
  `sudo src/mesos-execute --master=192.168.122.171:5050 --name=test 
--docker_image=library/busybox --command="sleep 30" --shell=true 
--networks=net1,net2`

The created container successfully joined the CNI networks `net1` and `net2`.


Thanks,

Qian Zhang

Re: Review Request 45726: Fixed a bug in the `flags::parse` function and added a test case.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 5, 2016, 2:57 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45726/
> ---
> 
> (Updated April 5, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5114
> https://issues.apache.org/jira/browse/MESOS-5114
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp 
> 6313b6b1f535822e1dee39f5780895a3691ff78c 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> e87bee24b8c57d15f3903ecd8d55e0e51e00f0ff 
> 
> Diff: https://reviews.apache.org/r/45726/diff/
> 
> 
> Testing
> ---
> 
> Added a new test case that fails before the changes to `parse.hpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45726: Fixed a bug in the `flags::parse` function and added a test case.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45726]

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

- Mesos ReviewBot


On April 5, 2016, 2:57 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45726/
> ---
> 
> (Updated April 5, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5114
> https://issues.apache.org/jira/browse/MESOS-5114
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp 
> 6313b6b1f535822e1dee39f5780895a3691ff78c 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> e87bee24b8c57d15f3903ecd8d55e0e51e00f0ff 
> 
> Diff: https://reviews.apache.org/r/45726/diff/
> 
> 
> Testing
> ---
> 
> Added a new test case that fails before the changes to `parse.hpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 44823: Remove `SlaveState` in `DockerContainerizer` during recover.

[haosdent huang]

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

(Updated April 4, 2016, 4:48 p.m.)


Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Till Toenshoff, 
and Timothy Chen.


Changes
---

Address @gyliu's comments.


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


Repository: mesos


Description
---

Remove `SlaveState` in `DockerContainerizer` during recover.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 89d450e10a84f24ddd46d517e2b4b46ab02c4fda 
  src/slave/containerizer/docker.cpp 9314d1f9e0b6077fe7c48b860783ab21acc48be6 
  src/tests/containerizer/docker_containerizer_tests.cpp 
b9c26b92c44e8ca718a5eb333855199bdf2a8e81 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44823: Remove `SlaveState` in `DockerContainerizer` during recover.

[haosdent huang]

> On April 3, 2016, 2:22 p.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, lines 825-826
> > 
> >
> > I think that here should be `||` but not `&&`
> 
> haosdent huang wrote:
> Actually these code existed before. If change to `||`, those containers 
> could be found in `existingContainers` and `executorInfo.has_container()` is 
> false would not be recovered.
> 
> Guangya Liu wrote:
> I think that when recover, we should fileter out the continers whose 
> executor does not include container info or the does not recognized by the 
> `existingContainers`?

If we filter out all executorInfo that doesn't contain `container()`, we would 
miss this case: the containerId exists in existingContainers and its 
`executorInfo` doesn't have container().


- haosdent


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


On April 4, 2016, 4:48 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44823/
> ---
> 
> (Updated April 4, 2016, 4:48 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Till 
> Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-5085
> https://issues.apache.org/jira/browse/MESOS-5085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove `SlaveState` in `DockerContainerizer` during recover.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 89d450e10a84f24ddd46d517e2b4b46ab02c4fda 
>   src/slave/containerizer/docker.cpp 9314d1f9e0b6077fe7c48b860783ab21acc48be6 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> b9c26b92c44e8ca718a5eb333855199bdf2a8e81 
> 
> Diff: https://reviews.apache.org/r/44823/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[haosdent huang]

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

(Updated April 4, 2016, 5:17 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Alex Clemmer, Ian Downes, 
Jie Yu, and Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Enable cgroups unified isolator in isolation.


Diffs (updated)
-

  src/CMakeLists.txt ff225c0d51a32b03a1b5f2ba31718ec2305c7ced 
  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45362: Add `cgroups::memory::pressure::Counter::snapshot` method.

[haosdent huang]

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

(Updated April 4, 2016, 5:21 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Add `cgroups::memory::pressure::Counter::snapshot` method.


Diffs (updated)
-

  src/linux/cgroups.hpp 53397fc1da11a71259df1c38526a613676a60301 
  src/linux/cgroups.cpp b7420c682970c4838e84973198ac4fe7af5f68f9 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44947: Add tests for XFS project quota utilities.

[James Peach]

> On April 1, 2016, 7:27 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/xfs_quota_tests.cpp, lines 292-294
> > 
> >
> > Instead of doing cleanups here, we can register the projectIds with a 
> > member variable:
> > 
> > ```
> > vector projectIds;
> > ```
> > 
> > This way in the `tearDown()` method we can go through all the them and 
> > call `clearProjectQuota()` on each.

I don't think we need this anymore since we are constructing a fresh filesystem 
each time.


> On April 1, 2016, 7:27 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/xfs_quota_tests.cpp, lines 273-280
> > 
> >
> > So for this we can also create
> > 
> > ```
> > directory
> >  |
> >  |- link
> > ```
> > 
> > Then:
> > ```
> > getProjectId(link);
> > setProjectId(directory);
> > getProjectId(link);
> > clearProjectId(directory);
> > getProjectId(link);
> > ```
> > 
> > Even if the reason for `EXPECT_ERROR(getProjectId(path));` to pass is 
> > due to FTS_PHYSICAL (so we didn't attempt to set it on the link instead of 
> > we cannot set it on the link), that's the situation we are ever going to 
> > run into in real scnearios anyways so it's OK.

These tests were restructured due to feedback in other reviews.


> On April 1, 2016, 7:27 a.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/xfs_quota_tests.cpp, line 215
> > 
> >
> > This test can be extended to include files:
> > 
> > ```
> > directory
> >   |
> >   |- file
> > ```
> > 
> > You can call `set|clearProjectId()` on the directory and verify 
> > `getProjectId()` on the file inside (and the directory).

These tests were restructured due to feedback in other reviews.


> On April 1, 2016, 7:27 a.m., Jiang Yan Xu wrote:
> > src/tests/cluster.cpp, lines 434-436
> > 
> >
> > This can be tackled in another review because a) the tests here don't 
> > use cluster.cpp and b) I see other issuse here as well (see below).
> 
> James Peach wrote:
> The isolator tests use ``cluster.cpp``. I hit this when I had a bug in 
> those tests that passed invalid flags arguments to the Slave. Let me know 
> whether you want me to move or just drop this diff. However, it is a fairly 
> annoying problem to debug.

Moved this change to a separate review.


- James


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


On April 1, 2016, midnight, James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44947/
> ---
> 
> (Updated April 1, 2016, midnight)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add tests for XFS project quota utilities.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
>   src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
>   src/tests/environment.cpp 90dbe9488bda6af26143934e196aab0d69dccec3 
> 
> Diff: https://reviews.apache.org/r/44947/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. These tests.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45572: Add labels to ExecutorInfo and deprecate source.

[haosdent huang]

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




CHANGELOG (line 49)


Usually we don't need change file. It would be changed by release manager.


- haosdent huang


On April 1, 2016, 1:42 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45572/
> ---
> 
> (Updated April 1, 2016, 1:42 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5029
> https://issues.apache.org/jira/browse/MESOS-5029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add labels to ExecutorInfo and deprecate source.
> 
> 
> Diffs
> -
> 
>   CHANGELOG b90078d41357c29c9102df00a735bde460e797bb 
>   include/mesos/mesos.proto e1fc02e05df531e29601c6764a5a48ba2b18569f 
>   include/mesos/v1/mesos.proto 35789e051608ea7f1be3ba5b63eaa1fc4e501c84 
>   src/tests/oversubscription_tests.cpp 
> ba036810758d99a6fb0034c5e2bc7829e2343a44 
> 
> Diff: https://reviews.apache.org/r/45572/diff/
> 
> 
> Testing
> ---
> 
> Added a test in oversubciption_tests to make sure executor labels are visible 
> to ResourceEstimator and QoSController.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[James Peach]

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

(Updated April 4, 2016, 5:26 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add autoconf tests for XFS project quotas.


Diffs (updated)
-

  configure.ac c693b825294f82ace8a14563cf2229820e159e3c 

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


Testing
---

Make check. Manual verification.


Thanks,

James Peach

Re: Review Request 44947: Add tests for XFS project quota utilities.

[James Peach]

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

(Updated April 4, 2016, 5:27 p.m.)


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


Changes
---

Rebased and addressed review comments.


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


Repository: mesos


Description
---

Add tests for XFS project quota utilities.


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
  src/tests/environment.cpp 90dbe9488bda6af26143934e196aab0d69dccec3 

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


Testing
---

Make check. Manual testing. These tests.


Thanks,

James Peach

Review Request 45690: Ensured the bind mount root is a shared mount in its own peer group.

[Jie Yu]

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

Review request for mesos, Ian Downes and Cong Wang.


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


Repository: mesos


Description
---

This is for the port mapping isolator.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
323c84a3d960a196d8ba87f753814e9d43a07957 

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


Testing
---

sudo make check on Fedora 23


Thanks,

Jie Yu

Review Request 45689: Make Cluster::Slave more tolerant of start failures.

[James Peach]

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

Review request for mesos, Joris Van Remoortere, Joseph Wu, and Jiang Yan Xu.


Repository: mesos


Description
---

If Cluster::Slave::start() fails, make sure we don't crash in the
destructor.


Diffs
-

  src/tests/cluster.cpp 14d0d34fcb4c408ad996672394c39c84fd2be918 

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


Testing
---

Make check.


Thanks,

James Peach

Re: Review Request 45520: Used realpath for the bind mount root in port mapping isolator.

[Cong Wang]

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


Ship it!




You don't have to pass the real path to all the places, other places than the 
mount table checking should be fine. But I am totally fine with this patch too.

Please consider to backport this to other releases.

- Cong Wang


On April 4, 2016, 5:29 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45520/
> ---
> 
> (Updated April 4, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Cong Wang.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used realpath for the bind mount root in port mapping isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0fe2f486eb733acf738c1c61fc44f820d7401afc 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e062daa9fcfc776144b48325daa1f1284c5e59a4 
> 
> Diff: https://reviews.apache.org/r/45520/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on Fedora23
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44822: Remove `SlaveState` in `MesosContainerizer` during recover.

[haosdent huang]

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

(Updated April 4, 2016, 4:47 p.m.)


Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Till Toenshoff, 
and Timothy Chen.


Changes
---

Address @gyliu's comments.


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


Repository: mesos


Description
---

Remove `SlaveState` in `MesosContainerizer` during recover.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
a6e3a8b43fb7591ff6f23b019a062696c8bfb719 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
09742ff21513dc2570684d384b257868dd57a9ce 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44822: Remove `SlaveState` in `MesosContainerizer` during recover.

[haosdent huang]

> On April 3, 2016, 2:08 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 426-429
> > 
> >
> > What about using reference here?
> > 
> > const ExecutorInfo& executorInfo = recoverable.executor_info();
> > const ContainerID& containerId = recoverable.container_id();
> > const FrameworkID& frameworkId = executorInfo.framework_id();
> > const ExecutorID& executorId = executorInfo.executor_id();
> 
> haosdent huang wrote:
> I remember mesos style guide didn't alllow const temporary refrence. 
> https://issues.apache.org/jira/browse/MESOS-2629
> 
> Guangya Liu wrote:
> But I saw that `We allow capturing non-temporaries by *constant 
> reference* when the intent is to **alias**.`, here most of the variables are 
> some **alias** I think. There are indeed some places using such style, such 
> as 
> https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/docker/runtime.cpp#L80

I think it may breake this rule
```
// 4: Don't use (since the T that got constructed is a temporary!).
const T& t = T("Hello").member();
```
in style guide. But I would update this because it really din't bring any hurts 
here if use `const &`.


- haosdent


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


On April 4, 2016, 4:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44822/
> ---
> 
> (Updated April 4, 2016, 4:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Till 
> Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-5085
> https://issues.apache.org/jira/browse/MESOS-5085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove `SlaveState` in `MesosContainerizer` during recover.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> a6e3a8b43fb7591ff6f23b019a062696c8bfb719 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5dd22380066aa85de04d485052084e2629681c0 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 09742ff21513dc2570684d384b257868dd57a9ce 
> 
> Diff: https://reviews.apache.org/r/44822/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 44543: Removed unnecessary MasterContender and MasterDetector definitions.

[Anurag Singh]

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

(Updated April 4, 2016, 5:07 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

MasterContender and MasterDetector are now defined in
include/mesos/master/contender.hpp and detector.hpp.


Diffs (updated)
-

  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44670: Added master_detector and master_contender flags.

[Anurag Singh]

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

(Updated April 4, 2016, 5:07 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

The master_detector and master_contender flags allow modules to be
used for specifying the MasterContender and MasterDetector
implementations to use.


Diffs (updated)
-

  src/master/flags.hpp f8d2cc4c6c8dab00e34ca737dbcb5b9ca3870d6d 
  src/master/flags.cpp 06852c9de68cce5d40f294f6402f7677ee6183d3 
  src/master/main.cpp 9dcdbda3fb21096ba954d57fec20a394a009a223 
  src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
  src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 
  src/slave/main.cpp f5f21422c7cb94506b78cebf7f7341b5a078f3b2 

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


Testing
---

In addition to all unit tests passing, we are currently using this 
functionality in our environment with a custom consensus stack. In our world, 
we have a C++ plugin that calls out to an HTTP REST service (implemented in 
Java/Scala, not that it matters).


Thanks,

Anurag Singh

Re: Review Request 44544: Moved contender and detector definitions into separate directories.

[Anurag Singh]

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

(Updated April 4, 2016, 5:08 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

Moved contender and detector definitions into separate directories.


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

[Anurag Singh]

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

(Updated April 4, 2016, 5:07 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 45083: Added a any mechanism for futures.

[haosdent huang]

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

(Updated April 4, 2016, 5:12 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Added a any mechanism for futures.


Diffs (updated)
-

  3rdparty/libprocess/include/process/collect.hpp 
5a92b72eb7668494dc832ec446a41b3d673a20cc 
  3rdparty/libprocess/src/tests/collect_tests.cpp 
155e0bb75cf723a0a6c29020f9f767e3ba3d7401 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45084: Add `Subsystem` abstraction for cgroups.

[haosdent huang]

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

(Updated April 4, 2016, 5:12 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Add `Subsystem` abstraction for cgroups.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45350: Add `--cgroups_subsystems` in agent flags.

[haosdent huang]

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

(Updated April 4, 2016, 5:14 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, Kevin 
Klues, and Neil Conway.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Add `--cgroups_subsystems` in agent flags.


Diffs (updated)
-

  docs/configuration.md da42eaf7069a016fa7eaf929fc285e1fa1f144e9 
  src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
  src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45085: Add cgroups unified isolator.

[haosdent huang]

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

(Updated April 4, 2016, 5:15 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Add cgroups unified isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45363: Add `MemorySubsystem` for cgroups unified isolator.

[haosdent huang]

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

(Updated April 4, 2016, 5:21 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Add `MemorySubsystem` for cgroups unified isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45364: Migrate test cases for `memory` subsystem to cgroups unified isolator.

[haosdent huang]

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

(Updated April 4, 2016, 5:21 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Migrate test cases for `memory` subsystem to cgroups unified isolator.


Diffs (updated)
-

  src/tests/containerizer/cgroups_isolator_tests.cpp 
c4e467c8227f9e4129b05d173812592f39a04e06 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

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

(Updated April 4, 2016, 5:28 p.m.)


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


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/containerizer.cpp 
a5dd22380066aa85de04d485052084e2629681c0 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
  src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

(Updated April 4, 2016, 5:27 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/tests/environment.cpp 90dbe9488bda6af26143934e196aab0d69dccec3 

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


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44950: Add XFS disk isolator documentation.

[James Peach]

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

(Updated April 4, 2016, 5:27 p.m.)


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


Changes
---

Rebased.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Add XFS disk isolator documentation.


Diffs (updated)
-

  docs/configuration.md da42eaf7069a016fa7eaf929fc285e1fa1f144e9 
  docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 

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


Testing
---

Make check. Source inspection.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated April 4, 2016, 5:28 p.m.)


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


Changes
---

Rebased and addressed some review comments.


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


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

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


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 45689: Make Cluster::Slave more tolerant of start failures.

[haosdent huang]

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




src/tests/cluster.cpp (line 437)


how about

```
if (!containerizer) {
  return;
}
```


- haosdent huang


On April 4, 2016, 5:31 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45689/
> ---
> 
> (Updated April 4, 2016, 5:31 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If Cluster::Slave::start() fails, make sure we don't crash in the
> destructor.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 14d0d34fcb4c408ad996672394c39c84fd2be918 
> 
> Diff: https://reviews.apache.org/r/45689/diff/
> 
> 
> Testing
> ---
> 
> Make check.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45689: Make Cluster::Slave more tolerant of start failures.

[James Peach]

> On April 4, 2016, 5:35 p.m., haosdent huang wrote:
> > src/tests/cluster.cpp, line 437
> > 
> >
> > how about
> > 
> > ```
> > if (!containerizer) {
> >   return;
> > }
> > ```
> 
> Jiang Yan Xu wrote:
> +1 this is better.

You can't early return because you have to call ``terminate()``.


- James


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


On April 4, 2016, 5:31 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45689/
> ---
> 
> (Updated April 4, 2016, 5:31 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If Cluster::Slave::start() fails, make sure we don't crash in the
> destructor.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 14d0d34fcb4c408ad996672394c39c84fd2be918 
> 
> Diff: https://reviews.apache.org/r/45689/diff/
> 
> 
> Testing
> ---
> 
> Make check.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45689: Make Cluster::Slave more tolerant of start failures.

[Jiang Yan Xu]

> On April 4, 2016, 10:35 a.m., haosdent huang wrote:
> > src/tests/cluster.cpp, line 437
> > 
> >
> > how about
> > 
> > ```
> > if (!containerizer) {
> >   return;
> > }
> > ```
> 
> Jiang Yan Xu wrote:
> +1 this is better.
> 
> James Peach wrote:
> You can't early return because you have to call ``terminate()``.

Alright, overlooked it :) However it looks like the lines above this suffer 
from the same problem:

```
if (!cleanUpContainersInDestructor) {
  return;
}
```

In generl I think this is OK:


How about 

```
if (!containerizer || !cleanUpContainersInDestructor) {
  terminate();
  return;
}

...
```


- Jiang Yan


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


On April 4, 2016, 10:31 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45689/
> ---
> 
> (Updated April 4, 2016, 10:31 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If Cluster::Slave::start() fails, make sure we don't crash in the
> destructor.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 14d0d34fcb4c408ad996672394c39c84fd2be918 
> 
> Diff: https://reviews.apache.org/r/45689/diff/
> 
> 
> Testing
> ---
> 
> Make check.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45033: Add a commit hook for checking non-printable characters (MESOS-4033).

[Yong Tang]

> On April 4, 2016, 8:36 a.m., Benjamin Bannier wrote:
> > support/mesos-style.py, line 120
> > 
> >
> > It would be great if we could `+ 1` the `chars`  here like we already 
> > do for the `line_number` in order to keep this all in one place. We could 
> > also use this opportunity to go for less incidental list formatting, e.g.,
> > 
> > chars=', '.join([str(char + 1) for char in char_errors])  # gives 
> > e.g., "Line: 10, Chars: 1, 2, 3: 
> 
> Yong Tang wrote:
> Thanks Benjamin. In Ln 112 the "+ 1" of the Chars has already been 
> processed ("offset + 1").
> 
> Benjamin Bannier wrote:
> Exactly! My point was that I wasn't sure that doing these operations on 
> `line_numer` and `char_errors` in different operations helped clarity.
> 
> Yong Tang wrote:
> Oh I see. Let me update the review request.

Thanks for the review Benjamin. The review request has been updated. Let me 
know if there are any other issues.


- Yong


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


On April 4, 2016, 3:21 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 4, 2016, 3:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an addition check in mesos-style.pl to check
> for non-printable characters. It scans .cpp, .hpp, .cc, .h
> files and reports an error if non-printable characters exist.
> 
> As part of this patch, two non-printable characters have been identified
> in versioning.md (one in Line 85 and another in Line 96) and are corrected
> accordingly.
> 
> Note: Scanning .md files is skipped based on feedback from reviews.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45534: Added per-role and quota share metrics to the DRFSorter.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45529, 45533, 45534]

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

- Mesos ReviewBot


On April 4, 2016, 12:32 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45534/
> ---
> 
> (Updated April 4, 2016, 12:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4760
> https://issues.apache.org/jira/browse/MESOS-4760
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added per-role and quota share metrics to the DRFSorter.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 76be32842e8b437fda40c8565a34ec4f8d8dfbcc 
>   src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/master/allocator/mesos/hierarchical.hpp 
> e979fdf60da1409d1c2d08f0e9f03cef067506dd 
>   src/master/allocator/sorter/drf/metrics.hpp PRE-CREATION 
>   src/master/allocator/sorter/drf/metrics.cpp PRE-CREATION 
>   src/master/allocator/sorter/drf/sorter.hpp 
> f316bb5b8bfe93311ecac57198392e104b234b04 
>   src/master/allocator/sorter/drf/sorter.cpp 
> c14f9a45b9e2ea8d79bd8d2f092d313afa4cbac3 
>   src/master/allocator/sorter/sorter.hpp 
> e2338d5297e11a1ca4f6e5d72a4526aa4579610c 
>   src/tests/hierarchical_allocator_tests.cpp 
> 8f78a204d296f94f515f21511710a35c33e27255 
> 
> Diff: https://reviews.apache.org/r/45534/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang trunk, not optimized)
> 
> I did also benchmark the slowdown of the allocator due to this addition with 
> the benchmark allocator https://reviews.apache.org/r/44853. There I saw that 
> for an unoptimized build this patch adds up to 70 ms to the time needed to 
> query the metrics endpoint (this was for the case of 5000 slaves and 1000 
> frameworks), though one could expected that an optimized build might perform 
> better. The numbers I got where
> 
> `#`slaves | #frameworks | old time [us] | new time [us] | slowdown
> |-|---|---|-
>1000 |   1 | 38980 | 23847 |0.6
>1000 |  50 | 27834 | 42091 |1.5
>1000 | 100 | 40060 | 47571 |1.2
>1000 | 200 | 63132 | 75806 |1.2
>1000 | 500 |145170 |171929 |1.2
>1000 |1000 |427721 |473822 |1.1
>5000 |   1 | 23249 | 21426 |0.9
>5000 |  50 | 41032 | 36318 |0.9
>5000 | 100 | 43636 | 45210 |1.0
>5000 | 200 | 60204 | 65570 |1.1
>5000 | 500 |121509 |196894 |1.6
>5000 |1000 |449476 |496641 |1.1
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44670: Added master_detector and master_contender flags.

[Anurag Singh]

> On March 31, 2016, 5:26 p.m., Kapil Arya wrote:
> > src/master/main.cpp, lines 247-248
> > 
> >
> > These two should fit on one line :-).

putting them on the same line is causing the line lenght to increase to 89.


- Anurag


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


On March 23, 2016, 11:06 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44670/
> ---
> 
> (Updated March 23, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master_detector and master_contender flags allow modules to be
> used for specifying the MasterContender and MasterDetector
> implementations to use.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp f8d2cc4c6c8dab00e34ca737dbcb5b9ca3870d6d 
>   src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 
>   src/master/main.cpp 61210d9f275d4073967c3468179307cf09e88551 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/main.cpp 33a1af84aeb079224b15e92caf97bcf081ea4646 
> 
> Diff: https://reviews.apache.org/r/44670/diff/
> 
> 
> Testing
> ---
> 
> In addition to all unit tests passing, we are currently using this 
> functionality in our environment with a custom consensus stack. In our world, 
> we have a C++ plugin that calls out to an HTTP REST service (implemented in 
> Java/Scala, not that it matters).
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44571: Added timeout for destroying Docker containers.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44571]

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

- Mesos ReviewBot


On April 4, 2016, 2:05 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44571/
> ---
> 
> (Updated April 4, 2016, 2:05 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4673
> https://issues.apache.org/jira/browse/MESOS-4673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Commands issued to the Docker daemon can hang, causing problems within Mesos.
> For example a hanging 'docker stop' can result in an unresponsive executor,
> causing the Mesos agent to issue an to run a 'docker stop' itself which might
> result in an unresponsive agent (see MESOS-4673).
> Adding a timeout can be used as a workaround.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 89d450e10a84f24ddd46d517e2b4b46ab02c4fda 
>   src/slave/containerizer/docker.cpp 9314d1f9e0b6077fe7c48b860783ab21acc48be6 
> 
> Diff: https://reviews.apache.org/r/44571/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh (to test if existing tests break due to the changed 
> behavior)
> 
> Because docker must hang for both the Mesos agent as well as the 
> `mesos-docker-executor`, it can't currently be tested as part of the Mesos 
> integration tests. Here's how to test that the timeout works:
> Run with Fedora 23 (Kernel 4.2.3, Docker 1.9.1)
> # Start a master
> ./bin/mesos-master.sh --work_dir=/tmp/mesos &
> 
> # Start an agent
> sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --containerizers=docker &
> 
> # Run a task using the docker containerizer
> ./src/mesos-execute --containerizer=docker --docker_image=alpine 
> --master=127.0.0.1:5050 --name="sleep" --command="sleep 1000" &
> # Note the pid of `mesos-execute` as well as the pid of the sleep task run by 
> docker (eg 3323 and 3474)
> 
> # Have mesos run `docker inspect` to gather the pid of the docker task
> curl -X GET localhost:5051/monitor/statistics
> 
> # Now overload docker by trying to run a lot of tasks in parallel
> for i in `seq 1 100`; do sudo docker run --rm alpine sleep 60 & done
> 
> # Wait until the first of these docker tasks finish, `sudo docker ps` should 
> be unresponsible now
> # Kill the `mesos-execute` task (eg 3323)
> kill 3323
> 
> # Watch the logs of the Mesos agent. At some point it will send a SIGKILL to 
> the docker task (eg 3474)
> # Make sure that the docker task is indeed termintad (using `ps fax` or the 
> like)
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44546: Moved functions in promises to a common header file.

[Anurag Singh]

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

(Updated April 4, 2016, 5:08 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

Moved functions in promises to a common header file.


Diffs (updated)
-

  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/detectors/zookeeper.cpp 9274435802d6292b183be48f42b43999476e016e 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44289: Added support for contender and detector modules.

[Anurag Singh]

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

(Updated April 4, 2016, 5:08 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

Added support for contender and detector modules.


Diffs (updated)
-

  include/mesos/module/contender.hpp PRE-CREATION 
  include/mesos/module/detector.hpp PRE-CREATION 
  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/examples/test_master_contender_module.cpp PRE-CREATION 
  src/examples/test_master_detector_module.cpp PRE-CREATION 
  src/local/local.cpp 06f6118a5038149f87e970b5cdf7987a270d3fae 
  src/master/contenders/zookeeper.cpp 846f516edcc63affd05158e9e5235e6d97f67f15 
  src/master/detectors/zookeeper.cpp PRE-CREATION 
  src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
  src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
  src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44669: Implemented create methods in MasterContender and MasterDetector.

[Anurag Singh]

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

(Updated April 4, 2016, 5:08 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


Summary (updated)
-

Implemented create methods in MasterContender and MasterDetector.


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


Repository: mesos


Description (updated)
---

The create method can be used to create a MasterContender/Detector from a
module (specified using the --modules flag on the command line). If the
contender/detector module names have not been specified, the method will
fall back to its prior behavior.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 44545: Separated standalone and zookeeper classes.

[Anurag Singh]

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

(Updated April 4, 2016, 5:08 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
---

Instead of keeping standalone and zookeper contender/detector class
definitions and implementations in the same file, separated them. Also
made the necessary changes in users of class headers to point to the
new locations.


Diffs (updated)
-

  src/Makefile.am f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/local/local.cpp 06f6118a5038149f87e970b5cdf7987a270d3fae 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 846f516edcc63affd05158e9e5235e6d97f67f15 
  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/contenders/standalone.cpp PRE-CREATION 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 
  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/main.cpp 9dcdbda3fb21096ba954d57fec20a394a009a223 
  src/master/master.hpp 1751ee1fc4d3e15df018be8bfd4f8ef2a0e32abd 
  src/slave/main.cpp f5f21422c7cb94506b78cebf7f7341b5a078f3b2 
  src/tests/authentication_tests.cpp 8143cd7a22bbdbcd0fc613cb44eae8b55fd458e7 
  src/tests/cluster.cpp 14d0d34fcb4c408ad996672394c39c84fd2be918 
  src/tests/containerizer/external_containerizer_test.cpp 
5e2116355418f5a0716cfd1573bab48ba75df596 
  src/tests/containerizer/isolator_tests.cpp 
7e16846e97e20a7ef058dd8ae4664fa1c554283d 
  src/tests/fault_tolerance_tests.cpp 67238876952bb9a694cb04722113417a4b4eb58d 
  src/tests/gc_tests.cpp 89a7f735b27b7a836b4ce3af10f26a6d63887279 
  src/tests/master_allocator_tests.cpp 9d74b9ed4b6a4c708f3a044a03ce631899acf4aa 
  src/tests/master_authorization_tests.cpp 
8b9b8991fbb8c5a5beb69416a9c4a4ef3525942d 
  src/tests/master_contender_detector_tests.cpp 
616a2c7316adc116071feaad95e3901ff66213ad 
  src/tests/master_slave_reconciliation_tests.cpp 
988f1d46580ab5a707fe801824e24f94d4f50da7 
  src/tests/master_tests.cpp bb599c952128e80e71beb2686e8665e5554856bc 
  src/tests/mesos.hpp 98f97101d89cb91ca5e773e7a91f04ee1b8a0d88 
  src/tests/oversubscription_tests.cpp ba036810758d99a6fb0034c5e2bc7829e2343a44 
  src/tests/partition_tests.cpp 349adbf67686e6044a2e6a4b673043ad74fce44e 
  src/tests/persistent_volume_tests.cpp 
7cb153cc6c0a8ca25cd31096686c63e829f746cf 
  src/tests/reconciliation_tests.cpp 5f541f5fe004ede943a1b022daab92f01d1f4853 
  src/tests/reservation_tests.cpp a9261bdf48c0af933e7fc303b7af356a60b49506 
  src/tests/scheduler_event_call_tests.cpp 
00e99777ba0294c9c12ac86594987afbd9388b51 
  src/tests/scheduler_http_api_tests.cpp 
b65790a9aad0ca68c6a93dd1d872442b906598fd 
  src/tests/scheduler_tests.cpp 0c66f65825a1fff01774bf0d94b08b9c22b1fbda 
  src/tests/slave_recovery_tests.cpp 0d59a06f8e32f3d88f6c3a222bc6756a889a142e 
  src/tests/slave_tests.cpp 57fc50360eae85819ae6ce714b0c3c4c1867b2b8 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 45350: Add `--cgroups_subsystems` in agent flags.

[Neil Conway]

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




src/slave/flags.cpp (line 335)


Remove the whitespace before "\n".


- Neil Conway


On April 4, 2016, 5:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45350/
> ---
> 
> (Updated April 4, 2016, 5:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, 
> Kevin Klues, and Neil Conway.
> 
> 
> Bugs: MESOS-5040
> https://issues.apache.org/jira/browse/MESOS-5040
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `--cgroups_subsystems` in agent flags.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md da42eaf7069a016fa7eaf929fc285e1fa1f144e9 
>   src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
>   src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 
> 
> Diff: https://reviews.apache.org/r/45350/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45353: Migrate test cases for `cpuacct` subsystem to cgroups unified isolator.

[haosdent huang]

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

(Updated April 4, 2016, 5:19 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Migrate test cases for `cpuacct` subsystem to cgroups unified isolator.


Diffs (updated)
-

  src/tests/containerizer/cgroups_isolator_tests.cpp 
c4e467c8227f9e4129b05d173812592f39a04e06 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45354: Migrate `UserCgroupIsolatorTest` to cgroups unified isolator.

[haosdent huang]

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

(Updated April 4, 2016, 5:19 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Migrate `UserCgroupIsolatorTest` to cgroups unified isolator.


Diffs (updated)
-

  src/tests/containerizer/cgroups_isolator_tests.cpp 
c4e467c8227f9e4129b05d173812592f39a04e06 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45520: Used realpath for the bind mount root in port mapping isolator.

[Jie Yu]

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

(Updated April 4, 2016, 5:29 p.m.)


Review request for mesos, Ian Downes and Cong Wang.


Changes
---

Split the patch per review comments.


Summary (updated)
-

Used realpath for the bind mount root in port mapping isolator.


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


Repository: mesos


Description (updated)
---

Used realpath for the bind mount root in port mapping isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
0fe2f486eb733acf738c1c61fc44f820d7401afc 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
323c84a3d960a196d8ba87f753814e9d43a07957 
  src/tests/containerizer/port_mapping_tests.cpp 
e062daa9fcfc776144b48325daa1f1284c5e59a4 

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


Testing
---

sudo make check on Fedora23


Thanks,

Jie Yu

Re: Review Request 45689: Make Cluster::Slave more tolerant of start failures.

[Jiang Yan Xu]

> On April 4, 2016, 10:35 a.m., haosdent huang wrote:
> > src/tests/cluster.cpp, line 437
> > 
> >
> > how about
> > 
> > ```
> > if (!containerizer) {
> >   return;
> > }
> > ```

+1 this is better.


- Jiang Yan


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


On April 4, 2016, 10:31 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45689/
> ---
> 
> (Updated April 4, 2016, 10:31 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If Cluster::Slave::start() fails, make sure we don't crash in the
> destructor.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 14d0d34fcb4c408ad996672394c39c84fd2be918 
> 
> Diff: https://reviews.apache.org/r/45689/diff/
> 
> 
> Testing
> ---
> 
> Make check.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45689: Make Cluster::Slave more tolerant of start failures.

[Jiang Yan Xu]

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



Not insisting on addressing it here but just would like to mention that we'd 
better not use assertions in here because we want to clean up all containers 
and not terminate early.

This can be done by addressing this 
[TODO](https://github.com/apache/mesos/blob/f921302b0a0d7c37ba1205ad12857ade614d04ff/3rdparty/libprocess/include/process/gtest.hpp#L206)
 and using it:
```
TODO(bmahler): Differentiate EXPECT and ASSERT here.
```

- Jiang Yan Xu


On April 4, 2016, 10:31 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45689/
> ---
> 
> (Updated April 4, 2016, 10:31 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If Cluster::Slave::start() fails, make sure we don't crash in the
> destructor.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 14d0d34fcb4c408ad996672394c39c84fd2be918 
> 
> Diff: https://reviews.apache.org/r/45689/diff/
> 
> 
> Testing
> ---
> 
> Make check.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45660: Changed some non-ascii characters to ascii equivalent.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45033, 45659, 45660]

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

- Mesos ReviewBot


On April 4, 2016, 3:22 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45660/
> ---
> 
> (Updated April 4, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is part of MESOS-4033: Add a commit hook for checking non-ascii 
> characters.
> 
> Note: The commit hook requires separate commits for libprocess and mesos.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 
>   src/uri/fetchers/curl.cpp cb50103775fb69e0696e0c389f2119f397a1c499 
>   src/uri/fetchers/docker.cpp fbfc96c3f86cd2d2d585516f37be12834c2bb3a4 
> 
> Diff: https://reviews.apache.org/r/45660/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44547: Added functions in promises to the future header.

[Anurag Singh]

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

(Updated April 4, 2016, 5:08 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


Summary (updated)
-

Added functions in promises to the future header.


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


Repository: mesos


Description (updated)
---

Added functions in promises to the future header.


Diffs (updated)
-

  3rdparty/libprocess/include/process/future.hpp 
ae4f2d659f1540c50dc1975f5771c6291c3b0c57 

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


Testing
---

See https://reviews.apache.org/r/44670/.


Thanks,

Anurag Singh

Re: Review Request 45351: Migrate test cases for `cpu` subsystem to cgroups unified isolator.

[haosdent huang]

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

(Updated April 4, 2016, 5:18 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Migrate test cases for `cpu` subsystem to cgroups unified isolator.


Diffs (updated)
-

  src/tests/containerizer/cgroups_isolator_tests.cpp 
c4e467c8227f9e4129b05d173812592f39a04e06 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45352: Add `CpuacctSubsystem` for cgroups unified isolator.

[haosdent huang]

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

(Updated April 4, 2016, 5:19 p.m.)


Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Add `CpuacctSubsystem` for cgroups unified isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45473: Migrate test cases for `net_cls` subsystem to cgroups unified isolator.

[haosdent huang]

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

(Updated April 4, 2016, 5:22 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, Guangya Liu, Ian 
Downes, Jie Yu, and Kevin Klues.


Changes
---

Rebase and update reviewers.


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


Repository: mesos


Description
---

Migrate test cases for `net_cls` subsystem to cgroups unified isolator.


Diffs (updated)
-

  src/tests/containerizer/cgroups_isolator_tests.cpp 
c4e467c8227f9e4129b05d173812592f39a04e06 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45033: Add a commit hook for checking non-printable characters (MESOS-4033).

[Yong Tang]

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

(Updated April 4, 2016, 3:21 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi Xiao.


Changes
---

Change the location of "+ 1" in chars for ease of readability.


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


Repository: mesos


Description
---

This patch adds an addition check in mesos-style.pl to check
for non-printable characters. It scans .cpp, .hpp, .cc, .h
files and reports an error if non-printable characters exist.

As part of this patch, two non-printable characters have been identified
in versioning.md (one in Line 85 and another in Line 96) and are corrected
accordingly.

Note: Scanning .md files is skipped based on feedback from reviews.

Note: This commit includes patches from Kevin Klues and haosdent.


Diffs (updated)
-

  docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
  support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 

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


Testing
---

Tested manually and found two non ascii characters in docs/versioning.md (fixed 
as part of this review request).


Thanks,

Yong Tang

Re: Review Request 45659: Changed some non-ascii characters in libprocess to ascii equivalent.

[Yong Tang]

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

(Updated April 4, 2016, 3:22 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi Xiao.


Changes
---

Update the review request as https://reviews.apache.org/r/45033/ has changed.


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


Repository: mesos


Description
---

This is part of MESOS-4033: Add a commit hook for checking non-ascii characters.

Note: The commit hook requires separate commits for libprocess and mesos.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/windows/format.hpp 
a1bfc808eb1b1c372acb028019901f0ab3f15238 

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


Testing
---

make check


Thanks,

Yong Tang

Re: Review Request 44674: Supported image name specified with private registry prefix.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 2, 2016, 8:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44674/
> ---
> 
> (Updated April 2, 2016, 8:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4939
> https://issues.apache.org/jira/browse/MESOS-4939
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported image name specified with private registry prefix.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 5fb6647cc02aa64fa9305b1ec6db9d6d81caf5f7 
>   src/docker/spec.cpp 07c0b87ee1a3b6981adfc96f5a7add6f625374c1 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> d3eccd9902a7896d52e6c79b13bd389bb6380b8d 
> 
> Diff: https://reviews.apache.org/r/44674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh --gtest_filter="ProvisionerDockerRegistryPullerTest"
> 
> Tested with private registry localhost:80/ubuntu
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>