mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dominic Hamon" <dha...@twopensource.com>
Subject Re: Review Request 27531: Update Master metrics to match task source and reason scheme.
Date Fri, 06 Feb 2015 17:39:42 GMT


> On Feb. 4, 2015, 12:41 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 4704
> > <https://reviews.apache.org/r/27531/diff/4/?file=768121#file768121line4704>
> >
> >     Not sure why we missed updating "metrics" for non-terminal states when we introduced
"metrics". This is a bug. I'll prep a fix for this.

there was a discussion about this in the reviews but i don't remember the outcome.


> On Feb. 4, 2015, 12:41 p.m., Vinod Kone wrote:
> > src/tests/master_authorization_tests.cpp, lines 365-373
> > <https://reviews.apache.org/r/27531/diff/4/?file=768126#file768126line365>
> >
> >     It is a bit weird to see a test for these metrics in authorization tests. Any
particular reason or did you randomly pick this file? I would recommend testing these in master_tests.cpp
(and in slave_tests.cpp once you implement the slave metrics) since that already has metrics
tests.

when implementing metrics, there was discussion around having metric tests or inserting metric
tests into existing tests. The conclusion there was that introducing metric tests would require
duplication of the logic from many other existing tests to trigger different updates, and
better metrics coverage could be tested by inserting metric tests into existing tests.

this is a prime example where expectations of task state, sources and reasons are well defined
by the existing tests.


- Dominic


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


On Feb. 5, 2015, 10:07 a.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27531/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 10:07 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1830
>     https://issues.apache.org/jira/browse/MESOS-1830
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Update metrics in Master to match the source and reason split for task statuses.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp dcfd38ae2fa9e1bd0b477e9719724dba37114d30 
>   src/master/master.cpp 234bbecc4205036d790b026abd59100eb188f913 
>   src/master/metrics.hpp 5e18f883fe82ae0b7ce1bd90419e271409867289 
>   src/master/metrics.cpp 5fe71e3545b48b26a7bc50252d5707cad453bffc 
>   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
>   src/slave/slave.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b 
>   src/tests/master_authorization_tests.cpp 20adaa954a0ddfe3459d8a3f9696b5ca9ae07f24 
>   src/tests/master_slave_reconciliation_tests.cpp 04806ede4e727fa4de1464017a06797d69b54e29

>   src/tests/master_tests.cpp 3cb7660b372c9846ae8c0aefbe559095f47e7794 
>   src/tests/mesos.hpp 17c2d8f0cb6326b08fc506143e823ee2c3a32e09 
>   src/tests/mesos.cpp 5ed4df530cf1bf11eec3b29542641822e0f702b2 
>   src/tests/rate_limiting_tests.cpp 7f5ca251fef5b1852bb273505f5eaed750578b9f 
>   src/tests/slave_tests.cpp 956ce64fcbee116a37eaa30b14edf580e7cff888 
> 
> Diff: https://reviews.apache.org/r/27531/diff/
> 
> 
> Testing
> -------
> 
> added metric tests to master tests
> make check
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


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