> On Dec. 22, 2015, 11:04 p.m., Rohini Palaniswamy wrote: > > core/src/main/java/org/apache/oozie/service/ZKLocksService.java, line 201 > > <https://reviews.apache.org/r/41623/diff/1/?file=1174154#file1174154line201> > > > > readWriteLock != null - Instead of adding this condition in multiple > > places, either put the whole logic in > > > > if (readWriteLock != null) { > > .... > > } > > > > block or make the InterProcessReadWriteLock also part of ZKLockToken > > like MemoryLockToken so that you don't have to retrieve from hashmap.
Not sure if i understand this. After removing lock.getParticipantNodes().size(), it should be ok. > On Dec. 22, 2015, 11:04 p.m., Rohini Palaniswamy wrote: > > core/src/test/java/org/apache/oozie/service/TestZKLocksService.java, line > > 508 > > <https://reviews.apache.org/r/41623/diff/1/?file=1174156#file1174156line508> > > > > Test case is most likely flaky. It needs to be changed to make the > > other thread acquire the lock after the thread that acquired the lock first > > released it. With current logic mostly only one thread will acquire the > > lock and release before the assert statement or if the other thread > > acquired the lock during sleep(1000), then test will fail. > > > > Also need a new testcase testReentrantSameThread() - to test reentrant > > behaviour in same thread. >Also need a new testcase testReentrantSameThread() - to test reentrant >behaviour in same thread. There is already a testcase for that, testReentrantMultipleCall. The behaviour of testcases is that only one thread should be able to acquire the lock. Upon release, entry should be removed from hashmap. This was an existing testcase, but testcases was incorrect, so I changed it. - Purshotam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41623/#review111682 ----------------------------------------------------------- On Dec. 22, 2015, 1:11 a.m., Purshotam Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41623/ > ----------------------------------------------------------- > > (Updated Dec. 22, 2015, 1:11 a.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1922 > https://issues.apache.org/jira/browse/OOZIE-1922 > > > Repository: oozie-git > > > Description > ------- > > OOZIE-1922 - MemoryLocksService fails if lock is acquired multiple times in > same thread and released > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/lock/MemoryLocks.java > ee564b3def2ce42b06f697c3384b1e102ac6a3ba > core/src/main/java/org/apache/oozie/service/MemoryLocksService.java > e3eccdb3f95e74198c3d42d19022b4acfc3d18e5 > core/src/main/java/org/apache/oozie/service/ZKLocksService.java > e3a6bcf04048e140d2ced9d8bdbfe08b01b323e4 > core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java > 0efe31033f63055c08e42202c56c336248271afe > core/src/test/java/org/apache/oozie/service/TestZKLocksService.java > 02cc1372dc015713f0bb1f6861b7e4b7455c4f13 > > Diff: https://reviews.apache.org/r/41623/diff/ > > > Testing > ------- > > TestZKLocksService.testReentrantMultipleCall already has reentrant test cases. > > > Thanks, > > Purshotam Shah > >
