[
https://issues.apache.org/jira/browse/OOZIE-1922?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15073111#comment-15073111
]
Purshotam Shah edited comment on OOZIE-1922 at 12/28/15 8:12 PM:
-----------------------------------------------------------------
{quote}
This only checks for if there are no waiting threads and if all locks held by
this thread are released before removing the lock. It does not take into
account if any other thread is holding read or write lock. You will also have
to make use of the following APIs in the logic.
{quote}
Sine we are in release function, which means that lock is acquired by current
thread. So, we don't have to check if lock is held by other thread or not.
{quote}
Even though acquire() may not cause a ZK call for a reentrant lock, looking at
the isAcquiredInThisProcess()/getParticipantNodes() APIs used in release()
call, I am concerned that those will be new calls to ZK and might cause
performance degradation if synchronous is removed in OOZIE-2394
{quote}
isAcquiredInThisProcess() doesn't make any ZK call.
{code:title=InterProcessMutex.java}
@Override
public boolean isAcquiredInThisProcess()
{
return (threadData.size() > 0);
}
{code}
getParticipantNodes() does. We may not need getParticipantNodes, since lock is
acquired by current host, it won't be to acquire by other hosts.
!readWriteLock.readLock().isAcquiredInThisProcess() &&
!readWriteLock.writeLock().isAcquiredInThisProcess() is enough.
I didn't remove getParticipantNodes because it was already part of the
condition and was not sure if it's break anything.
If you are really concern about performance, we can remove getParticipantNodes
from condition.
was (Author: puru):
{quote}
This only checks for if there are no waiting threads and if all locks held by
this thread are released before removing the lock. It does not take into
account if any other thread is holding read or write lock. You will also have
to make use of the following APIs in the logic.
{quote}
Sine we are in release function, which means that lock is acquired by current
thread. So, we don't have to check if lock is held by other thread or not.
{quote}
Even though acquire() may not cause a ZK call for a reentrant lock, looking at
the isAcquiredInThisProcess()/getParticipantNodes() APIs used in release()
call, I am concerned that those will be new calls to ZK and might cause
performance degradation if synchronous is removed in OOZIE-2394
{quote}
isAcquiredInThisProcess() doesn't make any ZK call.
{code:title=InterProcessMutex.java}
@Override
public boolean isAcquiredInThisProcess()
{
return (threadData.size() > 0);
}
getParticipantNodes() does. We may not need getParticipantNodes, since lock is
acquired by current host, it won't be to acquire by other hosts.
!readWriteLock.readLock().isAcquiredInThisProcess() &&
!readWriteLock.writeLock().isAcquiredInThisProcess() is enough.
I didn't remove getParticipantNodes because it was already part of the
condition and was not sure if it's break anything.
If you are really concern about performance, we can remove getParticipantNodes
from condition.
> MemoryLocksService fails if lock is acquired multiple times in same thread
> and released
> ---------------------------------------------------------------------------------------
>
> Key: OOZIE-1922
> URL: https://issues.apache.org/jira/browse/OOZIE-1922
> Project: Oozie
> Issue Type: Bug
> Reporter: Purshotam Shah
> Assignee: Purshotam Shah
> Attachments: OOZIE-1922-V1.patch, OOZIE-1922.1.patch,
> OOZIE-1922.2.patch, OOZIE-1922.3.patch
>
>
> ReentrantLock can be acquired multiple times in same thread. For multiple
> acquire call, ReentrantLock hold count is incremented by one.
> So if we acquire lock multiple time from same thread, all will be successful
> and hold count is increased for every call.
> But if we release lock, MemoryLocksService ignore the count and deletes the
> lock. Even if it's held by some command.
> Simple step can reproduce it.
> {code}
> service.getWriteLock("test", 5000); //writeHoldCount = 1
> MemoryLockToken lock = (MemoryLockToken)service.getWriteLock("test",
> 5000); //writeHoldCount = 2
> lock.release(); //writeHoldCount = 1
> lock = (MemoryLockToken)service.getWriteLock("test", 5000);
> //writeHoldCount = 1, it should be 2.
> {code}
> {code}
> @Override
> public void release() {
> int val = rwLock.getQueueLength();
> if (val == 0) {
> synchronized (locks) {
> locks.remove(resource);
> }
> }
> lock.unlock();
> }
> }
> {code}
> MemoryLocks should check hold count before removing lock.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
