falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Praveen Adlakha <adlakha.prav...@gmail.com>
Subject Re: Review Request 51424: Effective Time in Entity Update
Date Sun, 28 Aug 2016 19:10:18 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message