Return-Path: X-Original-To: apmail-falcon-dev-archive@minotaur.apache.org Delivered-To: apmail-falcon-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id EDABA188E5 for ; Mon, 19 Oct 2015 07:15:44 +0000 (UTC) Received: (qmail 12919 invoked by uid 500); 19 Oct 2015 07:15:44 -0000 Delivered-To: apmail-falcon-dev-archive@falcon.apache.org Received: (qmail 12879 invoked by uid 500); 19 Oct 2015 07:15:44 -0000 Mailing-List: contact dev-help@falcon.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@falcon.apache.org Delivered-To: mailing list dev@falcon.apache.org Received: (qmail 12865 invoked by uid 99); 19 Oct 2015 07:15:44 -0000 Received: from Unknown (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 19 Oct 2015 07:15:44 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 14F9FC23D7 for ; Mon, 19 Oct 2015 07:15:44 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -1.029 X-Spam-Level: X-Spam-Status: No, score=-1.029 tagged_above=-999 required=6.31 tests=[HEADER_FROM_DIFFERENT_DOMAINS=0.001, HTML_MESSAGE=3, KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, T_RP_MATCHES_RCVD=-0.01] autolearn=disabled Received: from mx1-us-west.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id qP3bDw8aZvdE for ; Mon, 19 Oct 2015 07:15:39 +0000 (UTC) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx1-us-west.apache.org (ASF Mail Server at mx1-us-west.apache.org) with SMTP id 199112118F for ; Mon, 19 Oct 2015 07:15:39 +0000 (UTC) Received: (qmail 12811 invoked by uid 99); 19 Oct 2015 07:15:38 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 19 Oct 2015 07:15:38 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id AC7471DD972; Mon, 19 Oct 2015 07:15:36 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============2177365536577669592==" MIME-Version: 1.0 Subject: Re: Review Request 35724: Base framework of the native scheduler From: "Pallavi Rao" To: "Srikanth Sundarrajan" Cc: "Falcon" , "Pallavi Rao" Date: Mon, 19 Oct 2015 07:15:36 -0000 Message-ID: <20151019071536.32263.72405@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Pallavi Rao" X-ReviewGroup: Falcon X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/35724/ X-Sender: "Pallavi Rao" References: <20150914184325.3774.96486@reviews.apache.org> In-Reply-To: <20150914184325.3774.96486@reviews.apache.org> Reply-To: "Pallavi Rao" X-ReviewRequest-Repository: falcon-git --===============2177365536577669592== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Sept. 14, 2015, 6:43 p.m., Srikanth Sundarrajan wrote: > > scheduler/pom.xml, line 57 > > > > > > 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 > > > > > > 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 > > > > > > 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 > > > > > > 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 > > > > > > 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 > > > > > > 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 > > > > > > 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 > > > > > > 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 > > > > > > 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 > > > > > > 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 > > --===============2177365536577669592==--