> On Sept. 12, 2014, 7:20 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java, line 
> > 299
> > <https://reviews.apache.org/r/25211/diff/3/?file=682132#file682132line299>
> >
> >     Are you better off using StringBuilder?
> 
> Ajay Yadava wrote:
>     Original code used StringBuffer, so I have retained it to be on safer 
> side, in case it was required for some concurrency scenario. Let me know if I 
> can safely change it to StringBuilder.

I think it can be safely switched to StringBuilder. The ealier use may be 
unintentional.


On Sept. 12, 2014, 7:20 a.m., Ajay Yadava wrote:
> > Haven't checked the actual evict logic. Can you confirm if it is merely 
> > moved from FeedEvictor into respective Storage classes or have they been 
> > modified as well ?
> 
> Ajay Yadava wrote:
>     No Changes have been made to any evict or related logic. I have kept the 
> code as close to original implementation as possible with moving it to 
> appropriate storage classes.

OK


- Srikanth


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


On Sept. 13, 2014, 4:43 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25211/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2014, 4:43 a.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 
> 58506ad 
>   common/src/main/java/org/apache/falcon/entity/Storage.java f88e139 
>   common/src/main/java/org/apache/falcon/retention/EvictionHelper.java 
> 5d6481c 
>   retention/src/main/java/org/apache/falcon/retention/FeedEvictor.java 
> 114071f 
> 
> Diff: https://reviews.apache.org/r/25211/diff/
> 
> 
> Testing
> -------
> 
> All FeedEvictor tests passed.
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>

Reply via email to