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

Jonathan Gray commented on HBASE-1655:
--------------------------------------

Patch looks pretty good.  Small nitpicky issues with some needless reorderings 
of stuff and also some tabbing oddities.  Can be fixed on commit.

I remember why we're using TreeMap now instead of HashMap.  HashMap with byte[] 
as a key does not work with default comparator, and you cannot pass one in.  
There is no real downside to using TreeMap unless you have hundreds or 
thousands of tables, and even then logarithmic on the client-side on the order 
of 100-1000 is negligible.  More efficient in memory but a little dirtier on 
the GC... however this is almost exclusively client-side (and there's ever only 
one) so really makes no difference IMO.  So, back to TreeMap.

The API seems to have grown quite a bit.  Do we need all these permutations of 
getTable() ?  Could we drop the String taking ones besides the simplest one?  
(This is why we don't support String in most of the HBase API now, leads to 
very long and ugly APIs)

The default getTable(byte [] tableName) also requires instantiating a new 
HBaseConfiguration() each time internally, even if we are reusing an existing 
HTable... Whether there is a significant overhead or not to that, we should 
avoid it when unnecessary.

Typical usage is probably to not supply your own HBC, so if someone wants that 
level of control, give them the ability to build the Pool themselves rather 
than expose the friendlier, higher-level methods with all the different ways to 
instantiate the pool.

Blah, this is better said in a new patch....

> Usability improvements to HTablePool
> ------------------------------------
>
>                 Key: HBASE-1655
>                 URL: https://issues.apache.org/jira/browse/HBASE-1655
>             Project: Hadoop HBase
>          Issue Type: Improvement
>          Components: client
>            Reporter: Ken Weiner
>         Attachments: HBASE-1655.patch
>
>
> A discussion on the HBase user mailing list 
> (http://markmail.org/thread/7leeha56ny5mwecg) led to some suggested 
> improvements for the org.apache.hadoop.hbase.client.HTablePool class.
> I will be submitting a patch that contains the following changes to 
> HTablePool:
> * Remove constructors that were not used.
> * Change access to remaining contstructor from public to private to enforce 
> use of the static factory method getPool.
> * Change internal map from TreeMap to HashMap because I couldn't see any 
> reason it needed to be sorted.
> * Remove HBaseConfiguration and tableName member variables since they aren't 
> really properties of the pool itself. They are associated with the HTable 
> that should get instantiated when one is requested from the pool, but not 
> already there.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to