Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.

[Paul Brett]

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

(Updated Aug. 21, 2015, 4:06 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Fix broken patch.


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


Repository: mesos


Description
---

Update perf tests to including testing the supported perf output formats.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 
  src/tests/containerizer/perf_tests.cpp 
6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.

[Klaus Ma]

> On 八月 21, 2015, 1:26 a.m., Klaus Ma wrote:
> > docs/docker-containerizer.md, line 21
> > 
> >
> > I'm also not a native speaker, but i'd suggest to:
> > 
> > If *iptables* enabled on slave, make sure *iptables* accept all traffic 
> > from docker bridge interface.
> > Example: 
> > ```
> > iptables -A INPUT -s 172.17.0.0/16 -i docker0 -p tcp -j ACCEPT
> > ```
> 
> haosdent huang wrote:
> Because the patch is submitted, if it is not urgent, could update next 
> time. :-)

sure, it's clear enough to end user :).


- Klaus


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


On 八月 20, 2015, 5:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37657/
> ---
> 
> (Updated 八月 20, 2015, 5:30 p.m.)
> 
> 
> Review request for mesos, Klaus Ma and Timothy Chen.
> 
> 
> Bugs: MESOS-3053
> https://issues.apache.org/jira/browse/MESOS-3053
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document iptable rule need added when use docker bridge network mode.
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 
> 
> Diff: https://reviews.apache.org/r/37657/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.

[haosdent huang]

> On Aug. 21, 2015, 1:26 a.m., Klaus Ma wrote:
> > docs/docker-containerizer.md, line 21
> > 
> >
> > I'm also not a native speaker, but i'd suggest to:
> > 
> > If *iptables* enabled on slave, make sure *iptables* accept all traffic 
> > from docker bridge interface.
> > Example: 
> > ```
> > iptables -A INPUT -s 172.17.0.0/16 -i docker0 -p tcp -j ACCEPT
> > ```

Because the patch is submitted, if it is not urgent, could update next time. :-)


- haosdent


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


On Aug. 20, 2015, 5:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37657/
> ---
> 
> (Updated Aug. 20, 2015, 5:30 p.m.)
> 
> 
> Review request for mesos, Klaus Ma and Timothy Chen.
> 
> 
> Bugs: MESOS-3053
> https://issues.apache.org/jira/browse/MESOS-3053
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document iptable rule need added when use docker bridge network mode.
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 
> 
> Diff: https://reviews.apache.org/r/37657/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 37669: Ignore overflow components in version parsing.

[haosdent huang]

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

Review request for mesos, Isabel Jimenez and Timothy Chen.


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


Repository: mesos


Description
---

Ignore overflow components in version parsing.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp 
51010845e8885ac52ce88ec0deb6b65a81122cba 
  3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp 
e8f8358f3c113b4e21e30cb5e9d6a3d229191484 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 37531: MESOS-3070

[Klaus Ma]

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



src/master/master.cpp (line 3218)


Yes; the TaskID will send back to framework by statusUpdate, so framework 
can use the UUID to kill a task which is not included in taskTag logic.


- Klaus Ma


On 八月 17, 2015, 1:28 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37531/
> ---
> 
> (Updated 八月 17, 2015, 1:28 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3070
> https://issues.apache.org/jira/browse/MESOS-3070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3070 (Master CHECK failure if a framework uses duplicated task id.)
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 
>   src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c 
> 
> Diff: https://reviews.apache.org/r/37531/diff/
> 
> 
> Testing
> ---
> 
> Draft code diff, will update UT cases later.
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37531: MESOS-3070

[Klaus Ma]

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

(Updated 八月 21, 2015, 2:10 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

MESOS-3070 (Master CHECK failure if a framework uses duplicated task id.)


Diffs (updated)
-

  include/mesos/mesos.proto 33e1b28 
  include/mesos/type_utils.hpp dafe1df 
  src/master/master.hpp 0432842 
  src/master/master.cpp 95207d2 
  src/master/validation.cpp ffb7bf0 
  src/messages/messages.proto 8977d8e 
  src/sched/sched.cpp 012af05 
  src/slave/slave.cpp 2a99abc 
  src/tests/master_tests.cpp 8a6b98b 

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


Testing
---

Draft code diff, will update UT cases later.


Thanks,

Klaus Ma

Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.

[Klaus Ma]

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



docs/docker-containerizer.md (line 21)


I'm also not a native speaker, but i'd suggest to:

If *iptables* enabled on slave, make sure *iptables* accept all traffic 
from docker bridge interface.
Example: 
```
iptables -A INPUT -s 172.17.0.0/16 -i docker0 -p tcp -j ACCEPT
```


- Klaus Ma


On 八月 20, 2015, 5:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37657/
> ---
> 
> (Updated 八月 20, 2015, 5:30 p.m.)
> 
> 
> Review request for mesos, Klaus Ma and Timothy Chen.
> 
> 
> Bugs: MESOS-3053
> https://issues.apache.org/jira/browse/MESOS-3053
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document iptable rule need added when use docker bridge network mode.
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 
> 
> Diff: https://reviews.apache.org/r/37657/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37462: Add support for version detection and parsing.

[Paul Brett]

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

(Updated Aug. 21, 2015, 12:48 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Add support for version detection and parsing.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check

Perf tests may fail on many machines because the tests are currently using the 
version of perf installed on the machine to choose decode.  The next patch will 
extend the perf tests to supply test cases for each of the supported perf 
versions.


Thanks,

Paul Brett

Re: Review Request 37462: Add support for version detection and parsing.

[Paul Brett]

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

(Updated Aug. 21, 2015, midnight)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Add support for version detection and parsing.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check

Perf tests may fail on many machines because the tests are currently using the 
version of perf installed on the machine to choose decode.  The next patch will 
extend the perf tests to supply test cases for each of the supported perf 
versions.


Thanks,

Paul Brett

Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.

[Paul Brett]

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

(Updated Aug. 20, 2015, 11:51 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Correct indentation.


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


Repository: mesos


Description
---

Update perf tests to including testing the supported perf output formats.


Diffs (updated)
-

  src/tests/containerizer/perf_tests.cpp 
6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37462: Add support for version detection and parsing.

[Paul Brett]

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

(Updated Aug. 20, 2015, 11:49 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Fix handling of empty fields.


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


Repository: mesos


Description
---

Add support for version detection and parsing.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check

Perf tests may fail on many machines because the tests are currently using the 
version of perf installed on the machine to choose decode.  The next patch will 
extend the perf tests to supply test cases for each of the supported perf 
versions.


Thanks,

Paul Brett

Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.

[Paul Brett]

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

(Updated Aug. 20, 2015, 11 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rework to use value parameterized tests.


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


Repository: mesos


Description
---

Update perf tests to including testing the supported perf output formats.


Diffs (updated)
-

  src/tests/containerizer/perf_tests.cpp 
6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37199: Added store interface and moved store implementation to LocalStore subclass.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [37196, 37197, 37198, 37199]

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

Error:
 2015-08-20 21:59:29 URL:https://reviews.apache.org/r/37199/diff/raw/ 
[8196/8196] -> "37199.patch" [1]
error: patch failed: src/slave/containerizer/provisioners/docker/store.hpp:39
error: src/slave/containerizer/provisioners/docker/store.hpp: patch does not 
apply
error: patch failed: src/slave/containerizer/provisioners/docker/store.cpp:48
error: src/slave/containerizer/provisioners/docker/store.cpp: patch does not 
apply
Failed to apply patch

- Mesos ReviewBot


On Aug. 17, 2015, 9:12 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37199/
> ---
> 
> (Updated Aug. 17, 2015, 9:12 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added store interface and moved store implementation to LocalStore subclass.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 37245: Refactor Docker Image to exclude path and manifest.

[Timothy Chen]

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


Can we squash this to ImageName commit?

- Timothy Chen


On Aug. 17, 2015, 9:13 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37245/
> ---
> 
> (Updated Aug. 17, 2015, 9:13 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-2850
> https://issues.apache.org/jira/browse/MESOS-2850
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactor Docker Image to exclude path and manifest.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37245/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 37200: Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers.

[Timothy Chen]

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



src/slave/containerizer/provisioners/docker.hpp (line 54)


I think I commented earlier, a remote registry might be part of the name 
with a ":" which specifies port.



src/slave/containerizer/provisioners/docker/backend.cpp (line 127)


You can support a std::ostream& operator<< override for the ImageName



src/slave/containerizer/provisioners/docker/store.cpp (line 130)


log the image name.



src/slave/containerizer/provisioners/docker/store.cpp (line 134)


I think we want to create a staging directory underneath a set staging 
directory, simialiar how appc does it.



src/slave/containerizer/provisioners/docker/store.cpp (line 402)


Use os::rename() instead of calling mv yourself


- Timothy Chen


On Aug. 19, 2015, 6:44 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37200/
> ---
> 
> (Updated Aug. 19, 2015, 6:44 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored DockerImage struct to store a list of layer ids instead of linked 
> list of DockerLayers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/docker.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
> 
> Diff: https://reviews.apache.org/r/37200/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 37199: Added store interface and moved store implementation to LocalStore subclass.

[Timothy Chen]

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



src/slave/containerizer/provisioners/docker/store.cpp (line 56)


"Unknown docker store: " + flags.docker_Store



src/slave/containerizer/provisioners/docker/store.cpp (line 365)


This should be "store path not found", local_store.cpp is already part of 
the log message.


- Timothy Chen


On Aug. 17, 2015, 9:12 p.m., Lily Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37199/
> ---
> 
> (Updated Aug. 17, 2015, 9:12 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-2849
> https://issues.apache.org/jira/browse/MESOS-2849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added store interface and moved store implementation to LocalStore subclass.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Re: Review Request 37532: Add QUIESCE call interface to the scheduler

[Marco Massenzio]

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



include/mesos/scheduler.hpp (line 273)


indent is off?



include/mesos/scheduler.hpp (line 424)


indent



include/mesos/scheduler/scheduler.proto (line 173)


micro-nit: the // is not aligned with lines above.



include/mesos/scheduler/scheduler.proto (lines 314 - 315)


This comments does not read well: what is the timeout? also, it would be 
good to have a bit of info about what the filters are

(eg, are they 'inclusion' or 'exclusion' filters? etc.)



src/master/master.cpp (line 2507)


IMO we should LOG(INFO) here to state this is not implemented yet or 
something.

Otherwise, say people see this and start using it, then nothing happens, 
look at the logs (which give the impression that the "Processing QUIESCE" was 
successfully called) and will report it as a bug.



src/sched/sched.cpp (line 1953)


indent off



src/tests/scheduler_tests.cpp (line 1003)


so, this is a good test, but I would really like to see one where we ask 
Master to keep quiet for a while and we don't get offers during that period of 
time, then we start getting again.

It may require some fiddling around with `Clock`s and all that jazz, but it 
would give us more confidence that this whole thing works.

Also - some tests around more complex filtering (if any? maybe this is 
there is, then it's fine).


- Marco Massenzio


On Aug. 17, 2015, 1:47 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37532/
> ---
> 
> (Updated Aug. 17, 2015, 1:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is just part of MESOS-3037, this patch only add the interface
> of QUIESCE call.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd 
>   include/mesos/scheduler/scheduler.proto 
> 89daf8a6b74057ee156b3ad691397e76fcb835b8 
>   include/mesos/v1/scheduler/scheduler.proto 
> bd5e82a614b1163b29f9b20e562208efa1ba4b55 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp c5e6c6f3304060d4c92d52851951f10bc432500e 
>   src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/37532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37462: Add support for version detection and parsing.

[Paul Brett]

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

(Updated Aug. 20, 2015, 7:17 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Improve readability for extract function.


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


Repository: mesos


Description
---

Add support for version detection and parsing.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing (updated)
---

sudo make check

Perf tests may fail on many machines because the tests are currently using the 
version of perf installed on the machine to choose decode.  The next patch will 
extend the perf tests to supply test cases for each of the supported perf 
versions.


Thanks,

Paul Brett

Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37657]

All tests passed.

- Mesos ReviewBot


On Aug. 20, 2015, 5:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37657/
> ---
> 
> (Updated Aug. 20, 2015, 5:30 p.m.)
> 
> 
> Review request for mesos, Klaus Ma and Timothy Chen.
> 
> 
> Bugs: MESOS-3053
> https://issues.apache.org/jira/browse/MESOS-3053
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document iptable rule need added when use docker bridge network mode.
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 
> 
> Diff: https://reviews.apache.org/r/37657/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Aug. 20, 2015, 5:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37657/
> ---
> 
> (Updated Aug. 20, 2015, 5:30 p.m.)
> 
> 
> Review request for mesos, Klaus Ma and Timothy Chen.
> 
> 
> Bugs: MESOS-3053
> https://issues.apache.org/jira/browse/MESOS-3053
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document iptable rule need added when use docker bridge network mode.
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 
> 
> Diff: https://reviews.apache.org/r/37657/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37460: Prevent perf test failures from killing the test harness.

[Paul Brett]

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

(Updated Aug. 20, 2015, 5:52 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Implement review comment.


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


Repository: mesos


Description
---

Prevent perf test failures from killing the test harness.


Diffs (updated)
-

  src/tests/containerizer/perf_tests.cpp 
6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37655]

All tests passed.

- Mesos ReviewBot


On Aug. 20, 2015, 5:03 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37655/
> ---
> 
> (Updated Aug. 20, 2015, 5:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3299
> https://issues.apache.org/jira/browse/MESOS-3299
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of using doubles, seconds and nanoseconds can be represented like
> `struct timespec`, with one field for seconds and one for nanoseconds.
> 
> This will be important if frameworks need to compare times to make decisions 
> (such as for maintenance primitives).
> 
> Note about the naming:
> 
> * Time will conflict with the Time class.
> * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict 
> with Duration.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
> 
> Diff: https://reviews.apache.org/r/37655/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 37442: Factor out the token extraction rules in prepartion for extending them to cope with multiple versions.

[Paul Brett]

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

(Updated Aug. 20, 2015, 5:31 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Remove redundant constructor.


Repository: mesos


Description
---

Factor out the token extraction rules in prepartion for extending them to cope 
with multiple versions.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Review Request 37657: Document iptable rule need added when use docker bridge network mode.

[haosdent huang]

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

Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

Document iptable rule need added when use docker bridge network mode.


Diffs
-

  docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

[haosdent huang]

> On Aug. 20, 2015, 5:02 p.m., Jie Yu wrote:
> > src/tests/mesos.hpp, line 965
> > 
> >
> > Could you please move this to a separate header file like?
> > ```
> > src/tests/containerizer/docker.hpp
> > ```

Thank you. Let me move it out.


- haosdent


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


On Aug. 20, 2015, 2:23 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36185/
> ---
> 
> (Updated Aug. 20, 2015, 2:23 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-2588
> https://issues.apache.org/jira/browse/MESOS-2588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create pre-launch hook before a docker container launches in slave.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 
>   src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd 
>   src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 
>   src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 
>   src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 
>   src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea 
>   src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb 
>   src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
> 
> Diff: https://reviews.apache.org/r/36185/diff/
> 
> 
> Testing
> ---
> 
> # Add a new unit test "HookTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook"
> make check -j8 GTEST_FILTER=-"*"
> sudo ./bin/mesos-tests.sh --verbose 
> --gtest_filter=HookTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.

[Joseph Wu]

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

(Updated Aug. 20, 2015, 10:03 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
Remoortere, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Instead of using doubles, seconds and nanoseconds can be represented like
`struct timespec`, with one field for seconds and one for nanoseconds.

This will be important if frameworks need to compare times to make decisions 
(such as for maintenance primitives).

Note about the naming:

* Time will conflict with the Time class.
* Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with 
Duration.


Diffs
-

  include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 

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


Testing
---

`make check`


Thanks,

Joseph Wu

Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

[Jie Yu]

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



src/tests/mesos.hpp (line 965)


Could you please move this to a separate header file like?
```
src/tests/containerizer/docker.hpp
```


- Jie Yu


On Aug. 20, 2015, 2:23 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36185/
> ---
> 
> (Updated Aug. 20, 2015, 2:23 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-2588
> https://issues.apache.org/jira/browse/MESOS-2588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create pre-launch hook before a docker container launches in slave.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 
>   src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd 
>   src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 
>   src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 
>   src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 
>   src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea 
>   src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb 
>   src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
> 
> Diff: https://reviews.apache.org/r/36185/diff/
> 
> 
> Testing
> ---
> 
> # Add a new unit test "HookTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook"
> make check -j8 GTEST_FILTER=-"*"
> sudo ./bin/mesos-tests.sh --verbose 
> --gtest_filter=HookTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

[Paul Brett]

> On Aug. 19, 2015, 1:42 a.m., Ben Mahler wrote:
> > src/linux/perf.cpp, lines 484-490
> > 
> >
> > Couple of things:
> > 
> > (1) Let's add a comment as to why we're using await here, since it is 
> > an anti-pattern. Namely, is it because making supported() asynchronous is a 
> > difficult change?
> > 
> > (2) Let's discard the future when it doesn't transition in the timeout.
> > 
> > (3) We're going to silently say false if the future fails, can we log 
> > the failure?
> > 
> > (4) 'if (' :)
> > 
> > (5) Can you add some newlines here to make it less dense?

What is the advantage of calling discard() on version here?  At the moment, we 
pass version as a const ref, so calling discard is not possible.  We could 
remove the const constraint, but then we would have to pass an lvalue from 
supported.


- Paul


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


On Aug. 20, 2015, 4:39 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37416/
> ---
> 
> (Updated Aug. 20, 2015, 4:39 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Perf supported() should be based on the version of perf, not the version of 
> the kernel.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 
> 
> Diff: https://reviews.apache.org/r/37416/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37427: Docker registry: adding TokenManager.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37426, 37427]

All tests passed.

- Mesos ReviewBot


On Aug. 20, 2015, 3:50 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37427/
> ---
> 
> (Updated Aug. 20, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes:
>   - Added Token implementation (RFC 7519).
>   - Added TokenManager implementation. This component keeps a cache of tokens
>   requested for any future requests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION 
>   src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37427/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

[Paul Brett]

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

(Updated Aug. 20, 2015, 4:39 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Fix bad patch.


Repository: mesos


Description
---

Perf supported() should be based on the version of perf, not the version of the 
kernel.


Diffs (updated)
-

  src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.

[Paul Brett]

> On Aug. 19, 2015, 1:42 a.m., Ben Mahler wrote:
> > src/linux/perf.cpp, lines 474-478
> > 
> >
> > Why `_supported` here that takes a version? Why not just have supported 
> > compute the version and then perform the necessary check?

We need two variants - perf::supported that computes the version and performs 
the necessary checks, and a version for testing that we can feed fake version 
information into so that we can test the parsing against multiple versions of 
perf output.


- Paul


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


On Aug. 20, 2015, 1:13 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37416/
> ---
> 
> (Updated Aug. 20, 2015, 1:13 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Perf supported() should be based on the version of perf, not the version of 
> the kernel.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 
> 
> Diff: https://reviews.apache.org/r/37416/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 37617: Added an error message to MesosContainerizerLaunch command.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Aug. 19, 2015, 6:26 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37617/
> ---
> 
> (Updated Aug. 19, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-3296
> https://issues.apache.org/jira/browse/MESOS-3296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an error message to MesosContainerizerLaunch command.
> 
> So that we can triage MESOS-3296.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> bd594d3f3d729aa000a1a7fbf9038f6cfcbb169b 
> 
> Diff: https://reviews.apache.org/r/37617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 37427: Docker registry: adding TokenManager.

[Jojy Varghese]

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

(Updated Aug. 20, 2015, 3:50 p.m.)


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


Changes
---

rebased with master


Repository: mesos


Description
---

Changes:
  - Added Token implementation (RFC 7519).
  - Added TokenManager implementation. This component keeps a cache of tokens
  requested for any future requests.


Diffs (updated)
-

  src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
  src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION 
  src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 36620: Added Non-Freezeer Task Killer.

[Joerg Schad]

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

(Updated Aug. 20, 2015, 11:42 a.m.)


Review request for mesos, Benjamin Hindman and Timothy Chen.


Summary (updated)
-

Added Non-Freezeer Task Killer.


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


Repository: mesos


Description
---

WIP Added Non-Freezeer Task Killer.


Diffs
-

  src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 

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


Testing
---

sudo make check
+ manual tests


Thanks,

Joerg Schad

Re: Review Request 37336: Simplified the caller interface to process::Subprocess

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37336]

All tests passed.

- Mesos ReviewBot


On Aug. 20, 2015, 8:03 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37336/
> ---
> 
> (Updated Aug. 20, 2015, 8:03 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-3035
> 
> The original API for `process::Subprocess` still left a lot of legwork
> to do for the caller; we have now added a `wait(Duration timeout)` method
> that returns a `CommandResult` (also introduced with this patch) which
> contains useful information about the command invocation; the exit code;
> stdout and, optionally, stderr too.
> 
> The `wait()` method will wait for both the process to terminate, and
> stdout/stderr to be available to read from; it also "unpacks" them into
> ready-to-use `string`s.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> d2341a53aac7c779668ee80983f73158fd44bff5 
>   3rdparty/libprocess/src/subprocess.cpp 
> d6ea62ed1c914d34e0e189395831c86fff8aac22 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ab7515325e5db0a4fd222bb982f51243d7b7e39d 
> 
> Diff: https://reviews.apache.org/r/37336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36185]

All tests passed.

- Mesos ReviewBot


On Aug. 20, 2015, 2:23 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36185/
> ---
> 
> (Updated Aug. 20, 2015, 2:23 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-2588
> https://issues.apache.org/jira/browse/MESOS-2588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create pre-launch hook before a docker container launches in slave.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 
>   src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd 
>   src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 
>   src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 
>   src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 
>   src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea 
>   src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb 
>   src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 
> 
> Diff: https://reviews.apache.org/r/36185/diff/
> 
> 
> Testing
> ---
> 
> # Add a new unit test "HookTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook"
> make check -j8 GTEST_FILTER=-"*"
> sudo ./bin/mesos-tests.sh --verbose 
> --gtest_filter=HookTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37336: Simplified the caller interface to process::Subprocess

[Marco Massenzio]

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

(Updated Aug. 20, 2015, 8:03 a.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

This now works and passes a sensible set of tests; still, there is some 
'surprising' behavior and I'd like to understand that before committing.


Summary (updated)
-

Simplified the caller interface to process::Subprocess


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


Repository: mesos


Description (updated)
---

Jira: MESOS-3035

The original API for `process::Subprocess` still left a lot of legwork
to do for the caller; we have now added a `wait(Duration timeout)` method
that returns a `CommandResult` (also introduced with this patch) which
contains useful information about the command invocation; the exit code;
stdout and, optionally, stderr too.

The `wait()` method will wait for both the process to terminate, and
stdout/stderr to be available to read from; it also "unpacks" them into
ready-to-use `string`s.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
d2341a53aac7c779668ee80983f73158fd44bff5 
  3rdparty/libprocess/src/subprocess.cpp 
d6ea62ed1c914d34e0e189395831c86fff8aac22 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
ab7515325e5db0a4fd222bb982f51243d7b7e39d 

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


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 37336: [WIP] Added `wait()` method to process::Subprocess

[Marco Massenzio]

> On Aug. 17, 2015, 11:01 p.m., Ben Mahler wrote:
> > In the same vein as os::shell, we should probably introduce an 'os' 
> > namespace in libprocess for asynchronous os utilities. In this case, 
> > process::os::shell which returns a Future of the output (although, ideally 
> > ).

Sounds good.
If I understand this correctly, the `process::os::shell` would take advantage 
of this refactoring, correct?
(also, how about returning a `Future`: this has all of  and a bit more).

Could you please take a look at the next revision I'm about to post: it now all 
works and passes the tests, but I'm somewhat confused as to why the `onFailed` 
never got called and another nit - they are marked as FIXME(marco) and will be 
obviously removed - I needed a way to mark those areas visibly: they are not 
TODOs).

Thanks!


- Marco


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


On Aug. 15, 2015, 2:02 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37336/
> ---
> 
> (Updated Aug. 15, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-3035
> 
> The original API for `process::Subprocess` still left a lot of legwork
> to do for the caller; we have now added a `wait(Duration timeout)` method
> that returns a `CommandResult` (also introduced with this patch) which
> contains useful information about the command invocation; the exit code;
> stdout and, optionally, stderr too.
> 
> The `wait()` method will wait for both the process to terminate, and
> stdout/stderr to be available to read from; it also "unpacks" them into
> ready-to-use `string`s.
> 
> This is still WIP as I'm seeing some unusual behavior and I'd like to discuss 
> with someone more expert on libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> 310cb4f8e4e2faa5545dffd196d7490c868bc5d6 
>   3rdparty/libprocess/src/subprocess.cpp 
> d6ea62ed1c914d34e0e189395831c86fff8aac22 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ab7515325e5db0a4fd222bb982f51243d7b7e39d 
> 
> Diff: https://reviews.apache.org/r/37336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 37617: Added an error message to MesosContainerizerLaunch command.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37617]

All tests passed.

- Mesos ReviewBot


On Aug. 19, 2015, 6:26 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37617/
> ---
> 
> (Updated Aug. 19, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Vinod Kone.
> 
> 
> Bugs: MESOS-3296
> https://issues.apache.org/jira/browse/MESOS-3296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an error message to MesosContainerizerLaunch command.
> 
> So that we can triage MESOS-3296.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> bd594d3f3d729aa000a1a7fbf9038f6cfcbb169b 
> 
> Diff: https://reviews.apache.org/r/37617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>