> On June 20, 2017, 7:16 p.m., Misha Dmitriev wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > > Line 62 (original), 62 (patched) > > <https://reviews.apache.org/r/60168/diff/3/?file=1754054#file1754054line62> > > > > One question, just in case: do you know what's the typical expected > > size of these lists? LinkedList may be more economical than ArrayList with > > default capacity when both are empty or contain only one element. So if > > there is a big chance that somewhere you may have a large number of > > empty/small list, it makes sense to at least instantiate ArrayLists with a > > capacity smaller than the default 10 elements.
Some objects may be very small (for DB/Table locations, but majority should be very long (partitions location). There are relatively small number of databases, way bigger number of tables and huge number of partitions. Difficult to say what is the optimal size. > On June 20, 2017, 7:16 p.m., Misha Dmitriev wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > > Line 83 (original), 83 (patched) > > <https://reviews.apache.org/r/60168/diff/3/?file=1754060#file1754060line84> > > > > Same question on whether the default ArrayList capacity is good here? I think the default size is good enough here - the number of privileges should be reasonably small, but we don't have to super-optimize since the whole thing is rather small. > On June 20, 2017, 7:16 p.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Line 2492 (original), 2491 (patched) > > <https://reviews.apache.org/r/60168/diff/3/?file=1754065#file1754065line2492> > > > > Same question on the default capacity. Here I think it is fine to have the default size. Even if we have a smaller list, it is temporaty, so allocating in one chunk seems a good idea. > On June 20, 2017, 7:16 p.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Line 3306 (original), 3305 (patched) > > <https://reviews.apache.org/r/60168/diff/3/?file=1754065#file1754065line3306> > > > > Same question about the default capacity. Same thing here. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60168/#review178415 ----------------------------------------------------------- On June 20, 2017, 6:35 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60168/ > ----------------------------------------------------------- > > (Updated June 20, 2017, 6:35 a.m.) > > > Review request for sentry, Brian Towles, Misha Dmitriev, kalyan kumar > kalvagadda, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1806 > https://issues.apache.org/jira/browse/SENTRY-1806 > > > Repository: sentry > > > Description > ------- > > SENTRY-1806: Improve memory handling for HDFS sync > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java > 6b09dc265324e574448dc2de0d59ad45fe481fb8 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/DeltaRetriever.java > 0e585933ab021dd12116c9a29fbe306cbdeedcb9 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 6b31f7a4bbff66f6640813774ef79958990d7a57 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java > 14a4a0f6372bb4016a3a1134b348f334273dea7c > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java > 7304fd8bb30a793aae2e6afcddf25c44fe5d226b > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > b8542b3ed7c1e4062fd89c6de12cc79b3d941062 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java > 5425114d38434dc8e6b1e4776be631b9b37fc6af > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java > a29fc749828d673f951994fbbc555fc7a00a714c > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > 5964f17e6316f8a72e701068a0b086e77763819f > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java > 49d2360a87399bfa4dfc5b2f5fd6f517dca2708b > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java > fc8bf4f6bce9b7c58a9152d3a340dfb687f23598 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java > 28d6f5baad590ceb00bfc0c3d50675d91879489a > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java > f51894bb7e109c37997e7134e07a82f46c0a3c44 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 8b19c88e52400ce096cf8f38fe2ca0313f7e46e1 > > > Diff: https://reviews.apache.org/r/60168/diff/3/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >
