----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51424/#review147106 -----------------------------------------------------------
Here are my some overall comments. 1. There are no unit tests and user documentation (I know you intend to add it, just recording it for my personal future reference). 2. Effective time is valid only for process update and not for other entities. Please add this validation at both - client and server end. 3. In the current form, this will break instance search. Once the instances are killed, as graph db is not updated with effective time. 4. The design doc mentions parallelism, however I couldn't find any parallelism in jar copying etc. This might be a performance issue like the past 5. Design doc also contains reference to certain features like running only one failed instance between successful instances by passing a command line option called appPath which is missing. In the current form there are several functional and correctness issues as listed below. 1. This feature will break entity sla features as it contains some instances for monitoring sla and they might be affected with entity update(and sla update along with it) 2. Touch command is also a way to do an update, however it is not updated to accept effectiveTime parameter. 3. This feature will also not work in cases where input feed instances may have been deleted by retention and on passing effectiveTime from past those instances will not succeed. 4. Instance search feature will also break by these changes as we are not taking care of existing graph db instances. 5. After several effective updates, it will be hard to keep track of which workflow/code worked with which input/jars. - Praveen Adlakha On Aug. 25, 2016, 9:20 a.m., sandeep samudrala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51424/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2016, 9:20 a.m.) > > > Review request for Falcon and Pallavi Rao. > > > Repository: falcon-git > > > Description > ------- > > Effective Time in Entity Update > > > Diffs > ----- > > cli/src/main/java/org/apache/falcon/cli/FalconCLI.java 0dd11f6 > cli/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java a8aea52 > client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java > 5d6eff5 > client/src/main/java/org/apache/falcon/client/FalconCLIConstants.java > 04f1599 > client/src/main/java/org/apache/falcon/client/FalconClient.java 8f77fad > common/pom.xml 96cb7f5 > common/src/main/java/org/apache/falcon/entity/EntityUtil.java aef1fd5 > common/src/main/java/org/apache/falcon/entity/ProcessHelper.java e563d18 > common/src/main/java/org/apache/falcon/entity/v0/EntityDictionaryUtil.java > PRE-CREATION > common/src/main/java/org/apache/falcon/entity/v0/EntityLibDictionary.java > PRE-CREATION > common/src/main/java/org/apache/falcon/update/UpdateHelper.java 266319f > > common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java > cccbe3b > > common/src/main/java/org/apache/falcon/workflow/engine/AbstractWorkflowEngine.java > 16a1753 > oozie/src/main/java/org/apache/falcon/oozie/ExportWorkflowBuilder.java > af7431a > oozie/src/main/java/org/apache/falcon/oozie/ImportWorkflowBuilder.java > 2d93189 > oozie/src/main/java/org/apache/falcon/oozie/OozieBundleBuilder.java 5f93cc2 > oozie/src/main/java/org/apache/falcon/oozie/OozieCoordinatorBuilder.java > f555b64 > oozie/src/main/java/org/apache/falcon/oozie/OozieEntityBuilder.java a856f8a > > oozie/src/main/java/org/apache/falcon/oozie/OozieOrchestrationWorkflowBuilder.java > 8d45d7a > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedBundleBuilder.java > c758411 > > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedReplicationWorkflowBuilder.java > db647aa > > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedRetentionWorkflowBuilder.java > fd51ed0 > > oozie/src/main/java/org/apache/falcon/oozie/process/HiveProcessWorkflowBuilder.java > 9f9579c > > oozie/src/main/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilder.java > f93a599 > > oozie/src/main/java/org/apache/falcon/oozie/process/PigProcessWorkflowBuilder.java > a1a7c12 > > oozie/src/main/java/org/apache/falcon/oozie/process/ProcessBundleBuilder.java > 6661dd5 > > oozie/src/main/java/org/apache/falcon/oozie/process/ProcessExecutionWorkflowBuilder.java > 20eeffd > > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java > c371d69 > > oozie/src/test/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilderTest.java > 05b513e > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java > aefd699 > > prism/src/main/java/org/apache/falcon/resource/extensions/ExtensionManager.java > 92b5531 > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java > 249c273 > > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java > 9ba62a1 > > shell/src/main/java/org/apache/falcon/shell/commands/FalconEntityCommands.java > 35a6f2a > src/build/checkstyle.xml 292a0a3 > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java 53073f0 > > unit/src/main/java/org/apache/falcon/unit/LocalSchedulableEntityManager.java > 7398c8a > unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 0bc7755 > webapp/src/main/java/org/apache/falcon/resource/ConfigSyncService.java > 7b32bd5 > > webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java > 657ef9e > webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java > 876ada5 > > Diff: https://reviews.apache.org/r/51424/diff/ > > > Testing > ------- > > > Thanks, > > sandeep samudrala > >
