> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/pom.xml, line 57
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023467#file1023467line57>
> >
> >     Why would we have oozie-adaptor dependency in Scheduler ?

The Builders are still part of that package and hence the dependency. Will take 
up the refactor a little later.


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java, 
> > line 104
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023473#file1023473line104>
> >
> >     How do we handle re-runs in this world ? Will ExecutionInstance be 
> > different for different runs ?

Yes. The plan is to have have multiple instances. So, attempt will be part of 
an instance's identity. Will be addressed in FALCON-1512.


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java,
> >  line 120
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023476#file1023476line120>
> >
> >     DI ?

The services are actually registered as services via the startup properties. 
The NotificationServicesRegistry is more like a convenience class.


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java,
> >  line 121
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023476#file1023476line121>
> >
> >     Can't quite get where the feed EL's being evaluated and paths being 
> > resolved.

That will come as part of FALCON-1230


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java,
> >  line 126
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023476#file1023476line126>
> >
> >     Not sure why data availability notification is being registered against 
> > ServicesRegistry.

The services are actually registered as services via the startup properties. 
The NotificationServicesRegistry is more like a convenience class.


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java, 
> > line 141
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023477#file1023477line141>
> >
> >     Would this not be common for both feed & process executors ?

Methods may be similar, but, objects used may be different. Will pull these 
methods up as and when we add support for Feed.


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/src/main/java/org/apache/falcon/notification/service/FalconNotificationService.java,
> >  line 43
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023478#file1023478line43>
> >
> >     Why is unregister() is on a different entity other than 
> > NotificationRequest

Don't want to be keeping the bulky Request object around as many fields aren't 
really required for unregister.


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/src/main/java/org/apache/falcon/notification/service/impl/JobCompletionService.java,
> >  line 72
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023486#file1023486line72>
> >
> >     If the job is not complete, is the notification request ignored ? Can't 
> > see any polling to check for job completion. Is this dependent on job end 
> > notifications ? Are we expecting that to be resilient to failures ?

This class uses WorkflowJobEndService and listens to notifications from Oozie 
for job status change (checked in as part of FALCON-1231). Hence, no polling 
required.


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan 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>
> >
> >     Is there a possiblity of removing running instances when the cache runs 
> > out of space. Am unclear on what the behavior would be

The cache is a loading cache.. so, even if instances are evicted, any attempt 
to retrieve them will retrieve it from state store.


> On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote:
> > scheduler/src/main/java/org/apache/falcon/notification/service/ServicesRegistry.java,
> >  line 63
> > <https://reviews.apache.org/r/35724/diff/2/?file=1023479#file1023479line63>
> >
> >     Can the NotificationService be registered with the general Services 
> > framework and accessed through Services::get()

The notification services are already registered with the general Services 
framework. The NotificationServicesRegistry is more a convenience class to 
access just the Notification Services.


- Pallavi


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


On Oct. 13, 2015, 11:53 a.m., Pallavi Rao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35724/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2015, 11:53 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/src/main/java/org/apache/falcon/entity/EntityUtil.java 3ab9339 
>   pom.xml 54e6cd1 
>   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/execution/SchedulerUtil.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/NotificationServicesRegistry.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/AlarmService.java
>  PRE-CREATION 
>   
> scheduler/src/main/java/org/apache/falcon/notification/service/impl/DataAvailabilityService.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/request/AlarmRequest.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/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/DAGEngineFactory.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/execution/MockDAGEngine.java 
> PRE-CREATION 
>   scheduler/src/test/java/org/apache/falcon/execution/SchedulerUtilTest.java 
> PRE-CREATION 
>   
> scheduler/src/test/java/org/apache/falcon/notification/service/AlarmServiceTest.java
>  PRE-CREATION 
>   
> scheduler/src/test/java/org/apache/falcon/notification/service/SchedulerServiceTest.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 
>   webapp/pom.xml e63aa44 
> 
> Diff: https://reviews.apache.org/r/35724/diff/
> 
> 
> Testing
> -------
> 
> New UTs have been added.
> 
> 
> Thanks,
> 
> Pallavi Rao
> 
>

Reply via email to