You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Peter Bacsko via Review Board <no...@reviews.apache.org> on 2018/05/17 13:37:24 UTC
Review Request 67183: OOZIE-3237 Flaky test
TestZKLocksService#testWriteReadLockThreads
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67183/
-----------------------------------------------------------
Review request for oozie, András Piros and Peter Cseh.
Repository: oozie-git
Description
-------
Because of fixed sleep() calls, these tests are not stable.
Diffs
-----
core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java 8c7b58eec0a839eb5f3ae3468f80d87986b45b49
core/src/test/java/org/apache/oozie/service/TestZKLocksService.java b7dee7e56efbac81a37b1ee04c48a448c49efe80
core/src/test/java/org/apache/oozie/util/LatchHandler.java PRE-CREATION
core/src/test/java/org/apache/oozie/util/Locker.java PRE-CREATION
Diff: https://reviews.apache.org/r/67183/diff/1/
Testing
-------
Thanks,
Peter Bacsko
Re: Review Request 67183: OOZIE-3237 Flaky test
TestZKLocksService#testWriteReadLockThreads
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67183/#review203330
-----------------------------------------------------------
core/src/test/java/org/apache/oozie/util/LatchHandler.java
Lines 23 (patched)
<https://reviews.apache.org/r/67183/#comment285460>
I extracted this class to avoid code duplication
core/src/test/java/org/apache/oozie/util/Locker.java
Lines 25 (patched)
<https://reviews.apache.org/r/67183/#comment285461>
I extracted this class to avoid code duplication
- Peter Bacsko
On máj. 17, 2018, 1:37 du, Peter Bacsko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67183/
> -----------------------------------------------------------
>
> (Updated máj. 17, 2018, 1:37 du)
>
>
> Review request for oozie, András Piros and Peter Cseh.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Because of fixed sleep() calls, these tests are not stable.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java 8c7b58eec0a839eb5f3ae3468f80d87986b45b49
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java b7dee7e56efbac81a37b1ee04c48a448c49efe80
> core/src/test/java/org/apache/oozie/util/LatchHandler.java PRE-CREATION
> core/src/test/java/org/apache/oozie/util/Locker.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67183/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Peter Bacsko
>
>
Re: Review Request 67183: OOZIE-3237 Flaky test
TestZKLocksService#testWriteReadLockThreads
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67183/#review203570
-----------------------------------------------------------
core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java
Line 29 (original), 29 (patched)
<https://reviews.apache.org/r/67183/#comment285852>
It's actually used.
- Peter Bacsko
On máj. 17, 2018, 1:37 du, Peter Bacsko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67183/
> -----------------------------------------------------------
>
> (Updated máj. 17, 2018, 1:37 du)
>
>
> Review request for oozie, András Piros and Peter Cseh.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Because of fixed sleep() calls, these tests are not stable.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java 8c7b58eec0a839eb5f3ae3468f80d87986b45b49
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java b7dee7e56efbac81a37b1ee04c48a448c49efe80
> core/src/test/java/org/apache/oozie/util/LatchHandler.java PRE-CREATION
> core/src/test/java/org/apache/oozie/util/Locker.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67183/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Peter Bacsko
>
>
Re: Review Request 67183: OOZIE-3237 Flaky test
TestZKLocksService#testWriteReadLockThreads
Posted by András Piros via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67183/#review203587
-----------------------------------------------------------
Ship it!
Ship It!
- András Piros
On May 22, 2018, 3:49 p.m., Peter Bacsko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67183/
> -----------------------------------------------------------
>
> (Updated May 22, 2018, 3:49 p.m.)
>
>
> Review request for oozie, András Piros and Peter Cseh.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Because of fixed sleep() calls, these tests are not stable.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java 8c7b58eec
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java b7dee7e56
> core/src/test/java/org/apache/oozie/util/Locker.java PRE-CREATION
> core/src/test/java/org/apache/oozie/util/LockerCoordinator.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67183/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Peter Bacsko
>
>
Re: Review Request 67183: OOZIE-3237 Flaky test
TestZKLocksService#testWriteReadLockThreads
Posted by Peter Bacsko via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67183/
-----------------------------------------------------------
(Updated máj. 22, 2018, 3:49 du)
Review request for oozie, András Piros and Peter Cseh.
Changes
-------
Addressed code review comments
Repository: oozie-git
Description
-------
Because of fixed sleep() calls, these tests are not stable.
Diffs (updated)
-----
core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java 8c7b58eec
core/src/test/java/org/apache/oozie/service/TestZKLocksService.java b7dee7e56
core/src/test/java/org/apache/oozie/util/Locker.java PRE-CREATION
core/src/test/java/org/apache/oozie/util/LockerCoordinator.java PRE-CREATION
Diff: https://reviews.apache.org/r/67183/diff/2/
Changes: https://reviews.apache.org/r/67183/diff/1-2/
Testing
-------
Thanks,
Peter Bacsko
Re: Review Request 67183: OOZIE-3237 Flaky test
TestZKLocksService#testWriteReadLockThreads
Posted by András Piros via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67183/#review203332
-----------------------------------------------------------
core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java
Line 29 (original), 29 (patched)
<https://reviews.apache.org/r/67183/#comment285462>
Unused import.
core/src/test/java/org/apache/oozie/util/LatchHandler.java
Lines 24 (patched)
<https://reviews.apache.org/r/67183/#comment285464>
`LATCH_TIMEOUT_SECONDS` would be a better name.
core/src/test/java/org/apache/oozie/util/LatchHandler.java
Lines 26-29 (patched)
<https://reviews.apache.org/r/67183/#comment285463>
Those can be `final`.
core/src/test/java/org/apache/oozie/util/Locker.java
Lines 25 (patched)
<https://reviews.apache.org/r/67183/#comment285467>
In the sense of composition over inheritance, I'd probably not let `Locker extends LatchHandler`, but have `Locker` a `private final LatchHandler` to be initialized inside constructor.
core/src/test/java/org/apache/oozie/util/Locker.java
Lines 26 (patched)
<https://reviews.apache.org/r/67183/#comment285465>
Why not `private static final`?
core/src/test/java/org/apache/oozie/util/Locker.java
Lines 27-28 (patched)
<https://reviews.apache.org/r/67183/#comment285466>
Why not `private final`?
- András Piros
On May 17, 2018, 1:37 p.m., Peter Bacsko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67183/
> -----------------------------------------------------------
>
> (Updated May 17, 2018, 1:37 p.m.)
>
>
> Review request for oozie, András Piros and Peter Cseh.
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Because of fixed sleep() calls, these tests are not stable.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/oozie/lock/TestMemoryLocks.java 8c7b58eec0a839eb5f3ae3468f80d87986b45b49
> core/src/test/java/org/apache/oozie/service/TestZKLocksService.java b7dee7e56efbac81a37b1ee04c48a448c49efe80
> core/src/test/java/org/apache/oozie/util/LatchHandler.java PRE-CREATION
> core/src/test/java/org/apache/oozie/util/Locker.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67183/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Peter Bacsko
>
>