[
https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Hiroshi Ikeda updated HBASE-6651:
---------------------------------
Attachment: HBASE-6651-V2.patch
Thanks for the reviews. Now I added a revised patch.
(1) In order to pass the tests for HTablePool, I changed both of HTablePool and
its tests, but I think it keeps backward compatibility from the point of view
of users.
I changed the behavior of HTablePool.closeTablePool(). The previous behavior is
vague and differs in its implementations, and I think that is far from
“shutdown” described in the javadoc. I changed the behavior to not only closing
idle objects at the moment, but making kind of reservation to close the rest of
objects when they return into the pool.
Also I removed the package private method HTablePool.getCurrentPoolSize(). Its
behavior differed in its implementations and exposed implementation details,
and it was only used in tests and hidden from users because of package private.
Instead, I created a class implementing HTableInterfaceFactory to count pooled
objects, and I added the class into tests and fixed the tests.
Fortunately HTableProol doesn’t expose details of the pooling, and the new
behavior is more eager to close objects than the previous behaviors, then I
think the new behavior has enough backward compatibility.
I think the tests will pass, but still I have no environment to run the tests.
(2) I created the previous patch with git with the option
--ignore-space-at-eol, and it seems to cause failure in applying the patch. I
created this patch without that option.
(3) Fixed line delimiters.
(4) Fixed java doc comments, and fixed to entirely use equals methods for
pooled objects.
Usually, objects we want to pool are heavy to create and are not replaceable.
And the operator == is preferable to equals() in order to identify them. But
there are few collections based on the sameness rather than equality, and
sticking to the sameness will require extra efforts to create collections based
on the sameness. Unless we must prepare malicious overriding equals/hashCode,
the extra effort is fruitless, and I wanted to make do with the notice in the
javadoc.
(5) Used @InterfaceStability.Evolving
(6) Once you get the pooled object by SharedMap.borrowObject() or register the
object by SharedMap.registerObject(), you have to declare the end of using the
borrowed object by calling SharedMap.returnObject() or
SharedMap.invalidateObject(). On the other hand, you can call
SharedMap.invalidateObject() and SharedMap.clear() at any place on any thread.
SharedMap.returnObject() returns false if the pool is already full, or
invalidateObject() or clear() method has been called explicitly in other place.
(7) Well, I need more time to study how to use https://reviews.apache.org
> Thread safety of HTablePool is doubtful
> ---------------------------------------
>
> Key: HBASE-6651
> URL: https://issues.apache.org/jira/browse/HBASE-6651
> Project: HBase
> Issue Type: Bug
> Components: Client
> Affects Versions: 0.94.1
> Reporter: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, sample.zip,
> sample.zip, sharedmap_for_hbaseclient.zip
>
>
> There are some operations in HTablePool to access to PoolMap in multiple
> times without any explict synchronization.
> For example HTablePool.closeTablePool() calles PoolMap.values(), and calles
> PoolMap.remove(). If other threads add new instances to the pool in the
> middle of the calls, the new added instances might be dropped.
> (HTablePool.closeTablePool() also has another problem that calling it by
> multple threads causes accessing HTable by multiple threads.)
> Moreover, PoolMap is not thread safe for the same reason.
> For example PoolMap.put() calles ConcurrentMap.get() and calles
> ConcurrentMap.put(). If other threads add a new instance to the concurent map
> in the middle of the calls, the new instance might be dropped.
> And also implementations of Pool have the same problems.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
