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:03 UTC
Review Request 43879: Added allocator metrics for number of
allocations made.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Guangya Liu <gy...@gmail.com>.
> On 二月 24, 2016, 1:50 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 286
> > <https://reviews.apache.org/r/43879/diff/1/?file=1265543#file1265543line286>
> >
> > some comments here
>
> Benjamin Bannier wrote:
> For variables which such narrow usage I would prefer to give them a speaking name; do you have a suggestion you find easier to understand?
The reason that I want to have some comments here is because the `allocation_runs` is a bit ambigous, i do not know if this counter was increased when there are allocations no matter succeed or not, or allocations succeed. I was that there are indeed some metrics include comments: https://github.com/apache/mesos/blob/master/src/master/metrics.hpp#L73-L77
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120431
-----------------------------------------------------------
On 二月 24, 2016, 8:46 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated 二月 24, 2016, 8:46 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 24, 2016, 2:50 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 286
> > <https://reviews.apache.org/r/43879/diff/1/?file=1265543#file1265543line286>
> >
> > some comments here
For variables which such narrow usage I would prefer to give them a speaking name; do you have a suggestion you find easier to understand?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120431
-----------------------------------------------------------
On Feb. 23, 2016, 6:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2016, 6:09 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120431
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.hpp (line 286)
<https://reviews.apache.org/r/43879/#comment181869>
some comments here
src/tests/hierarchical_allocator_tests.cpp (line 31)
<https://reviews.apache.org/r/43879/#comment181870>
This include should be put in a new section under `#include <process/queue.hpp>`
src/tests/hierarchical_allocator_tests.cpp (line 2288)
<https://reviews.apache.org/r/43879/#comment181871>
4 spaces
src/tests/hierarchical_allocator_tests.cpp (lines 2306 - 2310)
<https://reviews.apache.org/r/43879/#comment181874>
if (metrics.values.count(metric)) {
return metrics.find<JSON::Number>(metric)->as<double>();
}
return None();
- Guangya Liu
On 二月 23, 2016, 5:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated 二月 23, 2016, 5:09 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 24, 2016, 2:06 p.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2398
> > <https://reviews.apache.org/r/43879/diff/3/?file=1267167#file1267167line2398>
> >
> > I saw that most are using `Seconds(15)`
>
> Guangya Liu wrote:
> Can you please clarify why using `Seconds{15}` but not `Seconds(15)`? What is the difference?
Let's stay consistent with how constructor calls are written here now.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120492
-----------------------------------------------------------
On Feb. 24, 2016, 2:25 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 2:25 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Guangya Liu <gy...@gmail.com>.
> On 二月 24, 2016, 1:06 p.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2398
> > <https://reviews.apache.org/r/43879/diff/3/?file=1267167#file1267167line2398>
> >
> > I saw that most are using `Seconds(15)`
Can you please clarify why using `Seconds{15}` but not `Seconds(15)`? What is the difference?
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120492
-----------------------------------------------------------
On 二月 24, 2016, 1:25 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated 二月 24, 2016, 1:25 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 24, 2016, 2:06 p.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2398
> > <https://reviews.apache.org/r/43879/diff/3/?file=1267167#file1267167line2398>
> >
> > I saw that most are using `Seconds(15)`
>
> Guangya Liu wrote:
> Can you please clarify why using `Seconds{15}` but not `Seconds(15)`? What is the difference?
>
> Benjamin Bannier wrote:
> Let's stay consistent with how constructor calls are written here now.
`Seconds{15}` is a call to a constructor of type `Seconds` without narrowing, while `Seconds(15)` could be e.g., a constructor call, a function call, a macro invocation, with narrowing sprinkled in where the compiler might need it. That's why IMHO the first form is clearer and less magic.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120492
-----------------------------------------------------------
On Feb. 24, 2016, 2:25 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 2:25 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120492
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.hpp (lines 25 - 26)
<https://reviews.apache.org/r/43879/#comment181959>
not yours, but I think that it is better add a blank line between 25 and 26
src/tests/hierarchical_allocator_tests.cpp (lines 2390 - 2391)
<https://reviews.apache.org/r/43879/#comment181960>
What about `Clock::advance(Seconds(2))`
Also want to know why using 2s here?
src/tests/hierarchical_allocator_tests.cpp (line 2398)
<https://reviews.apache.org/r/43879/#comment181961>
I saw that most are using `Seconds(15)`
src/tests/hierarchical_allocator_tests.cpp (line 2430)
<https://reviews.apache.org/r/43879/#comment181962>
Days(365)
src/tests/hierarchical_allocator_tests.cpp (line 2435)
<https://reviews.apache.org/r/43879/#comment181963>
Why using double here? Should not this be int?
- Guangya Liu
On 二月 24, 2016, 10:22 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated 二月 24, 2016, 10:22 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 24, 2016, 9:17 p.m., Alexander Rojas wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 2395-2398
> > <https://reviews.apache.org/r/43879/diff/5/?file=1268385#file1268385line2395>
> >
> > Why isn't it an `AWAIT_READY` macro? you still use an `AWAIT_*` afterwards, so I doubt is the result value.
I want to *return* a `None` here if the response does not become ready while the macros you suggest do not return the correct type (`void` instead of `Option<T>`).
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120570
-----------------------------------------------------------
On Feb. 24, 2016, 9:42 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 9:42 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120570
-----------------------------------------------------------
On Feb. 24, 2016, 9:42 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 9:42 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120570
-----------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp (lines 2395 - 2398)
<https://reviews.apache.org/r/43879/#comment182035>
Why isn't it an `AWAIT_READY` macro? you still use an `AWAIT_*` afterwards, so I doubt is the result value.
src/tests/hierarchical_allocator_tests.cpp (line 2425)
<https://reviews.apache.org/r/43879/#comment182036>
from the C++ styleguide:
> Prefer trailing underscores for use as member fields (but not required). Some trailing underscores are used to distinguish between similar variables in the same scope (think prime symbols), but this should be avoided as much as possible, including removing existing instances in the code base.
I'd rather have the test's fixture `flags` being changed to `flags_`, but I'm oke with renaming this to `localFlags` or something along those lines.
src/tests/hierarchical_allocator_tests.cpp (line 2435)
<https://reviews.apache.org/r/43879/#comment182037>
Not an issue here, but equality comparison with doubles are a scary thing.
- Alexander Rojas
On Feb. 24, 2016, 8:25 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 8:25 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 27, 2016, 12:13 a.m., Alexander Rojas wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2395
> > <https://reviews.apache.org/r/43879/diff/8/?file=1272147#file1272147line2395>
> >
> > Not yours but we don't start function names with a capital letter and that threw me off for a while. I wasn't sure if a new object was being created every time.
> >
> > Do you think it makes sense to create a patch fixing that?
Filed as MESOS-4826.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120985
-----------------------------------------------------------
On March 2, 2016, 11:35 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 11:35 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120985
-----------------------------------------------------------
On March 2, 2016, 11:35 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 11:35 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120985
-----------------------------------------------------------
I like how the test feels much more readable than in earlier versions. However, I'm not sure why would we want this metric. I've been thinking about it and I cannot come up with a single use case in which I want to know how many times the allocator has run. I also don't see in the JIRA entry an use case for it. If you think of one you could add it there, if not, I don't think it makes sense to land this particular metric.
src/master/allocator/mesos/hierarchical.hpp (line 286)
<https://reviews.apache.org/r/43879/#comment182549>
_s/in flight/queued/_ When I read _"in flight"_ I expect that the dispatch events are being executed, but after checking the code, one realizes they are just waiting.
src/tests/hierarchical_allocator_tests.cpp (line 64)
<https://reviews.apache.org/r/43879/#comment182544>
Not used anywhere in the code.
src/tests/hierarchical_allocator_tests.cpp (line 2395)
<https://reviews.apache.org/r/43879/#comment182547>
Not yours but we don't start function names with a capital letter and that threw me off for a while. I wasn't sure if a new object was being created every time.
Do you think it makes sense to create a patch fixing that?
- Alexander Rojas
On Feb. 26, 2016, 6:03 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2016, 6:03 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review121639
-----------------------------------------------------------
src/master/allocator/mesos/metrics.hpp (line 46)
<https://reviews.apache.org/r/43879/#comment183386>
{ goes onto new line.
src/tests/hierarchical_allocator_tests.cpp (lines 2401 - 2402)
<https://reviews.apache.org/r/43879/#comment183387>
Blank line
src/tests/hierarchical_allocator_tests.cpp (lines 2409 - 2410)
<https://reviews.apache.org/r/43879/#comment183388>
Blank line.
- Alexander Rukletsov
On March 2, 2016, 10:35 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 10:35 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On March 3, 2016, 3:05 p.m., Alexander Rukletsov wrote:
> > docs/monitoring.md, line 872
> > <https://reviews.apache.org/r/43879/diff/12/?file=1279026#file1279026line872>
> >
> > s/the allocator was triggered/allocation was performed.
> >
> > Technically, we trigger allocator when we for example recover resources, but this does not trigger the allocaton.
Since calling `allocate(...)` does not necessarily perform allocations I went with `Number of times allocations where triggered`.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review121856
-----------------------------------------------------------
On March 3, 2016, 5:17 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 3, 2016, 5:17 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review121856
-----------------------------------------------------------
docs/monitoring.md (line 872)
<https://reviews.apache.org/r/43879/#comment183698>
s/the allocator was triggered/allocation was performed.
Technically, we trigger allocator when we for example recover resources, but this does not trigger the allocaton.
src/tests/hierarchical_allocator_tests.cpp (line 2383)
<https://reviews.apache.org/r/43879/#comment183699>
Wrap your braces, Benjamin! : )
- Alexander Rukletsov
On March 3, 2016, 1:12 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 3, 2016, 1:12 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review122043
-----------------------------------------------------------
Ship it!
Ship It!
- 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/43879/
> -----------------------------------------------------------
>
> (Updated March 3, 2016, 4:17 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review124937
-----------------------------------------------------------
Fix it, then Ship it!
Thanks! Just some minor adjustments I'll make before committing.
docs/monitoring.md (line 873)
<https://reviews.apache.org/r/43879/#comment187639>
How about:
```
Number of times the allocation algorithm has run
```
src/master/allocator/mesos/metrics.hpp (line 59)
<https://reviews.apache.org/r/43879/#comment187641>
How about:
// Number of times the allocation algorithm has run.
src/tests/hierarchical_allocator_tests.cpp (lines 2684 - 2686)
<https://reviews.apache.org/r/43879/#comment187643>
Could we put this next to the resource metrics test?
src/tests/hierarchical_allocator_tests.cpp (lines 2684 - 2685)
<https://reviews.apache.org/r/43879/#comment187645>
How about:
```
// This test checks that the number of times the allocation
// algorithm has run is correctly reflected in the metric.
```
src/tests/hierarchical_allocator_tests.cpp (line 2686)
<https://reviews.apache.org/r/43879/#comment187644>
s/Metrics/Metric/ in this patch since there's only one
src/tests/hierarchical_allocator_tests.cpp (line 2695)
<https://reviews.apache.org/r/43879/#comment187646>
We'll typically use size_t for this kind of variable
src/tests/hierarchical_allocator_tests.cpp (line 2699)
<https://reviews.apache.org/r/43879/#comment187647>
It's a bit unfortunate to use the [] operator here since it will insert the value if it was missing, could we switch to .contains? This would be consistent with the comments I made on the previous patches.
Also, why not use your allocations variable? s/0/allocations/
- Ben Mahler
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/43879/
> -----------------------------------------------------------
>
> (Updated March 21, 2016, 11:08 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated March 21, 2016, 12:08 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Review comments from @bmahler.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
Diffs (updated)
-----
docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated March 15, 2016, 3:51 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Addressed offline comments from Ben Mahler.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
Diffs (updated)
-----
docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review122170
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rojas
On March 3, 2016, 5:17 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 3, 2016, 5:17 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated March 3, 2016, 5:17 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Addressed comments form Alex.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description (updated)
-------
Added allocator metrics for number of allocations made.
Diffs (updated)
-----
docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated March 3, 2016, 2:12 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Tidied up includes.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(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-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description (updated)
-------
See summary.
Diffs (updated)
-----
docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated March 2, 2016, 11:35 a.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Style fixes.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review121255
-----------------------------------------------------------
On March 2, 2016, 11:35 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 11:35 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On March 1, 2016, 12:48 p.m., Alexander Rukletsov wrote:
> > docs/monitoring.md, lines 872-874
> > <https://reviews.apache.org/r/43879/diff/9/?file=1273629#file1273629line872>
> >
> > Why not putting these onto the same line?
>
> Benjamin Bannier wrote:
> Let's keep consistent formatting with other tables here.
>
> Alexander Rukletsov wrote:
> That is exactly my point. I see such strings on the same line with <td></td> in this file.
OK, filed an issue against my mental HTML parser. Fixed now.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review121255
-----------------------------------------------------------
On March 2, 2016, 4:43 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 4:43 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On March 1, 2016, 12:48 p.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2391
> > <https://reviews.apache.org/r/43879/diff/9/?file=1273632#file1273632line2391>
> >
> > This looks like a creation of an instance of some `Metrics` class. Let's rename it to somehting like `GetMetricsSnapshot()` in a separate clean-up patch.
> >
> > Also, I would move this down right before `EXPECT_EQ` for clarity.
MESOS-4826.
> On March 1, 2016, 12:48 p.m., Alexander Rukletsov wrote:
> > docs/monitoring.md, lines 872-874
> > <https://reviews.apache.org/r/43879/diff/9/?file=1273629#file1273629line872>
> >
> > Why not putting these onto the same line?
Let's keep consistent formatting with other tables here.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review121255
-----------------------------------------------------------
On March 2, 2016, 11:35 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 11:35 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On March 1, 2016, 11:48 a.m., Alexander Rukletsov wrote:
> > docs/monitoring.md, lines 872-874
> > <https://reviews.apache.org/r/43879/diff/9/?file=1273629#file1273629line872>
> >
> > Why not putting these onto the same line?
>
> Benjamin Bannier wrote:
> Let's keep consistent formatting with other tables here.
That is exactly my point. I see such strings on the same line with <td></td> in this file.
> On March 1, 2016, 11:48 a.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2391
> > <https://reviews.apache.org/r/43879/diff/9/?file=1273632#file1273632line2391>
> >
> > This looks like a creation of an instance of some `Metrics` class. Let's rename it to somehting like `GetMetricsSnapshot()` in a separate clean-up patch.
> >
> > Also, I would move this down right before `EXPECT_EQ` for clarity.
>
> Benjamin Bannier wrote:
> MESOS-4826.
I'd say renaming the function is not that long, let's do it now.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review121255
-----------------------------------------------------------
On March 2, 2016, 10:35 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated March 2, 2016, 10:35 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review121255
-----------------------------------------------------------
docs/monitoring.md (lines 859 - 862)
<https://reviews.apache.org/r/43879/#comment182935>
We already have one, `event_queue_dispatches`, do you want to list it here as well?
docs/monitoring.md (lines 861 - 862)
<https://reviews.apache.org/r/43879/#comment183077>
This is true only for the default hierarchical allocator, right?
docs/monitoring.md (lines 872 - 874)
<https://reviews.apache.org/r/43879/#comment183099>
Why not putting these onto the same line?
src/master/allocator/mesos/hierarchical.hpp (lines 276 - 277)
<https://reviews.apache.org/r/43879/#comment183100>
Let's maintain the same order everywhere: field declarations in the class, c-tor initializer list, c-tor body, d-tor body.
src/tests/hierarchical_allocator_tests.cpp (line 30)
<https://reviews.apache.org/r/43879/#comment183102>
Why do you pull in this one?
src/tests/hierarchical_allocator_tests.cpp (line 60)
<https://reviews.apache.org/r/43879/#comment183101>
Do you use it?
src/tests/hierarchical_allocator_tests.cpp (line 2390)
<https://reviews.apache.org/r/43879/#comment183103>
Why do we need a settle here?
src/tests/hierarchical_allocator_tests.cpp (line 2391)
<https://reviews.apache.org/r/43879/#comment183079>
This looks like a creation of an instance of some `Metrics` class. Let's rename it to somehting like `GetMetricsSnapshot()` in a separate clean-up patch.
Also, I would move this down right before `EXPECT_EQ` for clarity.
src/tests/hierarchical_allocator_tests.cpp (line 2393)
<https://reviews.apache.org/r/43879/#comment183107>
I don't see `using std::size_t` above : )
src/tests/hierarchical_allocator_tests.cpp (line 2396)
<https://reviews.apache.org/r/43879/#comment183080>
s/0/0u
Please, make sure types in gtest macros are the same!
src/tests/hierarchical_allocator_tests.cpp (line 2398)
<https://reviews.apache.org/r/43879/#comment183086>
If there is a single agent in the test, we do not enumerate for clarity.
src/tests/hierarchical_allocator_tests.cpp (line 2400)
<https://reviews.apache.org/r/43879/#comment183104>
If you write a proper comment for the test you may avoid putting trailing comments here : ).
src/tests/hierarchical_allocator_tests.cpp (lines 2415 - 2418)
<https://reviews.apache.org/r/43879/#comment183087>
I would format it differently: blank line between `settle` and sending the request, no blank line between sending the request and checking it.
- Alexander Rukletsov
On Feb. 28, 2016, 9:28 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2016, 9:28 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> 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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated Feb. 28, 2016, 10:28 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Addressed comments from Alexander.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated Feb. 26, 2016, 6:03 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Rebased and get metrics without a helper.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated Feb. 25, 2016, 12:01 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Added user documentation and reused read-out code for endpoint.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 9:42 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Renamed variable.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 8:25 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Addressed klaus1982's comments.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 24, 2016, 4:21 p.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2385
> > <https://reviews.apache.org/r/43879/diff/4/?file=1267401#file1267401line2385>
> >
> > Why a anonymous namespace? `static` should be fine.
In C `static` has more usages than just denoting internal linkage and the meaning needs to be inferred from the context, while C++ has anonymous namespaces just for that; so I would say it is rather "just anonymous namespace should be fine" than the other way around. Switched to `static` here anyway.
> On Feb. 24, 2016, 4:21 p.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2395
> > <https://reviews.apache.org/r/43879/diff/4/?file=1267401#file1267401line2395>
> >
> > What are you going to do for this comments? or just a notes?
Converted to `NOTE`.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120507
-----------------------------------------------------------
On Feb. 24, 2016, 2:25 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 2:25 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120507
-----------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp (line 2385)
<https://reviews.apache.org/r/43879/#comment181976>
Why a anonymous namespace? `static` should be fine.
src/tests/hierarchical_allocator_tests.cpp (line 2395)
<https://reviews.apache.org/r/43879/#comment181975>
What are you going to do for this comments? or just a notes?
- Klaus Ma
On Feb. 24, 2016, 9:25 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 9:25 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 2:25 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Addressed gyliu's comments.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 11:22 a.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/#review120473
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp (line 1156)
<https://reviews.apache.org/r/43879/#comment181933>
Just a question here, is it too early to get this value? What about only increaes this value when there are allocated resources send out?
What is the use of this metric if it includes all allocation cycles even if some cycles do not have resources allocated?
or else keep this metric and add a new metric which record the succeed allocations.
- Guangya Liu
On 二月 24, 2016, 8:46 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> -----------------------------------------------------------
>
> (Updated 二月 24, 2016, 8:46 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added allocator metrics for number of allocations made.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1
> src/master/allocator/mesos/hierarchical.cpp 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d
> src/tests/hierarchical_allocator_tests.cpp 5f771f02db9bd098f3cd36730cd84bf2f5e87a33
>
> Diff: https://reviews.apache.org/r/43879/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 43879: Added allocator metrics for number of
allocations made.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43879/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 9:46 a.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Addressed comments from Guangya Liu.
Bugs: MESOS-4718
https://issues.apache.org/jira/browse/MESOS-4718
Repository: mesos
Description
-------
Added allocator metrics for number of allocations made.
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/43879/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