falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From shwetha...@inmobi.com
Subject Re: Review Request 16511: Process update for wf changes
Date Tue, 31 Dec 2013 06:24:15 GMT


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 565
> > <https://reviews.apache.org/r/16511/diff/1/?file=404222#file404222line565>
> >
> >     Why note use ClusterHelper.getFileSystem

Added ClusterHelper.getFileSystem later, so missed it. Done


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 581
> > <https://reviews.apache.org/r/16511/diff/1/?file=404222#file404222line581>
> >
> >     Should this be modification or creation time ?

Both will work fine as the staging directory is never updated. FileStatus doesn't expose creation
time


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 601
> > <https://reviews.apache.org/r/16511/diff/1/?file=404222#file404222line601>
> >
> >     I would prefer lesser dependency on MR. Can we define our constant?

done


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/update/UpdateHelper.java, line 115
> > <https://reviews.apache.org/r/16511/diff/1/?file=404223#file404223line115>
> >
> >     ClusterHelper ?

done


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/update/UpdateHelper.java, line 123
> > <https://reviews.apache.org/r/16511/diff/1/?file=404223#file404223line123>
> >
> >     Why is getPaths also copying file ? The responsibility of the function is to
simply check if workflow is updated, if so why is file copy necessary ? This is not clear

During schedule, the same is used to copy the user wf/lib to staging directory, to avoid duplicate
traversals. Will rename


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/update/UpdateHelper.java, line 149
> > <https://reviews.apache.org/r/16511/diff/1/?file=404223#file404223line149>
> >
> >     This should perhaps be done recursively

done


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java,
line 111
> > <https://reviews.apache.org/r/16511/diff/1/?file=404236#file404236line111>
> >
> >     Is there a check to make sure that deletion of a entity and readdition doesn't
cause issues.

Yes, in findBundles(). Not sure why this check was added - this creates new coord if all instances
of old coord are killed, which results in over-lapping coords and the assumption that there
will be one coord action/wf per instance is not valid anymore


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java,
line 141
> > <https://reviews.apache.org/r/16511/diff/1/?file=404236#file404236line141>
> >
> >     If possible please remove references to FOC.SUCCESS constant.

done


> On Dec. 30, 2013, 3:58 p.m., Srikanth Sundarrajan wrote:
> > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java,
line 246
> > <https://reviews.apache.org/r/16511/diff/1/?file=404236#file404236line246>
> >
> >     There seems to be a different logic than before and this seems to be a different
problem also being resolved. Can this be elaborated in the description so that we know what
is being fixed.

updated comment


- shwethags


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


On Dec. 30, 2013, 7:35 a.m., shwethags wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16511/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2013, 7:35 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-206
>     https://issues.apache.org/jira/browse/FALCON-206
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> At process schedule, process mapper creates a checksum file that contains the list of
user wf/lib files and their checksum. The process mapper also copies the user wf and lib to
staging directory(where parent wf and coord xml are stored). The parent workflow references
the user workflow copied to staging dir. 
> Process update updates in oozie only if entity is updated or user wf/lib is updated.
> With this change, when user wf/lib is updated, the currently running instances will not
fail. This will also make adding new input to process easier (which includes updating process
and workflow.xml)
> Limitations: user oozie workflow should use only relative paths for libs/pig scripts/hive
scripts
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/entity/ClusterHelper.java e332aba 
>   common/src/main/java/org/apache/falcon/entity/EntityUtil.java ba80cac 
>   common/src/main/java/org/apache/falcon/update/UpdateHelper.java fc69933 
>   common/src/main/resources/log4j.xml 734d17c 
>   common/src/main/resources/startup.properties 5473f5d 
>   common/src/test/java/org/apache/falcon/entity/parser/FeedUpdateTest.java f39f300 
>   common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java e656772

>   common/src/test/java/org/apache/falcon/update/UpdateHelperTest.java 42bcee0 
>   common/src/test/resources/config/process/process-0.1.xml bb5cd35 
>   common/src/test/resources/config/process/process-0.2.xml c4cd83e 
>   common/src/test/resources/config/process/process-table.xml 1d6a8f0 
>   feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java 7b9095f

>   oozie/src/main/java/org/apache/falcon/service/SharedLibraryHostingService.java c47ec01

>   oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java 1978c53 
>   oozie/src/main/java/org/apache/falcon/workflow/engine/OozieHouseKeepingService.java
7e2f8a4 
>   oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java 1ed1ff7

>   prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java 7ec6cd1 
>   prism/src/main/resources/log4j.xml ac1d9e4 
>   process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java 8749f07 
>   process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java 1329733

>   process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java 7d5f4d1

>   src/bin/service-start.sh 430bb1a 
>   src/conf/log4j.xml 0b28ddd 
>   src/conf/startup.properties 69613f6 
>   webapp/src/main/resources/log4j.xml 6790576 
>   webapp/src/test/java/org/apache/falcon/logging/LogMoverIT.java 24c2959 
>   webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java cb2fcbb

>   webapp/src/test/java/org/apache/falcon/resource/TestContext.java d224f90 
>   webapp/src/test/java/org/apache/falcon/util/OozieTestUtils.java 690fb6b 
> 
> Diff: https://reviews.apache.org/r/16511/diff/
> 
> 
> Testing
> -------
> 
> UT, tested with embedded deployment
> 
> 
> Thanks,
> 
> shwethags
> 
>


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