Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-09-04 Thread András Piros via Review Board 

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


Ship it!




Ship It!

- András Piros


On Sept. 4, 2018, 3:45 p.m., Peter Bacsko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67885/
> ---
> 
> (Updated Sept. 4, 2018, 3:45 p.m.)
> 
> 
> Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
> Kanter.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> See upstream JIRA for details
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
>   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/10/
> 
> 
> Testing
> ---
> 
> 1. Executed TestCallableQueueService which passed completely.
> 2. New unit tests for ASyncXCommandExecutor.
> 3. Tried it on a 3-node cluster
> 
> 
> Thanks,
> 
> Peter Bacsko
> 
>

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-09-04 Thread Peter Bacsko via Review Board 

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

(Updated szept. 4, 2018, 3:45 du)


Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
Kanter.


Changes
---

Final changes


Repository: oozie-git


Description
---

See upstream JIRA for details


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
  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/10/

Changes: https://reviews.apache.org/r/67885/diff/9-10/


Testing
---

1. Executed TestCallableQueueService which passed completely.
2. New unit tests for ASyncXCommandExecutor.
3. Tried it on a 3-node cluster


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-09-04 Thread Peter Bacsko via Review Board 

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

(Updated szept. 4, 2018, 3:42 du)


Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
Kanter.


Repository: oozie-git


Description
---

See upstream JIRA for details


Diffs
-

  core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
  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/9/


Testing
---

1. Executed TestCallableQueueService which passed completely.
2. New unit tests for ASyncXCommandExecutor.
3. Tried it on a 3-node cluster


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-09-04 Thread András Piros via Review Board 

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




core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
Lines 1 (patched)


Apache 2.0 license header missing.



core/src/main/java/org/apache/oozie/service/CallableQueueService.java
Lines 562 (patched)


Typo: `by looking at the`



core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java
Lines 75 (patched)


Remove newline


- András Piros


On Sept. 4, 2018, 12:05 p.m., Peter Bacsko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67885/
> ---
> 
> (Updated Sept. 4, 2018, 12:05 p.m.)
> 
> 
> Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
> Kanter.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> See upstream JIRA for details
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
>   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/9/
> 
> 
> Testing
> ---
> 
> 1. Executed TestCallableQueueService which passed completely.
> 2. New unit tests for ASyncXCommandExecutor.
> 3. Tried it on a 3-node cluster
> 
> 
> Thanks,
> 
> Peter Bacsko
> 
>

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-09-04 Thread Peter Bacsko via Review Board 

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

(Updated szept. 4, 2018, 12:05 du)


Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
Kanter.


Changes
---

Addressed comments of Sala


Repository: oozie-git


Description
---

See upstream JIRA for details


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
  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/9/

Changes: https://reviews.apache.org/r/67885/diff/8-9/


Testing
---

1. Executed TestCallableQueueService which passed completely.
2. New unit tests for ASyncXCommandExecutor.
3. Tried it on a 3-node cluster


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-24 Thread Andras Salamon 

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




core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
Lines 140 (patched)


Should be using MIN_PRIORITY instead of 0



core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
Lines 243-261 (patched)


Would it be possible to eliminate the repeated code? Maybe create a new 
function and call it twice?



core/src/main/java/org/apache/oozie/service/CallableQueueService.java
Line 474 (original), 502 (patched)


Should be using PRIORITIES instead of 3



core/src/main/resources/oozie-default.xml
Line 1 (original), 1 (patched)


What about adding oozie.service.CallableQueueService.queue.oldImpl property 
to the oozie-default.xml file?



core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java
Lines 282 (patched)


Message can be confusing if pendingCommandsPerType.size() returns > 1 
(which is not very likely).


- Andras Salamon


On Aug. 17, 2018, 12:37 p.m., Peter Bacsko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67885/
> ---
> 
> (Updated Aug. 17, 2018, 12:37 p.m.)
> 
> 
> Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
> Kanter.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> See upstream JIRA for details
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
>   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/8/
> 
> 
> Testing
> ---
> 
> 1. Executed TestCallableQueueService which passed completely.
> 2. New unit tests for ASyncXCommandExecutor.
> 3. Tried it on a 3-node cluster
> 
> 
> Thanks,
> 
> Peter Bacsko
> 
>

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-17 Thread Peter Bacsko via Review Board 

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

(Updated aug. 17, 2018, 12:37 du)


Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
Kanter.


Repository: oozie-git


Description
---

See upstream JIRA for details


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
  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/8/

Changes: https://reviews.apache.org/r/67885/diff/7-8/


Testing
---

1. Executed TestCallableQueueService which passed completely.
2. New unit tests for ASyncXCommandExecutor.
3. Tried it on a 3-node cluster


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-17 Thread Peter Bacsko via Review Board 

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

(Updated aug. 17, 2018, 12:36 du)


Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
Kanter.


Repository: oozie-git


Description (updated)
---

See upstream JIRA for details


Diffs
-

  core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
  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/7/


Testing (updated)
---

1. Executed TestCallableQueueService which passed completely.
2. New unit tests for ASyncXCommandExecutor.
3. Tried it on a 3-node cluster


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-16 Thread Peter Bacsko via Review Board 

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

(Updated aug. 16, 2018, 3:55 du)


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 (updated)
-

  core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/main/resources/oozie-default.xml b69d2c9aa 
  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/7/

Changes: https://reviews.apache.org/r/67885/diff/6-7/


Testing
---

Executed TestCallableQueueService which passed completely.


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-15 Thread Peter Bacsko via Review Board 

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




core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
Lines 69 (patched)


Todo: probably this should be configurable (add property)



core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
Lines 127 (patched)


Extract constants



core/src/main/java/org/apache/oozie/service/CallableQueueService.java
Lines 473 (patched)


Constant


- Peter Bacsko


On aug. 15, 2018, 9:49 de, Peter Bacsko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67885/
> ---
> 
> (Updated aug. 15, 2018, 9:49 de)
> 
> 
> 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/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/6/
> 
> 
> Testing
> ---
> 
> Executed TestCallableQueueService which passed completely.
> 
> 
> Thanks,
> 
> Peter Bacsko
> 
>

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-15 Thread Peter Bacsko via Review Board 


> On aug. 10, 2018, 2:44 du, András Piros wrote:
> > core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
> > Lines 143 (patched)
> > 
> >
> > What about using a `CopyOnWriteArrayList` or a `ConcurrentHashSet` 
> > instead?

Changed List to ConcurrentHashSet, although a sync block is still necessary (at 
least it seems).


> On aug. 10, 2018, 2:44 du, András Piros wrote:
> > core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
> > Lines 154 (patched)
> > 
> >
> > Shouldn't we remove only when `executor.execute()` happened before?

This was the original implementation as well (that is, full --> throw away).


> On aug. 10, 2018, 2:44 du, András Piros wrote:
> > core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
> > Lines 163 (patched)
> > 
> >
> > Here we rely on the fact that `commandFinished()` is called only once 
> > per task. If called multiple times, we're in a problem.

Yeah, there are tests (actually there was a bug which I spotted after checking 
more thoroughly) to make sure that there is only one decrement for every 
increment.


> On aug. 10, 2018, 2:44 du, András Piros wrote:
> > core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
> > Lines 167 (patched)
> > 
> >
> > Sure we need to give reference to `ThreadPoolExecutor` in a `public` 
> > way?

It's just to make CallableQueueService happy (it has reference to an executor 
instance), this way we don't have to modify that class heavily.


> On aug. 10, 2018, 2:44 du, András Piros wrote:
> > core/src/main/java/org/apache/oozie/service/CallableQueueService.java
> > Lines 165 (patched)
> > 
> >
> > Unsure why we need an `AtomicInteger` here.

Probably not needed, this was the original implementation, didn't want to 
change it.


> On aug. 10, 2018, 2:44 du, András Piros wrote:
> > core/src/main/java/org/apache/oozie/service/CallableQueueService.java
> > Lines 182 (patched)
> > 
> >
> > Unsure why we need an `AtomicInteger` here.

See above.


> On aug. 10, 2018, 2:44 du, András Piros wrote:
> > core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java
> > Lines 207 (patched)
> > 
> >
> > Are there 4 priority levels, from 0 to 3?

Yes - added constants to make this clear.


- Peter


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


On aug. 15, 2018, 9:49 de, Peter Bacsko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67885/
> ---
> 
> (Updated aug. 15, 2018, 9:49 de)
> 
> 
> 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/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/6/
> 
> 
> Testing
> ---
> 
> Executed TestCallableQueueService which passed completely.
> 
> 
> Thanks,
> 
> Peter Bacsko
> 
>

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-15 Thread Peter Bacsko via Review Board 

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

(Updated aug. 15, 2018, 9:49 de)


Review request for oozie, András Piros, Peter Cseh, Kinga Marton, and Robert 
Kanter.


Changes
---

Addressing review comments


Repository: oozie-git


Description
---

Still just a POC.

Tests are almost completely missing.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.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/6/

Changes: https://reviews.apache.org/r/67885/diff/5-6/


Testing
---

Executed TestCallableQueueService which passed completely.


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-10 Thread András Piros via Review Board 

---
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)


What about using a `CopyOnWriteArrayList` or a `ConcurrentHashSet` instead?



core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
Lines 154 (patched)


Shouldn't we remove only when `executor.execute()` happened before?



core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
Lines 163 (patched)


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)


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)


`Integer#compareTo()` makes it more readable.



core/src/main/java/org/apache/oozie/service/AsyncXCommandExecutor.java
Lines 270 (patched)


`CopyOnWriteArrayList` or `ConcurrentHashSet` instead?



core/src/main/java/org/apache/oozie/service/CallableQueueService.java
Lines 122-126 (patched)


Dead code.



core/src/main/java/org/apache/oozie/service/CallableQueueService.java
Lines 165 (patched)


Unsure why we need an `AtomicInteger` here.



core/src/main/java/org/apache/oozie/service/CallableQueueService.java
Lines 182 (patched)


Unsure why we need an `AtomicInteger` here.



core/src/main/java/org/apache/oozie/service/CallableQueueService.java
Lines 559 (patched)


`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)


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)


Duplicate.



core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java
Lines 155-163 (patched)


Duplicate.



core/src/test/java/org/apache/oozie/service/TestAsyncXCommandExecutor.java
Lines 207 (patched)


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)


Dead code.



core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java
Line 425 (original), 455 (patched)


Dead code.



core/src/test/java/org/apache/oozie/service/TestCallableQueueService.java
Lines 1075 (patched)


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
> 
>

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-08-06 Thread Peter Bacsko via Review Board 

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

(Updated aug. 6, 2018, 3:28 du)


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 (updated)
-

  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/

Changes: https://reviews.apache.org/r/67885/diff/4-5/


Testing
---

Executed TestCallableQueueService which passed completely.


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-07-31 Thread Peter Bacsko via Review Board 

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

(Updated júl. 31, 2018, 11 de)


Review request for oozie, András Piros and Robert Kanter.


Changes
---

Added new tests to TestCallableQueueService


Repository: oozie-git


Description
---

Still just a POC.

Tests are almost completely missing.


Diffs (updated)
-

  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/TestCallableQueueService.java 
9c2a11d6f 


Diff: https://reviews.apache.org/r/67885/diff/4/

Changes: https://reviews.apache.org/r/67885/diff/3-4/


Testing
---

Executed TestCallableQueueService which passed completely.


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-07-13 Thread Peter Bacsko via Review Board 

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

(Updated júl. 13, 2018, 10:05 de)


Review request for oozie, András Piros and Robert Kanter.


Repository: oozie-git


Description
---

Still just a POC.

Tests are almost completely missing.


Diffs (updated)
-

  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/TestCallableQueueService.java 
9c2a11d6f 


Diff: https://reviews.apache.org/r/67885/diff/3/

Changes: https://reviews.apache.org/r/67885/diff/2-3/


Testing
---

Executed TestCallableQueueService which passed completely.


Thanks,

Peter Bacsko

Re: Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-07-13 Thread Peter Bacsko via Review Board 

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

(Updated júl. 13, 2018, 10:03 de)


Review request for oozie, András Piros and Robert Kanter.


Changes
---

Fixed some bugs


Repository: oozie-git


Description
---

Still just a POC.

Tests are almost completely missing.


Diffs (updated)
-

  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/TestCallableQueueService.java 
9c2a11d6f 


Diff: https://reviews.apache.org/r/67885/diff/2/

Changes: https://reviews.apache.org/r/67885/diff/1-2/


Testing
---

Executed TestCallableQueueService which passed completely.


Thanks,

Peter Bacsko

Review Request 67885: POC: OOZIE-3160 PriorityDelayQueue put()/take() can cause significant CPU load due to busy waiting

2018-07-11 Thread Peter Bacsko via Review Board 

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

Review request for oozie, András Piros 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/TestCallableQueueService.java 
9c2a11d6f 


Diff: https://reviews.apache.org/r/67885/diff/1/


Testing
---

Executed TestCallableQueueService which passed completely.


Thanks,

Peter Bacsko