mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 70591: Added `struct RoleInfo` to track role reservations and framework IDs.
Date Thu, 09 May 2019 20:58:39 GMT


> On May 6, 2019, 12:38 p.m., Meng Zhu wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 2119 (original), 2122 (patched)
> > <https://reviews.apache.org/r/70591/diff/1/?file=2142787#file2142787line2122>
> >
> >     Shouldn't this be:
> >     
> >     ```
> >     if (!roles.contains(role) || roles.at(role).frameworks.empty()) {
> >       ....
> >     }
> >     ```
> 
> Andrei Sekretenko wrote:
>     Why should it? 
>     After this check the code loops through frameworkSorters, which are empty if there
are no frameworks, so the `offerable` will be empty and no offers will be made.
>     
>     And checking for absence of any tracked frameworks now requires looping through the
`roles` hashmap, which is too expensive.
> 
> Meng Zhu wrote:
>     OK, that makes sense. I was just looking at the comment above it. Given the change,
maybe we should remove the comment.
> 
> Andrei Sekretenko wrote:
>     I'm thinking about removing this check. 
>     
>     It was initially introduced here: https://reviews.apache.org/r/37177/diff/8
>     and then modified here: https://reviews.apache.org/r/41075/diff/13
>     and I cannot see a rationale behind either version of this check.
>     
>     
>     The only functional difference introduced by this check is suppressing the log message
at the end of `deallocate()`:
>     `VLOG(2) << "No inverse offers to send out!";`
>     Suppressing this message in case of no frameworks makes some sense (fewer red herrings
in the log), but this (or something better) can be done in another way.

Sounds good to me. I am OK with removing that.


- Meng


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


On May 7, 2019, 6:58 a.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70591/
> -----------------------------------------------------------
> 
> (Updated May 7, 2019, 6:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9701
>     https://issues.apache.org/jira/browse/MESOS-9701
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch introduces `struct RoleInfo` which contains the framework IDs and    
> reservatons tracked under a role. Two separate hashmaps used for this purpose   
> previously are replaced by a single hashmap, thus the need to keep the          
> two consistent with each other is removed. This simplifies the role tracking    
> logic in the allocator. The patch introduces minimal to no performance impact.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp c2058baca5159da4cdcab77afd5de3c0d5ae6c48

>   src/master/allocator/mesos/hierarchical.cpp 64a076ddd29711437d539a06bb0470755828cc87

> 
> 
> Diff: https://reviews.apache.org/r/70591/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> Benchmarking: 5 runs of `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/5`
with the optimized build.
> 
> Performance impact in this benchmark seems to be negligible.
> 
> BEFORE:
> Added 3000 agents in 51.553929ms
> Added 3000 frameworks in 15.174748344secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.400805171secs
> Made 0 allocation in 12.5850238secs
> 
> Added 3000 agents in 55.739336ms
> Added 3000 frameworks in 14.730404769secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.563439682secs
> Made 0 allocation in 13.063555055secs
> 
> Added 3000 agents in 54.414733ms
> Added 3000 frameworks in 15.10136842secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 13.501664915secs
> Made 0 allocation in 12.89034382secs
> 
> Added 3000 agents in 52.58252ms
> Added 3000 frameworks in 14.048350298secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.299952145secs
> Made 0 allocation in 11.888248811secs
> 
> Added 3000 agents in 52.821439ms
> Added 3000 frameworks in 15.344450583secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.634000025secs
> Made 0 allocation in 12.427171541secs
> 
> 
> AFTER:
> 
> Added 3000 agents in 69.716648ms
> Added 3000 frameworks in 15.249001979secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.860494226secs
> Made 0 allocation in 12.228866329secs
> 
> Added 3000 agents in 52.639388ms
> Added 3000 frameworks in 15.207895482secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.504777266secs
> Made 0 allocation in 12.70388062secs
> 
> Added 3000 agents in 56.865794ms
> Added 3000 frameworks in 15.284003915secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 13.86859815secs
> Made 0 allocation in 12.538958231secs
> 
> Added 3000 agents in 56.028013ms
> Added 3000 frameworks in 13.892577869secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.341724418secs
> Made 0 allocation in 12.23022189secs
> 
> Added 3000 agents in 52.368219ms
> Added 3000 frameworks in 13.978581104secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.701682501secs
> Made 0 allocation in 12.141360313secs
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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