You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Hiroshi Ikeda (JIRA)" <ji...@apache.org> on 2012/08/24 05:14:41 UTC

[jira] [Created] (HBASE-6651) Thread safety of HTablePool is doubtful

Hiroshi Ikeda created HBASE-6651:
------------------------------------

             Summary: 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


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: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13483923#comment-13483923 ] 

Hadoop QA commented on HBASE-6651:
----------------------------------

{color:red}-1 overall{color}.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12550744/HBASE-6651-V4.patch
  against trunk revision .

    {color:green}+1 @author{color}.  The patch does not contain any @author tags.

    {color:green}+1 tests included{color}.  The patch appears to include 11 new or modified tests.

    {color:green}+1 hadoop2.0{color}.  The patch compiles against the hadoop 2.0 profile.

    {color:red}-1 javadoc{color}.  The javadoc tool appears to have generated 92 warning messages.

    {color:green}+1 javac{color}.  The applied patch does not increase the total number of javac compiler warnings.

    {color:green}+1 findbugs{color}.  The patch does not introduce any new Findbugs (version 1.3.9) warnings.

    {color:green}+1 release audit{color}.  The applied patch does not increase the total number of release audit warnings.

     {color:red}-1 core tests{color}.  The patch failed these unit tests:
                       org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3145//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3145//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3145//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3145//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3145//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3145//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3145//console

This message is automatically generated.
                
> 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, HBASE-6651-V3.patch, HBASE-6651-V4.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13478592#comment-13478592 ] 

Hiroshi Ikeda commented on HBASE-6651:
--------------------------------------

bq. Can you explain why calling clear() followed by returnObject() would result in return value of false ? There is space in SharedMap at this moment, right ?

SharedMap holds a collection of registered objects independent of whether the objects are pooled or borrowed, and SharedMap.invalidateObject() and SharedMap.clear() remove the specified object(s) from the collection. SharedMap.returnObject() always does nothing and returns false if the given object is not found in the collection. Filling a space of the registered objects is done by SharedMap.registerObject(), not SharedMap.returnObject(). 

                
> 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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13474366#comment-13474366 ] 

Ted Yu commented on HBASE-6651:
-------------------------------

Please use dos2unix or similar tool to remove trailing ^M's from the patch.
{code}
+ * the type {@code V} is assumed that different instances of it are not equal.^M
+ */^M
+@InterfaceAudience.Public^M
+@InterfaceStability.Unstable^M
+public class ReusableSharedMap<K, V> extends AbstractSharedMap<K, V> {^M
{code}
Did you mean 'the type {@code V} assumes that ...' above ?
Suggest tagging the class with @InterfaceStability.Evolving instead of Unstable
{code}
+   * Registers a new instance to this instance.^M
{code}
Suggest changing the above to 'a new Key/Value pair to this instance'.
{code}
+   * If this method returns false, the the registration of the given object expires.^M
+   * You might have to tear down the rejected object.^M
+   *^M
+   * @throws NullPointerException if {@code key} or {@code value} is null^M
+   */^M
+  boolean returnObject(K key, V value);^M
{code}
Can you tell us under what scenario returnObject() would cause given object to expire ?
I see the following check recurring in the patch:
{code}
+    if (counter == null || counter.value != value) {^M
{code}
Should value.equals() be used instead ?

Using https://reviews.apache.org would facilitate code review. You should select hbase-git repository.
                
> 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, 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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13478198#comment-13478198 ] 

Hadoop QA commented on HBASE-6651:
----------------------------------

{color:red}-1 overall{color}.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12549457/HBASE-6651-V2.patch
  against trunk revision .

    {color:green}+1 @author{color}.  The patch does not contain any @author tags.

    {color:green}+1 tests included{color}.  The patch appears to include 11 new or modified tests.

    {color:green}+1 hadoop2.0{color}.  The patch compiles against the hadoop 2.0 profile.

    {color:red}-1 javadoc{color}.  The javadoc tool appears to have generated 83 warning messages.

    {color:green}+1 javac{color}.  The applied patch does not increase the total number of javac compiler warnings.

    {color:red}-1 findbugs{color}.  The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.

    {color:green}+1 release audit{color}.  The applied patch does not increase the total number of release audit warnings.

    {color:green}+1 core tests{color}.  The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3065//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3065//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3065//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3065//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3065//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3065//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3065//console

This message is automatically generated.
                
> 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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13482472#comment-13482472 ] 

Hadoop QA commented on HBASE-6651:
----------------------------------

{color:red}-1 overall{color}.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12550486/HBASE-6651-V3.patch
  against trunk revision .

    {color:green}+1 @author{color}.  The patch does not contain any @author tags.

    {color:green}+1 tests included{color}.  The patch appears to include 11 new or modified tests.

    {color:green}+1 hadoop2.0{color}.  The patch compiles against the hadoop 2.0 profile.

    {color:red}-1 javadoc{color}.  The javadoc tool appears to have generated 88 warning messages.

    {color:green}+1 javac{color}.  The applied patch does not increase the total number of javac compiler warnings.

    {color:green}+1 findbugs{color}.  The patch does not introduce any new Findbugs (version 1.3.9) warnings.

    {color:green}+1 release audit{color}.  The applied patch does not increase the total number of release audit warnings.

    {color:green}+1 core tests{color}.  The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3126//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3126//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3126//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3126//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3126//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3126//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3126//console

This message is automatically generated.
                
> 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, HBASE-6651-V3.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13484735#comment-13484735 ] 

Hadoop QA commented on HBASE-6651:
----------------------------------

{color:red}-1 overall{color}.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12550927/HBASE-6651-V5.patch
  against trunk revision .

    {color:green}+1 @author{color}.  The patch does not contain any @author tags.

    {color:green}+1 tests included{color}.  The patch appears to include 11 new or modified tests.

    {color:green}+1 hadoop2.0{color}.  The patch compiles against the hadoop 2.0 profile.

    {color:red}-1 javadoc{color}.  The javadoc tool appears to have generated 95 warning messages.

    {color:green}+1 javac{color}.  The applied patch does not increase the total number of javac compiler warnings.

    {color:red}-1 findbugs{color}.  The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

    {color:green}+1 release audit{color}.  The applied patch does not increase the total number of release audit warnings.

     {color:red}-1 core tests{color}.  The patch failed these unit tests:
                       org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3154//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3154//console

This message is automatically generated.
                
> 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, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "stack (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13448874#comment-13448874 ] 

stack commented on HBASE-6651:
------------------------------

@Hiroshi Thank you for digging in here.  ThreadLocalPool was added by HBASE-2938 a while back.  On #1, what do you see as implications?  If its a pool of threads and all are using threadlocal, what would they need to share info?  Can you say more on points #2 and #3 above?  What do you suggest we do?  Purge ThreadLocalPool?  Thanks.
                
> 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
>
> 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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hiroshi Ikeda updated HBASE-6651:
---------------------------------

    Attachment: sharedmap_for_hbaseclient.zip

Added sample implementation which might be used to pool and share Connection instances between threads in HBaseClient. I use the name SharedMap, the old name of PoolMap.

In HBaseClient I think PoolMap with ThreadLocalPool leaks objects. Connection (extending Thead) automatically tries to remove itself from the pool at the end of its life, but its thread is different from the thread which created the instance of Connection and put into the pool.
                
> 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: 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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13458584#comment-13458584 ] 

Hiroshi Ikeda commented on HBASE-6651:
--------------------------------------

On #2, RoundRobinPool simply gives the next pooled resource for the next caller, and after taking a round it gives the pooled resource again, but the resource might be yet used. RoundRobinPool has the following code:

{code}
class RoundRobinPool<R> extends CopyOnWriteArrayList<R> implements Pool<R> {
...
@Override
public R get() {
  if (size() < maxSize) {
    return null;
  }
  nextResource %= size();
  R resource = get(nextResource++);
  return resource;
}
{code}

On #3, in general concurrent collections have the advantage of peformance in multi-threads because they are not blocked and don't cause context switches, which have large overhead. But in other word you cannot use concurrent collections to block other threads in order to make critical sections where only one thread may run. There are some tips to create unblocking codes with using concurrent utilities, but in general it is difficult for complex logics.

On #1, I'm not sure what to say, but ThreadLocal might bind tasks to specific threads, which is hard to use and is appropriate only for some restricted cases.
                
> 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: sample.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Lars Hofhansl (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13484681#comment-13484681 ] 

Lars Hofhansl commented on HBASE-6651:
--------------------------------------

[~karthick] Do you want to have a look?
                
> 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, HBASE-6651-V3.patch, HBASE-6651-V4.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

[jira] [Updated] (HBASE-6651) Improve thread safety of HTablePool

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ 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-V7.patch

Patch v7 from review board.
                
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>            Assignee: Hiroshi Ikeda
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.patch, HBASE-6651-V7.patch, sample.zip, sample.zip, sharedmap_for_hbaseclient.zip
>
>
> There are some operations in HTablePool accessing PoolMap in multiple places without any explicit synchronization. 
> For example HTablePool.closeTablePool() calls PoolMap.values(), and calls PoolMap.remove(). If other threads add new instances to the pool in the middle of the calls, the newly added instances might be dropped. (HTablePool.closeTablePool() also has another problem that calling it by multiple 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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ 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-V4.patch

Patch v4 from review board.
                
> 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, HBASE-6651-V3.patch, HBASE-6651-V4.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13478071#comment-13478071 ] 

Ted Yu commented on HBASE-6651:
-------------------------------

bq. SharedMap.returnObject() returns false if the pool is already full, or invalidateObject() or clear() method has been called explicitly in other place. 
Can you explain why calling clear() followed by returnObject() would result in return value of false ? There is space in SharedMap at this moment, right ?

I will put my other comments on the reviewboard.

Hadoop QA is running your patch: https://builds.apache.org/job/PreCommit-HBASE-Build/3065/parameters/
                
> 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

[jira] [Commented] (HBASE-6651) Improve thread safety of HTablePool

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13501817#comment-13501817 ] 

Hadoop QA commented on HBASE-6651:
----------------------------------

{color:red}-1 overall{color}.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12554470/HBASE-6651-V7.patch
  against trunk revision .

    {color:green}+1 @author{color}.  The patch does not contain any @author tags.

    {color:green}+1 tests included{color}.  The patch appears to include 15 new or modified tests.

    {color:green}+1 hadoop2.0{color}.  The patch compiles against the hadoop 2.0 profile.

    {color:red}-1 javadoc{color}.  The javadoc tool appears to have generated 109 warning messages.

    {color:green}+1 javac{color}.  The applied patch does not increase the total number of javac compiler warnings.

    {color:red}-1 findbugs{color}.  The patch appears to introduce 22 new Findbugs (version 1.3.9) warnings.

    {color:green}+1 release audit{color}.  The applied patch does not increase the total number of release audit warnings.

     {color:red}-1 core tests{color}.  The patch failed these unit tests:
     

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3384//console

This message is automatically generated.
                
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>            Assignee: Hiroshi Ikeda
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.patch, HBASE-6651-V7.patch, sample.zip, sample.zip, sharedmap_for_hbaseclient.zip
>
>
> There are some operations in HTablePool accessing PoolMap in multiple places without any explicit synchronization. 
> For example HTablePool.closeTablePool() calls PoolMap.values(), and calls PoolMap.remove(). If other threads add new instances to the pool in the middle of the calls, the newly added instances might be dropped. (HTablePool.closeTablePool() also has another problem that calling it by multiple 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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "stack (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13460726#comment-13460726 ] 

stack commented on HBASE-6651:
------------------------------

Hiroshi Your work is more palatable as a patch rather than zip file.  This might be of use to you: http://hbase.apache.org/book.html#submitting.patches  Thanks for looking into this stuff.
                
> 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: 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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ 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-V5.patch

Patch v5 from review board.
                
> 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, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.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

[jira] [Updated] (HBASE-6651) Improve thread safety of HTablePool

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ted Yu updated HBASE-6651:
--------------------------

    Priority: Major  (was: Minor)
    
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>            Assignee: Hiroshi Ikeda
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ 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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hiroshi Ikeda updated HBASE-6651:
---------------------------------

    Attachment: sample.zip

Added sample implementation to pool HTables.
                
> 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: sample.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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ted Yu updated HBASE-6651:
--------------------------

    Attachment: HBASE-6651-V3.patch

Patch v3 from review board.
                
> 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, HBASE-6651-V3.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13460612#comment-13460612 ] 

Hiroshi Ikeda commented on HBASE-6651:
--------------------------------------

Sorry I didn't realize that PoolMap is also used in HBaseClient, and it is inteneded that the round robin logic gives the same thread-safe object to different threads and sticks to the limit count of the resources (aside from the other factors to break thread-safety of PoolMap). Apparently the requirements of pooling differ in HTablePool and HBaseClient.
                
> 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: sample.zip, sample.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

[jira] [Commented] (HBASE-6651) Improve thread safety of HTablePool

Posted by "ramkrishna.s.vasudevan (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13487089#comment-13487089 ] 

ramkrishna.s.vasudevan commented on HBASE-6651:
-----------------------------------------------

[~ikeda]
Thanks for the patch and it is very good and of high quality.
As Ted said, if some one else also reviews this patch it would be great and i should be with Hiroshi's nice comments and javadocs thro out the code it should be easier.
The HBaseClient.java also has been touched so its better some one who is familiar in that side reviews it and we can get this in.
@Ted
You ok to commit this?
                
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>            Assignee: Hiroshi Ikeda
>            Priority: Minor
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13474319#comment-13474319 ] 

Ted Yu commented on HBASE-6651:
-------------------------------

I got two test failures from TestHTablePool:
{code}
testCloseTablePool(org.apache.hadoop.hbase.client.TestHTablePool$TestHTableReusablePool)  Time elapsed: 3.189 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<4> but was:<0>
  at junit.framework.Assert.fail(Assert.java:50)
  at junit.framework.Assert.failNotEquals(Assert.java:287)
  at junit.framework.Assert.assertEquals(Assert.java:67)
  at junit.framework.Assert.assertEquals(Assert.java:199)
  at junit.framework.Assert.assertEquals(Assert.java:205)
  at org.apache.hadoop.hbase.client.TestHTablePool$TestHTableReusablePool.testCloseTablePool(TestHTablePool.java:252)
...
testCloseTablePool(org.apache.hadoop.hbase.client.TestHTablePool$TestHTableThreadLocalPool)  Time elapsed: 3.095 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<1> but was:<0>
  at junit.framework.Assert.fail(Assert.java:50)
  at junit.framework.Assert.failNotEquals(Assert.java:287)
  at junit.framework.Assert.assertEquals(Assert.java:67)
  at junit.framework.Assert.assertEquals(Assert.java:199)
  at junit.framework.Assert.assertEquals(Assert.java:205)
  at org.apache.hadoop.hbase.client.TestHTablePool$TestHTableThreadLocalPool.testCloseTablePool(TestHTablePool.java:328)
{code}
BTW, the following hunk for HBaseClient.java should be rebased:
{code}
***************
*** 402,412 ****
        }
        this.header = builder.build();

-       this.setName("IPC Client (" + socketFactory.hashCode() +") connection to " +
          remoteId.getAddress().toString() +
          ((ticket==null)?" from an unknown user": (" from "
-         + ticket.getUserName())));
-       this.setDaemon(true);
      }

      private UserInformation getUserInfoPB(UserGroupInformation ugi) {
--- 482,491 ----
        }
        this.header = builder.build();

+       this.name = "IPC Client (" + socketFactory.hashCode() +") connection to " +
          remoteId.getAddress().toString() +
          ((ticket==null)?" from an unknown user": (" from "
+         + ticket.getUserName()));
      }

      private UserInformation getUserInfoPB(UserGroupInformation ugi) {
{code}
More review comments to follow.
                
> 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, 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

[jira] [Commented] (HBASE-6651) Improve thread safety of HTablePool

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13487093#comment-13487093 ] 

Ted Yu commented on HBASE-6651:
-------------------------------

I sent our review request to dev@hbase
Will wait some time before integration.
                
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>            Assignee: Hiroshi Ikeda
>            Priority: Minor
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ 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.patch

Added a patch.

Added a new interface SharedMap, and added its implementations and unit tests, and made HBaseClient and HTablePool use SharedMap instead of PoolMap. The API of HTablePool is marked as “stable” and some constructors of HTablePool uses a static nested class PoolMap.PoolType, and I’m not sure how to add Deprecated marker partially to PoolMap, so I did nothing about it.

I don’t have environment to run the whole test of HBase, and except tests added for SharedMap I only confirm that Eclipse can compile the patched code (where the projects are imported as “existing maven projects” from the trunk). I hope it will work well.

I purged ThreadLocal base logic from HBaseClient (that logic is terrible and I never believe that works well). Instead, I complicate the Round-Robin based logic to prefer to give idle connections to requesters. I think, this is the same reason why the ThreadLocal based logic makes high performance. Moreover, in order to find idle objects in the pool, SharedMap has different methods from PoolMap, and I rewrote and fixed many parts of HBaseClient. On the other hand fixing HTablePool to SharedMap is straightforward and I think I have no additional explanations about it.


                
> 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, 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

[jira] [Commented] (HBASE-6651) Improve thread safety of HTablePool

Posted by "Jesse Yates (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498376#comment-13498376 ] 

Jesse Yates commented on HBASE-6651:
------------------------------------

[~ikeda] this is looking pretty good. I added some comments on reviewboard along with a couple questions as to the implementation. Sorry if I'm being dense - this is tricky code and I'm a little late to this party :)
                
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>            Assignee: Hiroshi Ikeda
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.patch, sample.zip, sample.zip, sharedmap_for_hbaseclient.zip
>
>
> There are some operations in HTablePool accessing PoolMap in multiple places without any explicit synchronization. 
> For example HTablePool.closeTablePool() calls PoolMap.values(), and calls PoolMap.remove(). If other threads add new instances to the pool in the middle of the calls, the newly added instances might be dropped. (HTablePool.closeTablePool() also has another problem that calling it by multiple 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

[jira] [Assigned] (HBASE-6651) Improve thread safety of HTablePool

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ted Yu reassigned HBASE-6651:
-----------------------------

    Assignee: Hiroshi Ikeda
    
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>            Assignee: Hiroshi Ikeda
>            Priority: Minor
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13484812#comment-13484812 ] 

Hadoop QA commented on HBASE-6651:
----------------------------------

{color:red}-1 overall{color}.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12550936/HBASE-6651-V6.patch
  against trunk revision .

    {color:green}+1 @author{color}.  The patch does not contain any @author tags.

    {color:green}+1 tests included{color}.  The patch appears to include 15 new or modified tests.

    {color:green}+1 hadoop2.0{color}.  The patch compiles against the hadoop 2.0 profile.

    {color:red}-1 javadoc{color}.  The javadoc tool appears to have generated 95 warning messages.

    {color:green}+1 javac{color}.  The applied patch does not increase the total number of javac compiler warnings.

    {color:red}-1 findbugs{color}.  The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

    {color:green}+1 release audit{color}.  The applied patch does not increase the total number of release audit warnings.

    {color:green}+1 core tests{color}.  The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3155//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3155//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3155//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3155//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3155//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3155//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3155//console

This message is automatically generated.
                
> 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, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13448770#comment-13448770 ] 

Hiroshi Ikeda commented on HBASE-6651:
--------------------------------------

* I think ThreadLocalPool is useless and dangerous. You never access a content in ThreadLocal from other threads, and if you require information in the content to dispose its container object or something, you must collect the information by using all the thread that you ever used to access.

* RoundRobinPool might give the same object to different threads.

* It is bad to use conccurent collections. We should explictly lock larger sections to keep consistency, or remove synchronization concerns from PoolMap with using explicit locks from outside of PoolMap.

* PoolMap breaks the contract of Map; The actual behaviors of the methods of PoolMap are vague. Also filling out the methods of Map causes the code dirty. We should simplify the code by removing the needless implementation at the start.

                
> 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
>
> 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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ 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-V6.patch

Patch v6 from review board.
Sorry frequent update.
                
> 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, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.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

[jira] [Updated] (HBASE-6651) Improve thread safety of HTablePool

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ted Yu updated HBASE-6651:
--------------------------

    Fix Version/s: 0.96.0
     Hadoop Flags: Reviewed
          Summary: Improve thread safety of HTablePool  (was: Thread safety of HTablePool is doubtful)
    
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hiroshi Ikeda updated HBASE-6651:
---------------------------------

    Attachment: sample.zip

Added revised sample implementation to pool HTables; Fixed wrong name of classes, fixed memory leak, and renamed internal fields.
                
> 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: sample.zip, sample.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "stack (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13458964#comment-13458964 ] 

stack commented on HBASE-6651:
------------------------------

What you suggest we do instead Hiroshi?
                
> 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: sample.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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13484577#comment-13484577 ] 

Ted Yu commented on HBASE-6651:
-------------------------------

It would be nice for another committer to check out this nice patch.
                
> 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, HBASE-6651-V3.patch, HBASE-6651-V4.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

[jira] [Updated] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ted Yu updated HBASE-6651:
--------------------------

    Status: Patch Available  (was: Open)
    
> 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

[jira] [Commented] (HBASE-6651) Thread safety of HTablePool is doubtful

Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13459257#comment-13459257 ] 

Hiroshi Ikeda commented on HBASE-6651:
--------------------------------------

@stack I can only propose some ideas because I don't have an environment to build and test HBase, and I hope you create true codes.
                
> 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: sample.zip, sample.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

[jira] [Updated] (HBASE-6651) Improve thread safety of HTablePool

Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HBASE-6651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ted Yu updated HBASE-6651:
--------------------------

    Description: 
There are some operations in HTablePool accessing PoolMap in multiple places without any explicit synchronization. 

For example HTablePool.closeTablePool() calls PoolMap.values(), and calls PoolMap.remove(). If other threads add new instances to the pool in the middle of the calls, the newly added instances might be dropped. (HTablePool.closeTablePool() also has another problem that calling it by multiple 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.


  was:
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.


    
> Improve thread safety of HTablePool
> -----------------------------------
>
>                 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
>            Assignee: Hiroshi Ikeda
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6651.patch, HBASE-6651-V2.patch, HBASE-6651-V3.patch, HBASE-6651-V4.patch, HBASE-6651-V5.patch, HBASE-6651-V6.patch, sample.zip, sample.zip, sharedmap_for_hbaseclient.zip
>
>
> There are some operations in HTablePool accessing PoolMap in multiple places without any explicit synchronization. 
> For example HTablePool.closeTablePool() calls PoolMap.values(), and calls PoolMap.remove(). If other threads add new instances to the pool in the middle of the calls, the newly added instances might be dropped. (HTablePool.closeTablePool() also has another problem that calling it by multiple 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