----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33443/#review101543 -----------------------------------------------------------
common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java (line 411) <https://reviews.apache.org/r/33443/#comment159007> Quation: should addCounterToWF() be called only the counters are enabled in the Feed? common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java (line 434) <https://reviews.apache.org/r/33443/#comment159005> Question: Do we need to Fail the workflow if the counters file is not found? metrics/pom.xml (line 35) <https://reviews.apache.org/r/33443/#comment158999> GENERAL: Have you tested this on Secure Cluster? metrics/src/main/java/org/apache/falcon/job/JobCounters.java (line 73) <https://reviews.apache.org/r/33443/#comment158998> is there a specific reason for not using the following to create FileSystem? HadoopClientFactory.get().createProxiedFileSystem() This one handles authentication of proxy users across the system. metrics/src/main/java/org/apache/falcon/job/JobCountersHandler.java (line 33) <https://reviews.apache.org/r/33443/#comment158969> Instead of returning null, consider throwing an exception (NotSupported or IllegalArgument) so it will be easier to use this class and debug later. metrics/src/main/java/org/apache/falcon/job/ReplicationJobCountersList.java (line 27) <https://reviews.apache.org/r/33443/#comment158977> is it possible for the DistCP/Falcon Replication job to succeed, but still skip files/bytes? Should we grab the other metrics like CopyMapper.Counter.SKIP CopyMapper.Counter.BYTESSKIPPED FAIL (CopyMapper.Counter.FAIL, CopyMapper.Counter.BYTESFAILED) should fail the DistCp/Replication Job and so no need to get them. metrics/src/main/java/org/apache/falcon/job/ReplicationJobCountersList.java (line 48) <https://reviews.apache.org/r/33443/#comment159000> any specific reason not using ReplicationJobCountersList.valueOf()? oozie/src/main/java/org/apache/falcon/oozie/feed/FeedReplicationWorkflowBuilder.java (line 62) <https://reviews.apache.org/r/33443/#comment159001> nit: write defensive? "true".equalIgnoreCase(props.getValue()). so if the value is null for what ever reason we don't NPE. - Venkatesan Ramachandran On Sept. 29, 2015, 10:13 a.m., Peeyush Bishnoi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33443/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2015, 10:13 a.m.) > > > Review request for Falcon. > > > Bugs: FALCON-1102 > https://issues.apache.org/jira/browse/FALCON-1102 > > > Repository: falcon-git > > > Description > ------- > > FALCON-1102: Gather data transfer detail of replication job submitted from > HDFS recipe > > > Diffs > ----- > > > addons/recipes/hdfs-replication/src/main/resources/hdfs-replication-workflow.xml > 942421f > > common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java > 016c622 > common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionArgs.java > 9456fb9 > > common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java > 4454239 > > common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java > 89e8178 > metrics/pom.xml a0358db > metrics/src/main/java/org/apache/falcon/job/FSReplicationCounters.java > PRE-CREATION > metrics/src/main/java/org/apache/falcon/job/JobCounters.java PRE-CREATION > metrics/src/main/java/org/apache/falcon/job/JobCountersHandler.java > PRE-CREATION > metrics/src/main/java/org/apache/falcon/job/JobType.java PRE-CREATION > metrics/src/main/java/org/apache/falcon/job/ReplicationJobCountersList.java > PRE-CREATION > metrics/src/test/java/org/apache/falcon/job/FSReplicationCountersTest.java > PRE-CREATION > > oozie/src/main/java/org/apache/falcon/oozie/feed/FSReplicationWorkflowBuilder.java > b82f4e0 > > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedReplicationWorkflowBuilder.java > a7c19cd > > oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java > cfce1ae > oozie/src/test/resources/feed/fs-replication-feed-counters.xml PRE-CREATION > replication/pom.xml 3cc96fc > replication/src/main/java/org/apache/falcon/replication/FeedReplicator.java > a226058 > > Diff: https://reviews.apache.org/r/33443/diff/ > > > Testing > ------- > > Yes. Unit test cases added. > > > Thanks, > > Peeyush Bishnoi > >
