----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60373/#review178799 -----------------------------------------------------------
Fix it, then Ship it! The original patch is for sentry-ha-redesign branch, not master. Changes look good, some nits below. sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java Lines 193 (patched) <https://reviews.apache.org/r/60373/#comment253007> Naming - this is predicate, so may be use something like hasChildren()? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java Lines 198 (patched) <https://reviews.apache.org/r/60373/#comment253006> I think the official way is to return Collections.emptyList() why not use return (children != null) ? children.values() : COllections.emptySet(); sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java Line 462 (original), 504 (patched) <https://reviews.apache.org/r/60373/#comment253008> Use Collections.emptySet() sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java Lines 73 (patched) <https://reviews.apache.org/r/60373/#comment253009> Collections.emptySet() sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java Line 73 (original), 76 (patched) <https://reviews.apache.org/r/60373/#comment253010> Use isEmpty() instead of size() != 0 - Alexander Kolbasov On June 22, 2017, 11:29 p.m., Misha Dmitriev wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60373/ > ----------------------------------------------------------- > > (Updated June 22, 2017, 11:29 p.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1811 > https://issues.apache.org/jira/browse/SENTRY-1811 > > > Repository: sentry > > > Description > ------- > > SENTRY-1811: reduce memory consumed by data structures used in HDFS sync > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java > 3fb9b4e3829700ba8df7ca1a6f21b05185c33e6b > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java > 4e736ea45a763d0af846be2aa8773feb4ec1d1df > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java > f31ec215d20c5898976148c627e81e7d26151655 > > > Diff: https://reviews.apache.org/r/60373/diff/2/ > > > Testing > ------- > > > Thanks, > > Misha Dmitriev > >
