You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by pavan kumar kolamuri <pa...@gmail.com> on 2015/05/14 07:52:01 UTC
Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/
-----------------------------------------------------------
Review request for Falcon.
Repository: falcon-git
Description
-------
Handle transaction failures in Lineage , Rolling back transactions if they are partial.
Diffs
-----
common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
pom.xml 47718ba
Diff: https://reviews.apache.org/r/34192/diff/
Testing
-------
Thanks,
pavan kumar kolamuri
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/#review83835
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment134915>
Minor nit : Can you please catch NumberFormatException and throw a more readable FalconException. Since you are using int and long, parseInt and parseLong might be better than valueOf.
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment134921>
Perhaps we can add new method entityGraphBuilder.addEntity(Entity entity) which can then call entityGraphBuilder.addClusterEntity or entityGraphBuilder.addFeedEntity etc based on entity type. If we do this, we can avoid switch statement and repetition of code for both add and update scenarios.
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment134922>
Similar suggestion as onAdd, perhaps you can create entityGraphBuilder.updateEntity method
common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java
<https://reviews.apache.org/r/34192/#comment134926>
Can we add a test to simulate transactional failures and verify the rollback? For example, add a process entity with a non-existing input feed. This will fail with IllegalStateException("Feed entity vertex must exist: " + feedName).
Then verify that a vertex was not added for this process.
- Balu Vellanki
On May 14, 2015, 5:52 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34192/
> -----------------------------------------------------------
>
> (Updated May 14, 2015, 5:52 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Handle transaction failures in Lineage , Rolling back transactions if they are partial.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
> common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
> common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
> common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
> pom.xml 47718ba
>
> Diff: https://reviews.apache.org/r/34192/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/#review85329
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment136884>
Sure will do it.
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment136885>
Makes sense will do it. Switch case is still required right ?
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment136886>
Sure will do it
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment136887>
Yes pallavi will do it.
common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java
<https://reviews.apache.org/r/34192/#comment136888>
Thanks . Will do it
- pavan kumar kolamuri
On May 14, 2015, 5:52 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34192/
> -----------------------------------------------------------
>
> (Updated May 14, 2015, 5:52 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Handle transaction failures in Lineage , Rolling back transactions if they are partial.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
> common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
> common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
> common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
> pom.xml 47718ba
>
> Diff: https://reviews.apache.org/r/34192/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/#review83901
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment134979>
Agree with Balu here, all the methods, onAdd, onChange and onSuccess can be modified to make it less repetitive. Either create new methods in EntityGraphBuilder. Or, put the switch-case inside the method of the anonymous class. That way, the creation of anonymous class doesn't get repeated.
- Pallavi Rao
On May 14, 2015, 5:52 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34192/
> -----------------------------------------------------------
>
> (Updated May 14, 2015, 5:52 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Handle transaction failures in Lineage , Rolling back transactions if they are partial.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
> common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
> common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
> common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
> pom.xml 47718ba
>
> Diff: https://reviews.apache.org/r/34192/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by Ajay Yadava <aj...@gmail.com>.
> On May 28, 2015, 12:07 p.m., Ajay Yadava wrote:
> > pom.xml, line 935
> > <https://reviews.apache.org/r/34192/diff/2/?file=972530#file972530line935>
> >
> > Any reason for not moving to 2.6.0?
Something in my browser went haywire and a week old review got mixed with the current review. Please ignore. The patch looks good to me +1. There is a separate JIRA for upgrading, we will handle the upgrade there.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/#review83719
-----------------------------------------------------------
On May 27, 2015, 9:11 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34192/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 9:11 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Handle transaction failures in Lineage , Rolling back transactions if they are partial.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
> common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
> common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
> common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
> pom.xml 47718ba
>
> Diff: https://reviews.apache.org/r/34192/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/#review83719
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment134768>
We should add these properties to startup.properties so that users know that they can configure these properties.
pom.xml
<https://reviews.apache.org/r/34192/#comment134766>
Any reason for not moving to latest 2.6.0?
pom.xml
<https://reviews.apache.org/r/34192/#comment137131>
Any reason for not moving to 2.6.0?
- Ajay Yadava
On May 27, 2015, 9:11 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34192/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 9:11 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Handle transaction failures in Lineage , Rolling back transactions if they are partial.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
> common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
> common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
> common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
> pom.xml 47718ba
>
> Diff: https://reviews.apache.org/r/34192/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/
-----------------------------------------------------------
(Updated June 3, 2015, 7:23 a.m.)
Review request for Falcon.
Changes
-------
Added graph retry properties to startup.properties
Repository: falcon-git
Description
-------
Handle transaction failures in Lineage , Rolling back transactions if they are partial.
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
common/src/main/resources/startup.properties 65746d7
common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
pom.xml 47718ba
src/conf/startup.properties 64a7d27
Diff: https://reviews.apache.org/r/34192/diff/
Testing
-------
Thanks,
pavan kumar kolamuri
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by Ajay Yadava <aj...@gmail.com>.
> On May 27, 2015, 1:49 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java, line 78
> > <https://reviews.apache.org/r/34192/diff/2/?file=972528#file972528line78>
> >
> > Minor nit - Please use "private final int" and change the name to match checkstyle rules.
It's being populated in init() instead of the constructor so final won't work.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/#review85359
-----------------------------------------------------------
On May 27, 2015, 9:11 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34192/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 9:11 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Handle transaction failures in Lineage , Rolling back transactions if they are partial.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
> common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
> common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
> common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
> pom.xml 47718ba
>
> Diff: https://reviews.apache.org/r/34192/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/#review85359
-----------------------------------------------------------
The rest of the patch looks good to me.
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java
<https://reviews.apache.org/r/34192/#comment136904>
Minor nit - Please use "private final int" and change the name to match checkstyle rules.
- Balu Vellanki
On May 27, 2015, 9:11 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34192/
> -----------------------------------------------------------
>
> (Updated May 27, 2015, 9:11 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Handle transaction failures in Lineage , Rolling back transactions if they are partial.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
> common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
> common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
> common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
> pom.xml 47718ba
>
> Diff: https://reviews.apache.org/r/34192/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 34192: FALCON-1060 Handle transaction failures in
Lineage
Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34192/
-----------------------------------------------------------
(Updated May 27, 2015, 9:11 a.m.)
Review request for Falcon.
Changes
-------
Handle transaction failures in Lineage , added testcases and addressed comments
Repository: falcon-git
Description
-------
Handle transaction failures in Lineage , Rolling back transactions if they are partial.
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/metadata/EntityRelationshipGraphBuilder.java d90f7ec
common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java 2288443
common/src/main/java/org/apache/falcon/metadata/MetadataMappingService.java 9137fe0
common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java 11d27fe
pom.xml 47718ba
Diff: https://reviews.apache.org/r/34192/diff/
Testing
-------
Thanks,
pavan kumar kolamuri