mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 72508: Fixed performance of tracking resource totals in allocator's roles tree.
Date Thu, 14 May 2020 18:10:08 GMT

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


Ship it!




Looks good, feel free to split out the ScalarResourceTotals into a patch in front, to make
the fix more minimal and clear.


src/master/allocator/mesos/hierarchical.hpp
Lines 115 (patched)
<https://reviews.apache.org/r/72508/#comment309445>

    Hm.. "subset of scalar resources" makes me think of:
    
    some_subset(scalar resources)
    
    rather than what I think you intended:
    
    scalars(resources)
    
    Maybe just say tracking scalar totals across agents?
    Ideally spelling out if this is forgiving with the inputs or crashes if passing non scalars
(had to read the code to see):
    
    ```
    // Helper for tracking cross-agent scalar resource totals
    // (since directly summing scalar Resources across agents
    // is very expensive).
    //
    // This will implicitly filter out non-scalars from the
    // function inputs, the caller does not need to call
    // .scalars() beforehand.
    ```



src/master/allocator/mesos/hierarchical.hpp
Lines 124 (patched)
<https://reviews.apache.org/r/72508/#comment309447>

    `; }`



src/master/allocator/mesos/hierarchical.hpp
Lines 129 (patched)
<https://reviews.apache.org/r/72508/#comment309446>

    maybe:
    
    ```
    scalars
    scalarsTotal
    ```
    
    seeing total next to scalar might lead one to think that total is not involving only scalars



src/master/allocator/mesos/hierarchical.hpp
Lines 155-161 (patched)
<https://reviews.apache.org/r/72508/#comment309450>

    How about:
    
    `quotaOfferedPlusConsumed`
    `quotaOfferedOrConsumed`
    `quotaConsumedOrOffered`
    `quotaConsumedPlusOffered`
    
    The way I would want to think about this is that it's quota consumption with offered included
vs quota consumption without factoring in what's offered.


- Benjamin Mahler


On May 13, 2020, 7:43 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72508/
> -----------------------------------------------------------
> 
> (Updated May 13, 2020, 7:43 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10128
>     https://issues.apache.org/jira/browse/MESOS-10128
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before this patch, the roles tree was tracking total resources
> offered/allocated to a role as a single `Resources` objects.
> In the case when each agent has a limited number of unique resources
> (for example, a single persistent voulme), this resulted in poor
> asymptotic complexity of allocation versus the number of agents
> (O(N^2)) that was clearly observable in
> `HierarchicalAllocations_BENCHMARK_Test.PersistentVolumes`.
> In addition, the role tree code was violating the convention that
> `Resources` belonging to different agents should never be added.
> 
> This patch implements per-agent tracking of `Resources` in the roles
> tree, thus improving the performance of allocation (and getting rid of
> the potentially problematic O(N^2) asymptotic) in the case of many
> agents with a limited number of unique resources each.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 6454cdaa19f776365df34ecf83114f0d6fa20f27

>   src/master/allocator/mesos/hierarchical.cpp 5fe9ffcb518b8427d663ddae43e550795d290e3c

> 
> 
> Diff: https://reviews.apache.org/r/72508/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message