Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Date: Mon, 19 Nov 2012 02:36:58 +0000 (UTC)
From: "Hiroshi Ikeda (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: <37999834.1416.1353292618181.JavaMail.jiratomcat@arcas>
In-Reply-To: <1655112107.113047.1352883372238.JavaMail.jiratomcat@arcas>
Subject: [jira] [Commented] (HBASE-7160) Improve IdLock and remove its minor
 defects
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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