[
https://issues.apache.org/jira/browse/HBASE-21657?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16741712#comment-16741712
]
Zheng Hu edited comment on HBASE-21657 at 1/14/19 3:51 AM:
-----------------------------------------------------------
Thanks for your reply. [~stack].
bq. Do you need the change in NoTagsByteBufferKeyValue. It inherits from
ByteBufferKeyValue which has your change.
OK, seems no need to. will remove the getSerializedSize() in
NoTagsByteBufferKeyValue.
bq. Is it safe doing a return of this.length in SizeCachedKeyValue ? It caches
rowLen and keyLen... Does it cache this.length? Maybe its caching what is
passed in on construction?
Yeah, this.length was cached when constructing a KeyValue. the
getSerializedSize() means if have tags then return length with tags, if no tags
then return length without tags. So I think it's OK that just return the
this.size in SizeCachedKeyValue.
bq. That addition to addSize in RSRpcServices is crptic. We need that sir? Say
more why you are doing the accounting on the outside?
Well, I was thinking in the wrong way. No need this outside accounting any
more, because now the getSerializedSize() is very lightweight now.
bq. It should go back to branch-2.0?
Theoretically, I think it should. but it seems that there may be compatibility
issues? If users create their own Cell, the compile would not pass ? but if
not include in branch-2.0, seems the performance degradation isn't not
acceptable.
was (Author: openinx):
Thanks for your reply. [~stack].
bq. Do you need the change in NoTagsByteBufferKeyValue. It inherits from
ByteBufferKeyValue which has your change.
OK, seems no need to. will remove the getSerializedSize() in
NoTagsByteBufferKeyValue.
bq. Is it safe doing a return of this.length in SizeCachedKeyValue ? It caches
rowLen and keyLen... Does it cache this.length? Maybe its caching what is
passed in on construction?
Yeah, this.length was cached when constructing a KeyValue. the
getSerializedSize() means if have tags then return length with tags, if no tags
then return length without tags. So I think it's OK that just return the
this.size in SizeCachedKeyValue.
bq. That addition to addSize in RSRpcServices is crptic. We need that sir? Say
more why you are doing the accounting on the outside?
If read cell with tags, we will cost some cpu to get the serialiedSize(parse
the offset&len, especially ByteBufferKeyValue). Actually we can save it by just
get the delta between the scanner.nextRaw before and scanner.nextRaw after.
bq. It should go back to branch-2.0?
Theoretically, I think it should. but it seems that there may be compatibility
issues? If users create their own Cell, the compile would not pass ? but if
not include in branch-2.0, seems the performance degradation isn't not
acceptable.
> PrivateCellUtil#estimatedSerializedSizeOf has been the bottleneck in 100%
> scan case.
> ------------------------------------------------------------------------------------
>
> Key: HBASE-21657
> URL: https://issues.apache.org/jira/browse/HBASE-21657
> Project: HBase
> Issue Type: Bug
> Components: Performance
> Reporter: Zheng Hu
> Assignee: Zheng Hu
> Priority: Major
> Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5
>
> Attachments: HBASE-21657.v1.patch, HBASE-21657.v2.patch,
> HBASE-21657.v3.patch, HBASE-21657.v3.patch, HBASE-21657.v4.patch,
> HBASE-21657.v5.patch, HBASE-21657.v5.patch, HBASE-21657.v5.patch,
> HBASE-21657.v6.patch, HBase1.4.9-ssd-10000000-rows-flamegraph.svg,
> HBase1.4.9-ssd-10000000-rows-qps-latency.png,
> HBase2.0.4-patch-v2-ssd-10000000-rows-qps-and-latency.png,
> HBase2.0.4-patch-v2-ssd-10000000-rows.svg,
> HBase2.0.4-patch-v3-ssd-10000000-rows-flamegraph.svg,
> HBase2.0.4-patch-v3-ssd-10000000-rows-qps-and-latency.png,
> HBase2.0.4-patch-v4-ssd-10000000-rows-flamegraph.svg,
> HBase2.0.4-ssd-10000000-rows-flamegraph.svg,
> HBase2.0.4-ssd-10000000-rows-qps-latency.png, HBase2.0.4-with-patch.v2.png,
> HBase2.0.4-without-patch-v2.png, debug-the-ByteBufferKeyValue.diff,
> hbase2.0.4-ssd-scan-traces.2.svg, hbase2.0.4-ssd-scan-traces.svg,
> hbase20-ssd-100-scan-traces.svg, image-2019-01-07-19-03-37-930.png,
> image-2019-01-07-19-03-55-577.png, overview-statstics-1.png, run.log
>
>
> We are evaluating the performance of branch-2, and find that the throughput
> of scan in SSD cluster is almost the same as HDD cluster. so I made a
> FlameGraph on RS, and found that the
> PrivateCellUtil#estimatedSerializedSizeOf cost about 29% cpu, Obviously, it
> has been the bottleneck in 100% scan case.
> See the [^hbase20-ssd-100-scan-traces.svg]
> BTW, in our XiaoMi branch, we introduce a
> HRegion#updateReadRequestsByCapacityUnitPerSecond to sum up the size of cells
> (for metric monitor), so it seems the performance loss was amplified.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
