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 2019/01/23 12:12:43 UTC
Review Request 69819: Added equality operator for
`ResourceQuantities`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69819/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler and Meng Zhu.
Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523
Repository: mesos
Description
-------
This patch adds an `operator==` for `ResourceQuantities`. The operator
is declared as a `friend` of `ResourceQuantities` so we can directly
compare the two `ResourceQuantities::quantities` which can be less
expensive.
Diffs
-----
src/common/resource_quantities.hpp 11eb426104577bbbbb7977c2307df3e4917085cd
src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f
Diff: https://reviews.apache.org/r/69819/diff/1/
Testing
-------
`make check`
Thanks,
Benjamin Bannier
Re: Review Request 69819: Added equality operator for
`ResourceQuantities`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Jan. 28, 2019, 8:49 p.m., Benjamin Mahler wrote:
> > When we added `ResourceQuantities`, we wanted to support two use cases:
> >
> > ```
> > guarantees: absent == 0 (i.e. zero_by_default)
> > limits: absent == +inf (i.e. inf_by_default)
> > ```
> >
> > This is why zeros are currently stored (unlike `Resources` objects, where they are removed): having the ability to distinguish between 0 and absent allows us to support both use cases. However, the introduction of equality and containment operations are not so obvious:
> >
> > ```
> > equality: the approach in this patch is providing inf_by_default equality semantics AFAICT
> > containment: the approach in the subsequent patch is providing neither zero_by_default or inf_by_default equality semantics
> > ```
> >
> > I think that in order to introduce equality and containment, we may have to address some earlier feedback on `ResourceQuantities`, which was to require a contruction-time choice between `ZERO_BY_DEFAULT` and `INF_BY_DEFAULT` semantics. This would be stored as a variable in order to determine (1) whether to store zeros, (2) how to evaluate equality and containment between two quantities.
> >
> > Thoughts?
Yes, that makes sense. I am still unsure whether one should build limit- or guarantee-specific functionality into `ResourceQuantities` though as having a light-weight building block for more involved semantics, e.g., around quotas.
I am discarding this patch and https://reviews.apache.org/r/69820/ for now as they are not needed anymore.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69819/#review212390
-----------------------------------------------------------
On Jan. 29, 2019, 5:20 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69819/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2019, 5:20 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Meng Zhu.
>
>
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds an `operator==` for `ResourceQuantities`. The operator
> is declared as a `friend` of `ResourceQuantities` so we can directly
> compare the two `ResourceQuantities::quantities` which can be less
> expensive.
>
>
> Diffs
> -----
>
> src/common/resource_quantities.hpp 11eb426104577bbbbb7977c2307df3e4917085cd
> src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f
>
>
> Diff: https://reviews.apache.org/r/69819/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69819: Added equality operator for
`ResourceQuantities`.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69819/#review212390
-----------------------------------------------------------
When we added `ResourceQuantities`, we wanted to support two use cases:
```
guarantees: absent == 0 (i.e. zero_by_default)
limits: absent == +inf (i.e. inf_by_default)
```
This is why zeros are currently stored (unlike `Resources` objects, where they are removed): having the ability to distinguish between 0 and absent allows us to support both use cases. However, the introduction of equality and containment operations are not so obvious:
```
equality: the approach in this patch is providing inf_by_default equality semantics AFAICT
containment: the approach in the subsequent patch is providing neither zero_by_default or inf_by_default equality semantics
```
I think that in order to introduce equality and containment, we may have to address some earlier feedback on `ResourceQuantities`, which was to require a contruction-time choice between `ZERO_BY_DEFAULT` and `INF_BY_DEFAULT` semantics. This would be stored as a variable in order to determine (1) whether to store zeros, (2) how to evaluate equality and containment between two quantities.
Thoughts?
- Benjamin Mahler
On Jan. 23, 2019, 12:12 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69819/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2019, 12:12 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Meng Zhu.
>
>
> Bugs: MESOS-9523
> https://issues.apache.org/jira/browse/MESOS-9523
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds an `operator==` for `ResourceQuantities`. The operator
> is declared as a `friend` of `ResourceQuantities` so we can directly
> compare the two `ResourceQuantities::quantities` which can be less
> expensive.
>
>
> Diffs
> -----
>
> src/common/resource_quantities.hpp 11eb426104577bbbbb7977c2307df3e4917085cd
> src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f
>
>
> Diff: https://reviews.apache.org/r/69819/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69819: Added equality operator for
`ResourceQuantities`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69819/
-----------------------------------------------------------
(Updated Jan. 29, 2019, 5:20 p.m.)
Review request for mesos, Benjamin Mahler and Meng Zhu.
Changes
-------
Rebased.
Bugs: MESOS-9523
https://issues.apache.org/jira/browse/MESOS-9523
Repository: mesos
Description
-------
This patch adds an `operator==` for `ResourceQuantities`. The operator
is declared as a `friend` of `ResourceQuantities` so we can directly
compare the two `ResourceQuantities::quantities` which can be less
expensive.
Diffs (updated)
-----
src/common/resource_quantities.hpp 11eb426104577bbbbb7977c2307df3e4917085cd
src/common/resource_quantities.cpp 320983929cd7d14973c4b98d6ed5338de690ff5f
Diff: https://reviews.apache.org/r/69819/diff/2/
Changes: https://reviews.apache.org/r/69819/diff/1-2/
Testing
-------
`make check`
Thanks,
Benjamin Bannier