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

Enis Soztutar commented on HBASE-15518:
---------------------------------------

Thanks [~aliciashu] for the patch. Couple of comments: 
 - Please check the above hadoopqa warnings, and address those. 
 - Having both {{num}} and {{Count}} in the metric name does not make sense. 
Lets use {{readRequestCount}} / {{writeRequestCount}} to be consistent with 
RS-level metric names. Same thing for total request count. 
{code}+  String NUM_READ_REQUESTS_COUNT = "numReadRequestsCount";
 - Move this class inside the MetricsTableWrapperAggregateImpl. No need to be 
top level. It should be declared private. 
{code}
+public class MetricsTableValues {
{code} 
 - Instead of using the table aggregate wrapper directly in the regionserver, 
lets do a wrapper like the MetricsTable to hold the aggregate source and 
aggregate wrapper. We can instantiate that in the regionserver level. 
{code}
+  MetricsTableWrapperAggregate metricsTableWrapperAgg;
{code}
- You don't need this ConcurrentHashMap for metricsTableMap. It should be a 
locally allocated map inside the method that uses it. 
{code}
+  private ConcurrentHashMap<TableName, MetricsTableValues> metricsTableMap = 
new ConcurrentHashMap<>();
{code}
- otherwise looks good. 
 - Let me test this patch with my ycsb setup. 

> Add Per-Table metrics back
> --------------------------
>
>                 Key: HBASE-15518
>                 URL: https://issues.apache.org/jira/browse/HBASE-15518
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Enis Soztutar
>            Assignee: Alicia Ying Shu
>             Fix For: 2.0.0, 1.4.0
>
>         Attachments: HBASE-15518.patch
>
>
> We used to have per-table metrics, but it was removed in some restructuring. 
> We have per-region metrics, and per-regionserver metrics, but nothing in 
> between. 
> For majority of users, per-region is too granular, they are mostly interested 
> in table level aggregates. This is especially useful in multi-tenant cases 
> where a table's disk usage, number of requests, etc can be made much more 
> visible. 
> In this jira, we'll add the basic infrastructure to add a single (or a few) 
> per-table metrics. Than we can improve on that by adding remaining metrics 
> from the region server level. 
> The plan is to NOT aggregate per-table metrics at master for now. Just 
> aggregation of per-region metrics at the per-table level for every 
> regionserver. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to