falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peeyush Bishnoi" <bpeey...@yahoo.co.in>
Subject Re: Review Request 33025: FALCON-1146 : feed retention policy deleted everything all the way up to the root
Date Fri, 10 Apr 2015 11:01:23 GMT


> On April 10, 2015, 5:25 a.m., Pallavi Rao wrote:
> > retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java, line 392
> > <https://reviews.apache.org/r/33025/diff/1/?file=921767#file921767line392>
> >
> >     The code fix is fine. But, don't think this test tests the patch sufficiently.
If the retention limit is 10 days, nothing should ideally get deleted as we produce only 10
instances of data.

I agree that nothing should ideally get deleted if retention limit is 10 days with 10 instances.
But in this test case we have more than 10 instances thats why only 10 days data has been
retained after eviction.
$ ls retention/webapp/target/tmp-hadoop-pbishnoi/jail-fs/test/data/YYYY/feed1/mmHH/dd/MM/2015/04/
01	02	03	04	05	06	07	08	09	10

Also, here we have just re-enabled the testcase testEvictionWithEmptyDirs().


> On April 10, 2015, 5:25 a.m., Pallavi Rao wrote:
> > retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java, line 410
> > <https://reviews.apache.org/r/33025/diff/1/?file=921767#file921767line410>
> >
> >     You will notice that this check will pass even without your code fix (with just
the change in line 401), coz., no instances really get evicted in this test.

I have checked the logs and found that few instances has been evicted through this testcase.
As I mentioned, we have just re-enabled the test case.


> On April 10, 2015, 5:25 a.m., Pallavi Rao wrote:
> > retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java, line 412
> > <https://reviews.apache.org/r/33025/diff/1/?file=921767#file921767line412>
> >
> >     If we delete all instances and only the basePath is left behind, it should have
no children. So, afterDelCount should be zero.

For this test case as the retention limit is 10 days , 10 days data is available with total
directory count 32 as few older instances evicted.
$ ls retention/webapp/target/tmp-hadoop-pbishnoi/jail-fs/test/data/YYYY/feed1/mmHH/dd/MM/2015/04/
01	02	03	04	05	06	07	08	09	10
$ ls -Rlt retention/webapp/target/tmp-hadoop-pbishnoi/jail-fs/test/data/YYYY/feed1/mmHH/dd/MM/|grep
YYYY|wc -l
      32


- Peeyush


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33025/#review79641
-----------------------------------------------------------


On April 9, 2015, 5:54 p.m., Peeyush Bishnoi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33025/
> -----------------------------------------------------------
> 
> (Updated April 9, 2015, 5:54 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1146
>     https://issues.apache.org/jira/browse/FALCON-1146
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> FALCON-1146 : feed retention policy deleted everything all the way up to the root
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java a5caf8e 
>   retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java a2feccf 
> 
> Diff: https://reviews.apache.org/r/33025/diff/
> 
> 
> Testing
> -------
> 
> Yes
> 
> 
> Thanks,
> 
> Peeyush Bishnoi
> 
>


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