[HBASE-7460] Cleanup client connection layers - ASF JIRA

Voters

Watch issue

Watchers

Create sub-task

Link

Clone

Update Comment Author

Replace String in Comment

Update Comment Visibility

Delete Comments

XML

Word

Printable

JSON

Details

Type: Improvement
Status: Closed
Priority: Major
Resolution: Fixed
Affects Version/s: None
Fix Version/s: 0.95.0
Component/s: Client, IPC/RPC
Labels:
None

Hadoop Flags:

Incompatible change
Release Note:

Hide
Overriding of the client RPC engine (hbase.rpc.client.engine configuration property) is no longer supported. Instead the built-in RpcClientEngine is always used.

In some cases, where clients explicitly manage HConnection instance creation, the number of client RPC connections created may change. This issue changes the HConnection implementation to use it's own managed HBaseClient instance, with its own set of client RPC connections. So explicitly creating multiple HConnection instances (using HConnectionManager.createConnection()) will result in multiple HBaseClient instances. However, for the default behavior, using HTable with a single Configuration, there is no change.

Show
Overriding of the client RPC engine (hbase.rpc.client.engine configuration property) is no longer supported. Instead the built-in RpcClientEngine is always used. In some cases, where clients explicitly manage HConnection instance creation, the number of client RPC connections created may change. This issue changes the HConnection implementation to use it's own managed HBaseClient instance, with its own set of client RPC connections. So explicitly creating multiple HConnection instances (using HConnectionManager.createConnection()) will result in multiple HBaseClient instances. However, for the default behavior, using HTable with a single Configuration, there is no change.

Description

This issue originated from a discussion over in ~~HBASE-7442~~. We currently have a broken abstraction with HBaseClient, where it is bound to a single Configuration instance at time of construction, but then reused for all connections to all clusters. This is combined with multiple, overlapping layers of connection caching.

Going through this code, it seems like we have a lot of mismatch between the higher layers and the lower layers, with too much abstraction in between. At the lower layers, most of the ClientCache stuff seems completely unused. We currently effectively have an HBaseClient singleton (for SecureClient as well in 0.92/0.94) in the client code, as I don't see anything that calls the constructor or RpcEngine.getProxy() versions with a non-default socket factory. So a lot of the code around this seems like built up waste.

The fact that a single Configuration is fixed in the HBaseClient seems like a broken abstraction as it currently stands. In addition to cluster ID, other configuration parameters (max retries, retry sleep) are fixed at time of construction. The more I look at the code, the more it looks like the ClientCache and sharing the HBaseClient instance is an unnecessary complication. Why cache the HBaseClient instances at all? In HConnectionManager, we already have a mapping from Configuration to HConnection. It seems to me like each HConnection(Implementation) instance should have it's own HBaseClient instance, doing away with the ClientCache mapping. This would keep each HBaseClient associated with a single cluster/configuration and fix the current breakage from reusing the same HBaseClient against different clusters.

We need a refactoring of some of the interactions of HConnection(Implementation), HBaseRPC/RpcEngine, and HBaseClient. Off hand, we might want to expose a separate RpcEngine.getClient() method that returns a new RpcClient interface (implemented by HBaseClient) and move the RpcEngine.getProxy()/stopProxy() implementations into the client. So all proxy invocations can go through the same client, without requiring the static client cache. I haven't fully thought this through, so I could be missing other important aspects. But that approach at least seems like a step in the right direction for fixing the client abstractions.