Re: Review Request 42183: Introduced HttpEvent based filters in libprocess.

[Anand Mazumdar]

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

(Updated Jan. 22, 2016, 8:24 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated Depends On


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


Repository: mesos


Description
---

This change introduces the ability to filter `HTTPEvent` in libprocess similar 
to the already existing `MessageEvent` and `DispatchEvent`.


Diffs
-

  3rdparty/libprocess/include/process/gmock.hpp 
53fdb15ef8d0c6bb95a974be416ce1922a211064 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 42658: Restructured comments in allocator tests for clarity.

[Alexander Rukletsov]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Do not describe cluster state in advance to avoid confusion,
rather right after the state changes.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42172: Add documentation for logging and ContainerLogger.

[Joseph Wu]

> On Jan. 22, 2016, 11:16 a.m., Neil Conway wrote:
> > Can we also mention the common GLOG configuration options? For example, 
> > `GLOG_v`.

Besides `GLOG_v`, what else would you suggest?  There are quite a few, but I've 
only ever set this one specifically.

Note: That one is mentioned here:
https://reviews.apache.org/r/42172/diff/5#file1202582line125


- Joseph


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


On Jan. 19, 2016, 7:41 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42172/
> ---
> 
> (Updated Jan. 19, 2016, 7:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4206
> https://issues.apache.org/jira/browse/MESOS-4206
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moves (and updates) logging flags from configuration.md into the new 
> logging.md.
> 
> Add documentation for the ContainerLogger.
> Adds documentation about logging in Master/Agent/Framework and the 
> ContainerLogger.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 1834992907a29f571200fe8422f7c26eaac7d5a0 
>   docs/home.md ff797fb050d710283fa7e648515ec75779e58f65 
>   docs/logging.md PRE-CREATION 
>   docs/modules.md 1aad8e0958554c0219a287dffd6c6fb925edb025 
>   src/logging/flags.cpp b321c28492dbc9711b937f6cd9a8423ce557957f 
> 
> Diff: https://reviews.apache.org/r/42172/diff/
> 
> 
> Testing
> ---
> 
> Previewed on GitHub and via:
> ```
> docker build -t mesos/website support/site-docker
> docker run -it --rm -p 4567:4567 -v /path/to/mesos:/mesos mesos/website
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 42476: Introduced protobuf for set quota requests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42477, 42476]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 22, 2016, 3:43 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42476/
> ---
> 
> (Updated Jan. 22, 2016, 3:43 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
> Van Remoortere.
> 
> 
> Bugs: MESOS-4410
> https://issues.apache.org/jira/browse/MESOS-4410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/quota.md 208bfa06a9fc50843439bab9a041ccb557657b5d 
>   include/mesos/quota/quota.proto 338412ee967e14aa1957a47f4a50f2e19e4eca79 
>   src/master/quota_handler.cpp aa06cbc83d2c467c2422f1d27bb9507998953c44 
>   src/tests/master_quota_tests.cpp 7d07be355b9323f2a7f27f84020a6ecc7f061140 
>   src/tests/role_tests.cpp 979391306e2427aaa63a5df32704913f79e20e36 
> 
> Diff: https://reviews.apache.org/r/42476/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42181: Added a Slave constructor to pass process ID manually.

[Anand Mazumdar]

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

(Updated Jan. 22, 2016, 8:27 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated Depends On


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


Repository: mesos


Description
---

This change adds a constructor to Slave that takes in the process ID string as 
an argument. This is necessary for testing recovery of HTTP based executors. 
(Previously, every invocation of `StartSlave` used to generate a new ID via 
`ID::generate`). There was no process delegate set via `process::initialize` 
that led to problems for the existing HTTP based executor to connect back to 
the slave.

Also, modified the tests to introduce a new `StartSlave` function that takes 
this process ID as argument.


Diffs
-

  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
  src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
  src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
  src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41290: Modified `TestContainerizer` to handle HTTP based executors.

[Anand Mazumdar]

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

(Updated Jan. 22, 2016, 8:32 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Updated JIRA link


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


Repository: mesos


Description
---

This change modifies the `TestContainerizer` class to handle HTTP based 
executors. Previously, the containerizer test class only used to handle the 
driver interface.


Diffs
-

  src/tests/containerizer.hpp 25b64dacca65aa0ac49a05a2fbaf526aa0761471 
  src/tests/containerizer.cpp 15c8b19f3116e60d80671c64ff33580b552dc1ec 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41291: Modified the scheduler tests to use the new executor HTTP based library.

[Anand Mazumdar]

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

(Updated Jan. 22, 2016, 8:35 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Updated JIRA


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


Repository: mesos


Description
---

This change modifies the existing scheduler tests to use the new executor HTTP 
library instead of the old driver based interface.


Diffs
-

  src/tests/scheduler_tests.cpp 67432109c7df6be0aa76e94a03bd5b2e9c96d14e 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 42657: Corrected a typo in allocator tests.

[Alexander Rukletsov]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42184: Introduced `FUTURE_HTTP_*` for filtering HTTP based events.

[Anand Mazumdar]

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

(Updated Jan. 22, 2016, 8:17 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated JIRA


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


Repository: mesos


Description
---

This change introduces `FUTURE_HTTP_*` for filtering HTTP based events similar 
to the already existing filters for Messages/Dispatch events. Also, added 
`FUTURE_HTTP_CALL_*` for union based protobufs.


Diffs
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42185: Added an example executor based on the new V1 API.

[Anand Mazumdar]

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

(Updated Jan. 22, 2016, 8:25 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated Depends On


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


Repository: mesos


Description
---

This change adds a custom executor based on the new executor library.


Diffs
-

  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/examples/test_http_executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42503: Multiple Disk: Added Resource arithmetic tests for type 'PATH'.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Jan. 22, 2016, 9:31 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42503/
> ---
> 
> (Updated Jan. 22, 2016, 9:31 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4380
> https://issues.apache.org/jira/browse/MESOS-4380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Multiple Disk: Added Resource arithmetic tests for type 'PATH'.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
> 
> Diff: https://reviews.apache.org/r/42503/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42504: Multiple Disk: Modified 'DiskInfo' stripping logic.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Jan. 22, 2016, 9:31 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42504/
> ---
> 
> (Updated Jan. 22, 2016, 9:31 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored persistent volume tests to use a helper to generate disk
> resources.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/common/resources_utils.cpp d64db54a175299bf7c32ecbeeb1ce9a7afb1c88a 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/persistent_volume_tests.cpp 
> 7acf7ab29d64d891f3288f8042d267dcc82a32e9 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42504/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42593: Multiple Disk: Fixed invalid 'DiskResourcesTest's.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Jan. 21, 2016, 7:40 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42593/
> ---
> 
> (Updated Jan. 21, 2016, 7:40 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These were invalid as a resource should not have a 'DiskInfo' if it
> sets none of: persistence, volume, or source.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
> 
> Diff: https://reviews.apache.org/r/42593/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42605: Fixed a NULL pointer dereference bug in Slave.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Jan. 22, 2016, 3:57 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42605/
> ---
> 
> (Updated Jan. 22, 2016, 3:57 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4449
> https://issues.apache.org/jira/browse/MESOS-4449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes a `NULL` pointer dereference erroneously added as part of 
> introducing an output operator for the `Executor` struct in 
> https://reviews.apache.org/r/39569.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 
> 
> Diff: https://reviews.apache.org/r/42605/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42172: Add documentation for logging and ContainerLogger.

[Neil Conway]

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


Can we also mention the common GLOG configuration options? For example, 
`GLOG_v`.

- Neil Conway


On Jan. 20, 2016, 3:41 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42172/
> ---
> 
> (Updated Jan. 20, 2016, 3:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4206
> https://issues.apache.org/jira/browse/MESOS-4206
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moves (and updates) logging flags from configuration.md into the new 
> logging.md.
> 
> Add documentation for the ContainerLogger.
> Adds documentation about logging in Master/Agent/Framework and the 
> ContainerLogger.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 1834992907a29f571200fe8422f7c26eaac7d5a0 
>   docs/home.md ff797fb050d710283fa7e648515ec75779e58f65 
>   docs/logging.md PRE-CREATION 
>   docs/modules.md 1aad8e0958554c0219a287dffd6c6fb925edb025 
>   src/logging/flags.cpp b321c28492dbc9711b937f6cd9a8423ce557957f 
> 
> Diff: https://reviews.apache.org/r/42172/diff/
> 
> 
> Testing
> ---
> 
> Previewed on GitHub and via:
> ```
> docker build -t mesos/website support/site-docker
> docker run -it --rm -p 4567:4567 -v /path/to/mesos:/mesos mesos/website
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 42506: Multiple Disk: Parameterized persistent volume tests on disk source.

[Jie Yu]

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



src/tests/persistent_volume_tests.cpp (line 68)


'{' in next line.



src/tests/persistent_volume_tests.cpp (line 72)


2 lines apart please.



src/tests/persistent_volume_tests.cpp (line 80)


I think you can use environment->mkdtemp() so that you don't need to worry 
about teardown.



src/tests/persistent_volume_tests.cpp (line 81)


We try to avoid CHECK in test since it'll abort the entire test run. Can 
you de ASSERT instead?



src/tests/persistent_volume_tests.cpp (line 150)


'{' in next line.



src/tests/persistent_volume_tests.cpp (lines 150 - 167)


Do you still need this? Can you just use paths::getPersistentVolumePath?



src/tests/persistent_volume_tests.cpp (line 170)


Do you still need this?


- Jie Yu


On Jan. 22, 2016, 9:33 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42506/
> ---
> 
> (Updated Jan. 22, 2016, 9:33 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows us to leverage the existing persistent volume tests while
> also testing support for multiple disks.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 7acf7ab29d64d891f3288f8042d267dcc82a32e9 
> 
> Diff: https://reviews.apache.org/r/42506/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42670: Fixed filesystem isolator tests timing out.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Jan. 22, 2016, 7:47 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42670/
> ---
> 
> (Updated Jan. 22, 2016, 7:47 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed filesystem isolator tests timing out.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> f0c5b511ae20c720541351aba93f358ce26e0d51 
> 
> Diff: https://reviews.apache.org/r/42670/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 42590: Removed reserved() API.

[Joseph Wu]

> On Jan. 21, 2016, 10:37 a.m., Joseph Wu wrote:
> > include/mesos/v1/resources.hpp, line 227
> > 
> >
> > Instead of changing this to an Option, you should default to `"*"`.
> > 
> > Otherwise, you end up with two arguments (`None()` and `"*"`) that give 
> > you the same result.
> 
> Guangya Liu wrote:
> I think the `"*"` will return empty resources, am I missing something? 
> Please refer to 
> https://github.com/apache/mesos/blob/master/src/tests/resources_tests.cpp#L1426

Ah, yes.  I read the method incorrectly.  Dropping.


- Joseph


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


On Jan. 21, 2016, 7:37 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42590/
> ---
> 
> (Updated Jan. 21, 2016, 7:37 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4447
> https://issues.apache.org/jira/browse/MESOS-4447
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed reserved() API.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/master/http.cpp 12c1fe5a514903f657911302e8770e9b245fdbb7 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42590/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42505: Multiple Disk: Adjusted DiskInfo validation.

[Jie Yu]

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

Ship it!



src/common/resources.cpp (line 685)


insert a blank line above?



src/common/resources.cpp (line 687)


insert a blank line above?



src/v1/resources.cpp (line 685)


Ditto.



src/v1/resources.cpp (line 687)


Ditto.


- Jie Yu


On Jan. 22, 2016, 9:33 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42505/
> ---
> 
> (Updated Jan. 22, 2016, 9:33 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Multiple Disk: Adjusted DiskInfo validation.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/master/validation.cpp 222bb3451c133844c42caad0567c1e98de6ba778 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42505/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 42670: Fixed filesystem isolator tests timing out.

[Timothy Chen]

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

Review request for mesos, Bernd Mathiske and Jie Yu.


Repository: mesos


Description
---

Fixed filesystem isolator tests timing out.


Diffs
-

  src/tests/containerizer/filesystem_isolator_tests.cpp 
f0c5b511ae20c720541351aba93f358ce26e0d51 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 42472: Multiple Disk: Checkpoint persistent volume based on 'DiskInfo.Source'.

[Joris Van Remoortere]

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

(Updated Jan. 22, 2016, 9:29 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

Added `getPersistentVolumePath` overload to factor out common logic.


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


Repository: mesos


Description
---

We now create persistent volume directories based on 'DiskInfo.Source'
if it is present.


Diffs (updated)
-

  src/slave/paths.hpp 6b9a65d2e8ed771041adce6f8c6fb0601422c6e8 
  src/slave/paths.cpp 9315b4cf0664a381d02f5cc72083eb22055daf4c 
  src/slave/slave.cpp e23c3295c8ebed580751a5aabaf26e1773c54859 
  src/tests/paths_tests.cpp 5e8495f2a19a4641e258f78911772b991d34d85a 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 42473: Multiple Disk: Updated filesystem isolators to use 'DiskInfo.Source'.

[Joris Van Remoortere]

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

(Updated Jan. 22, 2016, 9:29 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

Leveraged new `getPersistentVolumePath` overload.


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


Repository: mesos


Description
---

Multiple Disk: Updated filesystem isolators to use 'DiskInfo.Source'.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
96c37ed7ac27074eb79e3eb5e0301b642ab653f2 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
d914587eedc9c66bc14b4088ec211b7f0eea63ea 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 42636: Replaced term 'periodic allocation' for consistency.

[Ben Mahler]

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


Hm.. do you think batch is a better term than periodic? I'm not so sure.

- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42636/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42636/diff/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42355: Removed the timeout from the filter.

[Ben Mahler]

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

Ship it!


- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42355/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4302
> https://issues.apache.org/jira/browse/MESOS-4302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Without the timeout, we rely on filter expiration only. This guarantees
> that filter removal is scheduled after `allocate()` if the allocator is
> backlogged given default parameters are used. Additionally we ensure the
> filter timeout is at least as big as the allocation interval.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 53d8784a3c4f0467756d7bd924e624ba4585e776 
> 
> Diff: https://reviews.apache.org/r/42355/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42634: Updated a comment for consistency.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42634/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42634/diff/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42631: Renamed resource offer timeout for clarity.

[Ben Mahler]

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

Ship it!


- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42631/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 53d8784a3c4f0467756d7bd924e624ba4585e776 
> 
> Diff: https://reviews.apache.org/r/42631/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42635: Removed misleading note from allocator tests.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42635/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42635/diff/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42630: Changed member visibility in offer filter classes.

[Ben Mahler]

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

Ship it!


- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42630/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 53d8784a3c4f0467756d7bd924e624ba4585e776 
> 
> Diff: https://reviews.apache.org/r/42630/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42637: Cleaned up formatting in allocator tests.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42637/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42637/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42629: Added tests for offer filters.

[Ben Mahler]

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

Ship it!


Great tests, thanks! I made some tweaks based on the suggestions below, and 
adjusted some comments as well.


src/tests/hierarchical_allocator_tests.cpp (lines 458 - 459)


If you can, try to wrap comments to reduce jaggedness, i.e. the following:

```
  // We put both frameworks into the same role, but we could also
  // have had separate roles; this should not influence the test.
```

Is less jagged than your existing one:

```
  // We put both frameworks into the same role, but we could also have had
  // separate roles; this should not influence the test.
```

Ditto for the rest of the comments throughout the changes here.



src/tests/hierarchical_allocator_tests.cpp (line 505)


It might be a bit easier to just say "the offer filter" here rather than 
trying to reference the `offerFilter` variable?



src/tests/hierarchical_allocator_tests.cpp (lines 509 - 515)


Looks like you only need the second advance here? Technically, you are 
triggering two batch allocations here, which doesn't appear to be the intent?

It would mean we need a Clock::settle after calling recoverResources.



src/tests/hierarchical_allocator_tests.cpp (line 525)


Some unicode character here?



src/tests/hierarchical_allocator_tests.cpp (line 529)


How about s/FilterTimeoutRespected/SmallOfferFilterTimeout/ since the 
existing name could accurately describe your other OfferFilter test as well?



src/tests/hierarchical_allocator_tests.cpp (line 542)


Explicitly typo



src/tests/hierarchical_allocator_tests.cpp (line 1415)


What is a Quarantee? ;)



src/tests/hierarchical_allocator_tests.cpp (lines 1497 - 1502)


Let's leave this for now, but it would be great to avoid this assumption 
and explicitly control the allocation interval duration, no? When you are 
advancing the clock below twice, you are triggering two batch allocations, when 
your intention appears to be to trigger only one batch allocation.

For now I'll fix this same issue in the OfferFilter test, and I'll leave 
this one as still triggering two allocations since it's an existing test.



src/tests/hierarchical_allocator_tests.cpp (lines 1513 - 1520)


This change actually belongs in the previous patch, since your last change 
breaks this test on its own.


- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42629/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4302
> https://issues.apache.org/jira/browse/MESOS-4302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42629/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check`
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.FilterTimeout" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure` passes with the patch and fails 
> without.
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42636: Replaced term 'periodic allocation' for consistency.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42636/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42636/diff/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42471: Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.

[Joris Van Remoortere]

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

(Updated Jan. 22, 2016, 9:27 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.


Diffs (updated)
-

  include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
  include/mesos/v1/mesos.hpp 961042d8e4944a475076b829966020d62175d726 
  src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

[Adam B]

> On Jan. 20, 2016, 1:51 a.m., Adam B wrote:
> > src/master/master.hpp, line 1577
> > 
> >
> > Can it ever return anything but `true`? Are there any error conditions 
> > at all?
> 
> Yongqiao Wang wrote:
> I have double checked that there are no any error conditions in our case.

Ok, looks like this function has to return a Try because that's what 
Operation defines for its interface. Makes sense for your to always return 
'true' in such a simple Operation. Dropping the review issue.


> On Jan. 20, 2016, 1:51 a.m., Adam B wrote:
> > src/master/weights_handler.cpp, lines 113-115
> > 
> >
> > Why are you reconstructing a roles `string` out of the weightInfos 
> > instead of just using a `vector`? Does `authorize()` really require a 
> > comma-delimited string?
> > 
> > What I was proposing before was that you call `authorize(principal, 
> > role)` for each weightInfo.role, and fail if any of those fails.
> 
> Yongqiao Wang wrote:
> Reconstructing a roles string is only for print all roles in 
> Master::WeightsHandler::authorize function.
> 
> In addition, authorize() is an asynchronous call and will return a 
> Future, in our logic, weights can only be updated in master, registry 
> and allocator if authorization passed for all specified roles. So it is 
> complex to block the authorization for each roles, then call _update() to 
> updates weights in registry and registry. So I authorize all roles together 
> at one time.

Printing doesn't seem like a good enough reason to have to translate roles back 
and forth. How about you pass a vector of roles to `authorize()` and just use 
`stringify` for the print string.?

And you wouldn't need to block sequentially for the calls, just wait for all of 
the futures to return. But I can see what that's unnecessary for 
WeightsHandler::authorize. I'll drop that idea.


- Adam


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


On Jan. 20, 2016, 6:16 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated Jan. 20, 2016, 6:16 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce HTTP endpoint /weights for updating weight.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 5ee3c7afadd131802c93febbb6b4dbad069c2d81 
>   include/mesos/authorizer/authorizer.proto 
> 84727e66dd14be9a25705ab1141e92eee72fae50 
>   src/CMakeLists.txt a52203ab9aa47315e6e5c58cc283a7b5df597c76 
>   src/Makefile.am 4fabe600d4ba38c95a777d622b0b675dd5811a53 
>   src/authorizer/local/authorizer.hpp 
> c7321c276d566eca6a91f45c546468bea1b0da15 
>   src/authorizer/local/authorizer.cpp 
> c1db9c2131ea8fbf835278203a240f108c6372c5 
>   src/master/http.cpp 34a70ee50553492fc8c3947497ab5922f4379d72 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
> 
> Diff: https://reviews.apache.org/r/41681/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully!
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
> --weights="role1=4.2,role2=3.1" --authenticate_http 
> --credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
> "roles": [
> {
> "frameworks": [ ], 
> "name": "*", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 1
> }, 
> {
> "frameworks": [ ], 
> "name": "role1", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 4.2
> }, 
> {
> "frameworks": [ ], 
> "name": "role2", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 3.1
> }
> ]
> }
> 
> Test update:
> $ curl 

Re: Review Request 42504: Multiple Disk: Modified 'DiskInfo' stripping logic.

[Joris Van Remoortere]

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

(Updated Jan. 22, 2016, 9:31 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

rebased.


Repository: mesos


Description
---

Refactored persistent volume tests to use a helper to generate disk
resources.


Diffs (updated)
-

  src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
  src/common/resources_utils.cpp d64db54a175299bf7c32ecbeeb1ce9a7afb1c88a 
  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
  src/tests/persistent_volume_tests.cpp 
7acf7ab29d64d891f3288f8042d267dcc82a32e9 
  src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 42505: Multiple Disk: Adjusted DiskInfo validation.

[Joris Van Remoortere]

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

(Updated Jan. 22, 2016, 9:33 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

Used an alias to make conditional more readable.
Removed extra `Resource::validation` check.


Repository: mesos


Description
---

Multiple Disk: Adjusted DiskInfo validation.


Diffs (updated)
-

  src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
  src/master/validation.cpp 222bb3451c133844c42caad0567c1e98de6ba778 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 42474: Multiple Disk: Updated Slave initialize to create DiskInfo paths.

[Joris Van Remoortere]

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

(Updated Jan. 22, 2016, 9:30 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

Added comment regarding `realpath` for `MOUNT` root.


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


Repository: mesos


Description (updated)
---

Create disk paths based on 'DiskInfo.Source' if the type is 'PATH'
and the directory does not yet exist.


Diffs (updated)
-

  src/slave/slave.cpp e23c3295c8ebed580751a5aabaf26e1773c54859 

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


Testing
---

make check


Thanks,

Joris Van Remoortere

Re: Review Request 42503: Multiple Disk: Added Resource arithmetic tests for type 'PATH'.

[Joris Van Remoortere]

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

(Updated Jan. 22, 2016, 9:31 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

rebased.


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


Repository: mesos


Description
---

Multiple Disk: Added Resource arithmetic tests for type 'PATH'.


Diffs (updated)
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
  src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 42506: Multiple Disk: Parameterized persistent volume tests on disk source.

[Joris Van Remoortere]

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

(Updated Jan. 22, 2016, 9:33 a.m.)


Review request for mesos, Jie Yu and Michael Park.


Changes
---

Rebased.


Repository: mesos


Description
---

This allows us to leverage the existing persistent volume tests while
also testing support for multiple disks.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
7acf7ab29d64d891f3288f8042d267dcc82a32e9 

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


Testing
---

make check


Thanks,

Joris Van Remoortere

Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

[Adam B]

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

Ship it!


Looks good except for a handful of nits. Fix these up and I'll be ready to 
commit it.


include/mesos/authorizer/authorizer.proto (line 129)


s/weight/weights/



src/master/master.hpp (lines 1540 - 1543)


Looks like this can all fit on one line



src/master/master.hpp (line 1547)


It's a shame we can't store the weights as a map in the registry, so we 
don't have to iterate through all the registry's weights for each weight being 
updated.



src/master/master.hpp (line 1568)


Technically, the bool returned represents whether the registry was mutated. 
If there were no weights, or all weights are "updated" to their existing 
values, then there is no mutation, so you should return false.
I'm not sure if these use cases are common enough to want to prevent a 
registrar update (and replication) though. What do you think?



src/master/master.cpp (line 1528)


"After dynamic weights are supported"?
Doesn't this change introduce support for it?
How about "After the Mesos master quorum..."



src/master/master.cpp (lines 1531 - 1532)


"..., so the `--weights` flag can be deprecated and this check can 
eventually be removed.



src/master/master.cpp (line 1535)


s/recover/recovering/
s/non-default //



src/master/master.cpp (lines 1537 - 1540)


Why not just `weights.clear()`?



src/master/master.cpp (line 1543)


s/non-default //



src/master/master.cpp (lines 1544 - 1550)


Why not combine these two loops rather than iterating twice?


- Adam B


On Jan. 20, 2016, 6:16 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated Jan. 20, 2016, 6:16 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce HTTP endpoint /weights for updating weight.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 5ee3c7afadd131802c93febbb6b4dbad069c2d81 
>   include/mesos/authorizer/authorizer.proto 
> 84727e66dd14be9a25705ab1141e92eee72fae50 
>   src/CMakeLists.txt a52203ab9aa47315e6e5c58cc283a7b5df597c76 
>   src/Makefile.am 4fabe600d4ba38c95a777d622b0b675dd5811a53 
>   src/authorizer/local/authorizer.hpp 
> c7321c276d566eca6a91f45c546468bea1b0da15 
>   src/authorizer/local/authorizer.cpp 
> c1db9c2131ea8fbf835278203a240f108c6372c5 
>   src/master/http.cpp 34a70ee50553492fc8c3947497ab5922f4379d72 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
> 
> Diff: https://reviews.apache.org/r/41681/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully!
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
> --weights="role1=4.2,role2=3.1" --authenticate_http 
> --credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
> "roles": [
> {
> "frameworks": [ ], 
> "name": "*", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 1
> }, 
> {
> "frameworks": [ ], 
> "name": "role1", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 4.2
> }, 
> {
> "frameworks": [ ], 
> "name": "role2", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 3.1
> }
> ]
> }
> 
> Test 

Re: Review Request 40553: Enable mesos tests installation.

[James Peach]

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

(Updated Jan. 22, 2016, 10:12 p.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Summary (updated)
-

Enable mesos tests installation.


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


Repository: mesos


Description
---

This patch enables the installation mesos-tests and its dependencies
and helper tool. The goal is to allow operators to build a separate
test package that can be run at deployment time to verify that Mesos
works in the deployment environment.

Since the build directory is searched first, to run it on a host
that has a build tree, you need to specify a non-existent tree:

~ $ $PREFIX/libexec/mesos/tests/mesos-tests \
--build_dir=/none \
--source_dir=/none

- Add --enable-tests-install
- Fix mesos-tests gmock dependencies
- Optionally install tests, helpers and test modules
- Add utility helpers to find various test resources


Diffs
-

  Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
  configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
  src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
  src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
  src/tests/balloon_framework_test.sh 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
  src/tests/containerizer/launch_tests.cpp 
c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
  src/tests/containerizer/memory_test_helper.cpp 
4a3de2e3c887aa6afc604588850e1386f92d8c11 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
677fcc1c654f83ad3e60e0f6172a1a1e4a1045b1 
  src/tests/containerizer/ns_tests.cpp 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
  src/tests/containerizer/port_mapping_tests.cpp 
fab16f697f90abc11d681222f10f518d70da908b 
  src/tests/environment.cpp 4de46bc48b245a4c5e89b0343578b0d31883a0ac 
  src/tests/event_call_framework_test.sh 
9d1211552734afbf15b376f8c4629bae8a2065af 
  src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
  src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
  src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
  src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475 
  src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
  src/tests/no_executor_framework_test.sh 
aebdc8c380abb2d041d6fc74dfac5a111c15267e 
  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 
  src/tests/persistent_volume_framework_test.sh 
84f02847a8d89400512d8a5714d33fb29cf5b03a 
  src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
  src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
  src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
  src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
  src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 

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


Testing
---


Thanks,

James Peach

Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.

[Jie Yu]

> On Jan. 15, 2016, 12:03 a.m., Zhitao Li wrote:
> > src/docker/docker.cpp, lines 410-420
> > 
> >
> > (Sorry I just got time to come back to this).
> > 
> > I don't exactly understand your suggestion about "add additional 
> > containerInfo.volumes() in 
> > DockerContainerizerProcess::Container::create()", because the latter 
> > function actually does not pass a `ContainerInfo`.
> > 
> > Because the volume is actually included in 
> > `TaskInfo::ContainerInfo::volumes`, we would need to mutate the `TaskInfo` 
> > before passing it to the `DockerExecutorProcess::launchTask()` function, 
> > which I don't really know how to do it, and I even doubt whether it's a 
> > good idea for logging/clarity purpose.
> > 
> > Please let me know if I didn't understand your suggestion, or if you 
> > think we should explore the other alternative (passing `hostPath` earlier 
> > in resource offer).

Sorry for being late on this.

My sugguestion is that we add some logics in 
DockerContainerProcess::Container::create(), the 'create' function has both 
TaskINfo and ExecutorInfo. So, you should be able to get the ContainerInfo from 
them. When you generate DockerContainerizerProcess::Container, you should be 
able to generate a new ContainerInfo that has persistent volumes in it. Let me 
know if you still don't understand.

FYI, docker just merged its mount propagation support for volumes. That means 
we can have full persistent volume support. We need to mount the sandbox as a 
shared mount into the docker container. ANy mounts under the sandbox mount will 
be automatically propergated to the docker container.


- Jie


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


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> ---
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes 
> from resources passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard 
> code slave's work_dir. I also checked that the inferred directory actually 
> exists to avoid framework messing up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start 
> the container, which seems to belong to slave.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/docker/executor.cpp 7512d07de6c8324340e6b5f3e5162ef00efc47fc 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/docker_tests.cpp 
> 83eceacaddc38d0ccfc42e65e700a09406d8df36 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41892/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Alexander Rukletsov]

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

(Updated Jan. 22, 2016, 10:51 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
Van Remoortere.


Changes
---

Rebased; more cleanups.


Repository: mesos


Description
---

Changes made:
- empty resource map promoted to a const class field;
- removed variable numeric suffixes where appropriate;
- added const where appropriate.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing (updated)
---

On Mac OS 10.10.4:

`make check` 

`GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`


Thanks,

Alexander Rukletsov

Re: Review Request 40553: Enable mesos tests installation

[James Peach]

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

(Updated Jan. 22, 2016, 10:03 p.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Changes
---

Updated to get ExampleFramework tests working. Verified on Fedora 23. All 
non-root tests pass both when you install and when you run from the build 
directory. When running as root, I got 2 failures because I have swap enabled, 
and 1 because the network isolator doesn't expect the net_prio cgroup.

```
[jpeach@jpeach build-install-tests]$ /opt/mesos/libexec/mesos/tests/mesos-tests 
 --build_dir=/none --source_dir=/none
...
[==] 896 tests from 119 test cases ran. (181364 ms total)
[  PASSED  ] 896 tests.
```


Summary (updated)
-

Enable mesos tests installation


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


Repository: mesos


Description (updated)
---

This patch enables the installation mesos-tests and its dependencies
and helper tool. The goal is to allow operators to build a separate
test package that can be run at deployment time to verify that Mesos
works in the deployment environment.

Since the build directory is searched first, to run it on a host
that has a build tree, you need to specify a non-existent tree:

~ $ $PREFIX/libexec/mesos/tests/mesos-tests \
--build_dir=/none \
--source_dir=/none

- Add --enable-tests-install
- Fix mesos-tests gmock dependencies
- Optionally install tests, helpers and test modules
- Add utility helpers to find various test resources


Diffs (updated)
-

  Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
  configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
  src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
  src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
  src/tests/balloon_framework_test.sh 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
  src/tests/containerizer/launch_tests.cpp 
c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
  src/tests/containerizer/memory_test_helper.cpp 
4a3de2e3c887aa6afc604588850e1386f92d8c11 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
677fcc1c654f83ad3e60e0f6172a1a1e4a1045b1 
  src/tests/containerizer/ns_tests.cpp 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
  src/tests/containerizer/port_mapping_tests.cpp 
fab16f697f90abc11d681222f10f518d70da908b 
  src/tests/environment.cpp 4de46bc48b245a4c5e89b0343578b0d31883a0ac 
  src/tests/event_call_framework_test.sh 
9d1211552734afbf15b376f8c4629bae8a2065af 
  src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
  src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
  src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
  src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475 
  src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
  src/tests/no_executor_framework_test.sh 
aebdc8c380abb2d041d6fc74dfac5a111c15267e 
  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 
  src/tests/persistent_volume_framework_test.sh 
84f02847a8d89400512d8a5714d33fb29cf5b03a 
  src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
  src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
  src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
  src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
  src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 

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


Testing
---


Thanks,

James Peach

Review Request 42518: Disabled "drop_log_memory" flag for glog.

[Kapil Arya]

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

Review request for mesos, Ben Mahler, Cody Maloney, and Joris Van Remoortere.


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


Repository: mesos


Description
---

When this flag is set, glog makes several hundred `posix_fadvise(...,
POSIX_FADV_DONTNEED)` calls per second to advise the kernel to drop
in-memory buffers related to log contents.


Diffs
-

  src/logging/logging.cpp f7619b18fa4a78b20951edad892ca5c616bbed55 

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


Testing
---

Verified with the patch by running under gdb and placing a breakpoint on 
posix_fadvise. The breakpoint doesn't hit anymore.


Thanks,

Kapil Arya

Re: Review Request 42519: Fixed race between SchedDriver.{stop(), abort()} and SchedDriver.join().

[Kevin Klues]

> On Jan. 22, 2016, 6:58 a.m., Ben Mahler wrote:
> > Logically looks good, just a couple of trivial comments and we can get this 
> > landed!

I was going to start fixing things up, but it looks like you took care of it 
for me.  Thanks!


- Kevin


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


On Jan. 19, 2016, 10:58 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42519/
> ---
> 
> (Updated Jan. 19, 2016, 10:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-4409
> https://issues.apache.org/jira/browse/MESOS-4409
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, it was possible for join() to return before a schedDriver
> was actually fully stopped or aborted (breaking the semantics of the
> join() call). The race came from a short circuit in join(), which
> simply checked for status != DRIVER_RUNNING before returning. It appears
> this short circuit was introduced to handle cases where initialize() or
> start() ended up aborting before ever starting the driver to begin with.
> However, it unintentionally covers cases where stop() or abort() were
> called *after* the driver started running as well.
> 
> The problem is that stop() and abort() will change the status
> to DRIVER_STOPPED or DRIVER_ABORTED before actually processing
> dispatched stop or abort events (which happen asynchronously in a
> libprocess thread). Under normal operation, join() would wait for these
> events to trigger a latch that allowed the join() call to return.
> However, with the short circuit, join() exits immediately even if the
> libprocess thread hasn't yet processed the stop() or abort() events.
> 
> This commit fixes the semantics of the join() call to avoid this race.
> We considered removing the latch completely and replacing it with
> process.wait(), but, unlike the latch, this wouldn't ensure that stop()
> or abort() was ever called in the first place.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 38940b7e2563a2156be2f8c228afdc27c45b6e83 
> 
> Diff: https://reviews.apache.org/r/42519/diff/
> 
> 
> Testing
> ---
> 
> Ran the entire 'make check' suite with no failures on both Mac OS X and 
> ubuntu 14.04.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42524: Simplified SchedulerDriver.run().

[Kevin Klues]

> On Jan. 20, 2016, 1:40 a.m., Greg Mann wrote:
> > src/sched/sched.cpp, lines 1929-1930
> > 
> >
> > Since `join()` returns the current status anyway, could we just do 
> > `return join()`?

We could. I just thought it looked cleaner to make the calls unconditionally, 
followed by a return of the status.


- Kevin


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


On Jan. 19, 2016, 10:58 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42524/
> ---
> 
> (Updated Jan. 19, 2016, 10:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-4409
> https://issues.apache.org/jira/browse/MESOS-4409
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the status check in run() was unnecessary due to the short
> circuit path in join(). This commit simplifies run() by removing the
> check completely.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 38940b7e2563a2156be2f8c228afdc27c45b6e83 
> 
> Diff: https://reviews.apache.org/r/42524/diff/
> 
> 
> Testing
> ---
> 
> Ran the entire 'make check' suite with no failures on both Mac OS X and 
> ubuntu 14.04.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42530, 42362]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 22, 2016, 5:03 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 22, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled, 
> both with and without authorization enabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> without authorization and without a principal; this patch introduces test 
> cases for this scenario: 
> `PersistentVolumeEndpointsTest.NoAuthenticationNoAuthorization` and 
> `PersistentVolumeEndpointsTest.NoAuthenticationWithAuthorization`. NOTE: due 
> to the current behavior of the HTTP authentication code, when HTTP 
> authentication is disabled, the endpoint callbacks always receive a principal 
> of `None()`, which means that authorization cannot be properly used when HTTP 
> authentication is disabled.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthorization`, was added to the 
> persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42557: Moved CachedImage to a separate file.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41958, 41959, 42156, 42157, 42274, 42554, 42557]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 22, 2016, 6:52 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42557/
> ---
> 
> (Updated Jan. 22, 2016, 6:52 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently CachedImage is private to the appc store. This will change as we
> implement image fetchers (eg. simple discovery).
> 
> This change will enable image fetcher to use the same structure.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/slave/containerizer/mesos/provisioner/appc/image_cache.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/appc/image_cache.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 73c4df858a70da3d4cc4a1cb15092165f6ff8fe4 
> 
> Diff: https://reviews.apache.org/r/42557/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42633: Corrected a comment in the allocator.

[Ben Mahler]

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


Why did you make this change?

- Ben Mahler


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42633/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 18f77bcf596c2ee17b4e407fac4367b0c737ce82 
> 
> Diff: https://reviews.apache.org/r/42633/diff/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42633: Corrected a comment in the allocator.

[Qian Zhang]

> On Jan. 22, 2016, 4:16 p.m., Ben Mahler wrote:
> > Why did you make this change?

I think the main reason should be, after optimistic offer phase 1 is done, we 
will have two types of revocable resource: usage slack (oversubscription 
resources) and allocation slack.


- Qian


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


On Jan. 22, 2016, 9:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42633/
> ---
> 
> (Updated Jan. 22, 2016, 9:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 18f77bcf596c2ee17b4e407fac4367b0c737ce82 
> 
> Diff: https://reviews.apache.org/r/42633/diff/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42633: Corrected a comment in the allocator.

[Alexander Rukletsov]

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

(Updated Jan. 22, 2016, 12:43 p.m.)


Review request for mesos, Ben Mahler and Qian Zhang.


Repository: mesos


Description (updated)
---

Revocable resources may come from different sources: usage slack (aka
oversubscribed resources), allocation slack (aka optimistic offers).
We should not use "oversubscribed" and "revocable" interchangeably.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
2d01034f43c3653f6233792ee6614fa311249e5c 

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


Testing (updated)
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42636: Replaced term 'periodic allocation' for consistency.

[Alexander Rukletsov]

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

(Updated Jan. 22, 2016, 12:43 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description (updated)
---

Though "periodic" may be a better name, we refer to such allocation
cycles as "batch" in the hierarchical allocator and some other tests.
For consistency, rename all occurrences in the code.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing (updated)
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42674: Replaced busybox with alpine in docker tests.

[Isabel Jimenez]

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




src/tests/health_check_tests.cpp (line 351)


Why can't we be consistent and use the official alpine image here too?


- Isabel Jimenez


On Jan. 23, 2016, 12:02 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42674/
> ---
> 
> (Updated Jan. 23, 2016, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced busybox with alpine in docker tests.
> Busybox is currently causing issues with Docker daemon with btrfs backed 
> (https://github.com/docker/docker/issues/16755), which is the default on 
> CentOS 7.
> Tested CentOS and Ubuntu 14.04 with alpine and all Docker tests passed.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 81ab3625a69644d9659d484f3c605aaa5a10b397 
>   src/tests/containerizer/docker_tests.cpp 
> 9583070d64e96f5a57db62b3fb69073ecc5b502b 
>   src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
>   src/tests/hook_tests.cpp 152984b01069acd4cf195bfce58835f0304a97f2 
> 
> Diff: https://reviews.apache.org/r/42674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 42672: Explicitly checked for the absence of allocation.

[Alexander Rukletsov]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Ensure the future for allocation is pending when there should be no
allocations. Reduce the number of allocations triggered in the test.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing
---

On Mac OS 10.10.4:

`make check`

`GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`


Thanks,

Alexander Rukletsov

Re: Review Request 42674: Replaced busybox with alpine in docker tests.

[Isabel Jimenez]

> On Jan. 23, 2016, 12:14 a.m., Isabel Jimenez wrote:
> >

+1 for this change. Alpine is great.


- Isabel


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


On Jan. 23, 2016, 12:02 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42674/
> ---
> 
> (Updated Jan. 23, 2016, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced busybox with alpine in docker tests.
> Busybox is currently causing issues with Docker daemon with btrfs backed 
> (https://github.com/docker/docker/issues/16755), which is the default on 
> CentOS 7.
> Tested CentOS and Ubuntu 14.04 with alpine and all Docker tests passed.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 81ab3625a69644d9659d484f3c605aaa5a10b397 
>   src/tests/containerizer/docker_tests.cpp 
> 9583070d64e96f5a57db62b3fb69073ecc5b502b 
>   src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
>   src/tests/hook_tests.cpp 152984b01069acd4cf195bfce58835f0304a97f2 
> 
> Diff: https://reviews.apache.org/r/42674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 42677: Updated ReviewBot to truncate the output of review.

[Vinod Kone]

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

Review request for mesos, Ben Mahler and Michael Park.


Repository: mesos


Description
---

Truncated the size of review that ReviewBot can post to 1 MB to avoid 
overloading the ASF ReviewBoard server.


Diffs
-

  support/verify_reviews.py 39661d7ee914ef3c48a2974bd309d2531209e7ea 

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


Testing
---

Tested locally with dummy arguments.

?  mesos git:(vinod/reviewbot_tail_output) 
BUILD_URL=https://builds.apache.org/job/mesos-reviewbot/10980/ 
./support/verify_reviews.py vinod kone 1
git rev-parse HEAD
Checking if review: 42474 needs verification
Skipping blocking review 42474
Checking if review: 42503 needs verification
Skipping blocking review 42503
Checking if review: 42590 needs verification
Latest diff timestamp: 2016-01-22 03:29:36
Latest dependency change timestamp: 2016-01-22 03:37:52
Verifying review 42590
Dependent review: https://reviews.apache.org/api/review-requests/40529/ 
Dependent review: https://reviews.apache.org/api/review-requests/42547/ 
Dependent review: https://reviews.apache.org/api/review-requests/41333/ 
Dependent review: https://reviews.apache.org/api/review-requests/41334/ 
Dependent review: https://reviews.apache.org/api/review-requests/40375/ 
Applying review 40375
./support/apply-review.sh -n -r 40375
Applying review 41334
./support/apply-review.sh -n -r 41334
Applying review 41333
./support/apply-review.sh -n -r 41333
Applying review 42547
./support/apply-review.sh -n -r 42547
Applying review 40529
./support/apply-review.sh -n -r 40529
Applying review 42590
./support/apply-review.sh -n -r 42590
export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; 
./support/docker_build.sh
Posting review: Bad patch!

Reviews applied: [40375, 41334, 41333, 42547, 40529, 42590]

Failed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

Error:
 ..
+ : ubuntu:14.04
+ : gcc
+ : --verbose
+++ dirname ./support/docker_build.sh
++ cd ./support/..
++ pwd
+ MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
+ cd /Users/vinodkone/workspace/mesos
+ DOCKERFILE=Dockerfile
+ rm -f Dockerfile
+ case $OS in
+ append_dockerfile 'FROM ubuntu:14.04'
+ echo FROM ubuntu:14.04
+ append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
+ echo RUN rm -rf '/var/lib/apt/lists/*'
+ append_dockerfile 'RUN apt-get update'
+ echo RUN apt-get update
+ append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
autoconf libtool'
+ echo RUN apt-get -y install build-essential clang git maven autoconf libtool
+ append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
libev-dev'
+ echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
+ append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
+ echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
+ append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
+ echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
+ append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
+ echo ENV GTEST_OUTPUT xml:report.xml
+ case $COMPILER in
+ append_dockerfile 'ENV CC gcc'
+ echo ENV CC gcc
+ append_dockerfile 'ENV CXX g++'
+ echo ENV CXX g++
+ append_dockerfile 'WORKDIR mesos'
+ echo WORKDIR mesos
+ append_dockerfile 'COPY . /mesos/'
+ echo COPY . /mesos/
+ append_dockerfile 'RUN chown -R mesos /mesos'
+ echo RUN chown -R mesos /mesos
+ append_dockerfile 'USER mesos'
+ echo USER mesos
+ append_dockerfile 'CMD ./bootstrap && ./configure --verbose && 
DISTCHECK_CONFIGURE_FLAGS="--verbose" GLOG_v=1 MESOS_VERBOSE=1 make -j8 
distcheck'
+ echo CMD ./bootstrap '&&' ./configure --verbose '&&' 
'DISTCHECK_CONFIGURE_FLAGS="--verbose"' GLOG_v=1 MESOS_VERBOSE=1 make -j8 
distcheck
++ date +%s
+ TAG=mesos-1453508830-10633
+ docker build --no-cache=true -t mesos-1453508830-10633 .
./support/docker_build.sh: line 117: docker: command not found

Full log: https://builds.apache.org/job/mesos-reviewbot/10980/console
Error handling URL 
https://reviews.apache.org/api/review-requests/42590/reviews/: UNAUTHORIZED 
({"stat": "fail", "err": {"msg": "The username or password was not correct", 
"code": 104}})
git clean -fd
git reset --hard c365743d459c2617a17ccc1b2bb3912ab9712e3c


Thanks,

Vinod Kone

Re: Review Request 42113: Added test cases for updateAllocation.

[Guangya Liu]

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

(Updated 一月 23, 2016, 5:48 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


Summary (updated)
-

Added test cases for updateAllocation.


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


Repository: mesos


Description (updated)
---

Added test cases for updateAllocation.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 41246: Enabled slave get USAGE_SLACK metrics.

[Guangya Liu]

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

(Updated 一月 23, 2016, 6:10 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
---

Enabled slave get USAGE_SLACK metrics.


Diffs (updated)
-

  src/slave/metrics.hpp 2c22de521309806d70f9bd124359756b5e4c75b4 
  src/slave/metrics.cpp 1b73121264de3ff6eabc8f620e5b4d5930ba5d43 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 1bd973a9cf4d8b922db9bd2f3df93bad7cc743bb 
  src/tests/metrics_tests.cpp 106bea58b0714ae745df73597c702e4815523938 

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


Testing
---

$ curl "http://192.168.0.107:5051/metrics/snapshot; 2>/dev/null| jq . | grep 
usage_slack
  "slave/cpus_usage_slack_percent": 0,
  "slave/cpus_usage_slack_total": 0,
  "slave/cpus_usage_slack_used": 0,
  "slave/disk_usage_slack_percent": 0,
  "slave/disk_usage_slack_total": 0,
  "slave/disk_usage_slack_used": 0,
  "slave/mem_usage_slack_percent": 0,
  "slave/mem_usage_slack_total": 0,
  "slave/mem_usage_slack_used": 0,


Thanks,

Guangya Liu

Re: Review Request 41848: Do not enable task and executor run on different resources.

[Guangya Liu]

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

(Updated 一月 23, 2016, 6:05 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description (updated)
---

Do not enable task and executor run on different resources.


Diffs (updated)
-

  src/master/validation.cpp 222bb3451c133844c42caad0567c1e98de6ba778 
  src/tests/master_validation_tests.cpp 
6fae01fa1833ae05ec82618a4ae28ac5bd275bd5 

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


Testing
---

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="TaskValidationTest.*" --verbose


Thanks,

Guangya Liu

Re: Review Request 42123: Enabled load qos controller use USAGE SLACK revocable resources.

[Guangya Liu]

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

(Updated 一月 23, 2016, 6:08 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

The current load qos controller is calculating executor used
resources via revocable() API, but this is not right as the
revocable() will include both USAGE SLACK and ALLOCATION SLACK.

This patch is updating the load qos controller only get USAGE
SLACK revocable resources for executors.

The unit test was already covered in oversubscription_tests.cpp.


Diffs (updated)
-

  src/slave/qos_controllers/load.cpp 52520d6220784423c09001b8bd59e090ebf787ec 
  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 42672: Explicitly checked for the absence of allocation.

[Guangya Liu]

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




src/tests/hierarchical_allocator_tests.cpp (line 1480)


remove the blank line?


- Guangya Liu


On 一月 22, 2016, 11:21 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42672/
> ---
> 
> (Updated 一月 22, 2016, 11:21 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure the future for allocation is pending when there should be no
> allocations. Reduce the number of allocations triggered in the test.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> b1cb955b7eb1213c7ba4a9c5181545bb49154f06 
> 
> Diff: https://reviews.apache.org/r/42672/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check`
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42491: Updated allocatable() to distinguish different resources.

[Guangya Liu]

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

(Updated 一月 23, 2016, 6:18 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

Updated allocatable() to distinguish different resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
65c7e6b15c5308c0910667e1b12f39b21293a316 
  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing
---

make
make check
GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose --gtest_repeat=100 --gtest_shuffle


Thanks,

Guangya Liu

Re: Review Request 42358: Logger Module: Refactored Sandbox logger initialization.

[Joseph Wu]

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

(Updated Jan. 22, 2016, 11:55 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Moved `Process` init into the constructor's initializer list.


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


Repository: mesos


Description
---

In retrospect, the `if (process != NULL)` initialization check in each sandbox 
logger function is unnecessary for such a simple module.


Diffs (updated)
-

  src/slave/container_loggers/sandbox.hpp 
17bb1d9791d46c834e7d074fc72c95f093cad990 
  src/slave/container_loggers/sandbox.cpp 
1b954d64ba97f650b1024eed383ca46093ca1b6b 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 41791: Updated allocation slack when dynamic reserve called.

[Guangya Liu]

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

(Updated 一月 23, 2016, 5:40 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


Summary (updated)
-

Updated allocation slack when dynamic reserve called.


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


Repository: mesos


Description (updated)
---

Updated allocation slack when dynamic reserve called.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
2d01034f43c3653f6233792ee6614fa311249e5c 
  src/master/allocator/mesos/hierarchical.cpp 
65c7e6b15c5308c0910667e1b12f39b21293a316 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 40553: Enable mesos tests installation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [39780, 39781, 39782, 40553]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 22, 2016, 10:12 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> ---
> 
> (Updated Jan. 22, 2016, 10:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
> 
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
> 
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests \
> --build_dir=/none \
> --source_dir=/none
> 
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
> 
> 
> Diffs
> -
> 
>   Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
>   configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
>   src/tests/balloon_framework_test.sh 
> 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
>   src/tests/containerizer/launch_tests.cpp 
> c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
>   src/tests/containerizer/memory_test_helper.cpp 
> 4a3de2e3c887aa6afc604588850e1386f92d8c11 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 677fcc1c654f83ad3e60e0f6172a1a1e4a1045b1 
>   src/tests/containerizer/ns_tests.cpp 
> 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
>   src/tests/containerizer/port_mapping_tests.cpp 
> fab16f697f90abc11d681222f10f518d70da908b 
>   src/tests/environment.cpp 4de46bc48b245a4c5e89b0343578b0d31883a0ac 
>   src/tests/event_call_framework_test.sh 
> 9d1211552734afbf15b376f8c4629bae8a2065af 
>   src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
>   src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
>   src/tests/module.cpp f0e64f7c462cf833839558665fdb60d9f2cbc475 
>   src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
>   src/tests/no_executor_framework_test.sh 
> aebdc8c380abb2d041d6fc74dfac5a111c15267e 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
>   src/tests/persistent_volume_framework_test.sh 
> 84f02847a8d89400512d8a5714d33fb29cf5b03a 
>   src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
>   src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
>   src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
>   src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
>   src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 
> 
> Diff: https://reviews.apache.org/r/40553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 42605: Fixed a NULL pointer dereference bug in Slave.

[Shuai Lin]

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

Ship it!


Ship It!

- Shuai Lin


On Jan. 22, 2016, 3:57 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42605/
> ---
> 
> (Updated Jan. 22, 2016, 3:57 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4449
> https://issues.apache.org/jira/browse/MESOS-4449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes a `NULL` pointer dereference erroneously added as part of 
> introducing an output operator for the `Executor` struct in 
> https://reviews.apache.org/r/39569.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 
> 
> Diff: https://reviews.apache.org/r/42605/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42471: Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.

[Jie Yu]

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



src/common/resources.cpp (line 310)


This is not correct. You've already checked left.disk() == right.disk(), 
why do you need this? Instead, you should check if left != right:

```
if (left.disk().has_source() &&
left.disk().source().type() == MOUNT &&
left != right) {
  return false;
}
```



src/v1/resources.cpp (lines 308 - 310)


Ditto.


- Jie Yu


On Jan. 22, 2016, 9:27 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42471/
> ---
> 
> (Updated Jan. 22, 2016, 9:27 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4380
> https://issues.apache.org/jira/browse/MESOS-4380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   include/mesos/v1/mesos.hpp 961042d8e4944a475076b829966020d62175d726 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42471: Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.

[Jie Yu]

> On Jan. 22, 2016, 5:03 p.m., Jie Yu wrote:
> > src/common/resources.cpp, line 314
> > 
> >
> > This is not correct. You've already checked left.disk() == 
> > right.disk(), why do you need this? Instead, you should check if left != 
> > right:
> > 
> > ```
> > if (left.disk().has_source() &&
> > left.disk().source().type() == MOUNT &&
> > left != right) {
> >   return false;
> > }
> > ```

Also, could you please add a test case for this?


- Jie


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


On Jan. 22, 2016, 9:27 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42471/
> ---
> 
> (Updated Jan. 22, 2016, 9:27 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4380
> https://issues.apache.org/jira/browse/MESOS-4380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Multiple Disk: Adjusted resource arithmetic for 'DiskInfo.Source'.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   include/mesos/v1/mesos.hpp 961042d8e4944a475076b829966020d62175d726 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/42471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42472: Multiple Disk: Checkpoint persistent volume based on 'DiskInfo.Source'.

[Jie Yu]

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

Ship it!



src/slave/slave.cpp (lines 2256 - 2258)


This fits in one line?


- Jie Yu


On Jan. 22, 2016, 9:29 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42472/
> ---
> 
> (Updated Jan. 22, 2016, 9:29 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4400
> https://issues.apache.org/jira/browse/MESOS-4400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now create persistent volume directories based on 'DiskInfo.Source'
> if it is present.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp 6b9a65d2e8ed771041adce6f8c6fb0601422c6e8 
>   src/slave/paths.cpp 9315b4cf0664a381d02f5cc72083eb22055daf4c 
>   src/slave/slave.cpp e23c3295c8ebed580751a5aabaf26e1773c54859 
>   src/tests/paths_tests.cpp 5e8495f2a19a4641e258f78911772b991d34d85a 
> 
> Diff: https://reviews.apache.org/r/42472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42473: Multiple Disk: Updated filesystem isolators to use 'DiskInfo.Source'.

[Jie Yu]

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

Ship it!



src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (lines 613 - 615)


This fits in one line?



src/slave/containerizer/mesos/isolators/filesystem/posix.cpp (lines 185 - 187)


Ditto. This fits in one line?


- Jie Yu


On Jan. 22, 2016, 9:29 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42473/
> ---
> 
> (Updated Jan. 22, 2016, 9:29 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4402
> https://issues.apache.org/jira/browse/MESOS-4402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Multiple Disk: Updated filesystem isolators to use 'DiskInfo.Source'.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 96c37ed7ac27074eb79e3eb5e0301b642ab653f2 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> d914587eedc9c66bc14b4088ec211b7f0eea63ea 
> 
> Diff: https://reviews.apache.org/r/42473/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42629: Added tests for offer filters.

[Alexander Rukletsov]

> On Jan. 22, 2016, 8:29 a.m., Ben Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 509-515
> > 
> >
> > Looks like you only need the second advance here? Technically, you are 
> > triggering two batch allocations here, which doesn't appear to be the 
> > intent?
> > 
> > It would mean we need a Clock::settle after calling recoverResources.

I was thinking about the following: if we remove the first advance, can there 
be a race when `expire()` is processed *after* the second allocation cycle? I 
think it can if we do not ensure the filter is set *before* the first batch 
allocation (your addition).

Re: triggering two batch allocations here, it doesn't matter because we are 
interested only in the next allocation, but I agree it may distract the reader.

Marking as "fixed".


> On Jan. 22, 2016, 8:29 a.m., Ben Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 525
> > 
> >
> > Some unicode character here?

:facepalm:


> On Jan. 22, 2016, 8:29 a.m., Ben Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 1415
> > 
> >
> > What is a Quarantee? ;)

double :facepalm: ...


> On Jan. 22, 2016, 8:29 a.m., Ben Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 1513-1522
> > 
> >
> > This change actually belongs in the previous patch, since your last 
> > change breaks this test on its own.

Correct, thanks Ben!


> On Jan. 22, 2016, 8:29 a.m., Ben Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 1497-1502
> > 
> >
> > Let's leave this for now, but it would be great to avoid this 
> > assumption and explicitly control the allocation interval duration, no? 
> > When you are advancing the clock below twice, you are triggering two batch 
> > allocations, when your intention appears to be to trigger only one batch 
> > allocation.
> > 
> > For now I'll fix this same issue in the OfferFilter test, and I'll 
> > leave this one as still triggering two allocations since it's an existing 
> > test.

Yeah, may be a bit misleading for the reader. I'll note it down and propose a 
patch some time in the future.


- Alexander


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


On Jan. 22, 2016, 1:24 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42629/
> ---
> 
> (Updated Jan. 22, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4302
> https://issues.apache.org/jira/browse/MESOS-4302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42629/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check`
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.FilterTimeout" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure` passes with the patch and fails 
> without.
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42353: Correctted typo in fetcher.md.

[Bernd Mathiske]

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



docs/fetcher.md (line 236)


Suggestions: 
s/the names/these variable names
s/all in/entirely in


- Bernd Mathiske


On Jan. 22, 2016, 5:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42353/
> ---
> 
> (Updated Jan. 22, 2016, 5:18 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected typo in fetcher.md.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md d9280e1611df13f8217c9a40a02a5391c44ed267 
> 
> Diff: https://reviews.apache.org/r/42353/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42353: Correctted typo in fetcher.md.

[Guangya Liu]

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

(Updated Jan. 22, 2016, 5:18 a.m.)


Review request for mesos and Bernd Mathiske.


Changes
---

Corrected typo in summary.


Repository: mesos


Description (updated)
---

Corrected typo in fetcher.md.


Diffs
-

  docs/fetcher.md d9280e1611df13f8217c9a40a02a5391c44ed267 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 42374: Logger Module: Add test filter for tests requiring `logrotate`.

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Jan. 19, 2016, 7:41 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42374/
> ---
> 
> (Updated Jan. 19, 2016, 7:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Artem 
> Harutyunyan.
> 
> 
> Bugs: MESOS-4136
> https://issues.apache.org/jira/browse/MESOS-4136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `logrotate` binary is used by the non-default `RotatingContainerLogger` 
> module.  On some systems, `logrotate` is not provided by default.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp a6322f260c23796ceaa5d2080126ea9fef0b5ac6 
> 
> Diff: https://reviews.apache.org/r/42374/diff/
> 
> 
> Testing
> ---
> 
> See later reviews.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 42440: Create a test macro to advance `Clock` for `Future`.

[Jian Qiu]

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



3rdparty/libprocess/include/process/gtest.hpp (line 63)


Maybe change this to advanceInterval?



3rdparty/libprocess/include/process/gtest.hpp (line 65)


If the clock is paused outside, there is no need to pause here


- Jian Qiu


On 一月 17, 2016, 4:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42440/
> ---
> 
> (Updated 一月 17, 2016, 4:30 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jian Qiu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4418
> https://issues.apache.org/jira/browse/MESOS-4418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create a test macro to advance `Clock` for `Future`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 049152c0535c78e6986346610735d301fb6876bc 
> 
> Diff: https://reviews.apache.org/r/42440/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42440: Create a test macro to advance `Clock` for `Future`.

[haosdent huang]

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

(Updated Jan. 22, 2016, 4:56 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jian Qiu, and 
Timothy Chen.


Changes
---

Address @qiujian's comments


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


Repository: mesos


Description
---

Create a test macro to advance `Clock` for `Future`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/gtest.hpp 
049152c0535c78e6986346610735d301fb6876bc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42440: Create a test macro to advance `Clock` for `Future`.

[haosdent huang]

> On Jan. 22, 2016, 1:54 p.m., Jian Qiu wrote:
> > 3rdparty/libprocess/include/process/gtest.hpp, line 65
> > 
> >
> > If the clock is paused outside, there is no need to pause here

Thank you very much, could you help review again?


- haosdent


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


On Jan. 22, 2016, 4:56 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42440/
> ---
> 
> (Updated Jan. 22, 2016, 4:56 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jian Qiu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4418
> https://issues.apache.org/jira/browse/MESOS-4418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create a test macro to advance `Clock` for `Future`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 049152c0535c78e6986346610735d301fb6876bc 
> 
> Diff: https://reviews.apache.org/r/42440/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42441: Decrease scheduler authenticate timeout.

[haosdent huang]

> On Jan. 22, 2016, 1:59 p.m., Jian Qiu wrote:
> > The reason I think is that the master starting from local drop the 
> > framework authentication message before it is fully started which cause the 
> > authentication timeout. I am not sure whether we can delay the start of 
> > framework until master has been fully started?

Because add a delay in a example looks strange and may let user confuse, I 
change the authenticate timeout here.


- haosdent


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


On Jan. 17, 2016, 6:27 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42441/
> ---
> 
> (Updated Jan. 17, 2016, 6:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jian Qiu.
> 
> 
> Bugs: MESOS-4155
> https://issues.apache.org/jira/browse/MESOS-4155
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Decrease scheduler authenticate timeout.
> 
> 
> Diffs
> -
> 
>   src/sched/constants.hpp 523c6d94571ae22b1aa8fe786f5c9f965039ea7a 
>   src/sched/constants.cpp c00b8ba31d5f24f99e99a9ec7034b641929246c0 
>   src/sched/flags.hpp 57dbab8ad7f8d4829d15627caf0e69ef91702081 
>   src/sched/sched.cpp 38940b7e2563a2156be2f8c228afdc27c45b6e83 
>   src/tests/event_call_framework_test.sh 
> 9d1211552734afbf15b376f8c4629bae8a2065af 
>   src/tests/java_exception_test.sh fd02322096d09cee6cc22340f18681ad372867b5 
>   src/tests/java_framework_test.sh 8fccc699133c6db6130153c367ce01ba3931c1a2 
>   src/tests/no_executor_framework_test.sh 
> aebdc8c380abb2d041d6fc74dfac5a111c15267e 
>   src/tests/persistent_volume_framework_test.sh 
> 84f02847a8d89400512d8a5714d33fb29cf5b03a 
>   src/tests/python_framework_test.sh dc3a50f4fe8d3340429996622e62f732941985a2 
>   src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
> 
> Diff: https://reviews.apache.org/r/42441/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*"
> [==] Running 8 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 8 tests from ExamplesTest
> [ RUN  ] ExamplesTest.TestFramework
> [   OK ] ExamplesTest.TestFramework (1162 ms)
> [ RUN  ] ExamplesTest.NoExecutorFramework
> [   OK ] ExamplesTest.NoExecutorFramework (1797 ms)
> [ RUN  ] ExamplesTest.EventCallFramework
> [   OK ] ExamplesTest.EventCallFramework (2289 ms)
> [ RUN  ] ExamplesTest.PersistentVolumeFramework
> [   OK ] ExamplesTest.PersistentVolumeFramework (4053 ms)
> [ RUN  ] ExamplesTest.JavaFramework
> [   OK ] ExamplesTest.JavaFramework (2601 ms)
> [ RUN  ] ExamplesTest.JavaException
> [   OK ] ExamplesTest.JavaException (782 ms)
> [ RUN  ] ExamplesTest.JavaLog
> [   OK ] ExamplesTest.JavaLog (2571 ms)
> [ RUN  ] ExamplesTest.PythonFramework
> [   OK ] ExamplesTest.PythonFramework (2034 ms)
> [--] 8 tests from ExamplesTest (17291 ms total)
> 
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42157: Changed ProvisionerAppcTest to use AppcStoreTest suite.

[Jojy Varghese]

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

(Updated Jan. 22, 2016, 5:36 p.m.)


Review request for mesos and Jie Yu.


Changes
---

rebased


Repository: mesos


Description
---

This change will enable ProvisionerAppcTest suite to reuse common code like
test image creation.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
e3d08d9e49df93d5290099c8bfd917f60c93e51b 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42554: Added 'dependencies' message to AppcImageManifest.

[Jojy Varghese]

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

(Updated Jan. 22, 2016, 5:38 p.m.)


Review request for mesos and Jie Yu.


Changes
---

rebased


Repository: mesos


Description (updated)
---

Added 'dependencies' message to AppcImageManifest.


Diffs (updated)
-

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 42662: Added common command utils file.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

This common file is a good place to add common command line utilities like tar,
digests(sha256, sha512, etc). Currently this functionality is spread in the code
base and this change would enable all those call sites to be replaced with a
common code.


Diffs
-

  src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
  src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
  src/common/command_utils.hpp PRE-CREATION 
  src/common/command_utils.cpp PRE-CREATION 
  src/tests/common/command_utils_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42474: Multiple Disk: Updated Slave initialize to create DiskInfo paths.

[Jie Yu]

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

Ship it!



src/slave/slave.cpp (line 364)


I would still prefer to capture the reference 'const 
Resource::DiskInfo::Source& source' first to avoid calling 
`resource.disk().source()` all the time:
```
const Resource::DiskInfo::Source& source = resource.disk.source();
if (source.type() == PATH) {
  CHECK(source.has_path());
  
  Try mkdir = os::mkdir(source.path().root());
  ...
}
```



src/slave/slave.cpp (lines 365 - 368)


We should validate the Resources (i.e., call `Resources::validate` in 
`Containerizer::resources(flag)`), and this should a CHECK instead.

Also, can you add a TODO to move 
`internal::validateCommandLineResources(result);` in Resources::parse to top 
level (e.g., in `Containerizer::resources`). I don't think that function 
belongs to the parse function.


- Jie Yu


On Jan. 22, 2016, 9:30 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42474/
> ---
> 
> (Updated Jan. 22, 2016, 9:30 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4403
> https://issues.apache.org/jira/browse/MESOS-4403
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create disk paths based on 'DiskInfo.Source' if the type is 'PATH'
> and the directory does not yet exist.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e23c3295c8ebed580751a5aabaf26e1773c54859 
> 
> Diff: https://reviews.apache.org/r/42474/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 40632: Enabled oversubscribed resources for reservations in allocator.

[Guangya Liu]

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

(Updated 一月 23, 2016, 3:18 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description (updated)
---

Enabled oversubscribed resources for reservations in allocator.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
65c7e6b15c5308c0910667e1b12f39b21293a316 
  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing
---

make
make check
GLOG_v=2  ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose --gtest_repeat=100 --gtest_shuffle


Thanks,

Guangya Liu

Re: Review Request 41847: Updated allocation slack when slave was updated.

[Guangya Liu]

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

(Updated 一月 23, 2016, 3:26 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

Updated allocation slack when slave was updated.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
65c7e6b15c5308c0910667e1b12f39b21293a316 
  src/tests/hierarchical_allocator_tests.cpp 
b1cb955b7eb1213c7ba4a9c5181545bb49154f06 

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


Testing
---

make
make check
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose


Thanks,

Guangya Liu

Re: Review Request 42518: Disabled "drop_log_memory" flag for glog.

[Kapil Arya]

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

(Updated Jan. 22, 2016, 10:59 p.m.)


Review request for mesos, Ben Mahler, Cody Maloney, and Joris Van Remoortere.


Changes
---

Update the flag only if env var is not set.


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


Repository: mesos


Description
---

When this flag is set, glog makes several hundred `posix_fadvise(...,
POSIX_FADV_DONTNEED)` calls per second to advise the kernel to drop
in-memory buffers related to log contents.


Diffs (updated)
-

  src/logging/logging.cpp f7619b18fa4a78b20951edad892ca5c616bbed55 

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


Testing
---

Verified with the patch by running under gdb and placing a breakpoint on 
posix_fadvise. The breakpoint doesn't hit anymore.


Thanks,

Kapil Arya

Re: Review Request 41291: Modified the scheduler tests to use the new executor HTTP based library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [41275, 41277, 41280, 41281, 41282, 41283, 41288, 41290, 41291]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 22, 2016, 8:35 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41291/
> ---
> 
> (Updated Jan. 22, 2016, 8:35 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4457
> https://issues.apache.org/jira/browse/MESOS-4457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change modifies the existing scheduler tests to use the new executor 
> HTTP library instead of the old driver based interface.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 67432109c7df6be0aa76e94a03bd5b2e9c96d14e 
> 
> Diff: https://reviews.apache.org/r/41291/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 42673: Silenced two more GMock warnings about shutdown expectations.

[Neil Conway]

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

In these tests:
* MasterTest.OrphanTasks
* StatusUpdateManagerTest.LatestTaskState


Diffs
-

  src/tests/master_tests.cpp cd426fda633c2d665b9533271610863062b1ed4e 
  src/tests/status_update_manager_tests.cpp 
24302d8627e635f3825a31d5383de2448ad9d557 

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


Testing
---

Verified that GMock warnings are observed w/o this review; after applying the 
review and running `--gtest_repeat=100`, no warnings were observed.


Thanks,

Neil Conway

Review Request 42674: Replaced busybox with alpine in docker tests.

[Timothy Chen]

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

Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Jie Yu.


Repository: mesos


Description
---

Replaced busybox with alpine in docker tests.
Busybox is currently causing issues with Docker daemon with btrfs backed 
(https://github.com/docker/docker/issues/16755), which is the default on CentOS 
7.
Tested CentOS and Ubuntu 14.04 with alpine and all Docker tests passed.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
81ab3625a69644d9659d484f3c605aaa5a10b397 
  src/tests/containerizer/docker_tests.cpp 
9583070d64e96f5a57db62b3fb69073ecc5b502b 
  src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
  src/tests/hook_tests.cpp 152984b01069acd4cf195bfce58835f0304a97f2 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 41772: Added helper function to flatten resources.

[Guangya Liu]

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

(Updated 一月 23, 2016, 2:17 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description (updated)
---

Added helper function to flatten allocation slack resources.


Diffs (updated)
-

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
  src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
---

make
make check

GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="ResourcesOperationTest.*" 
--verbose


Thanks,

Guangya Liu

Re: Review Request 42674: Replaced busybox with alpine in docker tests.

[Guangya Liu]

> On 一月 23, 2016, 1 a.m., Guangya Liu wrote:
> > I saw that many test cases are using busybox, why only updating this one's 
> > image? Is it possible to update all busybox to alpine? As the alpine was 
> > built around musl libc and BusyBox. The image is only 5 MB in size and has 
> > access to a package repository that is much more complete than other 
> > BusyBox based images.
> 
> Timothy Chen wrote:
> Hi Guangya, there aren't any tests that I know of still using busybox. If 
> you search for busybox in the tests folder, they are just references and not 
> actually pulling or using the busybox image itself. The intention of this is 
> to replace all busybox images to alpine.

What about this one 
https://github.com/apache/mesos/blob/master/src/examples/docker_no_executor_framework.cpp#L106
 ? This was not a unit test, but may also deserve a fix?


- Guangya


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


On 一月 23, 2016, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42674/
> ---
> 
> (Updated 一月 23, 2016, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced busybox with alpine in docker tests.
> Busybox is currently causing issues with Docker daemon with btrfs backed 
> (https://github.com/docker/docker/issues/16755), which is the default on 
> CentOS 7.
> Tested CentOS and Ubuntu 14.04 with alpine and all Docker tests passed.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 81ab3625a69644d9659d484f3c605aaa5a10b397 
>   src/tests/containerizer/docker_tests.cpp 
> 9583070d64e96f5a57db62b3fb69073ecc5b502b 
>   src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
>   src/tests/hook_tests.cpp 152984b01069acd4cf195bfce58835f0304a97f2 
> 
> Diff: https://reviews.apache.org/r/42674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 41772: Added helper function to flatten resources.

[Guangya Liu]

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

(Updated 一月 23, 2016, 2:59 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, and Klaus Ma.


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


Repository: mesos


Description
---

Added helper function to flatten allocation slack resources.


Diffs (updated)
-

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
  src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
  src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 

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


Testing
---

make
make check

GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="ResourcesOperationTest.*" 
--verbose


Thanks,

Guangya Liu

Re: Review Request 41783: Logger Module: Implement the rotating container logger module.

[Joseph Wu]

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

(Updated Jan. 22, 2016, 7:04 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Fix bug with launching the subprocess with the agent's environment.
Change subprocess's looping to have a future chain of max length 2.


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


Repository: mesos


Description
---

Adds a non-default ContainerLogger that constrains total log size by rotating 
logs (i.e. renaming the head log file).


Diffs (updated)
-

  src/slave/container_loggers/lib_logrotate.hpp PRE-CREATION 
  src/slave/container_loggers/lib_logrotate.cpp PRE-CREATION 
  src/slave/container_loggers/logrotate.hpp PRE-CREATION 
  src/slave/container_loggers/logrotate.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 42477: Corrected example in quota user doc.

[Guangya Liu]

> On 一月 22, 2016, 4:34 p.m., Bernd Mathiske wrote:
> > docs/quota.md, line 166
> > 
> >
> > Please explain what this seemingly redundant field is doing.

I think this is because the current Quota can only works for unreserved and non 
revocable resources. There is indeed a document 
https://github.com/apache/mesos/blob/master/docs/quota.md for this, make sense?


- Guangya


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


On 一月 22, 2016, 3:43 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42477/
> ---
> 
> (Updated 一月 22, 2016, 3:43 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/quota.md 208bfa06a9fc50843439bab9a041ccb557657b5d 
> 
> Diff: https://reviews.apache.org/r/42477/diff/
> 
> 
> Testing
> ---
> 
> None: Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42353: Correctted typo in fetcher.md.

[Guangya Liu]

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

(Updated 一月 23, 2016, 12:54 a.m.)


Review request for mesos and Bernd Mathiske.


Repository: mesos


Description (updated)
---

Correctted typo in fetcher.md.


Diffs (updated)
-

  docs/fetcher.md d9280e1611df13f8217c9a40a02a5391c44ed267 

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


Testing
---


Thanks,

Guangya Liu

Review Request 42678: Avoid `slave.total` do not contain `slave.allocated` in allocator.

[Klaus Ma]

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

Review request for mesos.


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


Repository: mesos


Description
---

Added test case for MESOS-4442.


Diffs
-

  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 42674: Replaced busybox with alpine in docker tests.

[Guangya Liu]

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



I saw that many test cases are using busybox, why only updating this one's 
image? Is it possible to update all busybox to alpine? As the alpine was built 
around musl libc and BusyBox. The image is only 5 MB in size and has access to 
a package repository that is much more complete than other BusyBox based images.

- Guangya Liu


On 一月 23, 2016, 12:02 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42674/
> ---
> 
> (Updated 一月 23, 2016, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced busybox with alpine in docker tests.
> Busybox is currently causing issues with Docker daemon with btrfs backed 
> (https://github.com/docker/docker/issues/16755), which is the default on 
> CentOS 7.
> Tested CentOS and Ubuntu 14.04 with alpine and all Docker tests passed.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 81ab3625a69644d9659d484f3c605aaa5a10b397 
>   src/tests/containerizer/docker_tests.cpp 
> 9583070d64e96f5a57db62b3fb69073ecc5b502b 
>   src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
>   src/tests/hook_tests.cpp 152984b01069acd4cf195bfce58835f0304a97f2 
> 
> Diff: https://reviews.apache.org/r/42674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 42678: WIP: Avoid `slave.total` do not contain `slave.allocated` in allocator.

[Klaus Ma]

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

(Updated Jan. 23, 2016, 9:16 a.m.)


Review request for mesos.


Changes
---

Add description


Summary (updated)
-

WIP: Avoid `slave.total` do not contain `slave.allocated` in allocator.


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


Repository: mesos


Description (updated)
---

__Phenomenon__:
In master log, `slave.allocated` may have more resources than `slave.total`.

__Root Cause__:
Not trace revocable resources in allocator; so the info maybe different between 
Master/Slave because of race condition. 

__Solution/Fix__:
Trace revocable resources in allocator.


Diffs
-

  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 

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


Testing (updated)
---

make check GTEST_FILTER="OversubscriptionTest.OverOffered"


Thanks,

Klaus Ma

Re: Review Request 42674: Replaced busybox with alpine in docker tests.

[Timothy Chen]

> On Jan. 23, 2016, 12:14 a.m., Isabel Jimenez wrote:
> > src/tests/health_check_tests.cpp, line 351
> > 
> >
> > Why can't we be consistent and use the official alpine image here too?

Sorry that was a mistake.


- Timothy


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


On Jan. 23, 2016, 12:02 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42674/
> ---
> 
> (Updated Jan. 23, 2016, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced busybox with alpine in docker tests.
> Busybox is currently causing issues with Docker daemon with btrfs backed 
> (https://github.com/docker/docker/issues/16755), which is the default on 
> CentOS 7.
> Tested CentOS and Ubuntu 14.04 with alpine and all Docker tests passed.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 81ab3625a69644d9659d484f3c605aaa5a10b397 
>   src/tests/containerizer/docker_tests.cpp 
> 9583070d64e96f5a57db62b3fb69073ecc5b502b 
>   src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
>   src/tests/hook_tests.cpp 152984b01069acd4cf195bfce58835f0304a97f2 
> 
> Diff: https://reviews.apache.org/r/42674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 42674: Replaced busybox with alpine in docker tests.

[Timothy Chen]

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

(Updated Jan. 23, 2016, 1:26 a.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Jie Yu.


Repository: mesos


Description
---

Replaced busybox with alpine in docker tests.
Busybox is currently causing issues with Docker daemon with btrfs backed 
(https://github.com/docker/docker/issues/16755), which is the default on CentOS 
7.
Tested CentOS and Ubuntu 14.04 with alpine and all Docker tests passed.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
81ab3625a69644d9659d484f3c605aaa5a10b397 
  src/tests/containerizer/docker_tests.cpp 
9583070d64e96f5a57db62b3fb69073ecc5b502b 
  src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
  src/tests/hook_tests.cpp 152984b01069acd4cf195bfce58835f0304a97f2 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 42674: Replaced busybox with alpine in docker tests.

[Timothy Chen]

> On Jan. 23, 2016, 1 a.m., Guangya Liu wrote:
> > I saw that many test cases are using busybox, why only updating this one's 
> > image? Is it possible to update all busybox to alpine? As the alpine was 
> > built around musl libc and BusyBox. The image is only 5 MB in size and has 
> > access to a package repository that is much more complete than other 
> > BusyBox based images.

Hi Guangya, there aren't any tests that I know of still using busybox. If you 
search for busybox in the tests folder, they are just references and not 
actually pulling or using the busybox image itself. The intention of this is to 
replace all busybox images to alpine.


- Timothy


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


On Jan. 23, 2016, 12:02 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42674/
> ---
> 
> (Updated Jan. 23, 2016, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced busybox with alpine in docker tests.
> Busybox is currently causing issues with Docker daemon with btrfs backed 
> (https://github.com/docker/docker/issues/16755), which is the default on 
> CentOS 7.
> Tested CentOS and Ubuntu 14.04 with alpine and all Docker tests passed.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 81ab3625a69644d9659d484f3c605aaa5a10b397 
>   src/tests/containerizer/docker_tests.cpp 
> 9583070d64e96f5a57db62b3fb69073ecc5b502b 
>   src/tests/health_check_tests.cpp 3606ce46bfa283ad0d5239fc25e02c5a9f8d1a53 
>   src/tests/hook_tests.cpp 152984b01069acd4cf195bfce58835f0304a97f2 
> 
> Diff: https://reviews.apache.org/r/42674/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>