[
https://issues.apache.org/jira/browse/OOZIE-1922?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15068848#comment-15068848
]
Rohini Palaniswamy commented on OOZIE-1922:
-------------------------------------------
See couple of issues with the patch.
1) rwLock.getQueueLength() == 0 && rwLock.getWriteHoldCount() == 0 &&
rwLock.getReadHoldCount() == 0
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.
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html#getReadLockCount()
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html#isWriteLocked()
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html#isWriteLockedByCurrentThread()
2)
{code}
& !readWriteLock.readLock().isAcquiredInThisProcess()
+ &&
!readWriteLock.writeLock().isAcquiredInThisProcess()
{code}
- Should be && instead of &.
- isAcquiredInThisProcess stands for if acquired in this JVM. You are trying
to remove it from current node's zkLocks HashMap if the node does not hold lock
anymore. That is good, but lock.getParticipantNodes().size() == 0 condition
will prevent it if another node is hold the lock. So you will have to remove
that condition.
Haven't looked at the test cases yet. Can you put the revised patch in review
board as it is hard to read logic for the test cases.
3) Related to OOZIE-2394 patch:
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. Can you run
one test case enabling debug logging for ZK and see how many calls are made
with and without removal of synchronous. If there is increase by even one ZK
call, I would like to retain the synchronous method. Cleanup of removing very
few lines of simple code is not worth for the cost of a ZK call.
> 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: Azrael Seoeun
> 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)
