falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "pavan kumar kolamuri" <pavan.kolam...@gmail.com>
Subject Re: Review Request 41401: Rerun API for Falcon Native Scheduler
Date Fri, 18 Dec 2015 06:45:09 GMT


> On Dec. 18, 2015, 5:20 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java, line 438
> > <https://reviews.apache.org/r/41401/diff/3/?file=1168361#file1168361line438>
> >
> >     instance is already cast to ProcessExecutionInstance in the above statement,
so checking here is redundant. May be this check should go up.

Will fix


> On Dec. 18, 2015, 5:20 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/notification/service/event/RerunEvent.java,
line 34
> > <https://reviews.apache.org/r/41401/diff/3/?file=1168363#file1168363line34>
> >
> >     Similarly can we make change here also to accept only InstanceID?

Sure


> On Dec. 18, 2015, 5:20 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/InstanceState.java, line 53
> > <https://reviews.apache.org/r/41401/diff/3/?file=1168366#file1168366line53>
> >
> >     Won't RERUN be more intuitive instead of EXTERNAL_TRIGGER?

We want to use same logic for extrenal triggers also which can be triggered by external service.


> On Dec. 18, 2015, 5:20 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/InstanceState.java, line 54
> > <https://reviews.apache.org/r/41401/diff/3/?file=1168366#file1168366line54>
> >
> >     Shouldn't rerun only be valid for terminal states? By that logic for waiting
instances, rerun should throw invalid state transition exception, right?

This is to make sure consistency with trigger and makes idempotent. As it is starting state


> On Dec. 18, 2015, 5:20 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/InstanceState.java, line 196
> > <https://reviews.apache.org/r/41401/diff/3/?file=1168366#file1168366line196>
> >
> >     Will it be more intuitive to name it rerun?

Explained


> On Dec. 18, 2015, 5:20 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/InstanceStateChangeHandler.java,
line 42
> > <https://reviews.apache.org/r/41401/diff/3/?file=1168367#file1168367line42>
> >
> >     Will it be more intuitive to rename it to onRerun?

Explained the reason


> On Dec. 18, 2015, 5:20 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java, line
145
> > <https://reviews.apache.org/r/41401/diff/3/?file=1168369#file1168369line145>
> >
> >     Isn't it instanceID?

No it is workflowID


- pavan kumar


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


On Dec. 17, 2015, 9 a.m., pavan kumar kolamuri wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41401/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2015, 9 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Currently rerun API was not supported in case of Native Scheduler in Falcon. Rerun of
instances should be implemented in FalconWorkflowEngine.
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/falcon/resource/InstancesResult.java e05eeeb 
>   scheduler/src/main/java/org/apache/falcon/execution/EntityExecutor.java c9c0f42 
>   scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java 3cc8a25

>   scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java f3beabc

>   scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java e446069 
>   scheduler/src/main/java/org/apache/falcon/notification/service/event/EventType.java
59f5cba 
>   scheduler/src/main/java/org/apache/falcon/notification/service/event/RerunEvent.java
PRE-CREATION 
>   scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java
fb11091 
>   scheduler/src/main/java/org/apache/falcon/predicate/Predicate.java 164fb0e 
>   scheduler/src/main/java/org/apache/falcon/state/InstanceState.java 7f2bda9 
>   scheduler/src/main/java/org/apache/falcon/state/InstanceStateChangeHandler.java 1f69fab

>   scheduler/src/main/java/org/apache/falcon/state/StateService.java c702cc3 
>   scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java 2f3aa3a

>   scheduler/src/main/java/org/apache/falcon/state/store/InstanceStateStore.java f1d1931

>   scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java 4bee269

>   scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java 0e3dfa9

>   scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java b2f8e80

>   scheduler/src/main/java/org/apache/falcon/workflow/engine/DAGEngine.java e0d2a0e 
>   scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java
c19cada 
>   scheduler/src/main/java/org/apache/falcon/workflow/engine/OozieDAGEngine.java a26eb77

>   scheduler/src/test/java/org/apache/falcon/execution/MockDAGEngine.java d274ad7 
>   scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java
6d5bd49 
>   webapp/src/test/java/org/apache/falcon/resource/AbstractSchedulerManagerJerseyIT.java
f5bcc54 
>   webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java
7959b63 
>   webapp/src/test/resources/local-process-noinputs-template.xml aabdc6a 
> 
> Diff: https://reviews.apache.org/r/41401/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pavan kumar kolamuri
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message