[
https://issues.apache.org/jira/browse/HBASE-4145?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13117609#comment-13117609
]
jirapos...@reviews.apache.org commented on HBASE-4145:
------------------------------------------------------
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/metrics/ScanMetrics.java,
line 48
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46377#file46377line48>
bq. >
bq. > Should be declared as implementing VersionedWritable.
The issue with VersionedWritable is it throws VersionMismatchException if the
version doesn't match.
public void readFields(DataInput in) throws IOException {
byte version = in.readByte(); // read version
if (version != getVersion())
throw new VersionMismatchException(getVersion(), version);
}
I want to make it backward compatible to support version <= getVersion(). The
program could catch VersionMismatchException, however, there is no way to find
out the expectedVersion and foundVersion, given they are private members.
public class VersionMismatchException extends IOException {
private byte expectedVersion;
private byte foundVersion;
...
}
Any other suggestions, or Is it something that need to be fixed in
VersionedWritable, VersionMismatchException?
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/metrics/ScanMetrics.java,
line 76
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46377#file46377line76>
bq. >
bq. > It is a bit hard to read this counter.
Fixed.
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/metrics/ScanMetrics.java,
line 94
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46377#file46377line94>
bq. >
bq. > This is count of regions scanned, right ?
bq. > If so, please name it that way.
Todd suggested to rename it from COUNT_OF_REGIONS to REGIONS, given the fact
that it is a counter is implicit in mapreduce framework.
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/metrics/ScanMetrics.java,
line 127
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46377#file46377line127>
bq. >
bq. > mb should be included in the exception.
Fixed.
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/metrics/ScanMetrics.java,
line 133
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46377#file46377line133>
bq. >
bq. > Why do we need this check again ?
Fixed.
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/metrics/ScanMetrics.java,
line 143
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46377#file46377line143>
bq. >
bq. > Value of version should be included here.
Fixed.
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/metrics/ScanMetrics.java,
line 151
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46377#file46377line151>
bq. >
bq. > I think we should have else block where the unsupported mb is logged.
Fixed.
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java,
line 52
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46380#file46380line52>
bq. >
bq. > This name doesn't really match the constant above. I think "HBase
mapreduce Counters" would be better.
The name should show up in mapreduce UI and report. Other group names don't
have "mapreduce". So keep it as "HBase Counters" and rename the variable.
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java,
line 83
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46380#file46380line83>
bq. >
bq. > This should not be a tongue twister.
bq. > How about naming it retrieveGetCounterWithStrings ?
Fixed.
bq. On 2011-09-29 04:18:54, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java,
line 232
bq. > <https://reviews.apache.org/r/1674/diff/4/?file=46380#file46380line232>
bq. >
bq. > Shall we create the Object array outside the for loop and only fill
in Metric name here ?
Fixed. Don't create Object at all, pass the parameters directly.
- Ming
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1674/#review2149
-----------------------------------------------------------
On 2011-09-29 21:00:18, Ming Ma wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1674/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-09-29 21:00:18)
bq.
bq.
bq. Review request for hbase.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. 1. Collect client-side scan related metrics during scan operation. It is
turned off by default.
bq. 2. TableInputFormat enables metrics collection on scan and pass the data
to mapreduce framework. It only works with new mapreduce APIs that allow
TableInputFormat to get access to mapreduce Counter.
bq. 3. Clean up some minor issues in tableInputFormat as well as test code.
bq.
bq.
bq. This addresses bug hbase-4145.
bq. https://issues.apache.org/jira/browse/hbase-4145
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
1176942
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java
1176942
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java
1176942
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
1176942
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/metrics/ScanMetrics.java
PRE-CREATION
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormatBase.java
1176942
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReader.java
1176942
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableRecordReaderImpl.java
1176942
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
1176942
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapred/TestTableInputFormat.java
1176942
bq.
bq. Diff: https://reviews.apache.org/r/1674/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. 1. Verified on a small cluster.
bq. 2. Existing unit tests.
bq. 3. Added new tests.
bq.
bq.
bq. Thanks,
bq.
bq. Ming
bq.
bq.
> Provide metrics for hbase client
> --------------------------------
>
> Key: HBASE-4145
> URL: https://issues.apache.org/jira/browse/HBASE-4145
> Project: HBase
> Issue Type: Improvement
> Reporter: Ming Ma
> Assignee: Ming Ma
> Attachments: HBaseClientSideMetrics.jpg
>
>
> Sometimes it is useful to get some metrics from hbase client point of view.
> This will help understand the metrics for scan/TableInputFormat map job
> scenario.
> What to capture, for example, for each ResultScanner object,
> 1. The number of RPC calls to RSs.
> 2. The delta time between consecutive RPC calls in the current serialized
> scan implementation.
> 3. The number of RPC retry to RSs.
> 4. The number of NotServingRegionException got.
> 5. The number of remote RPC calls. This excludes those call that hbase client
> calls the RS on the same machine.
> 6. The number of regions accessed.
> How to capture
> 1. Metrics framework works for a fixed number of metrics. It doesn't fit this
> scenario.
> 2. Use some TBD solution in HBase to capture such dynamic metrics. If we
> assume there is a solution in HBase that HBase client can use to log such
> kind of metrics, TableInputFormat can pass in mapreduce task ID as
> application scan ID to HBase client as small addition to existing scan API;
> and HBase client can log metrics accordingly with such ID. That will allow
> query, analysis later on the metrics data for specific map reduce job.
> 3. Expose via MapReduce counter. It lacks certain features, for example,
> there is no good way to access the metrics on per map instance; the MapReduce
> framework only performs sum on the counter values so it is tricky to find the
> max of certain metrics in all mapper instances. However, it might be good
> enough for now. With this approach, the metrics value will be available via
> MapReduce counter.
> a) Have ResultScanner return a new ResultScannerMetrics interface.
> b) TableInputFormat will access data from ResultScannerMetrics and populate
> MapReduce counters accordingly.
--
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
