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 71269: Added a role tree class in the allocator.
Date Fri, 16 Aug 2019 03:37:00 GMT


> On Aug. 13, 2019, 7:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 231 (patched)
> > <https://reviews.apache.org/r/71269/diff/2/?file=2160787#file2160787line231>
> >
> >     This can be `hashmap<std::string, Role>` which would avoid the potential
leak of forgetting `delete`, and avoid the extra memory allocation.
> 
> Meng Zhu wrote:
>     hmm, would prefer keep the map just as a simple lookup table. Feels somewhat convoluted
if also using it to manage the life cycle.
>     The simple destructors should be enough to guard against any leaks.
>     Also, the "get" call would need to return a coy of `Role` instead of a pointer, not
quite what we want.
> 
> Benjamin Mahler wrote:
>     > hmm, would prefer keep the map just as a simple lookup table. Feels somewhat
convoluted if also using it to manage the life cycle.
>     
>     The map is already managing lifecycle? No `Role*` should exist outside the map, right?
If we remove a `Role*` from the map, it needs to be `delete`ed right?
>     
>     > Also, the "get" call would need to return a coy of Role instead of a pointer,
not quite what we want.
>     
>     Why would you need to copy? You can just return a pointer to the value from the map,
that's no problem?

OK, didn't think deep about it. Updated the code to use the map for life cycle management.


- Meng


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


On Aug. 15, 2019, 8:36 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71269/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2019, 8:36 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The role concept in Mesos fits into a tree structure naturally.
> However, the role state in the allocator are currenstored
> in a hashmap. This is less efficient and harder to use and reason.
> 
> This patch introduced a `RoleTree` structure in the allocator
> and organizes all the roles in to a tree. This should simplify
> the code logic and opens further refactor and optimization
> opportunities.
> 
> In addition, the master code also lacks a proper tree structure
> for tracking roles. We should leverage the same role tree code
> here to simplify that as well.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 8be8dcee04e8fc5b97f730b2f058d14c81678788

>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378

> 
> 
> Diff: https://reviews.apache.org/r/71269/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> Benchmaring using optimized build shows no noticable performance change.
> 
> Benchmark: `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota`
> 
> ## Before:
> 
> Added 30 agents in 1.252621ms
> Added 30 frameworks in 7.747202ms
> Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
> Made 36 allocations in 12.791427ms
> 
> Added 300 agents in 9.765295ms
> Added 300 frameworks in 273.978325ms
> Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
> Made 350 allocations in 424.603736ms
> 
> Added 3000 agents in 79.646516ms
> Added 3000 frameworks in 19.17449514secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
> Made 3500 allocations in 31.687207066secs
> 
> ## After:
> 
> Added 30 agents in 1.376619ms
> Added 30 frameworks in 7.647487ms
> Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
> Made 36 allocations in 11.638116ms
> 
> Added 300 agents in 9.506101ms
> Added 300 frameworks in 263.008198ms
> Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
> Made 350 allocations in 418.962396ms
> 
> Added 3000 agents in 81.553527ms
> Added 3000 frameworks in 19.201708305secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
> Made 3500 allocations in 31.583417136secs
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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