You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Purshotam Shah <pu...@yahoo-inc.com> on 2016/09/22 17:04:33 UTC

Re: Review Request 47837: OOZIE-2501 ZK reentrant lock doesn't work for few cases


> On Aug. 8, 2016, 11:58 a.m., Peter Bacsko wrote:
> > core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java, line 352
> > <https://reviews.apache.org/r/47837/diff/4/?file=1461452#file1461452line352>
> >
> >     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 :)

I run in a loop, and I didn't see any issue. Anyway it's just test case, so let keep it for now and if its become flaky,  we can look it later on


> On Aug. 8, 2016, 11:58 a.m., Peter Bacsko wrote:
> > core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java, line 374
> > <https://reviews.apache.org/r/47837/diff/4/?file=1461452#file1461452line374>
> >
> >     OK, I'll leave this to you :)

I will fix it.


- Purshotam


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


On Aug. 3, 2016, 4:56 p.m., 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 p.m.)
> 
> 
> 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
> 
>