falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Venkatesan Ramachandran" <me.venk...@gmail.com>
Subject Re: Review Request 33443: FALCON-1102: Gather data transfer detail of replication job submitted from HDFS recipe
Date Tue, 06 Oct 2015 01:15:42 GMT

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


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