falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From shwetha...@inmobi.com
Subject Re: Review Request 22192: FALCON-369 Refactor workflow builder
Date Tue, 17 Jun 2014 07:27:47 GMT


> On June 16, 2014, 2:43 p.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 234
> > <https://reviews.apache.org/r/22192/diff/1/?file=602458#file602458line234>
> >
> >     This doesn't seem to be related to this refactoring. Can we exclude these changes
from here?

Feed replication with hcat doesn't work without this. I needed that to work to verify that
re-factoring works. So, I can't remove this


> On June 16, 2014, 2:43 p.m., Srikanth Sundarrajan wrote:
> > oozie/src/main/java/org/apache/falcon/oozie/FeedBundleBuilder.java, line 51
> > <https://reviews.apache.org/r/22192/diff/1/?file=602479#file602479line51>
> >
> >     Since this is internal to oozie, can't we have a better representation of the
coord to build the bundle out ? Seems like properties are being accumulated in a single property
bag across coords. Keeping them separated out might help in other ways if they are they are
independently needed.

Yes, I will need that for dry run. Will do


> On June 16, 2014, 2:43 p.m., Srikanth Sundarrajan wrote:
> > oozie/src/main/java/org/apache/falcon/oozie/FeedReplicationCoordinatorBuilder.java,
line 416
> > <https://reviews.apache.org/r/22192/diff/1/?file=602480#file602480line416>
> >
> >     Have assumed that functions are largely refactored and moved into newer classes
and not re-written. Please let me if that is not true, then will look through these a bit
more carefully. Currently concentrating only on the uber class collaborations/couplings

Yes, the functions are just moved/dropped when there is already one available. Haven't changed
the contents of functions


> On June 16, 2014, 2:43 p.m., Srikanth Sundarrajan wrote:
> > oozie/src/main/java/org/apache/falcon/oozie/FeedBundleBuilder.java, line 58
> > <https://reviews.apache.org/r/22192/diff/1/?file=602479#file602479line58>
> >
> >     Is doBuild a preparatory step ? Do you also want to copySharedLibs here ? Should
this be deferred to schedule function ?

I should rename doBuild to getCoords() as thats all it should do
Moving copySharedLibs to schedule will split the responsibility and the logic will spread
across different owners. Let the builders prepare everything required (including creating
wf/coord/bundle and copying jars required)


> On June 16, 2014, 2:43 p.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 438
> > <https://reviews.apache.org/r/22192/diff/1/?file=602459#file602459line438>
> >
> >     Should this be called LifeCycle Name Builder instead of Workflow Name Builder
?

Can be done as part of Lifecycle addition


- shwethags


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


On June 3, 2014, 6:21 a.m., shwethags wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22192/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 6:21 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-369
>     https://issues.apache.org/jira/browse/FALCON-369
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Refactoring of workflow builder
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 37a05cb 
>   common/src/main/java/org/apache/falcon/entity/EntityUtil.java b4bc07d 
>   common/src/main/java/org/apache/falcon/entity/FeedHelper.java 44d8d01 
>   common/src/main/java/org/apache/falcon/entity/ProcessHelper.java ece8982 
>   common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java 1f9a8c8 
>   common/src/test/java/org/apache/falcon/entity/CatalogStorageTest.java 972066d 
>   feed/pom.xml ab82b77 
>   feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java 6d36840

>   feed/src/main/resources/config/coordinator/replication-coordinator.xml 693b0bd 
>   feed/src/main/resources/config/workflow/falcon-table-export.hql 37fd1b7 
>   feed/src/main/resources/config/workflow/falcon-table-import.hql 653d580 
>   feed/src/main/resources/config/workflow/replication-workflow.xml 0748acf 
>   feed/src/main/resources/config/workflow/retention-workflow.xml 5138865 
>   feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java 5d6879a

>   feed/src/test/resources/feed.xml 4da222e 
>   feed/src/test/resources/fs-replication-feed.xml bada507 
>   feed/src/test/resources/src-cluster.xml 730f8d2 
>   feed/src/test/resources/table-replication-feed.xml 4c610f6 
>   feed/src/test/resources/trg-cluster-alpha.xml 1fb07cb 
>   feed/src/test/resources/trg-cluster-beta.xml 0bf0bcd 
>   feed/src/test/resources/trg-cluster.xml 8260fda 
>   oozie/src/main/java/org/apache/falcon/oozie/FeedBundleBuilder.java PRE-CREATION 
>   oozie/src/main/java/org/apache/falcon/oozie/FeedReplicationCoordinatorBuilder.java
PRE-CREATION 
>   oozie/src/main/java/org/apache/falcon/oozie/FeedReplicationWorkflowBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/FeedRetentionCoordinatorBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/FeedRetentionWorkflowBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/HiveProcessWorkflowBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/OozieBundleBuilder.java PRE-CREATION 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieCoordinatorBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/OozieEntityBuilder.java PRE-CREATION 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieOrchestrationWorkflowBuilder.java
PRE-CREATION 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieProcessWorkflowBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/PigProcessWorkflowBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/ProcessBundleBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/oozie/ProcessExecutionCoordinatorBuilder.java
PRE-CREATION 
>   oozie/src/main/java/org/apache/falcon/oozie/ProcessExecutionWorkflowBuilder.java PRE-CREATION

>   oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java 7d84938 
>   oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java 34192c0

>   oozie/src/main/resources/coordinator/replication-coordinator.xml PRE-CREATION 
>   oozie/src/main/resources/workflow/falcon-table-export.hql PRE-CREATION 
>   oozie/src/main/resources/workflow/falcon-table-import.hql PRE-CREATION 
>   oozie/src/main/resources/workflow/process-parent-workflow.xml PRE-CREATION 
>   oozie/src/main/resources/workflow/replication-workflow.xml PRE-CREATION 
>   oozie/src/main/resources/workflow/retention-workflow.xml PRE-CREATION 
>   oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java
PRE-CREATION 
>   oozie/src/test/java/org/apache/falcon/oozie/process/AbstractTestBase.java PRE-CREATION

>   oozie/src/test/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilderTest.java
PRE-CREATION 
>   oozie/src/test/resources/config/cluster/cluster-0.1.xml PRE-CREATION 
>   oozie/src/test/resources/config/feed/feed-0.1.xml PRE-CREATION 
>   oozie/src/test/resources/config/feed/hive-table-feed-out.xml PRE-CREATION 
>   oozie/src/test/resources/config/feed/hive-table-feed.xml PRE-CREATION 
>   oozie/src/test/resources/config/late/late-cluster.xml PRE-CREATION 
>   oozie/src/test/resources/config/late/late-feed1.xml PRE-CREATION 
>   oozie/src/test/resources/config/late/late-feed2.xml PRE-CREATION 
>   oozie/src/test/resources/config/late/late-feed3.xml PRE-CREATION 
>   oozie/src/test/resources/config/late/late-process1.xml PRE-CREATION 
>   oozie/src/test/resources/config/late/late-process2.xml PRE-CREATION 
>   oozie/src/test/resources/config/process/dumb-hive-process.xml PRE-CREATION 
>   oozie/src/test/resources/config/process/dumb-process.xml PRE-CREATION 
>   oozie/src/test/resources/config/process/hive-process-FSInputFeed.xml PRE-CREATION 
>   oozie/src/test/resources/config/process/hive-process-FSOutputFeed.xml PRE-CREATION

>   oozie/src/test/resources/config/process/hive-process.xml PRE-CREATION 
>   oozie/src/test/resources/config/process/pig-process-0.1.xml PRE-CREATION 
>   oozie/src/test/resources/config/process/pig-process-table.xml PRE-CREATION 
>   oozie/src/test/resources/config/process/process-0.1.xml PRE-CREATION 
>   oozie/src/test/resources/feed/feed.xml PRE-CREATION 
>   oozie/src/test/resources/feed/fs-replication-feed.xml PRE-CREATION 
>   oozie/src/test/resources/feed/src-cluster.xml PRE-CREATION 
>   oozie/src/test/resources/feed/table-replication-feed.xml PRE-CREATION 
>   oozie/src/test/resources/feed/trg-cluster-alpha.xml PRE-CREATION 
>   oozie/src/test/resources/feed/trg-cluster-beta.xml PRE-CREATION 
>   oozie/src/test/resources/feed/trg-cluster.xml PRE-CREATION 
>   pom.xml 7130352 
>   process/pom.xml c1ee74d 
>   process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java 70aeebd

>   process/src/main/resources/config/workflow/process-parent-workflow.xml 4a2495c 
>   process/src/test/java/org/apache/falcon/converter/AbstractTestBase.java 2c7ee8b 
>   process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java
2522ca3 
>   process/src/test/resources/config/cluster/cluster-0.1.xml 032cc77 
>   process/src/test/resources/config/feed/feed-0.1.xml fb9b707 
>   process/src/test/resources/config/feed/hive-table-feed-out.xml bd93a01 
>   process/src/test/resources/config/feed/hive-table-feed.xml 66d0742 
>   process/src/test/resources/config/late/late-cluster.xml ac0817f 
>   process/src/test/resources/config/late/late-feed1.xml c500c4c 
>   process/src/test/resources/config/late/late-feed2.xml 6ccffe2 
>   process/src/test/resources/config/late/late-feed3.xml 239f140 
>   process/src/test/resources/config/late/late-process1.xml aba5525 
>   process/src/test/resources/config/late/late-process2.xml bc507ad 
>   process/src/test/resources/config/process/dumb-hive-process.xml c504074 
>   process/src/test/resources/config/process/dumb-process.xml b71f089 
>   process/src/test/resources/config/process/hive-process-FSInputFeed.xml d871377 
>   process/src/test/resources/config/process/hive-process-FSOutputFeed.xml 23d96c3 
>   process/src/test/resources/config/process/hive-process.xml 4dac8e9 
>   process/src/test/resources/config/process/pig-process-0.1.xml 318f0da 
>   process/src/test/resources/config/process/pig-process-table.xml 37aca10 
>   process/src/test/resources/config/process/process-0.1.xml 6148441 
>   src/main/examples/app/hive/wordcount.hql 62d4f32 
>   src/main/examples/app/pig/hcat-wordcount.pig 023ce3d 
>   src/main/examples/data/hcat-generate.sh 957710a 
>   src/main/examples/entity/filesystem/pig-process.xml 0dbb558 
>   src/main/examples/entity/filesystem/replication-feed.xml PRE-CREATION 
>   src/main/examples/entity/filesystem/standalone-cluster.xml 3c3c9f1 
>   src/main/examples/entity/filesystem/standalone-target-cluster.xml PRE-CREATION 
>   src/main/examples/entity/hcat/hcat-in-feed.xml 77f70db 
>   src/main/examples/entity/hcat/hcat-out-feed.xml f09b2ed 
>   src/main/examples/entity/hcat/hcat-pig-process.xml 753e5b0 
>   src/main/examples/entity/hcat/hcat-replication-feed.xml PRE-CREATION 
>   src/main/examples/entity/hcat/hcat-standalone-cluster.xml 30f0cb1 
>   src/main/examples/entity/hcat/hcat-standalone-target-cluster.xml PRE-CREATION 
>   src/main/examples/entity/hcat/hive-process.xml 0f3c540 
>   webapp/pom.xml d1e1ec1 
> 
> Diff: https://reviews.apache.org/r/22192/diff/
> 
> 
> Testing
> -------
> 
> existings UTs + tested end to end the following cases:
> 1. feed retention for filesystem and hcat
> 2. feed replication for filesystem
> 3. process with oozie and pig engine for filesystem
> 4. process with hive and pig engine for hcat
> 
> 
> Thanks,
> 
> shwethags
> 
>


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