----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26908/#review59897 -----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java <https://reviews.apache.org/r/26908/#comment101214> An example to illustrate what is being stored, who help reader understand this more easily common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java <https://reviews.apache.org/r/26908/#comment101213> Not final ? common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java <https://reviews.apache.org/r/26908/#comment101216> final ? common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java <https://reviews.apache.org/r/26908/#comment101215> final ? common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java <https://reviews.apache.org/r/26908/#comment101217> final ? common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java <https://reviews.apache.org/r/26908/#comment101218> Do we need a corresponding thing for Catalog based feeds, can be done in an independent JIRA, but do we see utility ? common/src/main/java/org/apache/falcon/entity/store/FeedPathStore.java <https://reviews.apache.org/r/26908/#comment101219> Since you are using javax.annotation, why not annotate for the arguments for the methods and describe whether they are nullable or nonnull? common/src/main/java/org/apache/falcon/util/RadixNode.java <https://reviews.apache.org/r/26908/#comment101220> Please return an unmodifiableCollection common/src/main/java/org/apache/falcon/util/RadixTree.java <https://reviews.apache.org/r/26908/#comment101222> This seems very critical method, will go through this to check for boundary conditions common/src/main/java/org/apache/falcon/util/RadixTree.java <https://reviews.apache.org/r/26908/#comment101221> There should be a behavior in RadixNode::removeAll common/src/main/java/org/apache/falcon/util/RadixTree.java <https://reviews.apache.org/r/26908/#comment101224> When does this happen ? common/src/main/java/org/apache/falcon/util/RadixTree.java <https://reviews.apache.org/r/26908/#comment101225> Not sure if I understand this. Can we run through and find what happens if the tree has root-->aaa -->aab -->aac - and we search for root/aab/xyz, would it navigate into root/aaa and abort without finding the element ? common/src/main/java/org/apache/falcon/util/RadixTree.java <https://reviews.apache.org/r/26908/#comment101226> Can the find be implemented in such a way that both insert & remove can use, Currently each of these functions are slight variants of each other and bugs have to be fixed in each one of them. - Srikanth Sundarrajan On Oct. 19, 2014, 4:20 a.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26908/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2014, 4:20 a.m.) > > > Review request for Falcon and Srikanth Sundarrajan. > > > Repository: falcon-git > > > Description > ------- > > Store for feed path(key) and feed properties like feed name etc. as values > > > Diffs > ----- > > common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java > PRE-CREATION > common/src/main/java/org/apache/falcon/entity/store/FeedPathStore.java > PRE-CREATION > common/src/main/java/org/apache/falcon/util/RadixNode.java PRE-CREATION > common/src/main/java/org/apache/falcon/util/RadixTree.java PRE-CREATION > > common/src/test/java/org/apache/falcon/entity/store/FeedLocationStoreTest.java > PRE-CREATION > common/src/test/java/org/apache/falcon/util/RadixNodeTest.java PRE-CREATION > common/src/test/java/org/apache/falcon/util/RadixTreeTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/26908/diff/ > > > Testing > ------- > > Thorough unit testing for the patch submitted. > > > Thanks, > > Ajay Yadava > >
