hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ken Weiner (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HBASE-1655) Usability improvements to HTablePool
Date Wed, 15 Jul 2009 17:30:15 GMT

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

Ken Weiner commented on HBASE-1655:

- Even though TreeMap uses the comparator rather than the equals method to compare keys, using
a byte[] as the key seems to break the contract of a java.util.Map.  The last paragraph of
the [Map API|http://java.sun.com/javase/6/docs/api/java/util/Map.html] says that impls are
free to avoid calling equals() but the containsKey(Object key) method should "return true
if and only if this map contains a mapping for a key k such that (key==null ? k==null : key.equals(k))".
 As you mentioned, you'd need a byte[] wrapper in order to be compliant.  So a byte[] key
will work in a TreeMap (and not in a HashMap), but we'd break the Map contract.  Seems better
to just use String and HashMap which works well and satisfies the Map contract.
- Allowing both the static methods and the public construction would bring me back to the
complaint I had originally with this class. :)  It becomes confusing for a user trying to
quickly understand how he/she is supposed to interact with this class.  I guess this can all
be explained away with documentation, but, it doesn't feel "good".
- Also, yes, there would be a problem with re-instantiating the static map multiple times,
but this could be prevented by implementing a Singleton pattern so that the static access
operates on an internal singleton instance with a member Map rather than a static Map.
- I agree that there is no reason to construct HBC's for each invocation. An internal HBC
instance can be constructed and reused.
- Is there a checkstyle.xml file for HBase?  That would make it easy to check my code for
formatting problems before submitting patches.

> 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
>            Assignee: Ken Weiner
>         Attachments: HBASE-1655-v2-partial.patch, 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.

View raw message