-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41623/#review111682
-----------------------------------------------------------



core/src/main/java/org/apache/oozie/service/ZKLocksService.java (line 201)
<https://reviews.apache.org/r/41623/#comment171905>

    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.



core/src/test/java/org/apache/oozie/service/TestZKLocksService.java (line 484)
<https://reviews.apache.org/r/41623/#comment171907>

    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.


- Rohini Palaniswamy


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
> 
>

Reply via email to