> On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/pom.xml, line 73 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023467#file1023467line73> > > > > Snapshots should not be used right ?
Yes. My bad. Fixed it. > On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/src/main/java/org/apache/falcon/execution/EntityExecutor.java, > > line 33 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023472#file1023472line33> > > > > Should this extend InstanceChangeHandler also instead of process > > executor ? since FeedExecutor might also need this Good catch. > On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java, > > line 449 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023477#file1023477line449> > > > > Shouldn't be order reverse ? First invalidate and then destroy right Once we invalidate (the cache), the executor will not hold the reference to that instance. Hence, it is destroy and then invalidate. > On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java, > > line 184 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023487#file1023487line184> > > > > Few variables are unused like numRetries,retryDelayInMillis. Are these > > variables used for future todo's ? Those were intended to be used for failure scenarios when for some reason we couldn't schedule it on the DAG Engine. Since the implementation for that incomplete, I'll remove them. Will introduce them later when we implement failure handling. > On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java, > > line 220 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023487#file1023487line220> > > > > I don't think this method is called. Means the case where > > awaitedInstances queue is full is missing. Can you please check The awaitedInstances cache has a size limit. This method will be called by the cacheBuilder library, when any instance is removed from the cache because it exceeded the max. size. > On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/TimeNotificationService.java, > > line 65 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023488#file1023488line65> > > > > Shouldn't there be deletion also for notifications once that > > notification is completed. You are right. Added it in the unregisterForNotification method. > On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java, > > line 58 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023477#file1023477line58> > > > > I have checked the flow , flow from Instance Event Trigger to Instance > > Event Conditions Met is missing when there are awaited Conditions. Please > > correct me if i am missing something Any awaited predicates will be handled by the notify(Event event) of ProcessExecutionInstance > On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/src/test/java/org/apache/falcon/execution/FalconExecutionServiceTest.java, > > line 165 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023508#file1023508line165> > > > > In this test case there are places we should get the instance value > > from StateStore to get the updated status Believe you have already made these changes as part of state store impl. Thanks. > On Aug. 28, 2015, 12:14 p.m., pavan kumar kolamuri wrote: > > scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java, > > line 34 > > <https://reviews.apache.org/r/35724/diff/2/?file=1023473#file1023473line34> > > > > IMO, as exection instance is an object to be stored in Data store. If > > we add InstanceExecutor handles all notifications it will be good. Agreed. But, I want to handle that in a separate JIRA that refactors this code so it caters well to the state store too. - Pallavi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35724/#review96853 ----------------------------------------------------------- On July 28, 2015, 11:07 a.m., Pallavi Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35724/ > ----------------------------------------------------------- > > (Updated July 28, 2015, 11:07 a.m.) > > > Review request for Falcon and Srikanth Sundarrajan. > > > Bugs: FALCON-1213 > https://issues.apache.org/jira/browse/FALCON-1213 > > > Repository: falcon-git > > > Description > ------- > > The patch has the basic framework for the scheduler. Each of the individual > service needs to be implemented completely and will be done as separate > JIRAs. The intention of the patch is ensure the base framework satisfies all > use cases and get any early feedback in terms of course correction. > > > Diffs > ----- > > common/pom.xml 36de1f5 > common/src/main/java/org/apache/falcon/entity/EntityUtil.java b86d9d7 > > common/src/main/java/org/apache/falcon/workflow/WorkflowJobEndNotificationService.java > c4f8843 > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java cfdc84d > pom.xml 31997e8 > scheduler/pom.xml PRE-CREATION > scheduler/src/main/java/org/apache/falcon/exception/DAGEngineException.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/exception/InvalidStateTransitionException.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/exception/ServiceException.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/exception/StateStoreException.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/execution/EntityExecutor.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/execution/FalconExecutionService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/execution/NotificationHandler.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/FalconNotificationService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/ServicesRegistry.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/DataEvent.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/Event.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/JobCompletedEvent.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/JobScheduledEvent.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/event/TimeElapsedEvent.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/DataNotificationService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/JobCompletionService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/impl/TimeNotificationService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/DataNotificationRequest.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/JobCompletionNotificationRequest.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/JobScheduleNotificationRequest.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/NotificationRequest.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/notification/service/request/TimeNotificationRequest.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/predicate/Predicate.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/EntityState.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/EntityStateChangeHandler.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/ID.java PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/InstanceState.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/InstanceStateChangeHandler.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/StateMachine.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/StateService.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/AbstractStateStore.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/store/StateStore.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/workflow/engine/DAGEngine.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/workflow/engine/OozieDAGEngine.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/execution/FalconExecutionServiceTest.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/notification/service/SchedulerServiceTest.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/notification/service/TimeNotificationServiceTest.java > PRE-CREATION > scheduler/src/test/java/org/apache/falcon/predicate/PredicateTest.java > PRE-CREATION > scheduler/src/test/java/org/apache/falcon/state/EntityStateServiceTest.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/state/InstanceStateServiceTest.java > PRE-CREATION > scheduler/src/test/resources/config/cluster/cluster-0.1.xml PRE-CREATION > scheduler/src/test/resources/config/feed/feed-0.1.xml PRE-CREATION > scheduler/src/test/resources/config/process/process-0.1.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/35724/diff/ > > > Testing > ------- > > New UTs have been added. > > > Thanks, > > Pallavi Rao > >
