[ 
https://issues.apache.org/jira/browse/HBASE-5292?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Phabricator updated HBASE-5292:
-------------------------------

    Attachment: D1617.1.patch

zhiqiu requested code review of "[jira] [HBASE-5292] Prevent counting getSize 
on compactions".
Reviewers: Kannan, mbautin, Liyin, JIRA

  Added two separate metrics for both get() and next(). This is done by
  refactoring on internal next() API. To be more specific, only Get.get()
  and ResultScanner.next() passes the metric name ("getsize" and
  "nextsize" repectively) to
    HRegion::RegionScanner::next(List<KeyValue>, String)

  This will eventually hit StoreScanner()::next((List<KeyValue>,
  int, String) where the metrics are counted.

  And their call paths are:

  1) Get

  HTable::get(final Get get)
  => HRegionServer::get(byte [] regionName, Get get)
  => HRegion::get(final Get get, final Integer lockid)
  => HRegion::get(final Get get)      [pass METRIC_GETSIZE to the
  callee]

  => HRegion::RegionScanner::next(List<KeyValue> outResults, String
  metric)
  => HRegion::RegionScanner::next(List<KeyValue> outResults, int limit,
  String metric)
  => HRegion::RegionScanner::nextInternal(int limit, String metric)
  => KeyValueHeap::next(List<KeyValue> result, int limit, String
  metric)
  => StoreScanner::next(List<KeyValue> outResult, int limit, String
  metric)

  2) Next

  HTable::ClientScanner::next()
  => ScannerCallable::call()
  => HRegionServer::next(long scannerId)
  => HRegionServer::next(final long scannerId, int nbRows)  [pass
  METRIC_NEXTSIZE to the callee]

  => HRegion::RegionScanner::next(List<KeyValue> outResults, String
  metric)
  => HRegion::RegionScanner::next(List<KeyValue> outResults, int limit,
  String metric)
  => HRegion::RegionScanner::nextInternal(int limit, String metric)
  => KeyValueHeap::next(List<KeyValue> result, int limit, String
  metric)
  => StoreScanner::next(List<KeyValue> outResult, int limit, String
  metric)

  Task ID: #898948

  Blame Rev:

TEST PLAN
  1. Passed unit tests.
  2. Created a testcase TestRegionServerMetrics::testGetNextSize to
  guarantee:
   * Get/Next contributes to getsize/nextsize metrics
   * Both getsize/nextsize are per Column Family
   * Flush/compaction won't affect these two metrics

  Revert Plan:

  Tags:

REVISION DETAIL
  https://reviews.facebook.net/D1617

AFFECTED FILES
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
  src/main/java/org/apache/hadoop/hbase/regionserver/InternalScanner.java
  src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
  src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
  
src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java
  
src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
  
src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java

MANAGE HERALD DIFFERENTIAL RULES
  https://reviews.facebook.net/herald/view/differential/

WHY DID I GET THIS EMAIL?
  https://reviews.facebook.net/herald/transcript/3441/

Tip: use the X-Herald-Rules header to filter Herald messages in your client.

                
> 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, D1527.3.patch, 
> D1527.4.patch, D1617.1.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

        

Reply via email to