Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-20 Thread Benjamin Mahler 

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


Ship it!




Ship It!

- Benjamin Mahler


On Dec. 21, 2017, 2:25 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64003/
> ---
> 
> (Updated Dec. 21, 2017, 2:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-7099
> https://issues.apache.org/jira/browse/MESOS-7099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator now does fine-grained resource allocation up
> to the role's quota limit. And a role's quota is only
> considered to be satisfied if all of its quota resources
> are satisfied. Previously, a role's quota is considered
> to be met if any one of its quota resources reaches the limit.
> 
> Also fixed a few affected allocator tests.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1c767f7f778819dcfa6eff51765163aec1b70a2d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 173e4fbac184ad8d40c8adba19ad64225f11f1f2 
> 
> 
> Diff: https://reviews.apache.org/r/64003/diff/6/
> 
> 
> Testing
> ---
> 
> Fixed a few broken tests due to this patch.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-20 Thread Meng Zhu 

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

(Updated Dec. 20, 2017, 6:25 p.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.


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


Repository: mesos


Description (updated)
---

Allocator now does fine-grained resource allocation up
to the role's quota limit. And a role's quota is only
considered to be satisfied if all of its quota resources
are satisfied. Previously, a role's quota is considered
to be met if any one of its quota resources reaches the limit.

Also fixed a few affected allocator tests.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
1c767f7f778819dcfa6eff51765163aec1b70a2d 
  src/tests/hierarchical_allocator_tests.cpp 
173e4fbac184ad8d40c8adba19ad64225f11f1f2 


Diff: https://reviews.apache.org/r/64003/diff/5/

Changes: https://reviews.apache.org/r/64003/diff/4-5/


Testing
---

Fixed a few broken tests due to this patch.
make check.


Thanks,

Meng Zhu

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-20 Thread Benjamin Mahler 

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



Tests look good, do we want to add a test to ensure unchoppable resources don't 
get chopped?


src/tests/hierarchical_allocator_tests.cpp
Line 3831 (original), 3834 (patched)


Is this comment still accurate? More than its quota?



src/tests/hierarchical_allocator_tests.cpp
Lines 5198-5200 (original)


Should we keep the content after the first comma?


- Benjamin Mahler


On Dec. 20, 2017, 2:06 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64003/
> ---
> 
> (Updated Dec. 20, 2017, 2:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-7099
> https://issues.apache.org/jira/browse/MESOS-7099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator now does fine-grained resource allocation up
> to the role’s quota limit. And a role's quota is only
> considered to be satisfied if all of its quota resources
> are satisfied. Previously, a role's quota is considered
> to be met if any one of its quota resources reaches the limit.
> 
> Also fixed a few affected allocator tests.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1c767f7f778819dcfa6eff51765163aec1b70a2d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 173e4fbac184ad8d40c8adba19ad64225f11f1f2 
> 
> 
> Diff: https://reviews.apache.org/r/64003/diff/4/
> 
> 
> Testing
> ---
> 
> Fixed a few broken tests due to this patch.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-20 Thread Benjamin Mahler 

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




src/master/allocator/mesos/hierarchical.cpp
Lines 1681-1684 (original), 1671-1674 (patched)


I asked alexr about this TODO, it has become stale because we now have to 
allocate the reservations and can't "skip" satisfied roles anymore. Feel free 
to remove it in a different patch.



src/master/allocator/mesos/hierarchical.cpp
Line 1686 (original), 1676 (patched)


is -> is a



src/master/allocator/mesos/hierarchical.cpp
Lines 1741-1752 (original), 1728-1745 (patched)


Hm.. perhaps a single overall explanation might be more readable? Here's a 
suggestion with a TODO for checking headroom:

```
// We allocate the role's reservations as well as any unreserved
// resources while ensuring the role stays within its quota limits.
// This means that we'll "chop" the unreserved resources up to
// the quota limit if necessary.
//
// E.g. A role has no allocations or reservations yet and a 10 cpu
//  quota limit. We'll chop a 15 cpu agent down to only
//  allocate 10 cpus to the role to keep it within its limit.
//
// In the case that the role needs some of the resources on this
// agent to make progress towards its quota, we'll *also* allocate
// all of the resources for which it does not have quota.
//
// E.g. The agent has 1 cpu, 1024 mem, 1024 disk, 1 gpu, 5 ports
//  and the role has quota for 1 cpu, 1024 mem. We'll include
//  the disk, gpu, and ports in the allocation, despite the
//  role not having any quota guarantee for them.
//
// We have to do this for now because it's not possible to set
// quota on non-scalar resources, like ports. However, for scalar
// resources that can have quota set, we should ideally make
// sure that when we include them, we're not violating some
// other role's guarantes!
//
// TODO(mzhu): Check the headroom when we're including scalar
// resources that this role does not have quota for.
//
// TODO(mzhu): Since we're treating the resources with unset
// quota as having no guarantee and no limit, these should be
// also be allocated further in the second allocation "phase"
// below (above guarantee up to limit).
```



src/master/allocator/mesos/hierarchical.cpp
Line 1752 (original), 1737-1742 (patched)


Hm.. I'm kind of surprised we have to distinctly track this, isn't this 
always just equal to the unreserved non-revocable resources within `resources`?



src/master/allocator/mesos/hierarchical.cpp
Lines 1740-1741 (patched)


Don't think we need this note, given that it applies to `resources` 
already? Especially if we don't need to track this in a separate variable



src/master/allocator/mesos/hierarchical.cpp
Lines 1744-1746 (patched)


Hm.. this if condition means that if I have:

Quota Guarantee/Limit: 1 cpu, 1024 mem
Agent has: 1 cpu, 1024 mem, 1 gpu, 5 ports

Case (1):
Existing Allocation: 1 cpu, 1024 mem
I'll be allocated: (nothing)

Case (2):
Existing Allocation: (nothing)
I'll be allocated: 1 cpu, 1024 mem, 1 gpu, 5 ports

In other words, we are deciding that if you need some resources on this 
agent to sastisfy your quota, then we'll give you everything else that you 
don't have quota for.



src/master/allocator/mesos/hierarchical.cpp
Lines 1747 (patched)


Updating available seems a little odd? Maybe this should be `Resources 
unreserved = ...`?



src/master/allocator/mesos/hierarchical.cpp
Lines 1752-1754 (patched)


Hm.. we may want to shed some light on the disk case that motivated this?

```
  // When "chopping" resources, there is more than 1 "chop" that
  // can be done to satisfy the limits. Consider the case with
  // two `RAW` disks of 1GB and a "disk" quota of 1GB. We could
  // pick either of the disks here, but not both.
  //
  // In order to avoid repeatedly choosing the same "chop" of
  // the resources each time we allocate, we introduce some

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-19 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 64003`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64003

Relevant logs:

- 
[apply-review-64003-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64003/logs/apply-review-64003-stderr.log):

```
Traceback (most recent call last):
  File ".\support\apply-reviews.py", line 434, in 
main()
  File ".\support\apply-reviews.py", line 429, in main
reviewboard(options)
  File ".\support\apply-reviews.py", line 419, in reviewboard
apply_review(options)
  File ".\support\apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File ".\support\apply-reviews.py", line 271, in commit_patch
message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 
112: ordinal not in range(128)
```

- Mesos Reviewbot Windows


On Dec. 20, 2017, 2:06 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64003/
> ---
> 
> (Updated Dec. 20, 2017, 2:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-7099
> https://issues.apache.org/jira/browse/MESOS-7099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator now does fine-grained resource allocation up
> to the role’s quota limit. And a role's quota is only
> considered to be satisfied if all of its quota resources
> are satisfied. Previously, a role's quota is considered
> to be met if any one of its quota resources reaches the limit.
> 
> Also fixed a few affected allocator tests.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1c767f7f778819dcfa6eff51765163aec1b70a2d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 173e4fbac184ad8d40c8adba19ad64225f11f1f2 
> 
> 
> Diff: https://reviews.apache.org/r/64003/diff/4/
> 
> 
> Testing
> ---
> 
> Fixed a few broken tests due to this patch.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-19 Thread Meng Zhu 

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

(Updated Dec. 19, 2017, 6:06 p.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.


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


Repository: mesos


Description (updated)
---

Allocator now does fine-grained resource allocation up
to the role’s quota limit. And a role's quota is only
considered to be satisfied if all of its quota resources
are satisfied. Previously, a role's quota is considered
to be met if any one of its quota resources reaches the limit.

Also fixed a few affected allocator tests.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
1c767f7f778819dcfa6eff51765163aec1b70a2d 
  src/tests/hierarchical_allocator_tests.cpp 
173e4fbac184ad8d40c8adba19ad64225f11f1f2 


Diff: https://reviews.apache.org/r/64003/diff/4/

Changes: https://reviews.apache.org/r/64003/diff/3-4/


Testing
---

Fixed a few broken tests due to this patch.
make check.


Thanks,

Meng Zhu

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-18 Thread Benjamin Mahler 

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




src/master/allocator/mesos/hierarchical.cpp
Line 1750 (original), 1797 (patched)


limits



src/master/allocator/mesos/hierarchical.cpp
Line 1751 (original), 1798 (patched)


I think this patch also needs to update the quota satisfaction criteria now 
that we can chop. Breaking when only a single guarantee is reached was meant as 
a stop-gap for the gaming ticket reference on `someGuaranteesReached`. Now we 
could move back to considering quota satisfied only when all of the guarantees 
are reached.



src/master/allocator/mesos/hierarchical.cpp
Line 1752 (original), 1799 (patched)


Maybe: These are all scalar quantities



src/master/allocator/mesos/hierarchical.cpp
Lines 1800 (patched)


Maybe `unsatisfiedQuota`?



src/master/allocator/mesos/hierarchical.cpp
Lines 1803 (patched)


Do we need a note here to clarify why we don't care to use 
`allocatableTo(role)`?

```
// Note that since we currently only support top-level roles to
// have quota, there are no ancestor reservations involved here.
```



src/master/allocator/mesos/hierarchical.cpp
Lines 1806-1814 (patched)


The more important distinction for the reader here seems to be the 
resources that have a limit and those that don't?

(1) resources w/ limit: always choppable, chop up to limit

(2) resources w/o limit: offer entirely, may or may not be choppable but we 
don't care..?

I think with the current logic here, if I don't have quota limit for 
"some-scalar", because it's choppable it will get excluded during the 
intersection and I won't get it offered?

Also, as discussed offline, this is missing metadata?


- Benjamin Mahler


On Dec. 12, 2017, 2:33 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64003/
> ---
> 
> (Updated Dec. 12, 2017, 2:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-7099
> https://issues.apache.org/jira/browse/MESOS-7099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator now does fine-grained resource allocation up
> to the role’s quota limit.
> 
> Also fixed a few affected allocator tests.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> fccd71c96fe8e4d914e19b5ea8d8f69e9ebf2406 
>   src/tests/hierarchical_allocator_tests.cpp 
> f5fb47ed09682ebdd047aec7e79a86597ee09f53 
> 
> 
> Diff: https://reviews.apache.org/r/64003/diff/3/
> 
> 
> Testing
> ---
> 
> Fixed a few broken tests due to this patch.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-18 Thread Jie Yu 

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




src/master/allocator/mesos/hierarchical.cpp
Lines 1599 (patched)


RAW without volume_id is choppable.


- Jie Yu


On Dec. 12, 2017, 2:33 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64003/
> ---
> 
> (Updated Dec. 12, 2017, 2:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-7099
> https://issues.apache.org/jira/browse/MESOS-7099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator now does fine-grained resource allocation up
> to the role’s quota limit.
> 
> Also fixed a few affected allocator tests.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> fccd71c96fe8e4d914e19b5ea8d8f69e9ebf2406 
>   src/tests/hierarchical_allocator_tests.cpp 
> f5fb47ed09682ebdd047aec7e79a86597ee09f53 
> 
> 
> Diff: https://reviews.apache.org/r/64003/diff/3/
> 
> 
> Testing
> ---
> 
> Fixed a few broken tests due to this patch.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-14 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 64465.

Failed command: `python.exe .\support\apply-reviews.py -n -r 64465`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64003

Relevant logs:

- 
[apply-review-64465-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64003/logs/apply-review-64465-stderr.log):

```
Traceback (most recent call last):
  File ".\support\apply-reviews.py", line 434, in 
main()
  File ".\support\apply-reviews.py", line 429, in main
reviewboard(options)
  File ".\support\apply-reviews.py", line 419, in reviewboard
apply_review(options)
  File ".\support\apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File ".\support\apply-reviews.py", line 271, in commit_patch
message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 
131: ordinal not in range(128)
```

- Mesos Reviewbot Windows


On Dec. 12, 2017, 2:33 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64003/
> ---
> 
> (Updated Dec. 12, 2017, 2:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-7099
> https://issues.apache.org/jira/browse/MESOS-7099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator now does fine-grained resource allocation up
> to the role’s quota limit.
> 
> Also fixed a few affected allocator tests.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> fccd71c96fe8e4d914e19b5ea8d8f69e9ebf2406 
>   src/tests/hierarchical_allocator_tests.cpp 
> f5fb47ed09682ebdd047aec7e79a86597ee09f53 
> 
> 
> Diff: https://reviews.apache.org/r/64003/diff/3/
> 
> 
> Testing
> ---
> 
> Fixed a few broken tests due to this patch.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-11 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 64465.

Failed command: `python.exe .\support\apply-reviews.py -n -r 64465`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64003

Relevant logs:

- 
[apply-review-64465-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64003/logs/apply-review-64465-stderr.log):

```
Traceback (most recent call last):
  File ".\support\apply-reviews.py", line 434, in 
main()
  File ".\support\apply-reviews.py", line 429, in main
reviewboard(options)
  File ".\support\apply-reviews.py", line 419, in reviewboard
apply_review(options)
  File ".\support\apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File ".\support\apply-reviews.py", line 271, in commit_patch
message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 
131: ordinal not in range(128)
```

- Mesos Reviewbot Windows


On Dec. 11, 2017, 6:33 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64003/
> ---
> 
> (Updated Dec. 11, 2017, 6:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-7099
> https://issues.apache.org/jira/browse/MESOS-7099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator now does fine-grained resource allocation up
> to the role’s quota limit.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 49a3b9e7a187cf2afa5cee2e86f4d5e752aee5cb 
>   src/tests/hierarchical_allocator_tests.cpp 
> 862f4683da04d37d9fe9f471d6ec9cd7751f39ec 
> 
> 
> Diff: https://reviews.apache.org/r/64003/diff/2/
> 
> 
> Testing
> ---
> 
> Fixed a few broken tests due to this patch.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 64003: Made quota resource allocation fine-grained.

2017-12-11 Thread Meng Zhu 

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

(Updated Dec. 11, 2017, 6:33 p.m.)


Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description (updated)
---

Allocator now does fine-grained resource allocation up
to the role’s quota limit.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
49a3b9e7a187cf2afa5cee2e86f4d5e752aee5cb 
  src/tests/hierarchical_allocator_tests.cpp 
862f4683da04d37d9fe9f471d6ec9cd7751f39ec 


Diff: https://reviews.apache.org/r/64003/diff/2/

Changes: https://reviews.apache.org/r/64003/diff/1-2/


Testing (updated)
---

Fixed a few broken tests due to this patch.
make check.


Thanks,

Meng Zhu

Review Request 64003: Made quota resource allocation fine-grained.

2017-11-21 Thread Meng Zhu 

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Now an agent will offer no more resources than the role’s quota
even if the agent has more available resources. As a result, quota
limit will not be exceeded.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
cfeeb3bfa3ad7d78ff6e22cfc4adb1f0efa05629 
  src/tests/hierarchical_allocator_tests.cpp 
f0f95ba4f667bf8ea54e985d8cde913a4170d8ff 


Diff: https://reviews.apache.org/r/64003/diff/1/


Testing
---

Fixed a few broken tests due to this patch.
make check pass.


Thanks,

Meng Zhu