[
https://issues.apache.org/jira/browse/HADOOP-16912?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17058937#comment-17058937
]
Chao Sun commented on HADOOP-16912:
-----------------------------------
Thanks [~fengnanli] for the patch! Some comments:
1. Please mark the new class {{RpcDetailedMetrics}} with
{{InterfaceStability.Evolving}}
2. in {{MetricsRegistry#tag}} you passed {{ns}} as port, but it seems it is
more than that. Maybe we should use something different.
3. nit: "Initializing RPC stats for {} priorities" maybe change to
"Initializing RPC stats for {} priority levels"?
4. {{getQueueTime}} and {{getProcessingTime}} is called every time a new metric
is added, and they do string concatenation, which could be expensive. Can we
optimize this by, say, doing table lookup?
5. the queue and process metrics are called "FairCallQueuePriorityXXX" - should
we change that to "DecaySchedulerPriorityXXX" or something similar?
6. Same as above, I'm not sure if "RpcSchedulerDetailedMetrics" is a good name
for this since it is specificall for DecayRpcScheduler".
7. In {{getQueueTime}} and {{getProcessingTIme}}, please add comments on the
parameters and return types, or remove them if you think it is too obvious.
Also please add a blank line between the method description and the first
{{@param}}.
7. {{getRpcQueueRates}}, {{getRpcProcessingRates}} and {{getRegistry}} are not
used anywhere. Can we remove them?
8. Can we enhance the {{TestRpcSchedulerDetailedMetrics}} by testing that all
rate names are also registered via {{MutableRatesWithAggregation#init}}?
Also, it would be great if you can open a github PR instead of attaching the
patch. The former is much easier for reviewing :)
> Emit per priority rpc queue time and processing time from DecayRpcScheduler
> ---------------------------------------------------------------------------
>
> Key: HADOOP-16912
> URL: https://issues.apache.org/jira/browse/HADOOP-16912
> Project: Hadoop Common
> Issue Type: New Feature
> Components: common
> Reporter: Fengnan Li
> Assignee: Fengnan Li
> Priority: Major
> Labels: metrics
> Attachments: HADOOP-16912.001.patch, HADOOP-16912.002.patch
>
>
> At ipc Server level we have the overall rpc queue time and processing time
> for the whole CallQueueManager. In the case of using FairCallQueue, it will
> be great to know the per queue/priority level rpc queue time since many times
> we want to keep certain queues to meet some queue time SLA for customers.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
