hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Enis Soztutar (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HBASE-15518) Add Per-Table metrics back
Date Tue, 05 Apr 2016 03:04:25 GMT

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

Enis Soztutar edited comment on HBASE-15518 at 4/5/16 3:03 AM:
---------------------------------------------------------------

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";{code}
 - 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. 


was (Author: enis):
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)

Mime
View raw message