hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jurriaan Mous (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-12684) Add new AsyncRpcClient
Date Fri, 19 Dec 2014 09:21:14 GMT

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

Jurriaan Mous commented on HBASE-12684:

Why out of interest the below change?
830	return (value != null)? Integer.valueOf(value).intValue(): DEFAULT_TTL;	830	return (value
!= null) ? Integer.parseInt(value) : DEFAULT_TTL;

I was fixing findbugs issues and this performance issue was too easy to ignore. So I thought
why not change it while I was at it. Is it ok or should I revert it?
Findbug recommendation:
Boxing/unboxing to parse a primitive org.apache.hadoop.hbase.HColumnDescriptor.getMinVersions()
Bug type DM_BOXED_PRIMITIVE_FOR_PARSING (click for details) 
In class org.apache.hadoop.hbase.HColumnDescriptor
In method org.apache.hadoop.hbase.HColumnDescriptor.getMinVersions()
Called method Integer.intValue()
Should call Integer.parseInt(String) instead
At HColumnDescriptor.java:[line 846]

The below is not a repeat because we are replacing old rpc with this new one?
81	private static final byte[] MAGIC = new byte[]{ 'H', 'B', 'a', 's' }

That was my original thought. But I am now moving it to RpcClient interface as a constant
since it is so basic to the HBase RPC protocol.

bq. This looks great: 201	f.channel().pipeline().addFirst(saslHandler);

Yes I think that shows the nice pipeline architecture of Netty in 1 line of code :)

bq. Patch looks great. On the tests that are zombies, they are probably missing (timeout=XX)
annotations after @Test. Feel free to add if it will help you debug.

Will look into it. Those zombie tests were already indicating issues with the client. Could
add that parameter while I work on the remaining ones.

bq. When you get a chance, make a list of what you will be able to remove after this is all
working. I love removing code.

Will do! It will be mostly RpcClientImpl, Call and anything related with it. Some methods
in HBaseSaslRpcClient can be removed. More cleanup is possible when AsyncProcess is swapped
out for true async calls. 

Thanks for the quick review!

> Add new AsyncRpcClient
> ----------------------
>                 Key: HBASE-12684
>                 URL: https://issues.apache.org/jira/browse/HBASE-12684
>             Project: HBase
>          Issue Type: Improvement
>          Components: Client
>            Reporter: Jurriaan Mous
>            Assignee: Jurriaan Mous
>         Attachments: HBASE-12684-v1.patch, HBASE-12684-v2.patch, HBASE-12684-v3.patch,
HBASE-12684-v4.patch, HBASE-12684-v5.patch, HBASE-12684.patch
> With the changes in HBASE-12597 it is possible to add new RpcClients. This issue is about
adding a new Async RpcClient which would enable HBase to do non blocking protobuf service
> Besides delivering a new AsyncRpcClient I would also like to ask the question what it
would take to replace the current RpcClient? This would enable to simplify async code in some
next issues.

This message was sent by Atlassian JIRA

View raw message