----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67885/#review207076 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java Lines 143 (patched) <https://reviews.apache.org/r/67885/#comment290264> What about using a `CopyOnWriteArrayList` or a `ConcurrentHashSet` instead? core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java Lines 154 (patched) <https://reviews.apache.org/r/67885/#comment290263> Shouldn't we remove only when `executor.execute()` happened before? core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java Lines 163 (patched) <https://reviews.apache.org/r/67885/#comment290265> Here we rely on the fact that `commandFinished()` is called only once per task. If called multiple times, we're in a problem. core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java Lines 167 (patched) <https://reviews.apache.org/r/67885/#comment290266> Sure we need to give reference to `ThreadPoolExecutor` in a `public` way? core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java Lines 195 (patched) <https://reviews.apache.org/r/67885/#comment290267> `Integer#compareTo()` makes it more readable. core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java Lines 270 (patched) <https://reviews.apache.org/r/67885/#comment290268> `CopyOnWriteArrayList` or `ConcurrentHashSet` instead? core/src/main/java/org/apache/oozie/service/CallableQueueService.java Lines 122-126 (patched) <https://reviews.apache.org/r/67885/#comment290272> Dead code. core/src/main/java/org/apache/oozie/service/CallableQueueService.java Lines 165 (patched) <https://reviews.apache.org/r/67885/#comment290274> Unsure why we need an `AtomicInteger` here. core/src/main/java/org/apache/oozie/service/CallableQueueService.java Lines 182 (patched) <https://reviews.apache.org/r/67885/#comment290275> Unsure why we need an `AtomicInteger` here. core/src/main/java/org/apache/oozie/service/CallableQueueService.java Lines 559 (patched) <https://reviews.apache.org/r/67885/#comment290273> `LOG.debug()` on which implementation is running would be nice. core/src/main/java/org/apache/oozie/util/PriorityDelayQueue.java Line 67 (original), 67-68 (patched) <https://reviews.apache.org/r/67885/#comment290276> A javadoc about the difference between the semantics of `delay` and `initialDelay` would be nice. core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java Lines 103-111 (patched) <https://reviews.apache.org/r/67885/#comment290277> Duplicate. core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java Lines 155-163 (patched) <https://reviews.apache.org/r/67885/#comment290278> Duplicate. core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java Lines 207 (patched) <https://reviews.apache.org/r/67885/#comment290279> Are there 4 priority levels, from 0 to 3? core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java Lines 412-415 (original), 427-440 (patched) <https://reviews.apache.org/r/67885/#comment290280> Dead code. core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java Line 425 (original), 455 (patched) <https://reviews.apache.org/r/67885/#comment290281> Dead code. core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java Lines 1075 (patched) <https://reviews.apache.org/r/67885/#comment290286> Better use `taskCount / 3`. - András Piros On Aug. 6, 2018, 3:28 p.m., Peter Bacsko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67885/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2018, 3:28 p.m.) > > > Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert > Kanter. > > > Repository: oozie-git > > > Description > ------- > > Still just a POC. > > Tests are almost completely missing. > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java > PRE-CREATION > core/src/main/java/org/apache/oozie/service/CallableAccess.java > PRE-CREATION > core/src/main/java/org/apache/oozie/service/CallableQueueService.java > ef8d58da5 > core/src/main/java/org/apache/oozie/util/PriorityDelayQueue.java 75c20698c > core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java > PRE-CREATION > core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java > 9c2a11d6f > > > Diff: https://reviews.apache.org/r/67885/diff/5/ > > > Testing > ------- > > Executed TestCallableQueueService which passed completely. > > > Thanks, > > Peter Bacsko > >
