----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47837/#review144690 -----------------------------------------------------------
core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java (line 29) <https://reviews.apache.org/r/47837/#comment210728> Keep in mind that I'm going to make heavy changes in this class, you might want to incorporate them. See: https://issues.apache.org/jira/browse/OOZIE-2584 core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java (line 339) <https://reviews.apache.org/r/47837/#comment210730> Why two distinct variables? core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java (line 342) <https://reviews.apache.org/r/47837/#comment210733> extract magic number to a constant core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java (line 350) <https://reviews.apache.org/r/47837/#comment210729> I understand the purpose of this, but System.gc() is very dodgy especially in a test. I think if you want to test the fact that you use a Guava-based map with weak values, you're better off checking types. Guava-specific map is MapMakerInternalMap and the field that determines the "strength" of the values is valueStrength. It's not nice to access a package-private field with reflection but I think it's more acceptable and predictable than System.gc(). core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java (line 372) <https://reviews.apache.org/r/47837/#comment210734> Why does the method start with an underscore? Because of JUnit? Could be just simply checkLockRelease(). core/src/test/java/org/apache/oozie/service/TestZKLocksService.java (line 30) <https://reviews.apache.org/r/47837/#comment210737> This class suffers from the same problem as TestMemoryLocks, except that tests sleep for 1000ms which makes them less prone to errors. We might want to create a new JIRA to fix all these Thread.sleep() stuff. core/src/test/java/org/apache/oozie/service/TestZKLocksService.java (line 524) <https://reviews.apache.org/r/47837/#comment210735> See my comments in TestMemoryLocksService core/src/test/java/org/apache/oozie/service/TestZKLocksService.java (line 553) <https://reviews.apache.org/r/47837/#comment210738> Again, method naming - Peter Bacsko On aug. 3, 2016, 4:56 du, Purshotam Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47837/ > ----------------------------------------------------------- > > (Updated aug. 3, 2016, 4:56 du) > > > Review request for oozie. > > > Bugs: OOZIE-2501 > https://issues.apache.org/jira/browse/OOZIE-2501 > > > Repository: oozie-git > > > Description > ------- > > OOZIE-2501 ZK reentrant lock doesn't work for few cases > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/command/wf/ActionStartXCommand.java > 41f4430f69cb7a9b132a4000c3e5c8aa7573c0a0 > core/src/main/java/org/apache/oozie/lock/MemoryLocks.java > 7d65ac0e24a62086732ec91fc24f89b62469451d > core/src/main/java/org/apache/oozie/service/MemoryLocksService.java > d7c6a89fd47a219b2ec8ea4fe0caf05dc008943b > core/src/main/java/org/apache/oozie/service/ZKLocksService.java > 952b90d5dfbfeccf4600238f75885c792709ffc7 > core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java > 61fec19b346748b22df1b58f014c32b1c04c8c1f > core/src/test/java/org/apache/oozie/service/TestZKLocksService.java > d1acadfff36fff637fb9ccb8e3feffb24248c792 > > Diff: https://reviews.apache.org/r/47837/diff/ > > > Testing > ------- > > > Thanks, > > Purshotam Shah > >
