[
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)
