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/10/28 06:15:59 UTC
Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/
-----------------------------------------------------------
Review request for Falcon.
Bugs: FALCON-1560
https://issues.apache.org/jira/browse/FALCON-1560
Repository: falcon-git
Description
-------
Lifecycle does not allow feed with frequency greater than days(1)
Diffs
-----
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
Diff: https://reviews.apache.org/r/39711/diff/
Testing
-------
Added unit test for the scenarios.
Thanks,
Ajay Yadava
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104270
-----------------------------------------------------------
Ship it!
Ship It!
- sandeep samudrala
On Oct. 28, 2015, 9:32 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 9:32 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104271
-----------------------------------------------------------
Ship it!
Ship It!
- sandeep samudrala
On Oct. 28, 2015, 9:32 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 9:32 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
> On Oct. 28, 2015, 6:35 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 808
> > <https://reviews.apache.org/r/39711/diff/4/?file=1111629#file1111629line808>
> >
> > I don't fully understand the fix here. If retention stage is not defined for lifecycle shouldn't it fallback to old retention policy?
> >
> > Retetion policy frequency was not validated before. Now, with your code change retention validation happens even for those cases isn't if user defnes lifecycle without retention stage. Isn't it breaking the old behavior.
> >
> > If retention stage is not defined even when lifecycle is present then it shoudl fall back to old behavior. So validation should be skipped and retention should be set as in FeedRetentionCoordinatorBuilder. Am I missing something here?
>
> PRAGYA MITTAL wrote:
> validateRetentionFrequency is only used for validating Lifecycle related retention policy so it wont affect the old retention behaviour.
>
> Ajay Yadava wrote:
> 1. Retention is a mandatory stage for lifecycle, so it is not possible for it to be absent, so there is no fallback.
>
> 2. Even now, retention frequency is validated only for retention stage in lifecycle. It doesn't validate old retention frequencies, hence it is compatible with old behavior.
>
> 3. It is not required as retention stage is mandatory.
>
> Sowmya Ramesh wrote:
> If it's mandatory, when does it enter FeedHelper.getRetentionFrequency else part line 808? You are checking if (retentionStage != null && retentionStage.getFrequency() != null) and getRetentionStage returns null only if lifecycle is not enabled.
I was not clear in the previous comment. Doesn't t enter else part in getRetentionFrequency if lifecycle is not defined i.e. its old retention policy. If so it should fallback to old logic which seeting it to 6hrs or 1 day as in FeedRetentionCoordinatorBuilder isn't? Sorry, if I am missing something here. We can take this offline if its blocking the commit and my comments are not valid.
- Sowmya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104322
-----------------------------------------------------------
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Ajay Yadava <aj...@gmail.com>.
> On Oct. 28, 2015, 6:35 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 808
> > <https://reviews.apache.org/r/39711/diff/4/?file=1111629#file1111629line808>
> >
> > I don't fully understand the fix here. If retention stage is not defined for lifecycle shouldn't it fallback to old retention policy?
> >
> > Retetion policy frequency was not validated before. Now, with your code change retention validation happens even for those cases isn't if user defnes lifecycle without retention stage. Isn't it breaking the old behavior.
> >
> > If retention stage is not defined even when lifecycle is present then it shoudl fall back to old behavior. So validation should be skipped and retention should be set as in FeedRetentionCoordinatorBuilder. Am I missing something here?
>
> PRAGYA MITTAL wrote:
> validateRetentionFrequency is only used for validating Lifecycle related retention policy so it wont affect the old retention behaviour.
1. Retention is a mandatory stage for lifecycle, so it is not possible for it to be absent, so there is no fallback.
2. Even now, retention frequency is validated only for retention stage in lifecycle. It doesn't validate old retention frequencies, hence it is compatible with old behavior.
3. It is not required as retention stage is mandatory.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104322
-----------------------------------------------------------
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
> On Oct. 28, 2015, 6:35 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 808
> > <https://reviews.apache.org/r/39711/diff/4/?file=1111629#file1111629line808>
> >
> > I don't fully understand the fix here. If retention stage is not defined for lifecycle shouldn't it fallback to old retention policy?
> >
> > Retetion policy frequency was not validated before. Now, with your code change retention validation happens even for those cases isn't if user defnes lifecycle without retention stage. Isn't it breaking the old behavior.
> >
> > If retention stage is not defined even when lifecycle is present then it shoudl fall back to old behavior. So validation should be skipped and retention should be set as in FeedRetentionCoordinatorBuilder. Am I missing something here?
>
> PRAGYA MITTAL wrote:
> validateRetentionFrequency is only used for validating Lifecycle related retention policy so it wont affect the old retention behaviour.
>
> Ajay Yadava wrote:
> 1. Retention is a mandatory stage for lifecycle, so it is not possible for it to be absent, so there is no fallback.
>
> 2. Even now, retention frequency is validated only for retention stage in lifecycle. It doesn't validate old retention frequencies, hence it is compatible with old behavior.
>
> 3. It is not required as retention stage is mandatory.
If it's mandatory, when does it enter FeedHelper.getRetentionFrequency else part line 808? You are checking if (retentionStage != null && retentionStage.getFrequency() != null) and getRetentionStage returns null only if lifecycle is not enabled.
- Sowmya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104322
-----------------------------------------------------------
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by PRAGYA MITTAL <mi...@gmail.com>.
> On Oct. 28, 2015, 6:35 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 808
> > <https://reviews.apache.org/r/39711/diff/4/?file=1111629#file1111629line808>
> >
> > I don't fully understand the fix here. If retention stage is not defined for lifecycle shouldn't it fallback to old retention policy?
> >
> > Retetion policy frequency was not validated before. Now, with your code change retention validation happens even for those cases isn't if user defnes lifecycle without retention stage. Isn't it breaking the old behavior.
> >
> > If retention stage is not defined even when lifecycle is present then it shoudl fall back to old behavior. So validation should be skipped and retention should be set as in FeedRetentionCoordinatorBuilder. Am I missing something here?
validateRetentionFrequency is only used for validating Lifecycle related retention policy so it wont affect the old retention behaviour.
- PRAGYA
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104322
-----------------------------------------------------------
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104322
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 808)
<https://reviews.apache.org/r/39711/#comment162578>
I don't fully understand the fix here. If retention stage is not defined for lifecycle shouldn't it fallback to old retention policy?
Retetion policy frequency was not validated before. Now, with your code change retention validation happens even for those cases isn't if user defnes lifecycle without retention stage. Isn't it breaking the old behavior.
If retention stage is not defined even when lifecycle is present then it shoudl fall back to old behavior. So validation should be skipped and retention should be set as in FeedRetentionCoordinatorBuilder. Am I missing something here?
- Sowmya Ramesh
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104398
-----------------------------------------------------------
Ship it!
Ship It!
- pavan kumar kolamuri
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104316
-----------------------------------------------------------
Ship it!
Ship It!
- pavan kumar kolamuri
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Ajay Yadava <aj...@gmail.com>.
> On Oct. 28, 2015, 11:46 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 813
> > <https://reviews.apache.org/r/39711/diff/4/?file=1111629#file1111629line813>
> >
> > Sorry, for multiple comments. I didn't review Lifecycle feature so I didn't have the complete picture.
> >
> > Frequency in the retention stage is not mandatory and if teh frequency is not set by user then
> > 1> If feed frequency < 6 hrs its set to 6 hrs
> > 2> If its > 6 hrs its set to feed frequency
> >
> > Shouldn't it fallbaack to current behavior for retenting the data? < 6hrs set to 6 hrs and > 6hrs set to 1 day?
> >
> > This is required for 2 reasons
> > 1> Current understanding of users is that if feed frequency > 6 hrs , retention job will run every day. We shouldn't deviate from this.
> >
> > 2> I also spoke with Venkatesh about why was it set to 1 day. He mentioned in case retention fails and reruns fail too we don't want to keep the data till it runs next time if feed frequency is used. This can cause SEC retention vioalation and also cause memory issues if feed frequency is say one year. If job runs every day it catches up for the scenario mentioned above.
> >
> > Any specific reason to change the old behavior?
Sowmya and I had an offline discussion to address this. Updating the gist here.
We try to fall back to old behaviour as much as possible but it fails the extra validations in lifecycle retention. The current behaviour is to retain old behaviour as much as possible within new constraints (specifically retention shouldn't be more frequent than data availability).
Keeping retention frequency as a fallback to retries is not the best thing to do in such scenarios. If it fails all retries there is no guarantee that it will succeed next time as well. It means system is not able to recover on it's own and needs manual intervention. Best way to deal with such scenarios is to have appropriate monitoring and alerting (e.g. they can now have email alerts on failure of retention workflow).
The said kind of set up also fails for a majority of frequencies e.g. minutely, hourly, daily (all apart from roll ups like monthly) will not ensure the above guarantee from the reasoning mentioned. So the guarantee is already broken, if it was ever the intent.
Also, the above behaviour is a wastage of resources 99% of the times to solve for that rare 1% case. Coordinators will run and they will have nothing to do.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104372
-----------------------------------------------------------
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104372
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 813)
<https://reviews.apache.org/r/39711/#comment162628>
Sorry, for multiple comments. I didn't review Lifecycle feature so I didn't have the complete picture.
Frequency in the retention stage is not mandatory and if teh frequency is not set by user then
1> If feed frequency < 6 hrs its set to 6 hrs
2> If its > 6 hrs its set to feed frequency
Shouldn't it fallbaack to current behavior for retenting the data? < 6hrs set to 6 hrs and > 6hrs set to 1 day?
This is required for 2 reasons
1> Current understanding of users is that if feed frequency > 6 hrs , retention job will run every day. We shouldn't deviate from this.
2> I also spoke with Venkatesh about why was it set to 1 day. He mentioned in case retention fails and reruns fail too we don't want to keep the data till it runs next time if feed frequency is used. This can cause SEC retention vioalation and also cause memory issues if feed frequency is say one year. If job runs every day it catches up for the scenario mentioned above.
Any specific reason to change the old behavior?
- Sowmya Ramesh
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Ajay Yadava <aj...@gmail.com>.
> On Oct. 28, 2015, 10:17 p.m., Venkat Ranganathan wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 813
> > <https://reviews.apache.org/r/39711/diff/4/?file=1111629#file1111629line813>
> >
> > Shouldn't the default be the same before? Wouldn't this be change in behavior.
This doesn't affect the old retention. This behaviour is applicable only to retention in lifecycle. Even in lifecycle we try to fall back to old behaviour as much as possible (as seen from the diff on left side) but it causes issues as mentioned in this JIRA and hence the need for this change.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104358
-----------------------------------------------------------
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104358
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 813)
<https://reviews.apache.org/r/39711/#comment162619>
Shouldn't the default be the same before? Wouldn't this be change in behavior.
- Venkat Ranganathan
On Oct. 28, 2015, 11:04 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 11:04 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by PRAGYA MITTAL <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104326
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 808)
<https://reviews.apache.org/r/39711/#comment162583>
getRetentionFrequency is only used for validating Lifecycle related retention policy so it wont affect the old retention behaviour.
- PRAGYA MITTAL
On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 6:04 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/
-----------------------------------------------------------
(Updated Oct. 28, 2015, 6:04 p.m.)
Review request for Falcon.
Changes
-------
Addressed the corner case.
Bugs: FALCON-1560
https://issues.apache.org/jira/browse/FALCON-1560
Repository: falcon-git
Description
-------
Lifecycle does not allow feed with frequency greater than days(1)
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
Diff: https://reviews.apache.org/r/39711/diff/
Testing
-------
Added unit test for the scenarios.
Thanks,
Ajay Yadava
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Ajay Yadava <aj...@gmail.com>.
> On Oct. 28, 2015, 9:35 a.m., PRAGYA MITTAL wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 810
> > <https://reviews.apache.org/r/39711/diff/3/?file=1111485#file1111485line810>
> >
> > For instance if user wants to submit feed with frequency > 360 minutes, it will fail right ?
Yes. I will fix it. Sorry I somehow missed your comment earlier.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104269
-----------------------------------------------------------
On Oct. 28, 2015, 9:32 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 9:32 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by PRAGYA MITTAL <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104269
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 810)
<https://reviews.apache.org/r/39711/#comment162538>
For instance if user wants to submit feed with frequency > 360 minutes, it will fail right ?
- PRAGYA MITTAL
On Oct. 28, 2015, 9:32 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 9:32 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/
-----------------------------------------------------------
(Updated Oct. 28, 2015, 9:32 a.m.)
Review request for Falcon.
Bugs: FALCON-1560
https://issues.apache.org/jira/browse/FALCON-1560
Repository: falcon-git
Description
-------
Lifecycle does not allow feed with frequency greater than days(1)
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36
common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
Diff: https://reviews.apache.org/r/39711/diff/
Testing
-------
Added unit test for the scenarios.
Thanks,
Ajay Yadava
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/
-----------------------------------------------------------
(Updated Oct. 28, 2015, 9:28 a.m.)
Review request for Falcon.
Changes
-------
Addressed the review comments and other changes missing from previous patch.
Bugs: FALCON-1560
https://issues.apache.org/jira/browse/FALCON-1560
Repository: falcon-git
Description
-------
Lifecycle does not allow feed with frequency greater than days(1)
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
Diff: https://reviews.apache.org/r/39711/diff/
Testing
-------
Added unit test for the scenarios.
Thanks,
Ajay Yadava
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by Ajay Yadava <aj...@gmail.com>.
> On Oct. 28, 2015, 5:26 a.m., sandeep samudrala wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 810
> > <https://reviews.apache.org/r/39711/diff/1/?file=1111235#file1111235line810>
> >
> > In case of TimeUnit being minutes, if its 10 minutes, the frequency would be set to 10 hours.
Oops. I did it again! This patch is not the complete patch, it will cause unit test failures and I made some more changes after this but forgot to generate the updated patch. I will upload the latest patch, which addressed this issue.
> On Oct. 28, 2015, 5:26 a.m., sandeep samudrala wrote:
> > common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java, line 204
> > <https://reviews.apache.org/r/39711/diff/1/?file=1111236#file1111236line204>
> >
> > Can you change the test to test it for a frequency less than 6 hours , something in minutes?
I understand your intention, the suggested test won't help but I will add it :)
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104251
-----------------------------------------------------------
On Oct. 28, 2015, 9:28 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 9:28 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>
Re: Review Request 39711: Lifecycle does not allow feed with frequency
greater than days(1)
Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39711/#review104251
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 810)
<https://reviews.apache.org/r/39711/#comment162528>
In case of TimeUnit being minutes, if its 10 minutes, the frequency would be set to 10 hours.
common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java (line 204)
<https://reviews.apache.org/r/39711/#comment162529>
Can you change the test to test it for a frequency less than 6 hours , something in minutes?
- sandeep samudrala
On Oct. 28, 2015, 5:15 a.m., Ajay Yadava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39711/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2015, 5:15 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1560
> https://issues.apache.org/jira/browse/FALCON-1560
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Lifecycle does not allow feed with frequency greater than days(1)
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 905be68
>
> Diff: https://reviews.apache.org/r/39711/diff/
>
>
> Testing
> -------
>
> Added unit test for the scenarios.
>
>
> Thanks,
>
> Ajay Yadava
>
>