> On Dec. 14, 2015, 1:53 p.m., Peeyush Bishnoi wrote: > > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1015 > > <https://reviews.apache.org/r/41339/diff/1/?file=1162237#file1162237line1015> > > > > If there is any code commonality between getRetentionFrequency and > > getLifecycleRetentionFrequency, can you think to merge that and invoke it > > appropriately from single function. I know that the > > getLifecycleRetentionFrequency is for lifecycle and getRetentionFrequency > > is for normal retention. > > Narayan Periwal wrote: > If the retention stage is null, then in case of > getLifecycleRetentionFrequency, we return null, whereas for the > getRetentionFrequency, we return the retention limit of the cluster. If we > try to merge them, we won't be able to handle this case. So, IMO its better > if we keep them separate. What do you think?
@peeyush. I accept your point. There should be an api which would return retention frequency in general which would hide the hiererachy of various retentions. But this can be tracked in another jira as an improvement and go ahead with this patch, as that would mean changes at many places to hide this hierarchy. - sandeep ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41339/#review110231 ----------------------------------------------------------- On Dec. 14, 2015, 10:17 a.m., Narayan Periwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41339/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2015, 10:17 a.m.) > > > Review request for Falcon. > > > Bugs: FALCON-1617 > https://issues.apache.org/jira/browse/FALCON-1617 > > > Repository: falcon-git > > > Description > ------- > > Currently on enabling SLA monitoring it doesn't consider instances which had > nominal time in past for SLA monitoring. With this JIRA we would like to > enable this. > > > Diffs > ----- > > common/src/main/java/org/apache/falcon/entity/FeedHelper.java 29daff3 > prism/src/main/java/org/apache/falcon/service/FeedSLAMonitoringService.java > b302539 > > Diff: https://reviews.apache.org/r/41339/diff/ > > > Testing > ------- > > Done. > > > Thanks, > > Narayan Periwal > >
