----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47837/#review145089 -----------------------------------------------------------
core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java (line 342) <https://reviews.apache.org/r/47837/#comment211280> The value "5000" is repeated many times. It's better to extract it to a final variable. core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java (line 350) <https://reviews.apache.org/r/47837/#comment211284> Yes I understand that you want to make sure that weakrefs are cleared up after a minor GC. However System.gc() makes no guarantees about when/how it runs. It might work just fine in the current JDK, but what if something changes in the next version? The the test might become flaky. Also someone might come along and disable explicit GC for some reason (unlikely, but can happen). Unfortunately there's no super-reliable way to test weakrefs, that's why I recommended checking the types - if the types are OK, we know that we're using Guava and we can trust it (eg. we can trust the fact that if the strenght is "weak", we're using weak refs so it will work as expected). We're not testing Guava functionality this way. If the assertion fails, it will fail consistently and we know that something has changed. But I'm not super-religious about it - it's just the way how I would do this kind of thing. The final decision is yours :) core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java (line 372) <https://reviews.apache.org/r/47837/#comment211285> OK, I'll leave this to you :) - 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 > >
