----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62007/#review184304 -----------------------------------------------------------
Maybe I don't understand all the details, but here is what I think. I've just attempted my own exercise: I've changed MAuthzPathsMapping.getPathStrings() so that it returns a List instead of a Set, and then started to change Set -> List in all the dependent code just to satisfy type compatibility. I admit this has spread to a large number of files (and I didn't even touch the tests yet). Probably in most places the replacement could be done mechanically, but I also hit a method where I think Set functionality is important because of add/remove calls. Here Lists may not necessarily work well, because lookup in an ArrayList is not O(1). So my thinking is that this change needs to be done carefully. Basically in every method where we want to switch from Set to whatever, we need to understand the implications. Probably the code actually needs both Lists and Sets, depending on the operations and requirements. But then in some place(s) a conversion between these structures should happen. In that case, again, the code should be very explicit about the structures that it's using. Just blindly changing everything to Collection will cause problems (or perhaps causing them already). - Misha Dmitriev On Aug. 31, 2017, 4:14 p.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62007/ > ----------------------------------------------------------- > > (Updated Aug. 31, 2017, 4:14 p.m.) > > > Review request for sentry, Arjun Mishra, Brian Towles, Misha Dmitriev, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1909 > https://issues.apache.org/jira/browse/SENTRY-1909 > > > Repository: sentry > > > Description > ------- > > SENTRY-1909 Improvements for memory usage when full path snapshot is sent > from Sentry to NN > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > 898c7bec2e35e6f1424478666282ba78222da709 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java > 20b3e108cc976207a3809bc6a214a34e10788200 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java > e403d7cca5dca5c39785490f92e562dc9a3b4daf > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java > fee5279a7cd3d60df4e60a5f25d2e8604c37d04b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PathsImage.java > 409a557fe89d85df888c1f16b3f5aacdd9aa96b0 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 593b92f96b47f959266280bce3d0809f6a80c362 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java > 760a2b55e98ac1fb305666cbbedfba11d26f2fcc > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateModifier.java > 2cd18ea9d2ca40b5f46aaafde532fb3bf9769c07 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java > 4a8fb953ce486e1aeb1042884de56872b5539cd0 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 3da6a4e998d36cada7e9ea77285b11f07cea5f25 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateInitializer.java > 9d7bddd339513180e627286d8a749b7814c824f2 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateModifier.java > 7deccb064fd75b39af779796d9e95caf9c718b32 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java > f56384a99e128b3e9880cbc5692107d61f2f500f > > > Diff: https://reviews.apache.org/r/62007/diff/3/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >
