mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rojas <alexan...@mesosphere.io>
Subject Re: Review Request 43881: Added allocator metric for the number of allocations to a framework.
Date Wed, 24 Feb 2016 23:11:40 GMT

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




src/master/allocator/mesos/hierarchical.cpp (line 1204)
<https://reviews.apache.org/r/43881/#comment182061>

    Even though I really like the anonymous namespace, we don't use it for some reasons (I
think the main one is that symbols are still exported, even though with mangled names).
    
    All in all, there is only one place in non automatically generated code which uses them
(and I wonder how it managed).
    
    All this to say, I guess this function should be static.
    
    Moreover, does this need to be a free function? I much rather have a private method.



src/tests/hierarchical_allocator_tests.cpp (lines 2506 - 2514)
<https://reviews.apache.org/r/43881/#comment182063>

    Could you add a small comment, akin to the one in line 2488 on what is happening here?


- Alexander Rojas


On Feb. 24, 2016, 9:37 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43881/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4719
>     https://issues.apache.org/jira/browse/MESOS-4719
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metric for the number of allocations to a framework.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1

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

>   src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33

> 
> Diff: https://reviews.apache.org/r/43881/diff/
> 
> 
> Testing
> -------
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the allocator;
this is partially expected since the added code only inserts metrics in the allocator while
the actual work is perform asynchronously. These tests where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers`
on an optimized build under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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