You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Balu Vellanki <bv...@hortonworks.com> on 2016/01/20 23:43:52 UTC
Review Request 42574: Support feed listing for CatalogStorage
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/
-----------------------------------------------------------
Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
Bugs: Falcon-763
https://issues.apache.org/jira/browse/Falcon-763
Repository: falcon-git
Description
-------
Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
Diffs
-----
common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
Diff: https://reviews.apache.org/r/42574/diff/
Testing
-------
Added unit test, integration test and tested this end2end
Thanks,
Balu Vellanki
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Ying Zheng <yz...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review115901
-----------------------------------------------------------
Ship it!
LGTM + one nit pick.
common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java (line 299)
<https://reviews.apache.org/r/42574/#comment176952>
nitpick: You don't need this since the value of "size" is by defualt -1 during initialization.
- Ying Zheng
On Jan. 20, 2016, 10:43 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2016, 10:43 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Balu Vellanki <bv...@hortonworks.com>.
> On Jan. 22, 2016, 3:34 p.m., Peeyush Bishnoi wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 400
> > <https://reviews.apache.org/r/42574/diff/1/?file=1204002#file1204002line400>
> >
> > What will happen if exception message from e.getMessage() does not startwith with message from PARTITION_DOES_NOT_EXIST. Or exception message is different than PARTITION_DOES_NOT_EXIST.
Then Falcon throws the exception since Falcon was unable to fetch the actual information related to the partition.
- Balu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review115849
-----------------------------------------------------------
On Jan. 20, 2016, 10:43 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2016, 10:43 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review115849
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java (line 400)
<https://reviews.apache.org/r/42574/#comment176913>
What will happen if exception message from e.getMessage() does not startwith with message from PARTITION_DOES_NOT_EXIST. Or exception message is different than PARTITION_DOES_NOT_EXIST.
- Peeyush Bishnoi
On Jan. 20, 2016, 10:43 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2016, 10:43 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Balu Vellanki <bv...@hortonworks.com>.
> On Jan. 22, 2016, 11:39 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java, line 295
> > <https://reviews.apache.org/r/42574/diff/1/?file=1204001#file1204001line295>
> >
> > Can hCatPartition.getParameters() return null?
>From the code, it looks like parameters is always set. But I agree with you that we cant gaurantee it. Changing code to reflect that.
> On Jan. 22, 2016, 11:39 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 399
> > <https://reviews.apache.org/r/42574/diff/1/?file=1204002#file1204002line399>
> >
> > What if FalconException is not because of PARTITION_DOES_NOT_EXIST? Don't we have to rethrow the exception after catching it?
@peeyush and @sowmya - I was throwing exception in the patch I have on IDE, but somehow it didnt make it to the patch I submitted. Thanks for catching it.
> On Jan. 22, 2016, 11:39 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 432
> > <https://reviews.apache.org/r/42574/diff/1/?file=1204002#file1204002line432>
> >
> > CatalogServiceFactory.getCatalogService().getPartition can return null. Isn't there a possibility that partition can be null?
No, this partition is catalog partition created in HiveCatalogService, it cant be null.
> On Jan. 22, 2016, 11:39 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 451
> > <https://reviews.apache.org/r/42574/diff/1/?file=1204002#file1204002line451>
> >
> > Can result be null or empty?
Great catch, I found the same bug is in FileSystemStorage. Fixed it there as well.
- Balu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review115953
-----------------------------------------------------------
On Jan. 20, 2016, 10:43 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2016, 10:43 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review115953
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java (line 295)
<https://reviews.apache.org/r/42574/#comment176987>
Can hCatPartition.getParameters() return null?
common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java (line 296)
<https://reviews.apache.org/r/42574/#comment176988>
nit: use isNotBlank
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java (line 399)
<https://reviews.apache.org/r/42574/#comment176991>
What if FalconException is not because of PARTITION_DOES_NOT_EXIST? Don't we have to rethrow the exception after catching it?
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java (line 432)
<https://reviews.apache.org/r/42574/#comment176990>
CatalogServiceFactory.getCatalogService().getPartition can return null. Isn't there a possibility that partition can be null?
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java (line 451)
<https://reviews.apache.org/r/42574/#comment176989>
Can result be null or empty?
- Sowmya Ramesh
On Jan. 20, 2016, 10:43 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2016, 10:43 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review116534
-----------------------------------------------------------
Fix it, then Ship it!
Good work
common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java (line 299)
<https://reviews.apache.org/r/42574/#comment177542>
@Balu Vellanki, I know you wanted me to confirm on the size parameter to use. totalSize is the right table statistics to use. This is based on fileLength unlike the raw datasize.
- Venkat Ranganathan
On Jan. 26, 2016, 3:21 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2016, 3:21 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review116477
-----------------------------------------------------------
Ship it!
Ship It!
- Sowmya Ramesh
On Jan. 26, 2016, 11:21 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2016, 11:21 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/
-----------------------------------------------------------
(Updated Jan. 26, 2016, 11:21 p.m.)
Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
Changes
-------
Incorporated feedback from Sowmya
Bugs: Falcon-763
https://issues.apache.org/jira/browse/Falcon-763
Repository: falcon-git
Description
-------
Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
Diff: https://reviews.apache.org/r/42574/diff/
Testing
-------
Added unit test, integration test and tested this end2end
Thanks,
Balu Vellanki
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review116290
-----------------------------------------------------------
Ship it!
Ship It!
- Peeyush Bishnoi
On Jan. 25, 2016, 9:01 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2016, 9:01 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Balu Vellanki <bv...@hortonworks.com>.
> On Jan. 26, 2016, 6:55 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 453
> > <https://reviews.apache.org/r/42574/diff/1-2/?file=1204002#file1204002line453>
> >
> > result can be null isn't. get(0) will throw NPE if result is null. Check if result is null before trying to access it. Do the same for FileSystemStorage.
>
> Balu Vellanki wrote:
> result cannot be null, it is init to new ArrayList<FeedInstanceStatus>();
>
> Sowmya Ramesh wrote:
> list can be empty isn't? It will trhow java.lang.IndexOutOfBoundsException. Can you check if not empty and then do get(0)?
Good point will make that change.
- Balu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review116405
-----------------------------------------------------------
On Jan. 25, 2016, 9:01 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2016, 9:01 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Balu Vellanki <bv...@hortonworks.com>.
> On Jan. 26, 2016, 6:55 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 409
> > <https://reviews.apache.org/r/42574/diff/1-2/?file=1204002#file1204002line409>
> >
> > If this is required only for if can you move it inside if ? I know that control will not reach that line if it enters else but I think moving inside if will be more readable
This is required for the while condition. I dont understand your comment here, can you explain more?
> On Jan. 26, 2016, 6:55 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 453
> > <https://reviews.apache.org/r/42574/diff/1-2/?file=1204002#file1204002line453>
> >
> > result can be null isn't. get(0) will throw NPE if result is null. Check if result is null before trying to access it. Do the same for FileSystemStorage.
result cannot be null, it is init to new ArrayList<FeedInstanceStatus>();
- Balu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review116405
-----------------------------------------------------------
On Jan. 25, 2016, 9:01 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2016, 9:01 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
> On Jan. 26, 2016, 6:55 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 409
> > <https://reviews.apache.org/r/42574/diff/1-2/?file=1204002#file1204002line409>
> >
> > If this is required only for if can you move it inside if ? I know that control will not reach that line if it enters else but I think moving inside if will be more readable
>
> Balu Vellanki wrote:
> This is required for the while condition. I dont understand your comment here, can you explain more?
I didn't relaize there was a whike condition, sorry about that.
> On Jan. 26, 2016, 6:55 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java, line 453
> > <https://reviews.apache.org/r/42574/diff/1-2/?file=1204002#file1204002line453>
> >
> > result can be null isn't. get(0) will throw NPE if result is null. Check if result is null before trying to access it. Do the same for FileSystemStorage.
>
> Balu Vellanki wrote:
> result cannot be null, it is init to new ArrayList<FeedInstanceStatus>();
list can be empty isn't? It will trhow java.lang.IndexOutOfBoundsException. Can you check if not empty and then do get(0)?
- Sowmya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review116405
-----------------------------------------------------------
On Jan. 25, 2016, 9:01 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2016, 9:01 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/#review116405
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java (line 409)
<https://reviews.apache.org/r/42574/#comment177430>
If this is required only for if can you move it inside if ? I know that control will not reach that line if it enters else but I think moving inside if will be more readable
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java (line 453)
<https://reviews.apache.org/r/42574/#comment177431>
result can be null isn't. get(0) will throw NPE if result is null. Check if result is null before trying to access it. Do the same for FileSystemStorage.
- Sowmya Ramesh
On Jan. 25, 2016, 9:01 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42574/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2016, 9:01 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-763
> https://issues.apache.org/jira/browse/Falcon-763
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
> common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
> common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
> common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
> webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/42574/diff/
>
>
> Testing
> -------
>
> Added unit test, integration test and tested this end2end
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 42574: Support feed listing for CatalogStorage
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42574/
-----------------------------------------------------------
(Updated Jan. 25, 2016, 9:01 p.m.)
Review request for Falcon, Ajay Yadava, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
Bugs: Falcon-763
https://issues.apache.org/jira/browse/Falcon-763
Repository: falcon-git
Description
-------
Support feed listing capability for catalog storage, as outlined in FALCON-761 for FileSystemStorage. Falcon processes or feed replication provides status on instances. When the instance status is WAITING for input, users have difficulty in identifying what feed is missing. In general it would very helpful to users to get feed availability status natively through falcon.
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/catalog/CatalogPartition.java 9e35782
common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java b988c3e
common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 143d9b4
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8aa97ec
common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 200f71f
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java d565f94
webapp/src/test/java/org/apache/falcon/catalog/CatalogStorageIT.java PRE-CREATION
Diff: https://reviews.apache.org/r/42574/diff/
Testing
-------
Added unit test, integration test and tested this end2end
Thanks,
Balu Vellanki