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

stack commented on HBASE-12597:
-------------------------------

bq.  Is it ok to use "hbase.ipc.client.impl" as the setting name? And to embed 
its implementation in a RpcClientFactory class?

Yes.  Will you have a Configuration in your RpcClientFactory class context?  If 
so, read the class name from here?

bq. I would like to call it RpcClientAdapter and let RpcClientImpl implement 
it. Is this ok? 

Sounds good.  Is it an adapter or rpc configuration/settings?  What is it 
adapting?  We can always revisit naming.

On the patch, this will become a factory?

      this.rpcClient = new RpcClientImpl(this.conf, this.clusterId);

Looks like tests will want to have a different rpcclientimpl

    @VisibleForTesting RpcClient setRpcClient(final RpcClient rpcClient) {

We can then get rid of the above method and go factory route instead.  That'd 
be good cleanup

nit: Change this so its not inner exception?

import org.apache.hadoop.hbase.ipc.RpcClientImpl.FailedServerException;

I presume no changes in RpcClientImpl other than rename from RpcClient?

Would be good to move this stuff up out of RpcClient? 

          if (unwrappedDataLength == RpcClientImpl.PING_CALL_ID) {

and this stuff?

RpcClientImpl.FAILED_SERVER_EXPIRY_DEFAULT

Patch is great.

Move on to your factory basis and then lets commit that much?


> Add RpcClient interface and enable changing of RpcClient implementation
> -----------------------------------------------------------------------
>
>                 Key: HBASE-12597
>                 URL: https://issues.apache.org/jira/browse/HBASE-12597
>             Project: HBase
>          Issue Type: Improvement
>          Components: Client
>            Reporter: Jurriaan Mous
>         Attachments: HBASE-12597-v1.patch, HBASE-12597.patch
>
>
> Currently HConnectionImplementation works with the included RpcClient which 
> is a direct implementation and not defined by an interface.
> It would be great to be able to swap out the default RpcClient with another 
> implementation which can also be controlled by the default 
> HConnectionImplementation. 
> Suggested changes:
> - Create a RpcClient interface which defines all the ways 
> HConnectionImplementation interacts with an RPC client. Like getting a 
> blocking protobuf service interface or closing the client.
> - Define which RpcClient implementation to construct by setting a 
> configuration variable which defaults to the current RpcClient.
> - Possibly create an abstract RpcClient class to only load all the basic Rpc 
> layer configurations to be used in an implementation.
> Why? It enables experimentation with RpcClients which could enable new 
> features or could be more performant than the included client. 
> I created a new RpcClient implementation based on Netty which can also be 
> called asynchronously. It would be great to also be able to use this 
> RpcClient in all the default ways and tests to see if there are any issues 
> with it. 
> https://github.com/jurmous/async-hbase-client/
> https://github.com/jurmous/async-hbase-client/blob/master/src/main/java/org/apache/hadoop/hbase/ipc/AsyncRpcClient.java



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

Reply via email to