> 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. > > Pallavi Rao wrote: > 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"
Makes sense. But since we will now be showing Users a new status "READY" we should document it(all?) with detailed explanation. - Ajay ----------------------------------------------------------- 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 > >
