> On Sept. 15, 2015, midnight, Rohini Palaniswamy wrote:
> > core/src/main/java/org/apache/oozie/service/CallableQueueService.java, 
> > lines 515-517
> > <https://reviews.apache.org/r/37906/diff/3-4/?file=1061192#file1061192line515>
> >
> >     There is lot of unnecessary wrapping
> >     
> >     CallableWrapper extends PriorityDelayQueue.QueueElement<XCallable<?>> 
> > implements Runnable
> >     QueueElement<E> extends FutureTask<E> implements Delayed
> >     FutureTask<V> implements RunnableFuture<V>
> >     
> >     CallableWrapper is already a RunnableFuture and QueueElement. Why wrap 
> > it again into new QueueElement. Does not make sense.  Also I don't think we 
> > need QueueElement<E> extends FutureTask<E> in the first place.
> >     
> >     Can't CallableWrapper be just changed from Runnable to Callable and the 
> > expose CallableWrapper as public static class so that Fork command can 
> > directly use it? That will make it simple and get rid of lot of the changes.

CallableWrapper  can be a Cllable, but it still need to be futureTask. 
ExecutorService.newTaskFor() creates a futureTask for submit calls and if 
callable is not a futurtask, then it will throw ClassCastException. more 
detail@ 
http://stackoverflow.com/questions/11430574/how-to-have-an-unbound-sortable-queue-utilized-by-a-fixed-amount-of-threads.
To make things simple i have removed ExtendedcallableWrapper and made 
CallableWrapper as callable all other changes are needed.


- Purshotam


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


On Sept. 14, 2015, 8:52 p.m., Purshotam Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37906/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2015, 8:52 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2345
>     https://issues.apache.org/jira/browse/OOZIE-2345
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> We have few customers whose SLA is 8 min. They have around 30 actions. There 
> are 25 actions in fork.
> Though forked action jobs runs concurrently. forked action job submission is 
> sequential.
> Whenever NN is slow, job submission takes more time. Even if job submission 
> is delay for 30 sec. Total WF delay will be ~12 min.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/command/wf/ActionStartXCommand.java 
> e06649cf9bbc7c0997e1d4f94fa7e695533f8343 
>   core/src/main/java/org/apache/oozie/command/wf/ActionXCommand.java 
> 2616d322ea0dd85942fcb33c73d7d33af049b789 
>   
> core/src/main/java/org/apache/oozie/command/wf/ForkedActionStartXCommand.java 
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/main/java/org/apache/oozie/command/wf/SignalXCommand.java 
> d1fcd1ac933aaeaea22cdd3fcbe55be386281fbb 
>   core/src/main/java/org/apache/oozie/service/CallableQueueService.java 
> 830a58ebe152baed0a922e78855be51d3bf1b21c 
>   core/src/main/java/org/apache/oozie/util/ExtendedCallableWrapper.java 
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/main/java/org/apache/oozie/util/PriorityDelayQueue.java 
> ae545063045f70784089c27a69666ad445cb8e4e 
>   core/src/main/resources/oozie-default.xml 
> 32a1df044b9910ba9cfade88005a598e0ab3a7b3 
>   
> core/src/test/java/org/apache/oozie/command/wf/TestForkedActionStartXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/test/java/org/apache/oozie/command/wf/TestReRunXCommand.java 
> 02f61668e6d6d03674f5d516853057fb94fed518 
>   core/src/test/java/org/apache/oozie/command/wf/TestSignalXCommand.java 
> 4268b301d3efff05e870b3d833ba5fd5b9cc0d5a 
>   core/src/test/java/org/apache/oozie/util/TestPriorityDelayQueue.java 
> 857f4e37219c705b159c9f3f970fe14b090604b8 
> 
> Diff: https://reviews.apache.org/r/37906/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Reply via email to