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

Yu Li commented on HBASE-15180:
-------------------------------

{{TestTags#testFlushAndCompactionWithoutTags}} will fail with the latest patch.

After some investigation, it's caused by the bad design of the test case, that 
it assumes the KeyValue offset in the backup array is always 0, while after the 
below change in {{KeyValueCodec}} in this patch, the assumption no longer 
stands, thus causing the assertion to fail.
{code}
+        Cell c = createCell(buf.array(), buf.arrayOffset() + buf.position(), 
len);
{code}

The fix should be straight forward, that to change:
{code}
assertTrue(current.getValueOffset() + current.getValueLength() == 
current.getLength());
{code}
into:
{code}
assertTrue(current.getValueOffset() + current.getValueLength() == 
current.getOffset()
              + current.getLength());
{code}
in both line 222 and 240 of {{TestTags}}. [~anoop.hbase] mind take a look here?

What's more, the UT case TestTag is in the hbase-server module which depends on 
hbase-common, but all changes in the latest patch lie in hbase-common, and it 
seems Yetus only checked UT of the hbase-common module and neglected the 
failure. Is this something new to fix or a known issue [~busbey]? Thanks.

> Reduce garbage created while reading Cells from Codec Decoder
> -------------------------------------------------------------
>
>                 Key: HBASE-15180
>                 URL: https://issues.apache.org/jira/browse/HBASE-15180
>             Project: HBase
>          Issue Type: Sub-task
>          Components: regionserver
>    Affects Versions: 0.98.0
>            Reporter: Anoop Sam John
>            Assignee: Anoop Sam John
>             Fix For: 2.0.0, 1.3.0, 1.2.1
>
>         Attachments: HBASE-15180.patch, HBASE-15180_V2.patch, 
> HBASE-15180_V4.patch
>
>
> In KeyValueDecoder#parseCell (Default Codec decoder) we use 
> KeyValueUtil#iscreate to read cells from the InputStream. Here we 1st create 
> a byte[] of length 4 and read the cell length and then an array of Cell's 
> length and read in cell bytes into it and create a KV.
> Actually in server we read the reqs into a byte[] and CellScanner is created 
> on top of a ByteArrayInputStream on top of this. By default in write path, we 
> have MSLAB usage ON. So while adding Cells to memstore, we will copy the Cell 
> bytes to MSLAB memory chunks (default 2 MB size) and recreate Cells over that 
> bytes.  So there is no issue if we create Cells over the RPC read byte[] 
> directly here in Decoder.  No need for 2 byte[] creation and copy for every 
> Cell in request.
> My plan is to make a Cell aware ByteArrayInputStream which can read Cells 
> directly from it.  
> Same Codec path is used in client side also. There better we can avoid this 
> direct Cell create and continue to do the copy to smaller byte[]s path.  Plan 
> to introduce some thing like a CodecContext associated with every Codec 
> instance which can say the server/client context.



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

Reply via email to