You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2019/03/27 05:31:13 UTC

Review Request 70320: Used `ResourceQuantities` in `__allocate()` when possible.

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
-------

Replaced `Resources` with `ResourceQuantities` when possible
in `__allocate()`. This simplifies the code and improves
performance.


Diffs
-----

  src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
  src/master/allocator/mesos/hierarchical.cpp 8bc749903b8ceb09a02e260919377483479302b5 


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


Testing
-------

make check
Benchmark results coming in subsequent patches.


Thanks,

Meng Zhu


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when appropriate.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70320/#review214152
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70318, 70319, 70320]

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 March 27, 2019, 5:31 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 27, 2019, 5:31 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when
> appropriate in `__allocate()`. This simplifies the
> code and improves performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp 8bc749903b8ceb09a02e260919377483479302b5 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results in r/70330
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when appropriate.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70320/#review214242
-----------------------------------------------------------


Ship it!




Ship It!

- Benjamin Mahler


On March 29, 2019, 11:22 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 29, 2019, 11:22 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when
> appropriate in `__allocate()`. This simplifies the
> code and improves performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp ef4f968a94d3a23c010202e27cc48cd10bbec2c0 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results in r/70330
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when appropriate.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70320/
-----------------------------------------------------------

(Updated March 29, 2019, 4:22 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
-------

Addressed Ben's comment.


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


Repository: mesos


Description
-------

Replaced `Resources` with `ResourceQuantities` when
appropriate in `__allocate()`. This simplifies the
code and improves performance.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
  src/master/allocator/mesos/hierarchical.cpp ef4f968a94d3a23c010202e27cc48cd10bbec2c0 


Diff: https://reviews.apache.org/r/70320/diff/3/

Changes: https://reviews.apache.org/r/70320/diff/2-3/


Testing
-------

make check
Benchmark results in r/70330


Thanks,

Meng Zhu


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when possible.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70320/#review214088
-----------------------------------------------------------



PASS: Mesos patch 70320 was successfully built and tested.

Reviews applied: `['70318', '70319', '70320']`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3024/mesos-review-70320

- Mesos Reviewbot Windows


On March 27, 2019, 6:31 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 27, 2019, 6:31 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when possible
> in `__allocate()`. This simplifies the code and improves
> performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp 8bc749903b8ceb09a02e260919377483479302b5 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results coming in subsequent patches.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when possible.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70320/#review214089
-----------------------------------------------------------



Patch looks great!

Reviews applied: [70318, 70319, 70320]

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 March 27, 2019, 5:31 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 27, 2019, 5:31 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when possible
> in `__allocate()`. This simplifies the code and improves
> performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp 8bc749903b8ceb09a02e260919377483479302b5 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results coming in subsequent patches.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when appropriate.

Posted by Meng Zhu <mz...@mesosphere.io>.

> On March 27, 2019, 7:41 a.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1928 (patched)
> > <https://reviews.apache.org/r/70320/diff/1/?file=2134448#file2134448line1938>
> >
> >     Rather than adding this additional function, can't `shrinkResoures` take a `ResourceQuantities`?

We need to create a map-like a copy somewhere. `shrinkResoures` needs a copy of its own to do bookkeeping:
https://github.com/apache/mesos/blob/c10def96653fb99e309459476f5bc09c1da686ee/src/master/allocator/mesos/hierarchical.cpp#L1666

Or do you mean to create the map in the `shrinkResoures` function? My concern with that is currently `shrinkResources` has this semantic:
>If a resource does not have a target quantity provided, it will not be shrunk.

This is not compatible with `ResourceQuantities`. Given the semantic of `ResourceQuantities`, one would expect `shrinkResources` should drop (shrink to zero) for resources that are absent in the input `ResourceQuantities`.

Of course, since we are the only consumer of the function here, we could change the function semantic. But I do not see much benefits since, as mentioned above, we need to pass a map anyway.

Another thing to consider is we might need to get a copy of the scalar map somewhere else where direct mutations are needed.


> On March 27, 2019, 7:41 a.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2088-2091 (original), 2079-2082 (patched)
> > <https://reviews.apache.org/r/70320/diff/1/?file=2134448#file2134448line2106>
> >
> >     This looks like a `ResourceQuantities`?

Yes and no. Below:
```
if (!sufficientHeadroom) {
  toAllocate -= headroomToAllocate;
}
```
We need that as `Resources`.

But I do notice that we convert it to `ResourceQuantities` twice below. Might as well keep one in the scope.


- Meng


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


On March 26, 2019, 10:31 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 26, 2019, 10:31 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when
> appropriate in `__allocate()`. This simplifies the
> code and improves performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp 8bc749903b8ceb09a02e260919377483479302b5 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results in r/70330
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when appropriate.

Posted by Meng Zhu <mz...@mesosphere.io>.

> On March 27, 2019, 7:41 a.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1928 (patched)
> > <https://reviews.apache.org/r/70320/diff/1/?file=2134448#file2134448line1938>
> >
> >     Rather than adding this additional function, can't `shrinkResoures` take a `ResourceQuantities`?
> 
> Meng Zhu wrote:
>     We need to create a map-like a copy somewhere. `shrinkResoures` needs a copy of its own to do bookkeeping:
>     https://github.com/apache/mesos/blob/c10def96653fb99e309459476f5bc09c1da686ee/src/master/allocator/mesos/hierarchical.cpp#L1666
>     
>     Or do you mean to create the map in the `shrinkResoures` function? My concern with that is currently `shrinkResources` has this semantic:
>     >If a resource does not have a target quantity provided, it will not be shrunk.
>     
>     This is not compatible with `ResourceQuantities`. Given the semantic of `ResourceQuantities`, one would expect `shrinkResources` should drop (shrink to zero) for resources that are absent in the input `ResourceQuantities`.
>     
>     Of course, since we are the only consumer of the function here, we could change the function semantic. But I do not see much benefits since, as mentioned above, we need to pass a map anyway.
>     
>     Another thing to consider is we might need to get a copy of the scalar map somewhere else where direct mutations are needed.
> 
> Benjamin Mahler wrote:
>     > This is not compatible with ResourceQuantities. Given the semantic of ResourceQuantities, one would expect shrinkResources should drop (shrink to zero) for resources that are absent in the input ResourceQuantities.
>     
>     Seems like we want to pass in ResourceLimits then? (that makes sense given the argument name too :)). It seems weird to have the map function in the class, can we just do it manually here?
> 
> Benjamin Mahler wrote:
>     Something like this? (would need to add a constructor to ResourceLimits:
>     
>     ```
>         ResourceLimits limits = ResourceLimits(
>             vector<string,Value::Scalar>(
>                 unsatisfiedQuotaGuarantee.begin(),
>                 unsatisfiedQuotaGuarantee.end()));
>     
>         newQuotaAllocation = shrinkResources(newQuotaAllocation, limits);
>     ```

I think take in `ResourceQuantities` and drop resources absent in `ResourceQuantities` makes more sense. See https://reviews.apache.org/r/70345.


- Meng


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


On March 29, 2019, 4:22 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 29, 2019, 4:22 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when
> appropriate in `__allocate()`. This simplifies the
> code and improves performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp ef4f968a94d3a23c010202e27cc48cd10bbec2c0 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results in r/70330
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when appropriate.

Posted by Benjamin Mahler <bm...@apache.org>.

> On March 27, 2019, 2:41 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1928 (patched)
> > <https://reviews.apache.org/r/70320/diff/1/?file=2134448#file2134448line1938>
> >
> >     Rather than adding this additional function, can't `shrinkResoures` take a `ResourceQuantities`?
> 
> Meng Zhu wrote:
>     We need to create a map-like a copy somewhere. `shrinkResoures` needs a copy of its own to do bookkeeping:
>     https://github.com/apache/mesos/blob/c10def96653fb99e309459476f5bc09c1da686ee/src/master/allocator/mesos/hierarchical.cpp#L1666
>     
>     Or do you mean to create the map in the `shrinkResoures` function? My concern with that is currently `shrinkResources` has this semantic:
>     >If a resource does not have a target quantity provided, it will not be shrunk.
>     
>     This is not compatible with `ResourceQuantities`. Given the semantic of `ResourceQuantities`, one would expect `shrinkResources` should drop (shrink to zero) for resources that are absent in the input `ResourceQuantities`.
>     
>     Of course, since we are the only consumer of the function here, we could change the function semantic. But I do not see much benefits since, as mentioned above, we need to pass a map anyway.
>     
>     Another thing to consider is we might need to get a copy of the scalar map somewhere else where direct mutations are needed.
> 
> Benjamin Mahler wrote:
>     > This is not compatible with ResourceQuantities. Given the semantic of ResourceQuantities, one would expect shrinkResources should drop (shrink to zero) for resources that are absent in the input ResourceQuantities.
>     
>     Seems like we want to pass in ResourceLimits then? (that makes sense given the argument name too :)). It seems weird to have the map function in the class, can we just do it manually here?

Something like this? (would need to add a constructor to ResourceLimits:

```
    ResourceLimits limits = ResourceLimits(
        vector<string,Value::Scalar>(
            unsatisfiedQuotaGuarantee.begin(),
            unsatisfiedQuotaGuarantee.end()));

    newQuotaAllocation = shrinkResources(newQuotaAllocation, limits);
```


- Benjamin


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


On March 27, 2019, 5:31 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 27, 2019, 5:31 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when
> appropriate in `__allocate()`. This simplifies the
> code and improves performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp 8bc749903b8ceb09a02e260919377483479302b5 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results in r/70330
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when appropriate.

Posted by Benjamin Mahler <bm...@apache.org>.

> On March 27, 2019, 2:41 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1928 (patched)
> > <https://reviews.apache.org/r/70320/diff/1/?file=2134448#file2134448line1938>
> >
> >     Rather than adding this additional function, can't `shrinkResoures` take a `ResourceQuantities`?
> 
> Meng Zhu wrote:
>     We need to create a map-like a copy somewhere. `shrinkResoures` needs a copy of its own to do bookkeeping:
>     https://github.com/apache/mesos/blob/c10def96653fb99e309459476f5bc09c1da686ee/src/master/allocator/mesos/hierarchical.cpp#L1666
>     
>     Or do you mean to create the map in the `shrinkResoures` function? My concern with that is currently `shrinkResources` has this semantic:
>     >If a resource does not have a target quantity provided, it will not be shrunk.
>     
>     This is not compatible with `ResourceQuantities`. Given the semantic of `ResourceQuantities`, one would expect `shrinkResources` should drop (shrink to zero) for resources that are absent in the input `ResourceQuantities`.
>     
>     Of course, since we are the only consumer of the function here, we could change the function semantic. But I do not see much benefits since, as mentioned above, we need to pass a map anyway.
>     
>     Another thing to consider is we might need to get a copy of the scalar map somewhere else where direct mutations are needed.

> This is not compatible with ResourceQuantities. Given the semantic of ResourceQuantities, one would expect shrinkResources should drop (shrink to zero) for resources that are absent in the input ResourceQuantities.

Seems like we want to pass in ResourceLimits then? (that makes sense given the argument name too :)). It seems weird to have the map function in the class, can we just do it manually here?


- Benjamin


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


On March 27, 2019, 5:31 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 27, 2019, 5:31 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when
> appropriate in `__allocate()`. This simplifies the
> code and improves performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp 8bc749903b8ceb09a02e260919377483479302b5 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results in r/70330
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 70320: Used `ResourceQuantities` in `__allocate()` when possible.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70320/#review214106
-----------------------------------------------------------



Looks good, just noticed another headroom variable that seems to warrant `ResourceQuantities`. Also made a suggestion to avoid the createQuantitiesMap helper.

I saw the suggestion of posting benchmark results in subsequent patches, when are those patches going to get posted with the results? If not so soon, then we may just want to post the benchmark results in this patch so that we can ship it on its own sooner?


src/master/allocator/mesos/hierarchical.cpp
Line 1714 (original), 1713 (patched)
<https://reviews.apache.org/r/70320/#comment300278>

    s/Let sorter/Update sorter to/



src/master/allocator/mesos/hierarchical.cpp
Lines 1758 (patched)
<https://reviews.apache.org/r/70320/#comment300279>

    s/Let sorter/Update sorter to/



src/master/allocator/mesos/hierarchical.cpp
Lines 1765 (patched)
<https://reviews.apache.org/r/70320/#comment300280>

    s/Let sorter/Update sorter to/



src/master/allocator/mesos/hierarchical.cpp
Lines 1928 (patched)
<https://reviews.apache.org/r/70320/#comment300281>

    Rather than adding this additional function, can't `shrinkResoures` take a `ResourceQuantities`?



src/master/allocator/mesos/hierarchical.cpp
Lines 1956-1957 (original), 1949-1950 (patched)
<https://reviews.apache.org/r/70320/#comment300284>

    Clearer name, thanks!



src/master/allocator/mesos/hierarchical.cpp
Line 1962 (original), 1952 (patched)
<https://reviews.apache.org/r/70320/#comment300282>

    in the headroom surplus,



src/master/allocator/mesos/hierarchical.cpp
Lines 1961 (patched)
<https://reviews.apache.org/r/70320/#comment300283>

    Ditto here for just passing `ResourceQuantities`



src/master/allocator/mesos/hierarchical.cpp
Lines 2088-2091 (original), 2079-2082 (patched)
<https://reviews.apache.org/r/70320/#comment300285>

    This looks like a `ResourceQuantities`?


- Benjamin Mahler


On March 27, 2019, 5:31 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70320/
> -----------------------------------------------------------
> 
> (Updated March 27, 2019, 5:31 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9504
>     https://issues.apache.org/jira/browse/MESOS-9504
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replaced `Resources` with `ResourceQuantities` when possible
> in `__allocate()`. This simplifies the code and improves
> performance.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 36bf90baf413e99c1580d516dfac0f074335d322 
>   src/master/allocator/mesos/hierarchical.cpp 8bc749903b8ceb09a02e260919377483479302b5 
> 
> 
> Diff: https://reviews.apache.org/r/70320/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> Benchmark results coming in subsequent patches.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>