Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Qian Zhang]

> On Oct. 26, 2015, 9:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.
> 
> Alexander Rukletsov wrote:
> I'm not sure I got your point. If my mental compiler is correct, if a 
> framework in quota'ed role opts out, we do not immediately lay aside 
> resources. We do that after we have checked all the frameworks in the role in 
> a separate loop.
> 
> Qian Zhang wrote:
> Let me clarify my point with an example:
> Say in the Mesos cluster, there are 2 agents, a1 and a2, each has 4GB 
> memory. And there are 2 roles, r1 and r2, r1 has a quota set (4GB) and r2 has 
> not quota set. r1 has a framework f1 which currenlty has no allocation but 
> has a filter (4GB memory on a1), r2 also has a framework f2 which currently 
> has no allocation too and has no filter. And there is no static/dynamic 
> reservation and no revocable resources. Now with the logic in this patch, for 
> a1, in the quotaRoleSorter foreach loop, when we handle the quota for r1, we 
> will not allocate a1's resouces to f1 because f1 has a filter, so a1's 4GB 
> memory will be laid aside to satisfy r1's quota. And then in the roleSorter 
> foreach loop, we will NOT allocate a1's resources to f2 too since currently 
> a1 has no available resources due to all its 4GB memory has been laid aside 
> for r1. And then when we handle a2, its 4GB memory will be allocated to f1, 
> so f2 will not get anything in the end. So the result is, a1's 4GB memory is 
> laid aside
  to satisfy r1's quota, a2's 4GB is allocated to f1, and f2 gets nothing. But 
I think for this example, the expected result should be, f1 gets a2's 4GB (r1's 
quota is also satisfied) and f2 gets a1's 4GB.
> 
> Alexander Rukletsov wrote:
> This can happen during the allocation cycle, but we do not persist laid 
> aside resources between allocation cycles. Without refactoring `allocate()` 
> we do not know whether we get a suitable agent, hence we have to lay aside. 
> But at the next allocation cycle, `r1`'s quota is satisfied and `f2` will get 
> `a1`'s 4GB, which is OK in my opinion.

Yes, I understand ```f2``` will get 4GB at the ***next*** allocation cycle. But 
with the proposal in my first post, in a ***single*** allocation cycle, both 
```f1``` and ```f2``` can get 4GB respectively because when we find we can not 
allocate ```a1```'s 4GB to f1 due to the filter, we will NOT lay aside 
resources at this point, instead we will try ```a2``` and allocate ```a2```'s 
4GB to f1, and then when we handle the fair share, we will allocate ```a2```'s 
4GB to ```f2```. I think this proposal is also aligned with the principle 
mentioned in the design doc: quota first, fair share second. My understanding 
to this design principle is, we handle all role's quota for all slaves first, 
and then handle all role's fair share for all slaves (my proposal), rather than 
for ***each slave*** handle all role's quota and then all role's fair share 
(this patch).


- Qian


---
This i

Re: Review Request 39400: Quota: Implemented quota API.

[Qian Zhang]

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

Ship it!


Ship It!

- Qian Zhang


On Oct. 28, 2015, 3:17 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Oct. 28, 2015, 3:17 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37853]

All tests passed.

- Mesos ReviewBot


On Oct. 28, 2015, 3:58 a.m., Mei Wan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37853/
> ---
> 
> (Updated Oct. 28, 2015, 3:58 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.
> 
> 
> Bugs: MESOS-2971
> https://issues.apache.org/jira/browse/MESOS-2971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the overlay filesystem backend by layering the images as a 
> read-only filesystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6c7519b 
>   src/Makefile.am d6eb302 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 35ced4b 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 3347d58 
> 
> Diff: https://reviews.apache.org/r/37853/diff/
> 
> 
> Testing
> ---
> 
> I haven't done any official testing. When I was working off Ian's branch, I 
> tested it manually and the provisioning works.
> 
> 
> Thanks,
> 
> Mei Wan
> 
>

Re: Review Request 39709: Added logic to ensure the during a HTTP to PID scheduler downgrade, the previous HTTP instance gets an error message

[Anand Mazumdar]

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

(Updated Oct. 28, 2015, 4:54 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Repository: mesos


Description
---

Presently, when there is a downgrade from HTTP to PID based scheduler, we did 
not use to send the previous HTTP scheduler instance an error message. Modified 
the logic to send in an error message to the old HTTP based instance.


Diffs
-

  src/master/master.cpp 39ce9bc50ad0bd95ee83800a305b69670fba5ffb 

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


Testing
---

make check + a test to verify HTTP to PID downgrade in another review in the 
chain


Thanks,

Anand Mazumdar

Re: Review Request 39710: Added test to verify downgrade from HTTP to PID based schedulers

[Anand Mazumdar]

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

(Updated Oct. 28, 2015, 4:54 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Repository: mesos


Description
---

See summary


Diffs
-

  src/tests/scheduler_http_api_tests.cpp 
d338a1bc6fdb3aa0b40e5727991181b31b2e7592 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 39709: Added logic to ensure the during a HTTP to PID scheduler downgrade, the previous HTTP instance gets an error message

[Anand Mazumdar]

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

(Updated Oct. 28, 2015, 4:53 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
---

Added deps


Repository: mesos


Description
---

Presently, when there is a downgrade from HTTP to PID based scheduler, we did 
not use to send the previous HTTP scheduler instance an error message. Modified 
the logic to send in an error message to the old HTTP based instance.


Diffs
-

  src/master/master.cpp 39ce9bc50ad0bd95ee83800a305b69670fba5ffb 

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


Testing
---

make check + a test to verify HTTP to PID downgrade in another review in the 
chain


Thanks,

Anand Mazumdar

Re: Review Request 39710: Added test to verify downgrade from HTTP to PID based schedulers

[Anand Mazumdar]

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

(Updated Oct. 28, 2015, 4:53 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
---

Added dep


Repository: mesos


Description
---

See summary


Diffs
-

  src/tests/scheduler_http_api_tests.cpp 
d338a1bc6fdb3aa0b40e5727991181b31b2e7592 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 39710: Added test to verify downgrade from HTTP to PID based schedulers

[Anand Mazumdar]

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

Review request for mesos.


Repository: mesos


Description
---

See summary


Diffs
-

  src/tests/scheduler_http_api_tests.cpp 
d338a1bc6fdb3aa0b40e5727991181b31b2e7592 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 39709: Added logic to ensure the during a HTTP to PID scheduler downgrade, the previous HTTP instance gets an error message

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Repository: mesos


Description
---

Presently, when there is a downgrade from HTTP to PID based scheduler, we did 
not use to send the previous HTTP scheduler instance an error message. Modified 
the logic to send in an error message to the old HTTP based instance.


Diffs
-

  src/master/master.cpp 39ce9bc50ad0bd95ee83800a305b69670fba5ffb 

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


Testing
---

make check + a test to verify HTTP to PID downgrade in another review in the 
chain


Thanks,

Anand Mazumdar

Review Request 39708: Removed redundant code for conversion from PID to HTTP based frameworks

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Summary (updated)
-

Removed redundant code for conversion from PID to HTTP based frameworks


Repository: mesos


Description (updated)
---

This conversion logic is redundant and is already being taken care of as part 
of the `updateConnection` function.

https://github.com/apache/mesos/blob/master/src/master/master.hpp#L1736


Diffs (updated)
-

  src/master/master.cpp 39ce9bc50ad0bd95ee83800a305b69670fba5ffb 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mei Wan]

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

(Updated Oct. 28, 2015, 3:58 a.m.)


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


Changes
---

fixed indent


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  src/CMakeLists.txt 6c7519b 
  src/Makefile.am d6eb302 
  src/slave/containerizer/mesos/provisioner/backend.cpp 35ced4b 
  src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 3347d58 

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


Testing
---

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


Thanks,

Mei Wan

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [37853]

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

Error:
 2015-10-28 03:55:33 URL:https://reviews.apache.org/r/37853/diff/raw/ 
[12573/12573] -> "37853.patch" [1]
Successfully applied: Overlay filesystem provisioning backend

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


Review: https://reviews.apache.org/r/37853
Checking 4 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
src/tests/containerizer/provisioner_backend_tests.cpp:34:  Weird number of 
spaces at line-start.  Are you using a 2-space indent?  [whitespace/indent] [3]
Total errors found: 1
Failed to commit patch

- Mesos ReviewBot


On Oct. 28, 2015, 3:39 a.m., Mei Wan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37853/
> ---
> 
> (Updated Oct. 28, 2015, 3:39 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.
> 
> 
> Bugs: MESOS-2971
> https://issues.apache.org/jira/browse/MESOS-2971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the overlay filesystem backend by layering the images as a 
> read-only filesystem.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6c7519b 
>   src/Makefile.am d6eb302 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 35ced4b 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 3347d58 
> 
> Diff: https://reviews.apache.org/r/37853/diff/
> 
> 
> Testing
> ---
> 
> I haven't done any official testing. When I was working off Ian's branch, I 
> tested it manually and the provisioning works.
> 
> 
> Thanks,
> 
> Mei Wan
> 
>

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mei Wan]

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

(Updated Oct. 28, 2015, 3:39 a.m.)


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


Changes
---

removed binary files


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  src/CMakeLists.txt 6c7519b 
  src/Makefile.am d6eb302 
  src/slave/containerizer/mesos/provisioner/backend.cpp 35ced4b 
  src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 3347d58 

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


Testing
---

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


Thanks,

Mei Wan

Re: Review Request 37853: Overlay filesystem provisioning backend

[Mei Wan]

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

(Updated Oct. 28, 2015, 3:35 a.m.)


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


Changes
---

Started writing tests!


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  src/CMakeLists.txt 6c7519b 
  src/Makefile.am d6eb302 
  src/slave/.DS_Store PRE-CREATION 
  src/slave/containerizer/.DS_Store PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/backend.cpp 35ced4b 
  src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_backend_tests.cpp 3347d58 

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


Testing
---

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


Thanks,

Mei Wan

Re: Review Request 39707: Minor code refactor for fetcher.cpp.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39707]

All tests passed.

- Mesos ReviewBot


On Oct. 28, 2015, 1:45 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39707/
> ---
> 
> (Updated Oct. 28, 2015, 1:45 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made some small refactoring as I was reading through the fetcher code to make 
> the groking easy. No functional changes.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 8fb6c83981a141df9c0a8a6f8267230bef64f218 
> 
> Diff: https://reviews.apache.org/r/39707/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Review Request 39707: Minor code refactor for fetcher.cpp.

[Vinod Kone]

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

Review request for mesos, Bernd Mathiske and Jie Yu.


Repository: mesos


Description
---

Made some small refactoring as I was reading through the fetcher code to make 
the groking easy. No functional changes.


Diffs
-

  src/launcher/fetcher.cpp 8fb6c83981a141df9c0a8a6f8267230bef64f218 

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


Testing
---

make check


Thanks,

Vinod Kone

Re: Review Request 39703: [WIP] Exposed container-id via TaskStatus updates.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39702, 39703]

All tests passed.

- Mesos ReviewBot


On Oct. 27, 2015, 9:54 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39703/
> ---
> 
> (Updated Oct. 27, 2015, 9:54 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3688
> https://issues.apache.org/jira/browse/MESOS-3688
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The container-id is mesos-specific and so it's not quite clear what good 
> could it do. However, for Docker containers, one can use the "docker 
> container id" for various things such as "docker ps". This particular patch 
> doesn't expose docker container ids yet.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 94004343ea615d87d7c7d63a6a811cf8002e 
>   src/common/http.cpp f56d8a178e1f61d91adfeaad32b47718adbc4379 
>   src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5 
>   src/tests/master_tests.cpp ee2473997ccbd1c50d0cbf65d1259ea2dfe82971 
> 
> Diff: https://reviews.apache.org/r/39703/diff/
> 
> 
> Testing
> ---
> 
> make check with updated MasterTest.TaskStatusContainerStatus test.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39317, 38218, 39399, 39400, 39401, 39450]

All tests passed.

- Mesos ReviewBot


On Oct. 27, 2015, 7:29 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39450/
> ---
> 
> (Updated Oct. 27, 2015, 7:29 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 
> 
> Diff: https://reviews.apache.org/r/39450/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39415: Error out when root qdisc already exists

[Jie Yu]

> On Oct. 27, 2015, 5:51 p.m., Jie Yu wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1403-1404
> > 
> >
> > Can you explain more? What if the slave restarts after creating the 
> > qdisc, the slave will flap afterwards?
> 
> Cong Wang wrote:
> Hmm, if there any way to tell if this is a recover or normal start?
> 
> The problem I want to solve is:
> 
> 1) create an HTB qdisc on root;
> 2) start mesos slave with this flag, which will try to create fq_codel on 
> root too;
> 3) No error, because we ignore if root qdisc exists before us
> 4) But packets get dropped because our filters created for root qdisc are 
> not supposed to be used for HTB
> 
> The silent drop is definitely not acceptable...

Can you check if the root qdisc is fq_codel if egress root qdisc exists? I 
thought we have fq_codel::exists(link, EGRESS_ROOT)?


- Jie


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


On Oct. 18, 2015, 12:22 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39415/
> ---
> 
> (Updated Oct. 18, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When we enable --egress_unique_flow_per_container, we need to install an 
> fq_codel qdisc on root, but if a qdisc already exists on root, we should not 
> continue since it could be not fq_codel at all, so just exit with error.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39415/diff/
> 
> 
> Testing
> ---
> 
> Manual tests, with a pre-installed HTB qdisc and without.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39415: Error out when root qdisc already exists

[Cong Wang]

> On Oct. 27, 2015, 5:51 p.m., Jie Yu wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1403-1404
> > 
> >
> > Can you explain more? What if the slave restarts after creating the 
> > qdisc, the slave will flap afterwards?

Hmm, if there any way to tell if this is a recover or normal start?

The problem I want to solve is:

1) create an HTB qdisc on root;
2) start mesos slave with this flag, which will try to create fq_codel on root 
too;
3) No error, because we ignore if root qdisc exists before us
4) But packets get dropped because our filters created for root qdisc are not 
supposed to be used for HTB

The silent drop is definitely not acceptable...


- Cong


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


On Oct. 18, 2015, 12:22 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39415/
> ---
> 
> (Updated Oct. 18, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When we enable --egress_unique_flow_per_container, we need to install an 
> fq_codel qdisc on root, but if a qdisc already exists on root, we should not 
> continue since it could be not fq_codel at all, so just exit with error.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39415/diff/
> 
> 
> Testing
> ---
> 
> Manual tests, with a pre-installed HTB qdisc and without.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39416: Document --egress_unique_flow_per_container in docs/configuration.md

[Cong Wang]

> On Oct. 27, 2015, 5:35 p.m., Ian Downes wrote:
> > docs/configuration.md, lines 1533-1534
> > 
> >
> > Does it actually create a flow per container or is it really based on 
> > the 5 tuple, which will be different for different containers?

Yes, the flow is per container, rather than based on tuples.


- Cong


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


On Oct. 18, 2015, 12:24 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39416/
> ---
> 
> (Updated Oct. 18, 2015, 12:24 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Flag --egress_unique_flow_per_container is documented in help message, but 
> not in docs/configuration.md. We should document it there too, with more 
> details.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 
> 
> Diff: https://reviews.apache.org/r/39416/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39490: Always create non-IP egress filters

[Cong Wang]

> On Oct. 27, 2015, 5:38 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 2431
> > 
> >
> > Why is this no longer a failure?

They could be already created by the slave if the flag were enabled before 
restarting.


- Cong


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


On Oct. 20, 2015, 6:57 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> ---
> 
> (Updated Oct. 20, 2015, 6:57 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When rolling out the new flag --egress_unique_flow_per_container, I noticed, 
> on some slaves, only IP egress filters were created as expected, the reset 
> were not. Looking at the code, it looks like we skipped the creation if this 
> is not the first container we create, this is wrong for this case, because 
> egress filters were not created for previous containers yet. We should always 
> create them and ignore error if they exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Alexander Rukletsov]

> On Oct. 26, 2015, 2:07 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1004
> > 
> >
> > Just check the code, dynamically reserved resource are included in 
> > allocation.
> 
> Qian Zhang wrote:
> Really?:-) I think once a resource is dynamically reserved, it will be 
> removed from the framework's allocation and marked as reserved in slave's 
> total resources, so that in next allocation cycle it can be offered to the 
> framework of the role which reserves it.

A reserved resource can be allocated or not. For example, if a framework 
declines a reserved resource, it will be removed from allocated.


- Alexander


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


On Oct. 27, 2015, 7:27 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Oct. 27, 2015, 7:27 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39590: Made license-headers doxygen-compatible.

[Benjamin Bannier]

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

(Updated Oct. 27, 2015, 10:45 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Hindman, Bernd 
Mathiske, and Till Toenshoff.


Changes
---

Updated description, and rebased.


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


Repository: mesos


Description (updated)
---

This commit is the result of running

for f in `find . -name '*.hpp' -or -name '*.cpp'`; do
  sed -i -E '1 s~^\/\*\*~\/\*~' $f
done

plus manual cleanup not matching that pattern or in protobuf
definitions, and updating the C++ style guide.


Diffs (updated)
-

  docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
  include/mesos/attributes.hpp 78afcd51b22a9eeb741076a8affeb8f2ae4bdee3 
  include/mesos/authentication/authenticatee.hpp 
17fb7aa0f4d8a43f9cee0aab305af05f4fa7888f 
  include/mesos/authentication/authentication.hpp 
699aa886286bc7d9c05592e71232ab1c1084871f 
  include/mesos/authentication/authentication.proto 
a7db56d643aa01ca2e3cd3e4268bd75b54136727 
  include/mesos/authentication/authenticator.hpp 
aa3818c31fee3b2e7c17d80dcceb8d41a38bbd06 
  include/mesos/authorizer/authorizer.hpp 
d667a52f90f970a313580446a5a006cec4b5e25b 
  include/mesos/authorizer/authorizer.proto 
86bbb45f9d91b4098a262e3e50a793f3bb39497e 
  include/mesos/containerizer/containerizer.hpp 
9bf76e066f390c36392c469b3d2cd92e2d10f3c7 
  include/mesos/containerizer/containerizer.proto 
7cf6d2bb8c6636cbb3ea8c44fb45a41b93c8603d 
  include/mesos/executor.hpp 72eca97dd84fb1300b37764a3ef3a57fb5e676c2 
  include/mesos/executor/executor.hpp 85f181c72cdb5e80d6c549a4d663d9bad261693a 
  include/mesos/executor/executor.proto 
a9243c7a10c32f104c12cbce835bc23a7c75a275 
  include/mesos/fetcher/fetcher.hpp b7e6a717ed329d7f2586607cfe90342a96ae14a8 
  include/mesos/fetcher/fetcher.proto 1b2f4936d8dcd2b5e6d3ca2c85b3fa9df74a5797 
  include/mesos/hook.hpp 6d7fee85566cf6c057296b5f4a9c14c9fab31085 
  include/mesos/http.hpp 8b9b748fee5b2a8cc2261456cd6a74ebf9313164 
  include/mesos/maintenance/maintenance.hpp 
f676d01c2c81250b6e4740ab0934f966b50ed76d 
  include/mesos/maintenance/maintenance.proto 
aaca2513e2c30297bf624a831f5aa135f9f47e48 
  include/mesos/master/allocator.hpp dbceb53a3accd32762d09785ecae06667c3cb611 
  include/mesos/master/allocator.proto 224da71e9f34d2ea11a6e6e235d0f8196abaeb90 
  include/mesos/master/quota.hpp 5f7822f40af6fb23cdafdd0c205bcdc67e596935 
  include/mesos/master/quota.proto d2e3a45735e4ebcf257682556aff5075e6e3bf79 
  include/mesos/mesos.hpp 371d14a6cf802d1099be84f217a1af6554cc4eee 
  include/mesos/mesos.proto 94004343ea615d87d7c7d63a6a811cf8002e 
  include/mesos/module.hpp 01dd713941d504c3dfe606bfdf346d4702dc1495 
  include/mesos/module/allocator.hpp 376eb4860322582f911d7a07253b79b1c9aa9292 
  include/mesos/module/anonymous.hpp 22862bdba93537b7524f3143884b4d13d17c604a 
  include/mesos/module/authenticatee.hpp 
aafca1214cfdecb0479e4e8ab20fe9ffc8272473 
  include/mesos/module/authenticator.hpp 
b57938f8f4c5603b8e8e6d2e77f27a5eb302e99b 
  include/mesos/module/authorizer.hpp 7d8fc2123ac4132a7a698c855db03433eb77cea6 
  include/mesos/module/hook.hpp fdbc5b19fe04ac9456b4141d673d9fec03e9c70d 
  include/mesos/module/isolator.hpp 347d6d442debc70ff8accc4d89c944c2a2b7 
  include/mesos/module/module.hpp 6ef106ee6f1559f8e95b8309f36af2b2d6e2c48b 
  include/mesos/module/module.proto 821fc0e72ece7c497595859fc5efc1c64ea49b9b 
  include/mesos/module/qos_controller.hpp 
462f07608fb2b580cee9548b5506e9896ee7077a 
  include/mesos/module/resource_estimator.hpp 
c1e42b97d831093bb92f8666fdfd53c8a9cae83f 
  include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
  include/mesos/scheduler.hpp f571d42d1508632152473c4f4ade60ae3900fce1 
  include/mesos/scheduler/scheduler.hpp 
30de72b1b535831e074cd132b61e74fec2f4890a 
  include/mesos/scheduler/scheduler.proto 
417cfe6a9bca418b0ef77cb2268fafeb07867819 
  include/mesos/slave/isolator.hpp ea14bff0d31ffc440b0451675bfa440e09a524d8 
  include/mesos/slave/isolator.proto 0e71a9381061e3d0ece8f03ae6c9d2c82d48aba7 
  include/mesos/slave/oversubscription.hpp 
ffefaa08da1de27d9e0ccb0dc833e998e1638eef 
  include/mesos/slave/oversubscription.proto 
588316739018a4360d37d64e4bb1655c488b2456 
  include/mesos/slave/qos_controller.hpp 
7e280ccabd153eb10ae72cc48078d660df9f2011 
  include/mesos/slave/resource_estimator.hpp 
731ec3a470dcc8e90199e774d6fcd1d5635be296 
  include/mesos/type_utils.hpp 1076cbda64382be29348dd5679c0e3e414aa6e67 
  include/mesos/v1/attributes.hpp d8b35079f41adc4e22c45c5c40698d664810e5cd 
  include/mesos/v1/executor/executor.hpp 
f50533f8120908cb5b1144bd18a4e097c5336fde 
  include/mesos/v1/executor/executor.proto 
20c712c3f952d3299f2a7c23e185ed588ef13251 
  include/mesos/v1/mesos.hpp fe4c4b73d41505eeb977f818aecab3cd037668da 
  inc

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Alexander Rukletsov]

> On Oct. 26, 2015, 1:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.
> 
> Alexander Rukletsov wrote:
> I'm not sure I got your point. If my mental compiler is correct, if a 
> framework in quota'ed role opts out, we do not immediately lay aside 
> resources. We do that after we have checked all the frameworks in the role in 
> a separate loop.
> 
> Qian Zhang wrote:
> Let me clarify my point with an example:
> Say in the Mesos cluster, there are 2 agents, a1 and a2, each has 4GB 
> memory. And there are 2 roles, r1 and r2, r1 has a quota set (4GB) and r2 has 
> not quota set. r1 has a framework f1 which currenlty has no allocation but 
> has a filter (4GB memory on a1), r2 also has a framework f2 which currently 
> has no allocation too and has no filter. And there is no static/dynamic 
> reservation and no revocable resources. Now with the logic in this patch, for 
> a1, in the quotaRoleSorter foreach loop, when we handle the quota for r1, we 
> will not allocate a1's resouces to f1 because f1 has a filter, so a1's 4GB 
> memory will be laid aside to satisfy r1's quota. And then in the roleSorter 
> foreach loop, we will NOT allocate a1's resources to f2 too since currently 
> a1 has no available resources due to all its 4GB memory has been laid aside 
> for r1. And then when we handle a2, its 4GB memory will be allocated to f1, 
> so f2 will not get anything in the end. So the result is, a1's 4GB memory is 
> laid aside
  to satisfy r1's quota, a2's 4GB is allocated to f1, and f2 gets nothing. But 
I think for this example, the expected result should be, f1 gets a2's 4GB (r1's 
quota is also satisfied) and f2 gets a1's 4GB.

This can happen during the allocation cycle, but we do not persist laid aside 
resources between allocation cycles. Without refactoring `allocate()` we do not 
know whether we get a suitable agent, hence we have to lay aside. But at the 
next allocation cycle, `r1`'s quota is satisfied and `f2` will get `a1`'s 4GB, 
which is OK in my opinion.


- Alexander


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


On Oct. 27, 2015, 7:27 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Oct. 27, 2015, 7:27 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Test

Review Request 39703: [WIP] Exposed container-id via TaskStatus updates.

[Kapil Arya]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The container-id is mesos-specific and so it's not quite clear what good could 
it do. However, for Docker containers, one can use the "docker container id" 
for various things such as "docker ps". This particular patch doesn't expose 
docker container ids yet.


Diffs
-

  include/mesos/mesos.proto 94004343ea615d87d7c7d63a6a811cf8002e 
  src/common/http.cpp f56d8a178e1f61d91adfeaad32b47718adbc4379 
  src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5 
  src/tests/master_tests.cpp ee2473997ccbd1c50d0cbf65d1259ea2dfe82971 

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


Testing
---

make check with updated MasterTest.TaskStatusContainerStatus test.


Thanks,

Kapil Arya

Review Request 39702: Fixed incorrect signed vs. unsigned comparisons.

[Kapil Arya]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/executor_http_api_tests.cpp 
e429d84a4a7bf9248cd9b644a93be3b7d9058ae9 

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


Testing
---

Builds fine with the patch and fails with compilation errors without it.


Thanks,

Kapil Arya

Re: Review Request 39695: Relocate launcher and linux_launcher as MesosContainerizer specific

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39695]

All tests passed.

- Mesos ReviewBot


On Oct. 27, 2015, 6:30 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39695/
> ---
> 
> (Updated Oct. 27, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3129
> https://issues.apache.org/jira/browse/MESOS-3129
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Relocate launcher and linux_launcher as MesosContainerizer specific
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt e6169a0e3ad34dd0e4c3430a6532bd48c4bd04fd 
>   src/Makefile.am e797dac8093150871e9b68eba868f87df680ab43 
>   src/slave/containerizer/containerizer.cpp 
> 06753365e2ec7cb59edd1ed6ecfe1a794498ee9b 
>   src/slave/containerizer/external_containerizer.hpp 
> c00cebb93c5395bccbef558632d8f9c9bee8fffa 
>   src/slave/containerizer/launcher.hpp  
>   src/slave/containerizer/launcher.cpp 
> 668ae80d0723068f85f7c16e0cc57804bb55af16 
>   src/slave/containerizer/linux_launcher.hpp 
> 627df892728f127f37672991e1a57ee445008fe1 
>   src/slave/containerizer/linux_launcher.cpp 
> c0adb34771fdb5a85d087296a8f98b890254ddf7 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 91e4ea3a907ad165c359e7422135138737e14085 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 39008f620183d242407fea5377bfceffc57b 
>   src/tests/containerizer/isolator_tests.cpp 
> 4e1e90ba2aeb70d4a70c3e0cf9796bd1aa199147 
>   src/tests/containerizer/launcher.hpp 
> 5d34bab789bdafe71d94e8ec263710c50b83e180 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> b48133c3d0624b27bcdc2289669c7f0b1bcc12f4 
>   src/tests/containerizer/port_mapping_tests.cpp 
> ae2c0e613acf41614413e41e6989e9056328ff36 
> 
> Diff: https://reviews.apache.org/r/39695/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38577: Added synchronous validation for Call in Agent

[Vinod Kone]

> On Oct. 27, 2015, 1:13 a.m., Vinod Kone wrote:
> > src/slave/validation.cpp, line 75
> > 
> >
> > also print status.source()
> 
> Isabel Jimenez wrote:
> We don't have a stringify for this.
> 
> Anand Mazumdar wrote:
> Why not implement the `ostream` operator preferrably in `type_utils.hpp` 
> in a separate review and mark this one as dependent on it ? Otherwise, this 
> error string is hardly of any use to the end-user. What do you think?

+1 to anand's comment. but lets do that in a follow up review. i'll commit this 
chain now.


- Vinod


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


On Oct. 27, 2015, 1:20 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38577/
> ---
> 
> (Updated Oct. 27, 2015, 1:20 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added validation for Call protobuf message in Agent /api/v1/executor endpoint.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt e6169a0 
>   src/Makefile.am e797dac 
>   src/slave/http.cpp 3f7f71b 
>   src/slave/validation.hpp PRE-CREATION 
>   src/slave/validation.cpp PRE-CREATION 
>   src/tests/executor_http_api_tests.cpp e429d84 
> 
> Diff: https://reviews.apache.org/r/38577/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 39450: Quota: Added hierarchical allocator-specific tests.

[Alexander Rukletsov]

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

(Updated Oct. 27, 2015, 7:29 p.m.)


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


Changes
---

More tests.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Alexander Rukletsov]

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

(Updated Oct. 27, 2015, 7:27 p.m.)


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


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

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

(Updated Oct. 27, 2015, 7:17 p.m.)


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


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
cfd937ba306273c24fb5337dfeb1a15e1545169b 
  src/master/allocator/mesos/hierarchical.cpp 
f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Oct. 26, 2015, 6:52 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 897
> > 
> >
> > I think we are not *moving" the role into the quota'ed role sorter, 
> > instead, we are adding it into the quota'ed role sorter, i.e., quota'ed 
> > role will be placed in both quota'd role sorter and the non-quota'ed role 
> > sorter.

My thinking was to stress that by doing that a different allocation algorithm 
applies. But you're right, it's misleading.


- Alexander


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


On Oct. 23, 2015, 4:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Oct. 23, 2015, 4:38 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

[Joseph Wu]

> On Oct. 26, 2015, 1:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, 
> > lines 35-40
> > 
> >
> > Use `strings::endsWith`.
> > 
> > And you might also want to use a ternary conditional:
> > `currentPath = path + (strings::endsWith("\") ? "" : "\");`
> 
> Alex Clemmer wrote:
> The `strings::endsWith` suggestion is really good! I actually would like 
> to avoid the choice of the ternary operator if you don't mind. Even though I 
> used it in a recent review in general my personal opinion is that I tend to 
> find it too opaque and not much more readble.

That's fine too.  I just personally like using ternary for short statements 
like this.  (Marking as fixed.)


> On Oct. 26, 2015, 1:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 
> > 42
> > 
> >
> > `X:\blah*` isn't clear.  Did you mean `currentPath` + `*`?
> 
> Alex Clemmer wrote:
> I actually specifically avoided mentioning the variable name because I 
> thought it was not clear what the relationship of the Kleene star was to the 
> variable. I've expanded the path to something more clear. What do you think 
> of this solution? Is it more clear? Is it worse?

I think it's better.

You could also say that the "X" is an arbitrary drive letter.  I've seen "Z" 
drives before, so I'm guessing "X" drives aren't unheard of.


- Joseph


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


On Oct. 27, 2015, 1:33 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> ---
> 
> (Updated Oct. 27, 2015, 1:33 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::rmdir.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ba2836a72ceee948cb43364e80ada9f132f33d04 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> b6afe0e76366d0bc68d37097ced83a1e14828d84 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 3e6f2aafd0f541f512025dfa683ab4178701f7c4 
> 
> Diff: https://reviews.apache.org/r/39584/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39410: Added support for github to apply-reviews.py.

[Vinod Kone]

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



support/apply-reviews.py (line 38)


either this function should renamed to something review board specific or 
the function need to get both RB and GH users.



support/apply-reviews.py (lines 43 - 44)


update comment?



support/apply-reviews.py (lines 76 - 78)


bad rebase.



support/apply-reviews.py (line 105)


if 'dry_run' in options:



support/apply-reviews.py (lines 149 - 155)


i don't follow. why not just call 'shell(cmd)' if you want to override 
'dry_run'? is it because 'shell()' doesn't print the command?



support/apply-reviews.py (line 177)


s/populate_//



support/apply-reviews.py (line 192)


s/populate_//



support/apply-reviews.py (line 218)


s/populate_//



support/apply-reviews.py (line 228)


indentation?



support/apply-reviews.py (line 268)


s/mututally/mutually/ ?


- Vinod Kone


On Oct. 23, 2015, 11:13 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Oct. 23, 2015, 11:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Review Request 39695: Relocate launcher and linux_launcher as MesosContainerizer specific

[Gilbert Song]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Relocate launcher and linux_launcher as MesosContainerizer specific


Diffs
-

  src/CMakeLists.txt e6169a0e3ad34dd0e4c3430a6532bd48c4bd04fd 
  src/Makefile.am e797dac8093150871e9b68eba868f87df680ab43 
  src/slave/containerizer/containerizer.cpp 
06753365e2ec7cb59edd1ed6ecfe1a794498ee9b 
  src/slave/containerizer/external_containerizer.hpp 
c00cebb93c5395bccbef558632d8f9c9bee8fffa 
  src/slave/containerizer/launcher.hpp  
  src/slave/containerizer/launcher.cpp 668ae80d0723068f85f7c16e0cc57804bb55af16 
  src/slave/containerizer/linux_launcher.hpp 
627df892728f127f37672991e1a57ee445008fe1 
  src/slave/containerizer/linux_launcher.cpp 
c0adb34771fdb5a85d087296a8f98b890254ddf7 
  src/slave/containerizer/mesos/containerizer.hpp 
4aad8a3be43b331efc6b8157b2fae090df16c1b4 
  src/slave/containerizer/mesos/containerizer.cpp 
91e4ea3a907ad165c359e7422135138737e14085 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
39008f620183d242407fea5377bfceffc57b 
  src/tests/containerizer/isolator_tests.cpp 
4e1e90ba2aeb70d4a70c3e0cf9796bd1aa199147 
  src/tests/containerizer/launcher.hpp 5d34bab789bdafe71d94e8ec263710c50b83e180 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
b48133c3d0624b27bcdc2289669c7f0b1bcc12f4 
  src/tests/containerizer/port_mapping_tests.cpp 
ae2c0e613acf41614413e41e6989e9056328ff36 

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


Testing
---

make check (Ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 39415: Error out when root qdisc already exists

[Jie Yu]

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



src/slave/containerizer/isolators/network/port_mapping.cpp (lines 1403 - 1404)


Can you explain more? What if the slave restarts after creating the qdisc, 
the slave will flap afterwards?


- Jie Yu


On Oct. 18, 2015, 12:22 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39415/
> ---
> 
> (Updated Oct. 18, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When we enable --egress_unique_flow_per_container, we need to install an 
> fq_codel qdisc on root, but if a qdisc already exists on root, we should not 
> continue since it could be not fq_codel at all, so just exit with error.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39415/diff/
> 
> 
> Testing
> ---
> 
> Manual tests, with a pre-installed HTB qdisc and without.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39415: Error out when root qdisc already exists

[Ian Downes]

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

Ship it!


Ship It!

- Ian Downes


On Oct. 17, 2015, 5:22 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39415/
> ---
> 
> (Updated Oct. 17, 2015, 5:22 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When we enable --egress_unique_flow_per_container, we need to install an 
> fq_codel qdisc on root, but if a qdisc already exists on root, we should not 
> continue since it could be not fq_codel at all, so just exit with error.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39415/diff/
> 
> 
> Testing
> ---
> 
> Manual tests, with a pre-installed HTB qdisc and without.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39490: Always create non-IP egress filters

[Ian Downes]

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



src/slave/containerizer/isolators/network/port_mapping.cpp (line 2431)


Why is this no longer a failure?



src/slave/containerizer/isolators/network/port_mapping.cpp (line 2450)


ditto?


- Ian Downes


On Oct. 20, 2015, 11:57 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> ---
> 
> (Updated Oct. 20, 2015, 11:57 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When rolling out the new flag --egress_unique_flow_per_container, I noticed, 
> on some slaves, only IP egress filters were created as expected, the reset 
> were not. Looking at the code, it looks like we skipped the creation if this 
> is not the first container we create, this is wrong for this case, because 
> egress filters were not created for previous containers yet. We should always 
> create them and ignore error if they exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39416: Document --egress_unique_flow_per_container in docs/configuration.md

[Ian Downes]

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



docs/configuration.md (lines 1533 - 1534)


Does it actually create a flow per container or is it really based on the 5 
tuple, which will be different for different containers?


- Ian Downes


On Oct. 17, 2015, 5:24 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39416/
> ---
> 
> (Updated Oct. 17, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Flag --egress_unique_flow_per_container is documented in help message, but 
> not in docs/configuration.md. We should document it there too, with more 
> details.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 
> 
> Diff: https://reviews.apache.org/r/39416/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

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

(Updated Oct. 27, 2015, 2:27 p.m.)


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


Changes
---

collect untar futures.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 
  src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
1d3377e7462908246dfac90aa0c56314406529c9 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
  src/tests/containerizer/provisioner_docker_tests.cpp 
822aa77d0be0797ab62a5798527618b2cc4f6bf0 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

[Alex Clemmer]

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

(Updated Oct. 27, 2015, 8:33 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

Windows: Implemented `os::rmdir.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
ba2836a72ceee948cb43364e80ada9f132f33d04 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
b6afe0e76366d0bc68d37097ced83a1e14828d84 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
3e6f2aafd0f541f512025dfa683ab4178701f7c4 

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


Testing
---

`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.


Thanks,

Alex Clemmer

Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

[Alex Clemmer]

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

(Updated Oct. 27, 2015, 8:30 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

Windows: Implemented `os::rmdir.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
ba2836a72ceee948cb43364e80ada9f132f33d04 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
b6afe0e76366d0bc68d37097ced83a1e14828d84 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
3e6f2aafd0f541f512025dfa683ab4178701f7c4 

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


Testing
---

`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.


Thanks,

Alex Clemmer

Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

[Alex Clemmer]

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 138)


note that actually `::rmdir` here will deal appropriately with Unix-style 
paths, so there's no real need to normalize them.


- Alex Clemmer


On Oct. 27, 2015, 8:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> ---
> 
> (Updated Oct. 27, 2015, 8:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::rmdir.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ba2836a72ceee948cb43364e80ada9f132f33d04 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> b6afe0e76366d0bc68d37097ced83a1e14828d84 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 3e6f2aafd0f541f512025dfa683ab4178701f7c4 
> 
> Diff: https://reviews.apache.org/r/39584/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

[Alex Clemmer]

> On Oct. 26, 2015, 8:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 
> > 42
> > 
> >
> > `X:\blah*` isn't clear.  Did you mean `currentPath` + `*`?

I actually specifically avoided mentioning the variable name because I thought 
it was not clear what the relationship of the Kleene star was to the variable. 
I've expanded the path to something more clear. What do you think of this 
solution? Is it more clear? Is it worse?


> On Oct. 26, 2015, 8:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, 
> > lines 35-40
> > 
> >
> > Use `strings::endsWith`.
> > 
> > And you might also want to use a ternary conditional:
> > `currentPath = path + (strings::endsWith("\") ? "" : "\");`

The `strings::endsWith` suggestion is really good! I actually would like to 
avoid the choice of the ternary operator if you don't mind. Even though I used 
it in a recent review in general my personal opinion is that I tend to find it 
too opaque and not much more readble.


> On Oct. 26, 2015, 8:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 
> > 134
> > 
> >
> > You seem to be missing an `if (recursive)` here...

Wow, major +1 awesome catch I meant to go back and add this but totally spaced 
it. Great job!


- Alex


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


On Oct. 27, 2015, 8:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> ---
> 
> (Updated Oct. 27, 2015, 8:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::rmdir.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ba2836a72ceee948cb43364e80ada9f132f33d04 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> b6afe0e76366d0bc68d37097ced83a1e14828d84 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 3e6f2aafd0f541f512025dfa683ab4178701f7c4 
> 
> Diff: https://reviews.apache.org/r/39584/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

[Alex Clemmer]

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

(Updated Oct. 27, 2015, 8:23 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

Windows: Implemented `os::rmdir.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
ba2836a72ceee948cb43364e80ada9f132f33d04 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
b6afe0e76366d0bc68d37097ced83a1e14828d84 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
3e6f2aafd0f541f512025dfa683ab4178701f7c4 

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


Testing
---

`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.


Thanks,

Alex Clemmer

Re: Review Request 39583: Windows: Added `WindowsError` to parallel `ErrnoError`.

[Alex Clemmer]

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

(Updated Oct. 27, 2015, 8:17 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

Windows: Added `WindowsError` to parallel `ErrnoError`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
ba2836a72ceee948cb43364e80ada9f132f33d04 
  3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
fdd33512c8d8752093f72f597a7d647eb5e3c285 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
PRE-CREATION 

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


Testing
---

`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.


Thanks,

Alex Clemmer

Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.

[Alex Clemmer]

> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, 
> > line 48
> > 
> >
> > That `- 1` doesn't match the "size" described by the variable name.  
> > Seems like you're describing the `max_alphabet_index`.

Marking as fixed, but, I just want to verify: You meant `maxAlphabetIndex`, 
right? It looks like we're not using snake case. Have I missed something 
important?


- Alex


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


On Oct. 27, 2015, 8:14 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> ---
> 
> (Updated Oct. 27, 2015, 8:14 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::mkdtemp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ba2836a72ceee948cb43364e80ada9f132f33d04 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> b6afe0e76366d0bc68d37097ced83a1e14828d84 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 3e6f2aafd0f541f512025dfa683ab4178701f7c4 
> 
> Diff: https://reviews.apache.org/r/39559/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.

[Alex Clemmer]

> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, 
> > line 34
> > 
> >
> > `strlen()` might be better/more-readable.
> 
> Alex Clemmer wrote:
> So, I could definitely be mistaken, but it looks like `strlen` would have 
> to be recomputed every time we call the function, right? And it looks like 
> this code is normally run at initialization time?
> 
> If so it seems like this way has a clear benefit over the `strlen` 
> approach. What do you think? Maybe there is another way to make `strlen` an 
> init-time expression? (I already looked at `constexpr`.)

Post script: this exchange actually convinced me that we should compute the 
size of `alphabet` statically to! Let me know if you have a problem with this 
given the constraints I mentioned. :)


> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, 
> > lines 36-41
> > 
> >
> > The posix spec says that `XX` suffix is required, so 
> > `strings::endsWith` would be a better check.
> 
> Alex Clemmer wrote:
> Oh, good tip, I didn't know about that function.

Thus turns out to have been a great suggestion. Simplified the code greatly, so 
thanks!


- Alex


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


On Oct. 27, 2015, 8:14 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> ---
> 
> (Updated Oct. 27, 2015, 8:14 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::mkdtemp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ba2836a72ceee948cb43364e80ada9f132f33d04 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> b6afe0e76366d0bc68d37097ced83a1e14828d84 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 3e6f2aafd0f541f512025dfa683ab4178701f7c4 
> 
> Diff: https://reviews.apache.org/r/39559/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.

[Alex Clemmer]

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

(Updated Oct. 27, 2015, 8:14 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

Windows: Implemented `os::mkdtemp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
ba2836a72ceee948cb43364e80ada9f132f33d04 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
b6afe0e76366d0bc68d37097ced83a1e14828d84 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
3e6f2aafd0f541f512025dfa683ab4178701f7c4 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Qian Zhang]

> On Oct. 26, 2015, 10:07 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1004
> > 
> >
> > Just check the code, dynamically reserved resource are included in 
> > allocation.

Really?:-) I think once a resource is dynamically reserved, it will be removed 
from the framework's allocation and marked as reserved in slave's total 
resources, so that in next allocation cycle it can be offered to the framework 
of the role which reserves it.


- Qian


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


On Oct. 24, 2015, 12:38 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Oct. 24, 2015, 12:38 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.

[Alex Clemmer]

> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, 
> > line 34
> > 
> >
> > `strlen()` might be better/more-readable.

So, I could definitely be mistaken, but it looks like `strlen` would have to be 
recomputed every time we call the function, right? And it looks like this code 
is normally run at initialization time?

If so it seems like this way has a clear benefit over the `strlen` approach. 
What do you think? Maybe there is another way to make `strlen` an init-time 
expression? (I already looked at `constexpr`.)


- Alex


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


On Oct. 22, 2015, 6:23 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> ---
> 
> (Updated Oct. 22, 2015, 6:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::mkdtemp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> ba2836a72ceee948cb43364e80ada9f132f33d04 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 7f70c9ea7d57634b5bfd40523ba37561ec92a09a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> b6afe0e76366d0bc68d37097ced83a1e14828d84 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 3e6f2aafd0f541f512025dfa683ab4178701f7c4 
> 
> Diff: https://reviews.apache.org/r/39559/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Qian Zhang]

> On Oct. 26, 2015, 4:27 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1017-1020
> > 
> >
> > Why do we put these code inside the framework sorters foreach loop? I 
> > do not see it is related to framework.
> > If we really want to put these code here, then I think we also need to 
> > recalculate roleAllocatedResources every time when we allocate some 
> > resources to a framework of the role, and once the quota for the role is 
> > satifised, break.
> 
> Alexander Rukletsov wrote:
> There can be multiple frameworks in a role, hence quota may get satisfied 
> after we allocate resources to some frameworks.
> 
> > then I think we also need to recalculate roleAllocatedResources every 
> time when we allocate some resources to a framework
> 
> But we do that at the end of the loop, right?

I do not see we recalculate roleAllocatedResources in the frameworkSorters 
foreach loop, actually we do that outside of that loop (at the end of the 
quotaRoleSorter foreach loop), that means, even we allocate resources to some 
frameworks in the frameworkSorters foreach loop, the variable 
roleAllocatedResources will NOT be recalculated. That's why I suggested to 
recalculate roleAllocatedResources every time when we allocate some resources 
to a framework :-)


> On Oct. 26, 2015, 4:27 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 996
> > 
> >
> > These newly added code makes allocate() a huge method (more than 200 
> > lines), maybe move these codes into a separate method?
> 
> Alexander Rukletsov wrote:
> Absolutely! The reason why it's not done is because we have already 
> planned (but not yet scheduled) an allocator refactoring. Let me add a `TODO` 
> for now in order to increase the pressure on ourselves ; ).

Agree :-)


- Qian


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


On Oct. 24, 2015, 12:38 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Oct. 24, 2015, 12:38 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39678: Allow environment variables when lanuch Docker container.

[Mesos ReviewBot]

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Oct. 27, 2015, 6:51 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39678/
> ---
> 
> (Updated Oct. 27, 2015, 6:51 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3787
> https://issues.apache.org/jira/browse/MESOS-3787
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow environment variables when lanuch Docker container.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 4ebca660834492f99815a17e04cef6116654dedb 
>   src/tests/containerizer/docker_tests.cpp 
> babc7d8da4ed9d13b14bd69decd7f27fc7dfde89 
> 
> Diff: https://reviews.apache.org/r/39678/diff/
> 
> 
> Testing
> ---
> 
> Add a new test case: DockerTest.ROOT_DOCKER_RunWithEnv
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

[Qian Zhang]

> On Oct. 26, 2015, 9:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.
> 
> Alexander Rukletsov wrote:
> I'm not sure I got your point. If my mental compiler is correct, if a 
> framework in quota'ed role opts out, we do not immediately lay aside 
> resources. We do that after we have checked all the frameworks in the role in 
> a separate loop.

Let me clarify my point with an example:
Say in the Mesos cluster, there are 2 agents, a1 and a2, each has 4GB memory. 
And there are 2 roles, r1 and r2, r1 has a quota set (4GB) and r2 has not quota 
set. r1 has a framework f1 which currenlty has no allocation but has a filter 
(4GB memory on a1), r2 also has a framework f2 which currently has no 
allocation too and has no filter. And there is no static/dynamic reservation 
and no revocable resources. Now with the logic in this patch, for a1, in the 
quotaRoleSorter foreach loop, when we handle the quota for r1, we will not 
allocate a1's resouces to f1 because f1 has a filter, so a1's 4GB memory will 
be laid aside to satisfy r1's quota. And then in the roleSorter foreach loop, 
we will NOT allocate a1's resources to f2 too since currently a1 has no 
available resources due to all its 4GB memory has been laid aside for r1. And 
then when we handle a2, its 4GB memory will be allocated to f1, so f2 will not 
get anything in the end. So the result is, a1's 4GB memory is laid aside to sa
 tisfy r1's quota, a2's 4GB is allocated to f1, and f2 gets nothing. But I 
think for this example, the expected result should be, f1 gets a2's 4GB (r1's 
quota is also satisfied) and f2 gets a1's 4GB.


- Qian


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


On Oct. 24, 2015, 12:38 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Oct. 24, 2015, 12:38 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>