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



scheduler/pom.xml (line 73)
<https://reviews.apache.org/r/35724/#comment152533>

    Snapshots should not be used right ?



scheduler/src/main/java/org/apache/falcon/execution/EntityExecutor.java (line 
33)
<https://reviews.apache.org/r/35724/#comment152534>

    Should this extend InstanceChangeHandler also instead of process executor ? 
since FeedExecutor might also need this



scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java 
(line 34)
<https://reviews.apache.org/r/35724/#comment152537>

    IMO, as exection instance is an object to be stored in Data store. If we 
add InstanceExecutor handles all notifications it will be good.



scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java (line 
58)
<https://reviews.apache.org/r/35724/#comment152539>

    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



scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutor.java (line 
449)
<https://reviews.apache.org/r/35724/#comment152540>

    Shouldn't be order reverse ? First invalidate and then destroy right



scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java
 (line 184)
<https://reviews.apache.org/r/35724/#comment152543>

    Few variables are unused like numRetries,retryDelayInMillis. Are these 
variables used for future todo's ?



scheduler/src/main/java/org/apache/falcon/notification/service/impl/SchedulerService.java
 (line 220)
<https://reviews.apache.org/r/35724/#comment152542>

    I don't think this method is called. Means the case where awaitedInstances 
queue is full is missing. Can you please check



scheduler/src/main/java/org/apache/falcon/notification/service/impl/TimeNotificationService.java
 (line 65)
<https://reviews.apache.org/r/35724/#comment152545>

    Shouldn't there be deletion also for notifications once that notification 
is completed.



scheduler/src/test/java/org/apache/falcon/execution/FalconExecutionServiceTest.java
 (line 165)
<https://reviews.apache.org/r/35724/#comment152538>

    In this test case there are places we should get the instance value from 
StateStore to get the updated status


- pavan kumar kolamuri


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
> 
>

Reply via email to