> On Sept. 28, 2018, 4:56 p.m., Sergio Pena wrote: > > 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? > > Arjun Mishra wrote: > Sergio, the cache is releases as soon as Deltas are being sent to NN
what if there are no Deltas available to send in the next NN requests? The cache will live on memory until then. Look for Java soft or weak references (and the use of SoftReferences or WeakReferences). These objects might help on letting the GC to clean your cache if it is not needed anymore. > On Sept. 28, 2018, 4:56 p.m., Sergio Pena wrote: > > 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/diff/6/?file=2092470#file2092470line168> > > > > 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). > > Arjun Mishra wrote: > This is because the logic to invalidate cache is dependent on whether > delta updates are being sent or not. Since the decisision to send delta > updates is done in DBUpdateForwarder this logic sits in this piece of code Do you need the delta number to invalidate metadata for a snapshot? The PathImageRetriever can keep a cache of the current snapshot and invalidate its cache if the next snapshot ID is different, otherwise return the full path image from the cache (read a SoftReference example on how help clean the cache if it is not used). > On Sept. 28, 2018, 4:56 p.m., Sergio Pena wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java > > Lines 136-137 (patched) > > <https://reviews.apache.org/r/68547/diff/6/?file=2092473#file2092473line136> > > > > 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. > > Arjun Mishra wrote: > Sergio we haev 2 path classes PathImageRetriever, PathDeltaRetriever. > Even though we need cache to be in PathImageRetriever, we need the cache to > be visible to PathDeltaRetriever so it can be invalidated. Is a cache needed for deltas as well? I think the cache should be handled internally on each retriever. - Sergio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68547/#review209098 ----------------------------------------------------------- On Sept. 28, 2018, 7:56 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68547/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2018, 7:56 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-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/7/ > > > Testing > ------- > > > Thanks, > > Arjun Mishra > >
