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 Mahler <bm...@apache.org> on 2019/07/08 18:07:32 UTC
Review Request 71030: Added quota_consumption field to /roles
endpoint.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71030/
-----------------------------------------------------------
Review request for mesos, Andrei Sekretenko and Meng Zhu.
Bugs: MESOS-9871
https://issues.apache.org/jira/browse/MESOS-9871
Repository: mesos
Description
-------
Per the previous commit that adds the quota consumption computation,
quota consumption is:
Allocation + Unallocated Reservation ==
Reservations + Unreserved Allocation
That is, reservations count towards quota regardless of whether
they're allocated. Allocation counts towards quota. Offered resources
*do not* count towards quota, this is to (1) provide stability of the
quota consumption metrics in the face of offers flowing in and out,
and (2) to ensure that we treat offers as rescindable and therefore
not yet "counting" towards quota. Also, if in the future schedulers
are offered more than their quota to improve their choices, counting
offered resources as quota consumption will be problematic.
Diffs
-----
src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b
src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79
Diff: https://reviews.apache.org/r/71030/diff/1/
Testing
-------
Added a test in a subsequent patch.
Thanks,
Benjamin Mahler
Re: Review Request 71030: Added quota_consumption field to /roles
endpoint.
Posted by Benjamin Mahler <bm...@apache.org>.
> On July 9, 2019, 12:27 a.m., Meng Zhu wrote:
> > src/master/readonly_handler.cpp
> > Lines 738 (patched)
> > <https://reviews.apache.org/r/71030/diff/1/?file=2153964#file2153964line738>
> >
> > I am thinking we should just call it `consumption`, `quota_consumption` might throw some people off guard if a role has default quota (where user did not set any quota on). It is also more clean to me, as we say allocation, not quota_allocation.
> >
> > If we change this, should also change the accessor function name.
Per offline discussion, we can place this inside the quota field (and make the quota field always present like "weight"):
```
"quota": {
"consumption": {"cpus":1, "mem":32},
"guarantees": {},
"limits": {}
}
```
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71030/#review216435
-----------------------------------------------------------
On July 8, 2019, 6:07 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71030/
> -----------------------------------------------------------
>
> (Updated July 8, 2019, 6:07 p.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
>
>
> Bugs: MESOS-9871
> https://issues.apache.org/jira/browse/MESOS-9871
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Per the previous commit that adds the quota consumption computation,
> quota consumption is:
>
> Allocation + Unallocated Reservation ==
> Reservations + Unreserved Allocation
>
> That is, reservations count towards quota regardless of whether
> they're allocated. Allocation counts towards quota. Offered resources
> *do not* count towards quota, this is to (1) provide stability of the
> quota consumption metrics in the face of offers flowing in and out,
> and (2) to ensure that we treat offers as rescindable and therefore
> not yet "counting" towards quota. Also, if in the future schedulers
> are offered more than their quota to improve their choices, counting
> offered resources as quota consumption will be problematic.
>
>
> Diffs
> -----
>
> src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b
> src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79
>
>
> Diff: https://reviews.apache.org/r/71030/diff/1/
>
>
> Testing
> -------
>
> Added a test in a subsequent patch.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 71030: Added quota_consumption field to /roles
endpoint.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71030/#review216435
-----------------------------------------------------------
As mentioned in previous patch, add composition? A TODO is fine.
src/master/readonly_handler.cpp
Lines 738 (patched)
<https://reviews.apache.org/r/71030/#comment303673>
I am thinking we should just call it `consumption`, `quota_consumption` might throw some people off guard if a role has default quota (where user did not set any quota on). It is also more clean to me, as we say allocation, not quota_allocation.
If we change this, should also change the accessor function name.
- Meng Zhu
On July 8, 2019, 11:07 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71030/
> -----------------------------------------------------------
>
> (Updated July 8, 2019, 11:07 a.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
>
>
> Bugs: MESOS-9871
> https://issues.apache.org/jira/browse/MESOS-9871
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Per the previous commit that adds the quota consumption computation,
> quota consumption is:
>
> Allocation + Unallocated Reservation ==
> Reservations + Unreserved Allocation
>
> That is, reservations count towards quota regardless of whether
> they're allocated. Allocation counts towards quota. Offered resources
> *do not* count towards quota, this is to (1) provide stability of the
> quota consumption metrics in the face of offers flowing in and out,
> and (2) to ensure that we treat offers as rescindable and therefore
> not yet "counting" towards quota. Also, if in the future schedulers
> are offered more than their quota to improve their choices, counting
> offered resources as quota consumption will be problematic.
>
>
> Diffs
> -----
>
> src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b
> src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79
>
>
> Diff: https://reviews.apache.org/r/71030/diff/1/
>
>
> Testing
> -------
>
> Added a test in a subsequent patch.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 71030: Added quota.consumed field to /roles
endpoint.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71030/#review216467
-----------------------------------------------------------
Patch looks great!
Reviews applied: [71027, 71028, 71029, 71030]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On July 10, 2019, 1:19 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71030/
> -----------------------------------------------------------
>
> (Updated July 10, 2019, 1:19 a.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
>
>
> Bugs: MESOS-9871
> https://issues.apache.org/jira/browse/MESOS-9871
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Per the previous commit that adds the consumed quota computation,
> consumed quota is:
>
> Allocation + Unallocated Reservation ==
> Reservations + Unreserved Allocation
>
> That is, reservations count towards quota regardless of whether
> they're allocated. Allocation counts towards quota. Offered resources
> *do not* count towards quota, this is to (1) provide stability of the
> quota consumption metrics in the face of offers flowing in and out,
> and (2) to ensure that we treat offers as rescindable and therefore
> not yet "counting" towards quota. Also, if in the future schedulers
> are offered more than their quota to improve their choices, counting
> offered resources as consumed quota will be problematic.
>
>
> Diffs
> -----
>
> src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b
> src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79
>
>
> Diff: https://reviews.apache.org/r/71030/diff/2/
>
>
> Testing
> -------
>
> Added a test in a subsequent patch.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 71030: Added quota.consumed field to /roles
endpoint.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71030/
-----------------------------------------------------------
(Updated July 10, 2019, 1:19 a.m.)
Review request for mesos, Andrei Sekretenko and Meng Zhu.
Changes
-------
- Renamed to quota.consumption, and always expose quota field.
Summary (updated)
-----------------
Added quota.consumed field to /roles endpoint.
Bugs: MESOS-9871
https://issues.apache.org/jira/browse/MESOS-9871
Repository: mesos
Description (updated)
-------
Per the previous commit that adds the consumed quota computation,
consumed quota is:
Allocation + Unallocated Reservation ==
Reservations + Unreserved Allocation
That is, reservations count towards quota regardless of whether
they're allocated. Allocation counts towards quota. Offered resources
*do not* count towards quota, this is to (1) provide stability of the
quota consumption metrics in the face of offers flowing in and out,
and (2) to ensure that we treat offers as rescindable and therefore
not yet "counting" towards quota. Also, if in the future schedulers
are offered more than their quota to improve their choices, counting
offered resources as consumed quota will be problematic.
Diffs (updated)
-----
src/master/readonly_handler.cpp 0d1e3dc19352863263c3e8992a63852c7f27225b
src/tests/role_tests.cpp bf5b3cba59d38eee41ed222d14bc8c928fcf2a79
Diff: https://reviews.apache.org/r/71030/diff/2/
Changes: https://reviews.apache.org/r/71030/diff/1-2/
Testing
-------
Added a test in a subsequent patch.
Thanks,
Benjamin Mahler