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