[
https://issues.apache.org/jira/browse/HBASE-5292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13199307#comment-13199307
]
Phabricator commented on HBASE-5292:
------------------------------------
Kannan has commented on the revision "[jira] [HBASE-5292] [89-fb] Prevent
counting getSize on compactions".
Zhiqui: Looks very good. A few comments inlined...
INLINE COMMENTS
src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:299 i think
we can avoid this hashset and "contains" check later.
Since this is all internal code, we can assume that if caller pass a non-null
metric name, it is good to use.
src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java:200 we
need to a append a "." here for compatibility with old code, and for
readability.
Otherwise, the metric names will be
cf.column_familygetsize
instead of
cf.column_family.getsize.
src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java:136
nice test!
src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java:345 as
mentioned earlier, I think we can get rid of the hashset.
Also, can we pass in null (instead of "") for all the places you don't want
this metric to be tracked. And this check would simply become:
if (metric != null)
REVISION DETAIL
https://reviews.facebook.net/D1527
> getsize per-CF metric incorrectly counts compaction related reads as well
> --------------------------------------------------------------------------
>
> Key: HBASE-5292
> URL: https://issues.apache.org/jira/browse/HBASE-5292
> Project: HBase
> Issue Type: Bug
> Reporter: Kannan Muthukkaruppan
> Attachments: D1527.1.patch, D1527.2.patch
>
>
> The per-CF "getsize" metric's intent was to track bytes returned (to HBase
> clients) per-CF. [Note: We already have metrics to track # of HFileBlock's
> read for compaction vs. non-compaction cases -- e.g., compactionblockreadcnt
> vs. fsblockreadcnt.]
> Currently, the "getsize" metric gets updated for both client initiated
> Get/Scan operations as well for compaction related reads. The metric is
> updated in StoreScanner.java:next() when the Scan query matcher returns an
> INCLUDE* code via a:
> HRegion.incrNumericMetric(this.metricNameGetsize, copyKv.getLength());
> We should not do the above in case of compactions.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
