falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ajay Yadava" <ajayn...@gmail.com>
Subject Re: Review Request 25211: Delegated feed eviction to the appropriate Storage implementation
Date Tue, 02 Sep 2014 05:51:35 GMT


> On Sept. 2, 2014, 4:57 a.m., shwethags wrote:
> > common/src/main/java/org/apache/falcon/entity/Storage.java, line 102
> > <https://reviews.apache.org/r/25211/diff/2/?file=673444#file673444line102>
> >
> >     should have args - retentionLimit and timezone. It can probably return back
list of instances deleted. FeedEvictor class can then log these instances as its common to
both catalog storage and filesystem storage

Your suggestion is completely correct and the way this code is written is slightly convoluted
e.g. the OUT is being used by Tests (passed through feedevictor) As part of this JIRA I have
just separated the concerns to respective storage systems and kept true to original implementation.
I was planning to handle the refactoring of Tests and Evictor logic, in separate JIRA. Thoughts?


- Ajay


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


On Sept. 1, 2014, 1:31 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25211/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2014, 1:31 p.m.)
> 
> 
> Review request for Falcon and shwethags.
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Earlier FeedEviction class contained logic for both FileSystemStorage and TableStorage.
Corresponding code was being called using an if else. To make the code cleaner and more manageable
I moved the code to the appropriate storage class and delegated feed eviction to the appropriate
Storage implementation. Needed to add evict method to Storage Interface and make some minor
changes here and there.
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 7ad0716 
>   common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 4eb3d60 
>   common/src/main/java/org/apache/falcon/entity/Storage.java f88e139 
>   retention/src/main/java/org/apache/falcon/retention/FeedEvictor.java 4de7938 
>   retention/src/test/java/org/apache/falcon/retention/FeedEvictorFileSystemTest.java
PRE-CREATION 
>   retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java eb4173e 
> 
> Diff: https://reviews.apache.org/r/25211/diff/
> 
> 
> Testing
> -------
> 
> All FeedEvictor tests passed.
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


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