You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2017/01/17 20:42:32 UTC
Review Request 55625: Prevented certain kinds of gaming the quota
system.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55625/
-----------------------------------------------------------
Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
Bugs: MESOS-6432
https://issues.apache.org/jira/browse/MESOS-6432
Repository: mesos
Description
-------
In the certain coarse-grained allocation scheme it is possible to game
quota since the allocator will perform allocations until the quotas
for all resource kinds are satisfied. This can lead to allocations for
certain resource kinds far exceeding the set quota.
This patch changes the notion of what constitutes a "satisfied quota".
Where before a quota could only be satisfied if for all resource kinds
allocations had been me up to the set quota, we now consider a quota
satisfied as soon as at least one resource kinds is allocated up to
the set quota. We here take only "common" resource kinds into account
where a common resource is a resource present on every agent node. The
notion of common resource kinds is needed to avoid strong bias from
rare resources in the introduced quota allocation scheme.
Ultimately this change can only be temporary and should be replaced
with a more complete fix, e.g., chunked allocations, see MESOS-3765.
Diffs
-----
src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
src/tests/hierarchical_allocator_tests.cpp 1edd0ecc8a93cd41532e1cf3641f67c780ab23a5
Diff: https://reviews.apache.org/r/55625/diff/
Testing
-------
Tested of various Linux configurations in internal CI.
Thanks,
Benjamin Bannier
Re: Review Request 55625: Prevented certain kinds of gaming the quota
system.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55625/#review163617
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55625]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Jan. 30, 2017, 10:27 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55625/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2017, 10:27 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
>
>
> Bugs: MESOS-6432
> https://issues.apache.org/jira/browse/MESOS-6432
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In the certain coarse-grained allocation scheme it is possible to game
> quota since the allocator will perform allocations until the quotas
> for all resource kinds are satisfied. This can lead to allocations for
> certain resource kinds far exceeding the set quota.
>
> This patch changes the notion of what constitutes a "satisfied quota".
> Where before a quota could only be satisfied if for all resource kinds
> allocations had been me up to the set quota, we now consider a quota
> satisfied as soon as at least one resource kinds is allocated up to
> the set quota. We here take only "common" resource kinds into account
> where a common resource is a resource present on every agent node. The
> notion of common resource kinds is needed to avoid strong bias from
> rare resources in the introduced quota allocation scheme.
>
> Ultimately this change can only be temporary and should be replaced
> with a more complete fix, e.g., chunked allocations, see MESOS-3765.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp f471b6848bebae601a7a0509e9c6ad5eab4fa4a2
> src/tests/hierarchical_allocator_tests.cpp e04d1998679fcf022bb3741676a62da8b01ce97c
>
> Diff: https://reviews.apache.org/r/55625/diff/
>
>
> Testing
> -------
>
> Tested of various Linux configurations in internal CI.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 55625: Prevented certain kinds of gaming the quota
system.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55625/#review164287
-----------------------------------------------------------
Fix it, then Ship it!
Thanks Benjamin! High level thought here: there seems to be some inconsistent language around the case of one or more (but not all) of the quota guarantees being met. We should probably avoid saying that the quota is "satisfied" in the case that not all of the guarantees is reached. "Partially satisfied" seems more correct but it is ambiguous as to whether this refers to one of the guarantees having been met vs some allocation but none of the guarantees being met. "Minimally satisfied" has an implication of there being a "maximally" or "fully satisfied", this works but then we don't have a way to refer to the case between minimum and maximum.
Looks like the description needs an update to reflect that we no longer special case common vs scarce resources. E.g.
```
Prevent gaming of the quota system due to demand:quota ratio mismatch.
It is currently possible to game the quota system since the allocator
will perform allocations until the quotas for all resource types are
satisfied. This can lead to allocations for certain resource type
far exceeding the set quota.
In order to prevent this issue, this patch changes the notion of what
constitutes a "satisfied quota". Where before a quota could only be
satisfied when allocations had been met up to the set quota for every
resource type, we now consider a quota satisfied as soon as at least
one of the resource types is allocated up to the set quota. What this
means is that if the allocation and quota vectors are of the same
proportion, this new technique behaves the same as the previous
technique. However, if the allocation and quota vectors have different
resource type ratios, then the quota becomes satisfied as soon as
the any quota guarantee is hit.
Longer term, we can consider allocating the resource types that have
not reached the quota guarantee, but this is non-trivial (consider
the case of having to allocate only memory, this means we likely
should only offer it in such a manner that it allows an existing
container to increase its memory; it cannot be used to create a
new container unless there is a mechanism to obtain best-effort
cpu/disk).
Also note that the quota guarantee is currently applied as a limit,
whereas longer term we would like to (1) make these distinct, and
(2) distinguish quota and fair sharing via non-revocable and
revocable resources, respectively.
```
src/master/allocator/mesos/hierarchical.cpp (lines 1368 - 1372)
<https://reviews.apache.org/r/55625/#comment235971>
Looks like we don't need this after my suggestion below.
src/master/allocator/mesos/hierarchical.cpp (lines 1394 - 1397)
<https://reviews.apache.org/r/55625/#comment235970>
Hm.. not related to this change but I couldn't follow this TODO? Do you understand it?
src/master/allocator/mesos/hierarchical.cpp (line 1398)
<https://reviews.apache.org/r/55625/#comment235981>
Per my suggestion above, maybe "someGuaranteesReached" to be a bit clearer that quota isn't satisfied?
src/master/allocator/mesos/hierarchical.cpp (lines 1400 - 1401)
<https://reviews.apache.org/r/55625/#comment235969>
Seems we could incorporate this into the existing comment? We should perhaps include a reference to MESOS-6432 for posterity. Also, maybe a bit of discussion about the longer term:
```
// Longer term, we could ideally allocate what remains
// unsatisfied to allow an existing container to scale
// vertically, or to allow the launching of a container
// with best-effort cpus/mem/disk/etc.
```
src/master/allocator/mesos/hierarchical.cpp (lines 1530 - 1533)
<https://reviews.apache.org/r/55625/#comment235968>
Hm.. what was the reasoning for adding this? While it tries to hold back fewer resources than necessary given the current quota allocation to the role, it seems a bit suprising that it no longer holds back sufficient resources to satisfy the quota should the allocation change.
E.g.
Quota: 200cpus, 200mem
Allocation: 100cpus, 200mem
Previously, we would hold back 100 cpus, but with this change we don't hold any back. So if this role were to re-size its tasks to increase the cpu we didn't hold sufficient resources back.
I'm inclined to leave this out, let me know your thoughts!
src/tests/hierarchical_allocator_tests.cpp (lines 3538 - 3542)
<https://reviews.apache.org/r/55625/#comment235973>
How about the following?
```
// This tests the behavior of quota when the allocation and
// quota are disproportionate. The current approach (see MESOS-6432)
// is to consider the quota satisfied once one of the resource
// guarantees is reached. We test the following example:
//
// Quota: cpus:4;mem:1024
// Allocation: cpus:2;mem:1024
//
// Here the quota is considered satisfied since the memory
// guarantee is reached. Longer term, we could offer the
// unsatisfied cpus to allow an existing container to scale
// vertically, or to allow the launching of a container with
// best-effort memory/disk, etc.
```
src/tests/hierarchical_allocator_tests.cpp (line 3543)
<https://reviews.apache.org/r/55625/#comment235972>
Hm.. minimally might be a bit unclear, how about something like "DisproportionateQuotaVsAllocation"?
src/tests/hierarchical_allocator_tests.cpp (lines 3558 - 3559)
<https://reviews.apache.org/r/55625/#comment235966>
I was going to suggest storing Resources directly rather than a string, but it seems createQuota only takes a string, that's unfortunate.
src/tests/hierarchical_allocator_tests.cpp (lines 3573 - 3574)
<https://reviews.apache.org/r/55625/#comment235974>
Maybe something like this?
"With that QUOTA_ROLE will not receive more resources since we currently do not have an ability to offer out only the cpus."
src/tests/hierarchical_allocator_tests.cpp (lines 3579 - 3582)
<https://reviews.apache.org/r/55625/#comment235977>
We can use the allocation equality / construction that was recently introduced:
```
Allocation expected = Allocation(
framework1.id(),
{{"role1", {{agent1.id(), agent1.resources()}}}});
AWAIT_EXPECT_EQ(expected, allocations.get());
```
- Benjamin Mahler
On Jan. 30, 2017, 10:27 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55625/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2017, 10:27 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
>
>
> Bugs: MESOS-6432
> https://issues.apache.org/jira/browse/MESOS-6432
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In the certain coarse-grained allocation scheme it is possible to game
> quota since the allocator will perform allocations until the quotas
> for all resource kinds are satisfied. This can lead to allocations for
> certain resource kinds far exceeding the set quota.
>
> This patch changes the notion of what constitutes a "satisfied quota".
> Where before a quota could only be satisfied if for all resource kinds
> allocations had been me up to the set quota, we now consider a quota
> satisfied as soon as at least one resource kinds is allocated up to
> the set quota. We here take only "common" resource kinds into account
> where a common resource is a resource present on every agent node. The
> notion of common resource kinds is needed to avoid strong bias from
> rare resources in the introduced quota allocation scheme.
>
> Ultimately this change can only be temporary and should be replaced
> with a more complete fix, e.g., chunked allocations, see MESOS-3765.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp f471b6848bebae601a7a0509e9c6ad5eab4fa4a2
> src/tests/hierarchical_allocator_tests.cpp e04d1998679fcf022bb3741676a62da8b01ce97c
>
> Diff: https://reviews.apache.org/r/55625/diff/
>
>
> Testing
> -------
>
> Tested of various Linux configurations in internal CI.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 55625: Prevented certain kinds of gaming the quota
system.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55625/
-----------------------------------------------------------
(Updated Jan. 30, 2017, 11:27 p.m.)
Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
Bugs: MESOS-6432
https://issues.apache.org/jira/browse/MESOS-6432
Repository: mesos
Description
-------
In the certain coarse-grained allocation scheme it is possible to game
quota since the allocator will perform allocations until the quotas
for all resource kinds are satisfied. This can lead to allocations for
certain resource kinds far exceeding the set quota.
This patch changes the notion of what constitutes a "satisfied quota".
Where before a quota could only be satisfied if for all resource kinds
allocations had been me up to the set quota, we now consider a quota
satisfied as soon as at least one resource kinds is allocated up to
the set quota. We here take only "common" resource kinds into account
where a common resource is a resource present on every agent node. The
notion of common resource kinds is needed to avoid strong bias from
rare resources in the introduced quota allocation scheme.
Ultimately this change can only be temporary and should be replaced
with a more complete fix, e.g., chunked allocations, see MESOS-3765.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.cpp f471b6848bebae601a7a0509e9c6ad5eab4fa4a2
src/tests/hierarchical_allocator_tests.cpp e04d1998679fcf022bb3741676a62da8b01ce97c
Diff: https://reviews.apache.org/r/55625/diff/
Testing
-------
Tested of various Linux configurations in internal CI.
Thanks,
Benjamin Bannier
Re: Review Request 55625: Prevented certain kinds of gaming the quota
system.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55625/#review163548
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55625]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Jan. 30, 2017, 4:55 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55625/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2017, 4:55 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
>
>
> Bugs: MESOS-6432
> https://issues.apache.org/jira/browse/MESOS-6432
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In the certain coarse-grained allocation scheme it is possible to game
> quota since the allocator will perform allocations until the quotas
> for all resource kinds are satisfied. This can lead to allocations for
> certain resource kinds far exceeding the set quota.
>
> This patch changes the notion of what constitutes a "satisfied quota".
> Where before a quota could only be satisfied if for all resource kinds
> allocations had been me up to the set quota, we now consider a quota
> satisfied as soon as at least one resource kinds is allocated up to
> the set quota. We here take only "common" resource kinds into account
> where a common resource is a resource present on every agent node. The
> notion of common resource kinds is needed to avoid strong bias from
> rare resources in the introduced quota allocation scheme.
>
> Ultimately this change can only be temporary and should be replaced
> with a more complete fix, e.g., chunked allocations, see MESOS-3765.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp f471b6848bebae601a7a0509e9c6ad5eab4fa4a2
> src/tests/hierarchical_allocator_tests.cpp e04d1998679fcf022bb3741676a62da8b01ce97c
>
> Diff: https://reviews.apache.org/r/55625/diff/
>
>
> Testing
> -------
>
> Tested of various Linux configurations in internal CI.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 55625: Prevented certain kinds of gaming the quota
system.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55625/
-----------------------------------------------------------
(Updated Jan. 30, 2017, 5:55 p.m.)
Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
Changes
-------
Removed the notion of "common" resource kinds.
Bugs: MESOS-6432
https://issues.apache.org/jira/browse/MESOS-6432
Repository: mesos
Description
-------
In the certain coarse-grained allocation scheme it is possible to game
quota since the allocator will perform allocations until the quotas
for all resource kinds are satisfied. This can lead to allocations for
certain resource kinds far exceeding the set quota.
This patch changes the notion of what constitutes a "satisfied quota".
Where before a quota could only be satisfied if for all resource kinds
allocations had been me up to the set quota, we now consider a quota
satisfied as soon as at least one resource kinds is allocated up to
the set quota. We here take only "common" resource kinds into account
where a common resource is a resource present on every agent node. The
notion of common resource kinds is needed to avoid strong bias from
rare resources in the introduced quota allocation scheme.
Ultimately this change can only be temporary and should be replaced
with a more complete fix, e.g., chunked allocations, see MESOS-3765.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.cpp f471b6848bebae601a7a0509e9c6ad5eab4fa4a2
src/tests/hierarchical_allocator_tests.cpp e04d1998679fcf022bb3741676a62da8b01ce97c
Diff: https://reviews.apache.org/r/55625/diff/
Testing
-------
Tested of various Linux configurations in internal CI.
Thanks,
Benjamin Bannier
Re: Review Request 55625: Prevented certain kinds of gaming the quota
system.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55625/#review161981
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55625]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos ReviewBot
On Jan. 17, 2017, 8:42 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55625/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2017, 8:42 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
>
>
> Bugs: MESOS-6432
> https://issues.apache.org/jira/browse/MESOS-6432
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In the certain coarse-grained allocation scheme it is possible to game
> quota since the allocator will perform allocations until the quotas
> for all resource kinds are satisfied. This can lead to allocations for
> certain resource kinds far exceeding the set quota.
>
> This patch changes the notion of what constitutes a "satisfied quota".
> Where before a quota could only be satisfied if for all resource kinds
> allocations had been me up to the set quota, we now consider a quota
> satisfied as soon as at least one resource kinds is allocated up to
> the set quota. We here take only "common" resource kinds into account
> where a common resource is a resource present on every agent node. The
> notion of common resource kinds is needed to avoid strong bias from
> rare resources in the introduced quota allocation scheme.
>
> Ultimately this change can only be temporary and should be replaced
> with a more complete fix, e.g., chunked allocations, see MESOS-3765.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
> src/tests/hierarchical_allocator_tests.cpp 1edd0ecc8a93cd41532e1cf3641f67c780ab23a5
>
> Diff: https://reviews.apache.org/r/55625/diff/
>
>
> Testing
> -------
>
> Tested of various Linux configurations in internal CI.
>
>
> Thanks,
>
> Benjamin Bannier
>
>