hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anoop Sam John (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-17012) Handle Offheap cells in CompressedKvEncoder
Date Thu, 24 Nov 2016 15:41:58 GMT

    [ https://issues.apache.org/jira/browse/HBASE-17012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15693567#comment-15693567
] 

Anoop Sam John commented on HBASE-17012:
----------------------------------------

Did not read the patch fully..  First level comment
{code}
// TODO : Should this be moved to CellUtil? seems very specific to SecureWAL.
212	      if (cell instanceof ByteBufferedCell) {
213	        bbwos.write(((ByteBufferedCell) cell).getRowByteBuffer(),
214	          ((ByteBufferedCell) cell).getRowPosition(), cell.getRowLength());
215	      } else {
216	        bbwos.write(cell.getRowArray(), cell.getRowOffset(), cell.getRowLength());
217	      }
 StreamUtils.writeRawVInt32(bbwos, cell.getFamilyLength());
219	      if (cell instanceof ByteBufferedCell) {
220	        bbwos.write(((ByteBufferedCell) cell).getFamilyByteBuffer(),
221	          ((ByteBufferedCell) cell).getFamilyPosition(), cell.getFamilyLength());
222	      } else {
223	        bbwos.write(cell.getFamilyArray(), cell.getFamilyOffset(), cell.getFamilyLength());
224	      }
..
{code}
We can avoid this..  CellUtil already having method like writeRow(DataOutputStream out, Cell
cell, short rlength), writeFamily(DataOutputStream out, Cell cell, byte flength)..   You can
make use of them here.  If u see we can change DataOutputStream signature be OutputStream..
 It should be very much fine. I believe we added it in 2.0 and there is no BC break also.

compressCellForWal  -> Do we need to move this entire logic to here?  Let the logic be
there in the original place.. You can add way to do this in CellUtil
write(out, ((ByteBufferedCell) cell).getRowByteBuffer(),
1957	        ((ByteBufferedCell) cell).getRowPosition(), cell.getRowLength(), rowDict);

Add API like writeRow(OS,Cell)  .. And the actual logic of check the Dict and add entry to
Dict can be moved to some CompressionContext class.. We have TagCompressionContext..  May
be this should be renamed and made generic to be CellComressionContext?   Let CellUtil know
abt this context only and do the Dict specific work talking with this context.  But the CellUtil
impl can check whether Cell is BBCell or not and can decide to pass BB or byte[]


> Handle Offheap cells in CompressedKvEncoder
> -------------------------------------------
>
>                 Key: HBASE-17012
>                 URL: https://issues.apache.org/jira/browse/HBASE-17012
>             Project: HBase
>          Issue Type: Sub-task
>          Components: regionserver
>    Affects Versions: 2.0.0
>            Reporter: Anoop Sam John
>            Assignee: ramkrishna.s.vasudevan
>             Fix For: 2.0.0
>
>         Attachments: HBASE-17012_1.patch, HBASE-17012_2.patch
>
>
> When we deal with off heap cells we will end up copying Cell components on heap
> {code}
> public void write(Cell cell) throws IOException {
> .................
>       write(cell.getRowArray(), cell.getRowOffset(), cell.getRowLength(), compression.rowDict);
>       write(cell.getFamilyArray(), cell.getFamilyOffset(), cell.getFamilyLength(),
>           compression.familyDict);
>       write(cell.getQualifierArray(), cell.getQualifierOffset(), cell.getQualifierLength(),
>           compression.qualifierDict);
> ......
>       out.write(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength());
> ...
> {code}
> We need to avoid this.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message