> 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)) > > Arjun Mishra wrote: > 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.
The corrent logic should actually be if both Path and Perm update state is at SENDING_DELTA_UPDATES. That is a garuntee that full updates have already been sent, since deltaas are requested only after full updates were 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 > >
