----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68547/#review209098 -----------------------------------------------------------
What if NN does not request full images anymore? How can the cache be released from memory to avoid unused cache for the rest of the process life? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java Lines 53-55 (patched) <https://reviews.apache.org/r/68547/#comment293355> Why is a configuration needed to whether invalidate the cache or not? Cache invalidation should be done by a certain condition that happens on the server. For instance, if the snapshot ID read from the DB is different from the cache, then it needs to invalidate the whole snapshot in the cache, or if the latest notification in the DB is newer than the cache, then it invalidates. I prefer not to add a new configuration for this. sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java Line 158 (original), 168-172 (patched) <https://reviews.apache.org/r/68547/#comment293360> I don't think this logic whether use a cache or not should be in this clas. The retriever should call the cache instead (or call the SentryStore if it needs to invalidate the current cache image). sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java Lines 136-137 (patched) <https://reviews.apache.org/r/68547/#comment293358> Why is the cache needed to be instantiated and passed as aparameter instead of instatiating the cache inside the PathImageRetriever and PermImageRetriever instead? A retriever returns the paths, but if they're cache, then it returns the ones from the cache. - Sergio Pena On Sept. 27, 2018, 10:29 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68547/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2018, 10:29 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena. > > > Bugs: SENTRY-2370 > https://issues.apache.org/jira/browse/SENTRY-2370 > > > Repository: sentry > > > Description > ------- > > When NN requests path updates from sentry and if it exceeds the time > threshold, on consecutive attempts sentry will attempt to fetch the full > update from scratch. Instead it should cache it and update the cache before > sending it to NN > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageCache.java > PRE-CREATION > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java > 2d21411e2 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > 08b16a4df > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathsUpdateImageCache.java > PRE-CREATION > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermissionsUpdateImageCache.java > PRE-CREATION > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java > b8f5ce7db > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java > f86ce6f83 > > > Diff: https://reviews.apache.org/r/68547/diff/6/ > > > Testing > ------- > > > Thanks, > > Arjun Mishra > >
