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/23 18:09:11 UTC
Review Request 43883: Added allocator metrics for number of filters
per framework.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/
-----------------------------------------------------------
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 filters per framework.
Diffs
-----
src/master/allocator/mesos/hierarchical.hpp 0d39d3f3b5f4ff7f62f9de7200d062845c71818a
src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
src/tests/hierarchical_allocator_tests.cpp 1af5c9870430eb05ca0b19ece0ca2957a05093ff
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
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. 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 Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 25, 2016, 12:59 p.m., Guangya Liu wrote:
> > docs/monitoring.md, line 905
> > <https://reviews.apache.org/r/43883/diff/5/?file=1270813#file1270813line905>
> >
> > s/framework_filters/framework_offer_filters
> >
> > and ditto for the following
> >
> > There are both offer filter and inverse offer filter in allocator, it is better to clarify that this is offer filter.
> >
> > BTW: Do u have any plan to add inverse offer filter metrics?
Renamed.
Also filled MESOS-4775 for inverse offer gauges.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43883/#review120688
-----------------------------------------------------------
On Feb. 25, 2016, 4:27 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> 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.
>
>
> 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
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/#review120688
-----------------------------------------------------------
docs/monitoring.md (line 905)
<https://reviews.apache.org/r/43883/#comment182147>
s/framework_filters/framework_offer_filters
and ditto for the following
There are both offer filter and inverse offer filter in allocator, it is better to clarify that this is offer filter.
BTW: Do u have any plan to add inverse offer filter metrics?
src/master/allocator/mesos/hierarchical.hpp (line 402)
<https://reviews.apache.org/r/43883/#comment182148>
s/filters/offer filters
src/master/allocator/mesos/hierarchical.cpp (line 265)
<https://reviews.apache.org/r/43883/#comment182149>
move the comments to the below line
- Guangya Liu
On 二月 25, 2016, 11:01 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> -----------------------------------------------------------
>
> (Updated 二月 25, 2016, 11: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 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
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. 25, 2016, 12:01 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Added documentation, fixed ws, and stayed away from `const` locals.
Bugs: MESOS-4722
https://issues.apache.org/jira/browse/MESOS-4722
Repository: mesos
Description
-------
Added allocator metrics for number of 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
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. 24, 2016, 9:37 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Renamed variable.
Bugs: MESOS-4722
https://issues.apache.org/jira/browse/MESOS-4722
Repository: mesos
Description
-------
Added allocator metrics for number of filters per framework.
Diffs (updated)
-----
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
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. 24, 2016, 11:53 a.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Just whitespace (no functional changes)
Bugs: MESOS-4722
https://issues.apache.org/jira/browse/MESOS-4722
Repository: mesos
Description
-------
Added allocator metrics for number of filters per framework.
Diffs (updated)
-----
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
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. 24, 2016, 11:21 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 filters per framework.
Diffs (updated)
-----
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