hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars George (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-8782) Thrift2 can not parse values when using framed transport
Date Sun, 23 Jun 2013 10:41:20 GMT

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

Lars George commented on HBASE-8782:
------------------------------------

Hi [~madani], I looked at the patch and have a few questions:

- Performance

My biggest concern is that you are invoking a full copy by the looks using Bytes.getBytes(),
which in turn calls readBytes() doing the copying. It is part of the critical path and will
mean a lot of churn since it is called for every operation. If it is really needed, at the
least we need to add a local (size bound) cache that keeps the most common ones for lookup
instead of doing a copy-read on every operation. You can do that by adding a new byteBufferToBytes()
for example, that check the cache first, and only on misses does the getBytes() call and then
stores the result. 

- Fix Issue

Having said the above, I am not sure why the patch actually is fixing your problem. Looking
at the original ByteBuffer.array(), which returns the internal array, or using Bytes.readBytes(),
which internally iterates over ByteBuffer.get() calls, returning the same array byte by byte.

Why do you see a difference there? How did you come to the conclusion that this was caused
by the framed transport option of Thrift?

More importantly, can we create a test for this? We have some skilled Thrift'lers here in
HBase land, so we could ask for help on staging that from the Thrift side?

- Nits

Please check the imports while you are at it, there seems to be unused imports (Delete here).
Please kindly clean those little things up too.


Overall, I am concerned about the patch as it is. It will not change the outcome of the test
suite, but seems to cause a performance regressions.
                
> Thrift2 can not parse values when using framed transport
> --------------------------------------------------------
>
>                 Key: HBASE-8782
>                 URL: https://issues.apache.org/jira/browse/HBASE-8782
>             Project: HBase
>          Issue Type: Bug
>          Components: Thrift
>    Affects Versions: 0.95.1
>            Reporter: Hamed Madani
>         Attachments: HBASE_8782.patch
>
>
> ThriftHBaseServiceHandler.java use .array() on table names , and values (family , qualifier
in checkandDelete , etc) which resulted in incorrect values with framed transport. Replacing
.array() with getBytes() fixed this problem. I've attached the patch
> EDIT: updated the patch to cover checkAndPut(), checkAndDelete()

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message