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/11/14 09:56:12 UTC
[jira] [Created] (HBASE-7160) Improve IdLock and remove its minor
defects
Hiroshi Ikeda created HBASE-7160:
------------------------------------
Summary: Improve IdLock and remove its minor defects
Key: HBASE-7160
URL: https://issues.apache.org/jira/browse/HBASE-7160
Project: HBase
Issue Type: Improvement
Reporter: Hiroshi Ikeda
Priority: Minor
Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should recreate the class and make it more understandable.
In the current code, I find the following minor defects:
(1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
(2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiroshi Ikeda updated HBASE-7160:
---------------------------------
Description:
Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
In the current code, I find the following minor defects:
(1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
(2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
was:
Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should recreate the class and make it more understandable.
In the current code, I find the following minor defects:
(1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
(2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiroshi Ikeda updated HBASE-7160:
---------------------------------
Attachment: HBASE-7160-V3.patch
Patch v3 from review board.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch, HBASE-7160-V2.patch, HBASE-7160-V3.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13499173#comment-13499173 ]
Ted Yu commented on HBASE-7160:
-------------------------------
The snippet I quoted was from the patch.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496970#comment-13496970 ]
Hiroshi Ikeda commented on HBASE-7160:
--------------------------------------
Created a review board.
https://reviews.apache.org/r/8052/
Thanks.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should recreate the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13503614#comment-13503614 ]
Hadoop QA commented on HBASE-7160:
----------------------------------
{color:red}-1 overall{color}. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12554815/HBASE-7160-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 5 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 98 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 24 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.TestFromClientSide
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3399//console
This message is automatically generated.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch, HBASE-7160-V2.patch, HBASE-7160-V3.patch, HBASE-7160-V4.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiroshi Ikeda updated HBASE-7160:
---------------------------------
Status: Patch Available (was: Open)
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should recreate the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "ramkrishna.s.vasudevan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
ramkrishna.s.vasudevan reassigned HBASE-7160:
---------------------------------------------
Assignee: Hiroshi Ikeda
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiroshi Ikeda updated HBASE-7160:
---------------------------------
Attachment: HBASE-7160-V2.patch
Patch v2 from review board.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch, HBASE-7160-V2.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiroshi Ikeda updated HBASE-7160:
---------------------------------
Attachment: HBASE-7160-V4.patch
Patch v4 from review board.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch, HBASE-7160-V2.patch, HBASE-7160-V3.patch, HBASE-7160-V4.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Lars Hofhansl (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13499166#comment-13499166 ]
Lars Hofhansl commented on HBASE-7160:
--------------------------------------
You in addition to this patch?
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498978#comment-13498978 ]
Ted Yu commented on HBASE-7160:
-------------------------------
Looking at Entry.release():
{code}
+ public void release() {
+ if (released.compareAndSet(false, true)) {
{code}
Would this do the trick ?
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13502652#comment-13502652 ]
Hadoop QA commented on HBASE-7160:
----------------------------------
{color:red}-1 overall{color}. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12554644/HBASE-7160-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 5 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 98 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 24 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/3397//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3397//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3397//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3397//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3397//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3397//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3397//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3397//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3397//console
This message is automatically generated.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch, HBASE-7160-V2.patch, HBASE-7160-V3.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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] [Comment Edited] (HBASE-7160) Improve IdLock and remove its
minor defects
Posted by "Lars Hofhansl (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13499166#comment-13499166 ]
Lars Hofhansl edited comment on HBASE-7160 at 11/16/12 9:48 PM:
----------------------------------------------------------------
You mean in addition to this patch?
was (Author: lhofhansl):
You in addition to this patch?
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13500080#comment-13500080 ]
Hadoop QA commented on HBASE-7160:
----------------------------------
{color:red}-1 overall{color}. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12554129/HBASE-7160-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 3 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 99 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 24 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/3363//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3363//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3363//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3363//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3363//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3363//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3363//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3363//console
This message is automatically generated.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch, HBASE-7160-V2.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496972#comment-13496972 ]
Hadoop QA commented on HBASE-7160:
----------------------------------
{color:red}-1 overall{color}. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12553476/HBASE-7160.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 3 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 93 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 17 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.regionserver.TestSplitTransaction
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3335//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3335//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3335//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3335//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3335//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3335//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3335//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3335//console
This message is automatically generated.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should recreate the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13499993#comment-13499993 ]
Hiroshi Ikeda commented on HBASE-7160:
--------------------------------------
bq. Hiroshi Ikeda This is interesting. There is indeed busy waiting while an Entry is being unlocked. On the other hand I find it hard to prove by visual inspection that the new code has no such busy waiting.
The following pattern is commonly used for Atomic* for the purpose of non-blocking:
{code}
while(true) {
value = valueReference.get();
// some light code
if(valueReference.compareAndSet(value, nextValue)) {
break;
}
}
{code}
and the above "if" condition is true and breaks the infinite loop if other threads don't interfere in the valueReference while executing "some light code".
If other threads interfere we have to start over the loop, (and we cannot prevent it because of non-blocking even if the "some light code" is very small,) but in practice we can ignore the situation that other threads continue to interfere and we have to loop many times.
The new code merely combines sort of the above pattern.
bq. Are you planning some multithreaded performance testing with the patch to demonstrate that it is better?
Sorry, I have no idea how to do it.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Lars Hofhansl (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498933#comment-13498933 ]
Lars Hofhansl commented on HBASE-7160:
--------------------------------------
[~ikeda] This is interesting. There is indeed busy waiting while an Entry is being unlocked. On the other hand I find it hard to prove by visual inspection that the new code has no such busy waiting.
Are you planning some multithreaded performance testing with the patch to demonstrate that it is better?
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13499994#comment-13499994 ]
Hiroshi Ikeda commented on HBASE-7160:
--------------------------------------
{quote}
Looking at Entry.release():
+ public void release() {
+ if (released.compareAndSet(false, true)) {
Would this do the trick ?
{quote}
Calling compareAndSet() in the Entry.release() is a mere safety net.
Otherwise multiple calling Entry.release() can break the exclusiveness of IdLock.
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should re-create the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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-7160) Improve IdLock and remove its minor
defects
Posted by "Hiroshi Ikeda (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-7160?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiroshi Ikeda updated HBASE-7160:
---------------------------------
Attachment: HBASE-7160.patch
Added a patch.
I recreated IdLock with only non-blocking methods except for blocking at the locked entry. (And consequently the code might be a little tedious. It might be overkill.)
> Improve IdLock and remove its minor defects
> -------------------------------------------
>
> Key: HBASE-7160
> URL: https://issues.apache.org/jira/browse/HBASE-7160
> Project: HBase
> Issue Type: Improvement
> Reporter: Hiroshi Ikeda
> Priority: Minor
> Attachments: HBASE-7160.patch
>
>
> Combination of synchronizations and concurrent collections complicates the code, and it is hard to trace the code and to confirm its correctness. We should recreate the class and make it more understandable.
> In the current code, I find the following minor defects:
> (1) In the case that there is a waiting thread for a lock in getLockEntry() and another thread is releasing the lock by calling releaseLockEntry(), trying to get the lock with a 3rd thread by calling getLockEntry() falls into a busy loop until the waiting thread wakes up and gets the lock.
> Even if notify() wakes up the blocked thread and causes a context switch to the waked thread immediately, synchronization might block the waked thread and cause another context switch, and the busy loop might continue for a while.
> (2) In the same case as (1), since releasing the lock is merely notifying without removing the lock-entry from the map, interrupting the waiting thread might leave an unused lock-entry (entry.numWaiters == 0) in the map. This is a memory leak unless the id of the lock is used again.
--
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