You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2016/02/25 16:27:50 UTC

Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

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

(Updated Feb. 25, 2016, 4:27 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Rename framework_filters to framework_offer_filters.


Summary (updated)
-----------------

Added allocator metrics for number of offer filters per framework.


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


Repository: mesos


Description (updated)
-------

Added allocator metrics for number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  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/43883/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


Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/#review121058
-----------------------------------------------------------




src/tests/hierarchical_allocator_tests.cpp (line 2614)
<https://reviews.apache.org/r/43883/#comment182650>

    4 spaces
    
    and ditto for the following



src/tests/hierarchical_allocator_tests.cpp (line 2641)
<https://reviews.apache.org/r/43883/#comment182651>

    what about move this comments above `Clock::advance(flags.allocation_interval);
    ` and update it to:
    
    // OfferFilter refuse1 will be expired after allocation_interval.
    
    ditto for the following


- Guangya Liu


On 二月 26, 2016, 5:01 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated 二月 26, 2016, 5:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metrics for number of offer filters per framework.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   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/43883/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
> 
>


Re: Review Request 43883: Added a metric for querying the number offer filters for a role.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/#review125485
-----------------------------------------------------------



The code looks pretty clean, thanks! However, there were two bugs here:

(1) The value of the metric only counts how many slaves have offer filters, rather than the total number of filters (note the value type of `offerFilters` is a set).
(2) removeRole was not removing the role from the map of gauges

I made some minor adjustments based on the comments here and fixed these two bugs, will commit shortly!


docs/monitoring.md (line 955)
<https://reviews.apache.org/r/43883/#comment188276>

    In the interest of consistency and avoiding repeating ourselves in the name, perhaps the following?
    
    ```
    allocator/mesos/filters/roles/<role>/active
    ```



src/master/allocator/mesos/hierarchical.hpp (line 298)
<https://reviews.apache.org/r/43883/#comment188273>

    Hm.. why the "role" naming prefix here but no "role" naming prefix on `_quota_allocated`?



src/master/allocator/mesos/hierarchical.cpp (line 1742)
<https://reviews.apache.org/r/43883/#comment188275>

    2 spaces here



src/master/allocator/mesos/hierarchical.cpp (line 1743)
<https://reviews.apache.org/r/43883/#comment188281>

    How about wrapping the argument on the next line here for consistency?



src/master/allocator/mesos/hierarchical.cpp (line 1747)
<https://reviews.apache.org/r/43883/#comment188277>

    whoops, extra newline?



src/master/allocator/mesos/hierarchical.cpp (line 1753)
<https://reviews.apache.org/r/43883/#comment188278>

    Whoops, this looks like a bug. `offerFilters` is a `hashmap<SlaveID, hashset<OfferFilter*>>` and so it looks like we need to loop over the values and use the sum of the sizes, since there may be more than one filter per slave:
    
    ```
    foreachkey (const SlaveID& slaveId, framework.offerFilters) {
      result += framework.offerFilters[slaveId].size();
    }
    ```



src/master/allocator/mesos/metrics.hpp (lines 51 - 52)
<https://reviews.apache.org/r/43883/#comment188274>

    Very nice!



src/master/allocator/mesos/metrics.cpp (line 113)
<https://reviews.apache.org/r/43883/#comment188272>

    s/(/ (/



src/master/allocator/mesos/metrics.cpp (line 175)
<https://reviews.apache.org/r/43883/#comment188279>

    `offer_filter` here but `gauge` in removeRole?



src/master/allocator/mesos/metrics.cpp (line 185)
<https://reviews.apache.org/r/43883/#comment188271>

    2 spaces here



src/master/allocator/mesos/metrics.cpp (lines 188 - 192)
<https://reviews.apache.org/r/43883/#comment188285>

    This looks like a bug: we also have to remove the role from the map here.



src/tests/hierarchical_allocator_tests.cpp (line 568)
<https://reviews.apache.org/r/43883/#comment188280>

    Since the metric is declared higher up it's a bit tricker to follow what is being checked here, how about s/metric/filters_active/ ?



src/tests/hierarchical_allocator_tests.cpp (lines 2655 - 2657)
<https://reviews.apache.org/r/43883/#comment188283>

    Thanks for the test!



src/tests/hierarchical_allocator_tests.cpp (lines 2700 - 2707)
<https://reviews.apache.org/r/43883/#comment188282>

    Some newlines here would be nice:
    
    ```
      Future<Allocation> allocation = allocations.get();
    
      AWAIT_READY(allocation);
      ASSERT_EQ(framework1.id(), allocation->frameworkId);
    
      allocator->recoverResources(
          allocation->frameworkId,
          agent.id(),
          allocation->resources.get(agent.id()).get(),
          offerFilter);
    ```


- Ben Mahler


On March 24, 2016, 1:36 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 1:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a metric for querying the number offer filters for a role.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md dcf19e6ce06b02373a2bd1af81a451a35743fa76 
>   src/master/allocator/mesos/hierarchical.hpp e4604f4da59166da24709a68b8cd4e56bf55f97f 
>   src/master/allocator/mesos/hierarchical.cpp 39a290d0db2c22e179a8f933b1a78e3a2dcefdc3 
>   src/master/allocator/mesos/metrics.hpp b5f2806cff99ee2ee46c4ac8a13174ef699969aa 
>   src/master/allocator/mesos/metrics.cpp 2f3012a5bd40a4100e790e1d373832015c80b993 
>   src/tests/hierarchical_allocator_tests.cpp e9cfcfc0ad8b0b89bbac459b7db39183f6c189be 
> 
> Diff: https://reviews.apache.org/r/43883/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
> 
>


Re: Review Request 43883: Added a metric for querying the number offer filters for a role.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 24, 2016, 2:36 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
-------

Added a metric for querying the number offer filters for a role.


Diffs (updated)
-----

  docs/monitoring.md dcf19e6ce06b02373a2bd1af81a451a35743fa76 
  src/master/allocator/mesos/hierarchical.hpp e4604f4da59166da24709a68b8cd4e56bf55f97f 
  src/master/allocator/mesos/hierarchical.cpp 39a290d0db2c22e179a8f933b1a78e3a2dcefdc3 
  src/master/allocator/mesos/metrics.hpp b5f2806cff99ee2ee46c4ac8a13174ef699969aa 
  src/master/allocator/mesos/metrics.cpp 2f3012a5bd40a4100e790e1d373832015c80b993 
  src/tests/hierarchical_allocator_tests.cpp e9cfcfc0ad8b0b89bbac459b7db39183f6c189be 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added a metric for querying the number offer filters for a role.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 24, 2016, 2:35 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Changed to report offer filters per role as opposed to per framework.


Summary (updated)
-----------------

Added a metric for querying the number offer filters for a role.


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


Repository: mesos


Description (updated)
-------

Added a metric for querying the number offer filters for a role.


Diffs (updated)
-----

  docs/monitoring.md dcf19e6ce06b02373a2bd1af81a451a35743fa76 
  src/master/allocator/mesos/hierarchical.hpp e4604f4da59166da24709a68b8cd4e56bf55f97f 
  src/master/allocator/mesos/hierarchical.cpp 39a290d0db2c22e179a8f933b1a78e3a2dcefdc3 
  src/master/allocator/mesos/metrics.hpp b5f2806cff99ee2ee46c4ac8a13174ef699969aa 
  src/master/allocator/mesos/metrics.cpp 2f3012a5bd40a4100e790e1d373832015c80b993 
  src/tests/hierarchical_allocator_tests.cpp e9cfcfc0ad8b0b89bbac459b7db39183f6c189be 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

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



Patch looks great!

Reviews applied: [44850, 44851, 44852, 43884, 43880, 43879, 43881, 43882, 43883]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 21, 2016, 11:08 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated March 21, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metrics for the number of offer filters per framework.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 0c622b8569bc79ae4e365a57e463ca5349356713 
>   src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
>   src/tests/hierarchical_allocator_tests.cpp 459e02576f6d05abbbcc83ae5cabac5c66e93f05 
> 
> Diff: https://reviews.apache.org/r/43883/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
> 
>


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 21, 2016, 12:08 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Review comments from @bmahler.


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


Repository: mesos


Description
-------

Added allocator metrics for the number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 0c622b8569bc79ae4e365a57e463ca5349356713 
  src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
  src/master/allocator/mesos/metrics.hpp d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
  src/master/allocator/mesos/metrics.cpp 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
  src/tests/hierarchical_allocator_tests.cpp 459e02576f6d05abbbcc83ae5cabac5c66e93f05 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

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



Patch looks great!

Reviews applied: [44850, 44851, 44852, 43884, 43880, 43879, 43881, 43882, 43883]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 15, 2016, 2:52 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated March 15, 2016, 2:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metrics for the number of offer filters per framework.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 52b3a9bfbe73d24968f7ddb0672aee1e05636ab0 
>   src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
>   src/tests/hierarchical_allocator_tests.cpp 459e02576f6d05abbbcc83ae5cabac5c66e93f05 
> 
> Diff: https://reviews.apache.org/r/43883/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
> 
>


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 15, 2016, 3:52 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Addressed offline comments from Ben Mahler.


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


Repository: mesos


Description (updated)
-------

Added allocator metrics for the number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 52b3a9bfbe73d24968f7ddb0672aee1e05636ab0 
  src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
  src/master/allocator/mesos/metrics.hpp d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
  src/master/allocator/mesos/metrics.cpp 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
  src/tests/hierarchical_allocator_tests.cpp 459e02576f6d05abbbcc83ae5cabac5c66e93f05 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 4, 2016, 5:01 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Adressed comments from Alex.


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


Repository: mesos


Description (updated)
-------

See summary.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
  src/master/allocator/mesos/metrics.hpp PRE-CREATION 
  src/master/allocator/mesos/metrics.cpp PRE-CREATION 
  src/tests/hierarchical_allocator_tests.cpp 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On March 4, 2016, 2:05 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 288-297
> > <https://reviews.apache.org/r/43883/diff/13/?file=1279215#file1279215line288>
> >
> >     I suggest we create a helper in `Metrics` for this. Also, minor nit: in previous patches you didn't separate `.put()` from `process::metrics::add()` with a blank line.

I introduced a number of helpers in the latest incarnation of this series, among others this one. Blank lines should be consistent now.


> On March 4, 2016, 2:05 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 293
> > <https://reviews.apache.org/r/43883/diff/13/?file=1279215#file1279215line293>
> >
> >     I believe this one requires `->self()` as well.

Code got moved to helper, and adjusted there.


> On March 4, 2016, 2:05 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 368-369
> > <https://reviews.apache.org/r/43883/diff/13/?file=1279215#file1279215line368>
> >
> >     Same here.

Done.


> On March 4, 2016, 2:05 p.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 538
> > <https://reviews.apache.org/r/43883/diff/13/?file=1279218#file1279218line538>
> >
> >     Why do explicitly use double `0` here? I thought you rely on implicit conversions and promotions in these patches.

This was a simple oversight, fixed now.


> On March 4, 2016, 2:05 p.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 2462-2466
> > <https://reviews.apache.org/r/43883/diff/13/?file=1279218#file1279218line2462>
> >
> >     Here you can also check that 2 counters for offers filters were installed.

We do now.


- Benjamin


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


On March 4, 2016, 5:01 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated March 4, 2016, 5:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp PRE-CREATION 
>   src/master/allocator/mesos/metrics.cpp PRE-CREATION 
>   src/tests/hierarchical_allocator_tests.cpp 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43883/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
> 
>


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/#review122056
-----------------------------------------------------------


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.cpp (lines 288 - 297)
<https://reviews.apache.org/r/43883/#comment183911>

    I suggest we create a helper in `Metrics` for this. Also, minor nit: in previous patches you didn't separate `.put()` from `process::metrics::add()` with a blank line.



src/master/allocator/mesos/hierarchical.cpp (line 293)
<https://reviews.apache.org/r/43883/#comment183928>

    I believe this one requires `->self()` as well.



src/master/allocator/mesos/hierarchical.cpp (lines 368 - 369)
<https://reviews.apache.org/r/43883/#comment183912>

    Same here.



src/tests/hierarchical_allocator_tests.cpp (line 538)
<https://reviews.apache.org/r/43883/#comment183914>

    Why do explicitly use double `0` here? I thought you rely on implicit conversions and promotions in these patches.



src/tests/hierarchical_allocator_tests.cpp (lines 2462 - 2466)
<https://reviews.apache.org/r/43883/#comment183915>

    Here you can also check that 2 counters for offers filters were installed.


- Alexander Rukletsov


On March 3, 2016, 4:17 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated March 3, 2016, 4:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metrics for the number of offer filters per framework.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp PRE-CREATION 
>   src/master/allocator/mesos/metrics.cpp PRE-CREATION 
>   src/tests/hierarchical_allocator_tests.cpp 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43883/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
> 
>


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 3, 2016, 5:17 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Addressed comments form Alex.


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


Repository: mesos


Description (updated)
-------

Added allocator metrics for the number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
  src/master/allocator/mesos/metrics.hpp PRE-CREATION 
  src/master/allocator/mesos/metrics.cpp PRE-CREATION 
  src/tests/hierarchical_allocator_tests.cpp 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 3, 2016, 2:12 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Tidied up includes.


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


Repository: mesos


Description (updated)
-------

See summary.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 70291075c00a9a557529c2562dedcfc6c6c3ec32 
  src/master/allocator/mesos/metrics.hpp PRE-CREATION 
  src/master/allocator/mesos/metrics.cpp PRE-CREATION 
  src/tests/hierarchical_allocator_tests.cpp 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 2, 2016, 4:43 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Rebased, and minor style fixes.


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


Repository: mesos


Description
-------

Added allocator metrics for the number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 24fa50f62dec10ed43089297473cc386d6ba2f78 
  src/master/allocator/mesos/metrics.hpp PRE-CREATION 
  src/master/allocator/mesos/metrics.cpp PRE-CREATION 
  src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated March 2, 2016, 11:34 a.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Incorporated test into existing one; style fixes.


Summary (updated)
-----------------

Added allocator metrics for the number of offer filters per framework.


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


Repository: mesos


Description (updated)
-------

Added allocator metrics for the number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 24fa50f62dec10ed43089297473cc386d6ba2f78 
  src/master/allocator/mesos/metrics.hpp PRE-CREATION 
  src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On March 1, 2016, 12:56 a.m., Alexander Rojas wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 276-282
> > <https://reviews.apache.org/r/43883/diff/9/?file=1274393#file1274393line276>
> >
> >     As in previous reviews, let's go for:
> >     
> >     ```c++
> >     metrics.framework_offer_filters.put(
> >         frameworkId,
> >         process::metrics::Gauge(
> >             strings::join(
> >                 "/", 
> >                 "allocator/framework_offer_filters",
> >                 frameworkId.value()),
> >             process::defer(self(), [this, frameworkId]() {
> >               return this->
> >                 frameworks[frameworkId].offerFilters.size();
> >             })));
> >     ```

Fixed as explained previously.


- Benjamin


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


On March 2, 2016, 11:34 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated March 2, 2016, 11:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metrics for the number of offer filters per framework.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 24fa50f62dec10ed43089297473cc386d6ba2f78 
>   src/master/allocator/mesos/metrics.hpp PRE-CREATION 
>   src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43883/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
> 
>


Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/#review121363
-----------------------------------------------------------




src/master/allocator/mesos/hierarchical.cpp (lines 276 - 282)
<https://reviews.apache.org/r/43883/#comment183033>

    As in previous reviews, let's go for:
    
    ```c++
    metrics.framework_offer_filters.put(
        frameworkId,
        process::metrics::Gauge(
            strings::join(
                "/", 
                "allocator/framework_offer_filters",
                frameworkId.value()),
            process::defer(self(), [this, frameworkId]() {
              return this->
                frameworks[frameworkId].offerFilters.size();
            })));
    ```


- Alexander Rojas


On Feb. 29, 2016, 8:59 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated Feb. 29, 2016, 8:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metrics for number of offer filters per framework.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 24fa50f62dec10ed43089297473cc386d6ba2f78 
>   src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43883/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
> 
>


Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated Feb. 29, 2016, 8:59 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Added allocator metrics for number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 24fa50f62dec10ed43089297473cc386d6ba2f78 
  src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated Feb. 28, 2016, 10:28 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Addressed comments from Klaus.


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


Repository: mesos


Description
-------

Added allocator metrics for number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
  src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 24fa50f62dec10ed43089297473cc386d6ba2f78 
  src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

Diff: https://reviews.apache.org/r/43883/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


Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

Posted by Alexander Rojas <al...@mesosphere.io>.

> On Feb. 28, 2016, 4:49 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 384
> > <https://reviews.apache.org/r/43883/diff/7/?file=1272161#file1272161line384>
> >
> >     Should we remove this `Gauge` when remove framework?

I think given this comment in the patch:

> // Gauges for the number of active offer filters for each framework.

It most certainly should be removed. Moreover, gauges measure the state at a single point in time.


- Alexander


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


On Feb. 29, 2016, 8:59 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated Feb. 29, 2016, 8:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metrics for number of offer filters per framework.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 24fa50f62dec10ed43089297473cc386d6ba2f78 
>   src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43883/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
> 
>


Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/#review121148
-----------------------------------------------------------




src/master/allocator/mesos/hierarchical.hpp (line 384)
<https://reviews.apache.org/r/43883/#comment182801>

    Should we remove this `Gauge` when remove framework?


- Klaus Ma


On Feb. 27, 2016, 1:01 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2016, 1:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
>     https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added allocator metrics for number of offer filters per framework.
> 
> 
> Diffs
> -----
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   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/43883/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
> 
>


Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------

(Updated Feb. 26, 2016, 6:01 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
-------

Rebased and get metrics without a helper.


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


Repository: mesos


Description
-------

Added allocator metrics for number of offer filters per framework.


Diffs (updated)
-----

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  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/43883/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