> 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. > > Arjun Mishra wrote: > 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
Here is the logic makes sure that thar both PathUpdaterState and PermUpdaterState are in SENDING_FULL_UPDATES and there was a request from Namenode asking for path and perm sequences what which are greater than the sequence numbers that were served. If we change the logic as you suggested it could take longer as there need not be updates after a full update was sent. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68622/#review208319 ----------------------------------------------------------- On Jan. 24, 2019, 6:38 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68622/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2019, 6:38 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 > 81c614a345c122e067ec0a19b8f75766390b2ad4 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > 3532ef33dde8b119ab095861381fa52fa5520f4c > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java > 8d6713acd61f99940f72c4985098dfeabb9fc832 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > 443434127b14fadaeb27717ad2370dcdc10ca70c > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java > 0cd405b54ee2b4bf788dffb1ac606362614f6efe > > 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 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryStateBank.java > 2afe919c31b0c5f495d54c6448593abad38eddc2 > > > Diff: https://reviews.apache.org/r/68622/diff/2/ > > > Testing > ------- > > > Thanks, > > kalyan kumar kalvagadda > >
