You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Ajay Yadava <aj...@gmail.com> on 2015/06/15 12:56:30 UTC
Review Request 35452: FALCON-1260 InstanceDependency API gives
incorrect results.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35452/
-----------------------------------------------------------
Review request for Falcon and Srikanth Sundarrajan.
Repository: falcon-git
Description
-------
There were some bugs in the current implementation.
Diffs
-----
client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java 2a7ecdb
common/src/main/java/org/apache/falcon/entity/EntityUtil.java 7ebf39e
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 9f4eb61
common/src/main/java/org/apache/falcon/entity/ProcessHelper.java fe78bc8
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java f70edfb
common/src/test/java/org/apache/falcon/entity/ProcessHelperTest.java 0d396ae
docs/src/site/twiki/FalconCLI.twiki 50dce84
prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 72f9fe4
prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java 1a8396c
Diff: https://reviews.apache.org/r/35452/diff/
Testing
-------
Unit tests added for all the issues and tests were also added for newer scenarios.
Thanks,
Ajay Yadava
Re: Review Request 35452: FALCON-1260 InstanceDependency API gives
incorrect results.
Posted by Ajay Yadava <aj...@gmail.com>.
> On June 26, 2015, 8:21 a.m., Srikanth Sundarrajan wrote:
> > common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java, line 183
> > <https://reviews.apache.org/r/35452/diff/1/?file=984614#file984614line183>
> >
> > Does it make sense to assert on the actual values besides the size of the set. Also see if other such asserts introduced by the patch can be changed.
Absolutely. Will make the necessary changes.
> On June 26, 2015, 8:21 a.m., Srikanth Sundarrajan wrote:
> > prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java, line 392
> > <https://reviews.apache.org/r/35452/diff/1/?file=984618#file984618line392>
> >
> > Though not an issue caused by this patch, there seems to be no logging of the actual exception. It would be useful to log the full exception since we aren't passing it up.
Thanks for surfacing it. Will add logging.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35452/#review88027
-----------------------------------------------------------
On June 15, 2015, 10:57 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35452/
> -----------------------------------------------------------
>
> (Updated June 15, 2015, 10:57 a.m.)
>
>
> Review request for Falcon and Srikanth Sundarrajan.
>
>
> Bugs: FALCON-1260
> https://issues.apache.org/jira/browse/FALCON-1260
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> There were some bugs in the current implementation.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java 2a7ecdb
> common/src/main/java/org/apache/falcon/entity/EntityUtil.java 7ebf39e
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 9f4eb61
> common/src/main/java/org/apache/falcon/entity/ProcessHelper.java fe78bc8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java f70edfb
> common/src/test/java/org/apache/falcon/entity/ProcessHelperTest.java 0d396ae
> docs/src/site/twiki/FalconCLI.twiki 50dce84
> prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 72f9fe4
> prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java 1a8396c
>
> Diff: https://reviews.apache.org/r/35452/diff/
>
>
> Testing
> -------
>
> Unit tests added for all the issues and tests were also added for newer scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 35452: FALCON-1260 InstanceDependency API gives
incorrect results.
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35452/#review88027
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/EntityUtil.java (line 811)
<https://reviews.apache.org/r/35452/#comment140434>
Assert that referenceTime is > startTime, if lesser getInstanceSequence returns -1
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java (line 183)
<https://reviews.apache.org/r/35452/#comment142117>
Does it make sense to assert on the actual values besides the size of the set. Also see if other such asserts introduced by the patch can be changed.
prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java (line 392)
<https://reviews.apache.org/r/35452/#comment142116>
Though not an issue caused by this patch, there seems to be no logging of the actual exception. It would be useful to log the full exception since we aren't passing it up.
- Srikanth Sundarrajan
On June 15, 2015, 10:57 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35452/
> -----------------------------------------------------------
>
> (Updated June 15, 2015, 10:57 a.m.)
>
>
> Review request for Falcon and Srikanth Sundarrajan.
>
>
> Bugs: FALCON-1260
> https://issues.apache.org/jira/browse/FALCON-1260
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> There were some bugs in the current implementation.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java 2a7ecdb
> common/src/main/java/org/apache/falcon/entity/EntityUtil.java 7ebf39e
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 9f4eb61
> common/src/main/java/org/apache/falcon/entity/ProcessHelper.java fe78bc8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java f70edfb
> common/src/test/java/org/apache/falcon/entity/ProcessHelperTest.java 0d396ae
> docs/src/site/twiki/FalconCLI.twiki 50dce84
> prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 72f9fe4
> prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java 1a8396c
>
> Diff: https://reviews.apache.org/r/35452/diff/
>
>
> Testing
> -------
>
> Unit tests added for all the issues and tests were also added for newer scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 35452: FALCON-1260 InstanceDependency API gives
incorrect results.
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35452/#review89708
-----------------------------------------------------------
Ship it!
Ship It!
- Srikanth Sundarrajan
On June 29, 2015, 6:59 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35452/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 6:59 a.m.)
>
>
> Review request for Falcon and Srikanth Sundarrajan.
>
>
> Bugs: FALCON-1260
> https://issues.apache.org/jira/browse/FALCON-1260
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> There were some bugs in the current implementation.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java 2a7ecdb
> common/src/main/java/org/apache/falcon/entity/EntityUtil.java 7ebf39e
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 9f4eb61
> common/src/main/java/org/apache/falcon/entity/ProcessHelper.java fe78bc8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java f70edfb
> common/src/test/java/org/apache/falcon/entity/ProcessHelperTest.java 0d396ae
> docs/src/site/twiki/FalconCLI.twiki 50dce84
> prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 72f9fe4
> prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java 1a8396c
>
> Diff: https://reviews.apache.org/r/35452/diff/
>
>
> Testing
> -------
>
> Unit tests added for all the issues and tests were also added for newer scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 35452: FALCON-1260 InstanceDependency API gives
incorrect results.
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35452/#review89709
-----------------------------------------------------------
Ship it!
+1
- Srikanth Sundarrajan
On June 29, 2015, 6:59 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35452/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 6:59 a.m.)
>
>
> Review request for Falcon and Srikanth Sundarrajan.
>
>
> Bugs: FALCON-1260
> https://issues.apache.org/jira/browse/FALCON-1260
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> There were some bugs in the current implementation.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java 2a7ecdb
> common/src/main/java/org/apache/falcon/entity/EntityUtil.java 7ebf39e
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 9f4eb61
> common/src/main/java/org/apache/falcon/entity/ProcessHelper.java fe78bc8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java f70edfb
> common/src/test/java/org/apache/falcon/entity/ProcessHelperTest.java 0d396ae
> docs/src/site/twiki/FalconCLI.twiki 50dce84
> prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 72f9fe4
> prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java 1a8396c
>
> Diff: https://reviews.apache.org/r/35452/diff/
>
>
> Testing
> -------
>
> Unit tests added for all the issues and tests were also added for newer scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 35452: FALCON-1260 InstanceDependency API gives
incorrect results.
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35452/
-----------------------------------------------------------
(Updated June 29, 2015, 6:59 a.m.)
Review request for Falcon and Srikanth Sundarrajan.
Changes
-------
Addressed the review comments.
Bugs: FALCON-1260
https://issues.apache.org/jira/browse/FALCON-1260
Repository: falcon-git
Description
-------
There were some bugs in the current implementation.
Diffs (updated)
-----
client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java 2a7ecdb
common/src/main/java/org/apache/falcon/entity/EntityUtil.java 7ebf39e
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 9f4eb61
common/src/main/java/org/apache/falcon/entity/ProcessHelper.java fe78bc8
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java f70edfb
common/src/test/java/org/apache/falcon/entity/ProcessHelperTest.java 0d396ae
docs/src/site/twiki/FalconCLI.twiki 50dce84
prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 72f9fe4
prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java 1a8396c
Diff: https://reviews.apache.org/r/35452/diff/
Testing
-------
Unit tests added for all the issues and tests were also added for newer scenarios.
Thanks,
Ajay Yadava
Re: Review Request 35452: FALCON-1260 InstanceDependency API gives
incorrect results.
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35452/
-----------------------------------------------------------
(Updated June 15, 2015, 10:57 a.m.)
Review request for Falcon and Srikanth Sundarrajan.
Bugs: FALCON-1260
https://issues.apache.org/jira/browse/FALCON-1260
Repository: falcon-git
Description
-------
There were some bugs in the current implementation.
Diffs
-----
client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java 2a7ecdb
common/src/main/java/org/apache/falcon/entity/EntityUtil.java 7ebf39e
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 9f4eb61
common/src/main/java/org/apache/falcon/entity/ProcessHelper.java fe78bc8
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java f70edfb
common/src/test/java/org/apache/falcon/entity/ProcessHelperTest.java 0d396ae
docs/src/site/twiki/FalconCLI.twiki 50dce84
prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 72f9fe4
prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java 1a8396c
Diff: https://reviews.apache.org/r/35452/diff/
Testing
-------
Unit tests added for all the issues and tests were also added for newer scenarios.
Thanks,
Ajay Yadava