falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pallavi Rao" <pallavi....@inmobi.com>
Subject Re: Review Request 41401: Rerun API for Falcon Native Scheduler
Date Fri, 18 Dec 2015 08:41:20 GMT


> On Dec. 18, 2015, 8:01 a.m., Ajay Yadava wrote:
> > After an offline discussion with @pavan kumar kolamuri, here are some key points
which I am not sure are the best way to do it.
> > 
> > 1. A new WorkflowStatus has been added, since we are changing only scheduler, a
new status for workflow(READY) looks odd to me. It is not used anywhere in OozieWorkflowEngine,
so the two engines api are now out of sync.
> > 2. There is a bad coupling between two Enums - InstanceState.STATE, and InstancesResult.WorkflowStatus
e.g. in the following code (It may not be the only place where this is being done, but I am
taking this as I used it for discussion with Pavan Kumar Kolamuri)
> > 
> > ```case RERUN:
> >             executor.rerun(instance, userProps, isForced);
> >             instanceInfo.status =
> >                     InstancesResult.WorkflowStatus.valueOf(STATE_STORE
> >                             .getExecutionInstance(instance.getId()).getCurrentState().name());
> >             break;
> > ```
> > STATE_STORE.getExecutionInstance(instance.getId()).getCurrentState() returns InstanceState.STATE
and it's name is used to generate an enum of type InstancesResult.WorkflowStatus.  Both the
enums have some states which don't make sense for others. 
> > 
> > InstancesResult.WorkflowStatus was earlier used to return workflow status(which
was never ready), now it seems to be used to return status for what earlier were Coordinator
Action status which were. I think we should maintain that separation for the sake of compatibility
across two engines.
> 
> Pallavi Rao wrote:
>     +1 to your 2nd point.
>     
>     Regarding your 1st point, we will need to expose a READY status to users when we
absorb the scheduling. If we do not add READY to WorkflowStatus and introduce a new Status
(lets say, InstanceStatus), the InstanceResult will need to be changed to handle this new
type and then we'll have to worry about how we merge these 2 when we send it to the user.
It is easier to preserve backward-compatibility with the addition of READY.

WorkflowStatus itself is a wrong variable name in the first place, coz., WAITING is not really
not a "Workflow" status either. It is more of a co-ordinator function. It should have ideally
been "InstanceStatus"


- Pallavi


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


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