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



scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java
 (line 112)
<https://reviews.apache.org/r/41401/#comment170609>

    Not a good idea to expose this directly. It should remain a private method. 
If you need this to be invoked, you can add another method (rerun, may be) and 
have it invoke this.



scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java (line 
284)
<https://reviews.apache.org/r/41401/#comment170636>

    If we update the existing instance, we will be relying on Oozie to give us 
all the runs for an instance and details on each run. 
    
    Have you evaluated this approach against creating a brand new instance 
every time and Falcon having the information that of runs and run-info?



scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java (line 
286)
<https://reviews.apache.org/r/41401/#comment170627>

    You don't need to update the STATE_STORE explicitly. It is the 
responsibility of the StateService to do that (handleStateChange method). Just 
update the cache let the State transition take care of updating the state.



scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java (line 
440)
<https://reviews.apache.org/r/41401/#comment170610>

    As mentioned above, create a new "trigger" method in 
ProcessExecutionInstance and have that invoke this method.



scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java
 (line 281)
<https://reviews.apache.org/r/41401/#comment170619>

    We already have the instance, why are we retrieving it again from state 
store?
    
    If the rerun method of ProcessExecutor updates its cache, the instance will 
contain the properties.



scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java
 (line 285)
<https://reviews.apache.org/r/41401/#comment170612>

    Shouldn't this be FALCON_FORCE_RERUN?



scheduler/src/main/java/org/apache/falcon/state/StateService.java (line 133)
<https://reviews.apache.org/r/41401/#comment170618>

    Shouldn't this be EVENT.EXTERNAL_TRIGGER?



scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java 
(line 275)
<https://reviews.apache.org/r/41401/#comment170631>

    Add empty properties check too.



scheduler/src/main/java/org/apache/falcon/workflow/engine/OozieDAGEngine.java 
(line 246)
<https://reviews.apache.org/r/41401/#comment170634>

    Why remove these? These shouldn't be present in the first place, right?


- Pallavi Rao


On Dec. 15, 2015, 3:08 p.m., pavan kumar kolamuri wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41401/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2015, 3:08 p.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
> 
>

Reply via email to