[ 
https://issues.apache.org/jira/browse/HADOOP-9090?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13506582#comment-13506582
 ] 

Ravi Prakash commented on HADOOP-9090:
--------------------------------------

This could be very useful. Thanks for taking this up Mostafa.

Minor nit. In MetricsSystem.java:
{code}
public abstract void publishMetricsNow();
{code}
IMHO we shouldn't put that method that high in the heirarchy. How would 
implementations of MetricsSystem not concerned with real-time implement this 
method?

Does the description of this JIRA need an update? The classes aren't abstract 
after your patch.

Otherwise code looks good to me.
                
> Refactor MetricsSystemImpl to allow for an on-demand publish system
> -------------------------------------------------------------------
>
>                 Key: HADOOP-9090
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9090
>             Project: Hadoop Common
>          Issue Type: New Feature
>          Components: metrics
>            Reporter: Mostafa Elhemali
>            Priority: Minor
>         Attachments: HADOOP-9090.2.patch, 
> HADOOP-9090.justEnhanceDefaultImpl.2.patch, 
> HADOOP-9090.justEnhanceDefaultImpl.3.patch, 
> HADOOP-9090.justEnhanceDefaultImpl.4.patch, 
> HADOOP-9090.justEnhanceDefaultImpl.patch, HADOOP-9090.patch
>
>
> We have a need to publish metrics out of some short-living processes, which 
> is not really well-suited to the current metrics system implementation which 
> periodically publishes metrics asynchronously (a behavior that works great 
> for long-living processes). Of course I could write my own metrics system, 
> but it seems like such a waste to rewrite all the awesome code currently in 
> the MetricsSystemImpl and supporting classes.
> The way I'm proposing to solve this is to:
> 1. Refactor the MetricsSystemImpl class into an abstract base 
> MetricsSystemImpl class (common configuration and other code) and a concrete 
> PeriodicPublishMetricsSystemImpl class (timer thread).
> 2. Refactor the MetricsSinkAdapter class into an abstract base 
> MetricsSinkAdapter class (common configuration and other code) and a concrete 
> AsyncMetricsSinkAdapter class (asynchronous publishing using the SinkQueue).
> 3. Derive a new simple class OnDemandPublishMetricsSystemImpl from 
> MetricsSystemImpl, that just exposes a synchronous publish() method to do all 
> the work.
> 4. Derive a SyncMetricsSinkAdapter class from MetricsSinkAdapter to just 
> synchronously push metrics to the underlying sink.
> Does that sound reasonable? I'll attach the patch with all this coded up and 
> simple tests (could use some polish I guess, but wanted to get everyone's 
> opinion first). Notice that this is somewhat of a breaking change since 
> MetricsSystemImpl is public (although it's marked with 
> InterfaceAudience.Private); if the breaking change is a problem I could just 
> rename the refactored classes so that PeriodicPublishMetricsSystemImpl is 
> still called MetricsSystemImpl (and MetricsSystemImpl -> 
> BaseMetricsSystemImpl).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to