You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@mesos.apache.org by Dominic Hamon <dh...@twopensource.com> on 2014/05/09 21:39:20 UTC

Review Request 21279: Added task gauges to Master.

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

Review request for mesos and Ben Mahler.


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


Repository: mesos-git


Description
-------

see summary


Diffs
-----

  src/master/master.hpp 0a350b0b402edb3ca648c91c920043f66c08fe0e 
  src/master/master.cpp d851a7291acce950ea9391ddfb8813a432aeda34 

Diff: https://reviews.apache.org/r/21279/diff/


Testing
-------

make check

ran locally and manually verified.


Thanks,

Dominic Hamon

Re: Review Request 21279: Added task gauges to Master.

Posted by Vinod Kone <vi...@gmail.com>.

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



src/master/master.cpp
<https://reviews.apache.org/r/21279/#comment76782>

    I would kill this.
    
    When a slave re-registers with a failed over master, it doesn't currently send information about completed executors or completed tasks of a running framework. It only sends info about completed frameworks (and their completed executors and tasks).
    
    So, updating the terminal task counts with the latter but not the former seems misleading.
    
    Lets update the counters after we fix the slave to do the right thing.
    


- Vinod Kone


On May 13, 2014, 7:35 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21279/
> -----------------------------------------------------------
> 
> (Updated May 13, 2014, 7:35 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1332
>     https://issues.apache.org/jira/browse/MESOS-1332
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 923552e3c7f9942d6be7a0446b44e40f53c11433 
>   src/master/master.cpp 569995c2eaf758f5171c4e0d082d4e519a143025 
> 
> Diff: https://reviews.apache.org/r/21279/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> ran locally and manually verified.
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Re: Review Request 21279: Added task gauges to Master.

Posted by Vinod Kone <vi...@gmail.com>.

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

Ship it!


Ship It!

- Vinod Kone


On May 13, 2014, 9:31 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21279/
> -----------------------------------------------------------
> 
> (Updated May 13, 2014, 9:31 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1332
>     https://issues.apache.org/jira/browse/MESOS-1332
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 923552e3c7f9942d6be7a0446b44e40f53c11433 
>   src/master/master.cpp 569995c2eaf758f5171c4e0d082d4e519a143025 
> 
> Diff: https://reviews.apache.org/r/21279/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> ran locally and manually verified.
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Re: Review Request 21279: Added task gauges to Master.

Posted by Ben Mahler <be...@gmail.com>.

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

Ship it!


Looks good, would like to see some consistency around post-increment vs. pre-increment for metrics and counters.


src/master/master.cpp
<https://reviews.apache.org/r/21279/#comment77003>

    No period here



src/master/master.cpp
<https://reviews.apache.org/r/21279/#comment77006>

    Now we're pre-incrementing 'count' here, post-incrementing 'count' below, pre-incrementing some 'metrics', post-incrementing some 'metrics' (in master and slave).
    
    We should stick with one technique, and I think the preference in Mesos has been to post-increment for "readability" which is arguable but is the current state (avoiding the thought of optimization here as it's definitely premature and the compiler will often take care of it).
    
    We're not consistent across the code by any means, and my personal preference is to only use post-increment when necessary, however the easiest change here would be to just post-increment in this change.
    


- Ben Mahler


On May 14, 2014, 8:50 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21279/
> -----------------------------------------------------------
> 
> (Updated May 14, 2014, 8:50 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1332
>     https://issues.apache.org/jira/browse/MESOS-1332
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 12111cf7b183438f540bb0aad17e50e3f367558a 
>   src/master/master.cpp 2f0e9027c5bea8066ac13996451d4905a422336e 
>   src/tests/master_tests.cpp dcda0c79a6693bc3c0b69b4c9b148f9608342738 
> 
> Diff: https://reviews.apache.org/r/21279/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> ran locally and manually verified.
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>