[
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)
