[
https://issues.apache.org/jira/browse/HADOOP-15481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16739858#comment-16739858
]
Erik Krogen commented on HADOOP-15481:
--------------------------------------
Thanks for tackling this [~cgregori]! My comments on the v001 patch are below.
* I think the metrics context should be "rpc" rather than "dfs", since FCQ
lives at the IPC layer of the stack and isn't HDFS-specific
* Making this implement {{MetricsSource}} seems like a good move to be able to
easily use the current info available in conjunction with the metrics proxy,
but I don't understand why both {{FairCallQueue}} and {{MetricsProxy}}
implement it. It seems like only the {{MetricsProxy}} should be a
{{MetricsSource}}?
* In {{Metrics.md}}, I think this needs to be under the "rpc" H1 section (as
discussed above), and should have its own H2 header -- there's one of these for
each class which emits metrics. It should probably include some discussion of
the fact that these will only exist if FCQ is enabled.
* The name of the metric should probably use {{getClass().getSimpleName()}}
instead of {{getClass().getName()}} so that it looks like
{{context.FairCallQueue}} instead of including the package name
* Since a process can theoretically have multiple FairCallQueue instances
running simultaneously, we need to include the port information. Take a look at
{{RpcMetrics}} for how it includes the port number as a "tag" that we can later
use to differentiate metrics from different FCQ instances.
* I know checkstyle didn't complain, but we typically discourage the use of
star-imports like you used for {{MetricsAsserts}} in your test, can you make
them individual static imports instead?
* The test looks great! My one comment would be that it's a little confusing
right now that the assertions appear within the catch block -- it would read
more easily if the catch block just had a comment saying that the exception was
expected, then the assertions outside of the try-catch.
> Emit FairCallQueue stats as metrics
> -----------------------------------
>
> Key: HADOOP-15481
> URL: https://issues.apache.org/jira/browse/HADOOP-15481
> Project: Hadoop Common
> Issue Type: Improvement
> Components: metrics, rpc-server
> Reporter: Erik Krogen
> Assignee: Christopher Gregorian
> Priority: Major
> Attachments: HADOOP-15481.001.patch, HADOOP-15481.001.patch
>
>
> Currently FairCallQueue has some statistics which are exported via JMX: the
> size of each queue, and the number of overflowed calls per queue. These are
> useful statistics to track over time to determine, for example, if queues
> need to be resized. We should emit them via the standard metrics system.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
