[
https://issues.apache.org/jira/browse/HBASE-13977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14613118#comment-14613118
]
Anoop Sam John commented on HBASE-13977:
----------------------------------------
Most of the comments are not direct on your changes Ram.. But those changes we
can do now which will help our read performance. So pls pardon me for making
you to do it in this patch. :-)
bq.return CellUtil.toString(getLastKey(), true)
nit : Can pass false no?
See in HFileReaderImpl
We have getLastKey() returning Cell representation of the last key. The the API
which returns the current key is named as getKeyAsCell. We can uniform them?
Make both as 'AsCell' or just leave the old name as is. Am fine with both kind
of naming.
bq.KeyValue kv = KeyValueUtil.ensureKeyValue(scanner.getCell());
Avoid call to ensureKeyValue() now?
{quote}
firstOnRow = new KeyValue(lastKV.getRowArray(), lastKV.getRowOffset(),
lastKV.getRowLength(),
1851 HConstants.LATEST_TIMESTAMP);
firstOnRow = new KeyValue(kv.getRowArray(), kv.getRowOffset(),
kv.getRowLength(),
1867 HConstants.LATEST_TIMESTAMP);
{quote}
Can use CellUtil#createFirstOnRow now?
{quote}
// TODO this is in hot path? Optimize and avoid 2 extra object creations.
Cell firstKeyKV = this.getFirstKey();
Cell lastKeyKV = this.getLastKey();
{quote}
I think we are good now and that TODO can go away. This is great!
{quote}
KeyValue firstKeyOfPreviousRow = KeyValueUtil.createFirstOnRow(hfs.getCell()
430 .getRowArray(), hfs.getCell().getRowOffset(),
hfs.getCell().getRowLength());
{quote}
Not your issue Ram. But we have hope for improvement here. Every call to
hfs.getCell() creates an object. This should be called once and use that Cell's
details to create first row. Infact for that creation of the 1st cell in row,
we can use CellUtil#createFirstOnRow API.
bq.byte[] boundary = existingWriters.isEmpty() ? left :
CellUtil.cloneRow(cell); // make a copy
This is good. You are avoiding reference to deprecated methods. I see it many
places in this patch.
bq. ((KeyValue.KeyOnlyKeyValue)reader.getLastKey()).getKey())
Any such unconditional type casting looks suspicious.. But ya we discussed abt
this already in another Jira. As of now ok.. Let us revisit this area later.
> Convert getKey and related APIs to Cell
> ---------------------------------------
>
> Key: HBASE-13977
> URL: https://issues.apache.org/jira/browse/HBASE-13977
> Project: HBase
> Issue Type: Sub-task
> Reporter: ramkrishna.s.vasudevan
> Assignee: ramkrishna.s.vasudevan
> Attachments: HBASE-13977.patch, HBASE-13977_1.patch,
> HBASE-13977_2.patch, HBASE-13977_3.patch, HBASE-13977_4.patch,
> HBASE-13977_4.patch
>
>
> During the course of changes for HBASE-11425 felt that more APIs can be
> converted to return Cell instead of BB like getKey, getLastKey.
> We can also rename the getKeyValue to getCell.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)