falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sowmya Ramesh" <sram...@hortonworks.com>
Subject Re: Review Request 39711: Lifecycle does not allow feed with frequency greater than days(1)
Date Wed, 28 Oct 2015 20:21:26 GMT


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


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