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



src/master/master.hpp
<https://reviews.apache.org/r/19504/#comment76443>

    I would propose the following order (keeps the message counters together 
and tries to group them in an intuitive way) and let's use this same order in 
the initializer list, constructor body, and destructor body:
    
        process::metrics::Gauge uptime_secs;
        process::metrics::Gauge elected;
    
        process::metrics::Gauge active_slaves;
        process::metrics::Gauge inactive_slaves;
    
        process::metrics::Gauge active_frameworks;
        process::metrics::Gauge inactive_frameworks;
    
        process::metrics::Gauge outstanding_offers;
    
        // Message counters.
        // TODO(bmahler): Add counters for other messages: kill task,
        // status update, etc.
        process::metrics::Counter dropped_messages;
    
        process::metrics::Counter framework_registration_messages;
        process::metrics::Counter framework_reregistration_messages;
    
        process::metrics::Counter slave_registration_messages;
        process::metrics::Counter slave_reregistration_messages;
    
        process::metrics::Counter valid_framework_messages;
        process::metrics::Counter invalid_framework_messages;
    
        process::metrics::Counter valid_status_updates;
        process::metrics::Counter invalid_status_updates;
    
        // Recovery counters.
        process::metrics::Counter recovery_slave_removals;
    
        // Process metrics.
        process::metrics::Gauge event_queue_size;



src/master/master.hpp
<https://reviews.apache.org/r/19504/#comment76428>

    Hm, these are a bit confusing because they sound like they are referring to 
_all_ framework messages but they're only referring to framework -> executor 
messages, let's update these names to:
    
    valid_framework_to_executor_messages
    invalid_framework_to_executor_messages
    
    Let's also follow up on the slave metrics to make a similar naming change 
(these were missed in my list in MESOS-1332 because they weren't in the JSON).



src/master/master.hpp
<https://reviews.apache.org/r/19504/#comment76444>

    'slaves.deactivated' is actually the _removed_ slaves!
    
    We'll want 'active_slaves' to count those slaves in 'slaves.activated' that 
are connected, and 'inactive_slaves' to count those slaves in 
'slaves.activated' that are disconnected (no offers being sent for them).


- Ben Mahler


On May 9, 2014, 5:37 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19504/
> -----------------------------------------------------------
> 
> (Updated May 9, 2014, 5:37 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1132 and MESOS-1332
>     https://issues.apache.org/jira/browse/MESOS-1132
>     https://issues.apache.org/jira/browse/MESOS-1332
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp f54c841d003d7a4d2eda5e93405afa358707f4e0 
>   src/master/master.hpp 0a350b0b402edb3ca648c91c920043f66c08fe0e 
>   src/master/master.cpp d851a7291acce950ea9391ddfb8813a432aeda34 
>   src/tests/master_tests.cpp 326392ed03f3cb1321ec21af57a5b0253cf29ef9 
> 
> Diff: https://reviews.apache.org/r/19504/diff/
> 
> 
> Testing
> -------
> 
> ran locally and checked /metrics/snapshot vs master/stats.json
> 
> make check
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Reply via email to