> On Sept. 4, 2018, 6:32 p.m., Arjun Mishra wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java > > Lines 74 (patched) > > <https://reviews.apache.org/r/68622/diff/1/?file=2082214#file2082214line74> > > > > This condition should be after getUpdatesFrom methods for path and perm > > are called. That is where state SENDING_FULL_UPDATES is set. > > > > Also this should be an OR for Path Updates or Perm updates. We are > > checking for sending full update state as opposed to the inital HDFS ACL > > sync > > kalyan kumar kalvagadda wrote: > This logic is hit when the client sends a request after sentry server > sent the full update for perms and paths. > > Arjun Mishra wrote: > Ok got it. So it is logged on the cosecutive call. I still see a problem > though. I think its not fair to say PathUpdate and PermUpdate will be set to > SENDING_FULL_UPDATES at the same time. Your logic is assuming that current > path and perm update state is SENDING_FULL_UPDATES but it can be the case > that PathUpdate is still at FETCHING_FULL_UPDATES, while PermUpdate state > (which usually occurs faster) has completed sending full updates and is now > at SENDING_DELTA_UPDATE. That means when PathUpdate completes sending full > update, this condition will not be true > (SentryStateBank.isEnabled(PathUpdaterState.COMPONENT, > PermUpdaterState.SENDING_FULL_UPDATES) && > SentryStateBank.isEnabled(PathUpdaterState.COMPONENT, > PermUpdaterState.SENDING_FULL_UPDATES))
I think instead of verifying if HDFS ACL sync is complete, we can only log if Path Full Update has been sent, and Perm Full Update has been sent. - Arjun ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68622/#review208319 ----------------------------------------------------------- On Sept. 4, 2018, 4:42 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68622/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2018, 4:42 p.m.) > > > Review request for sentry and Arjun Mishra. > > > Bugs: SENTRY-2287 > https://issues.apache.org/jira/browse/SENTRY-2287 > > > Repository: sentry > > > Description > ------- > > Currently there is no way to confirm that HDFS ACL synchronization is > complete when snapshot is initiated. We need to identify that and log in > console and log file as well. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > 08b16a4df3ea9126f21248365d6096fcdb83f21e > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java > 0d39300fe0fddd205e5a1ed868ee818475628132 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > 2b1618134921a594e137a0339cf517f7ccd9bc03 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java > b9405ccd23594db6218af2cd184c82ce59ae5ec4 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > f3a2d5028a3e429b450894b3fe12526a1392e40a > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java > 5e2d5c5ee6bd5a65aebc6d00e6e3f4a506cf2b07 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/PathUpdaterState.java > PRE-CREATION > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/PermUpdaterState.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/68622/diff/1/ > > > Testing > ------- > > > Thanks, > > kalyan kumar kalvagadda > >
