> 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
This logic is hit when the client sends a request after sentry server sent the full update for perms and paths. > 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 76 (patched) > > <https://reviews.apache.org/r/68622/diff/1/?file=2082214#file2082214line76> > > > > Do we need the isClientRequestingNextSeqId check? The current states > > should be enough This logic is hit when the client sends a request after sentry server sent the full update for perms and paths. This check will make sure that client has succefully processed the full update and is asking for new updates. > 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 77 (patched) > > <https://reviews.apache.org/r/68622/diff/1/?file=2082214#file2082214line77> > > > > This log message should be sending full path/permission updates as > > opposed to ACL synchorication complete. Also it should be after the > > retVal.set method Please check method "updateState" which is invoked in getPathsUpdatesFrom and getPermissionsUpdatesFrom where the state is set to SENDING_FULL_UPDATES. > 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 215 (patched) > > <https://reviews.apache.org/r/68622/diff/1/?file=2082214#file2082214line215> > > > > Path and Perm sequence won't necessarily have a difference of 1. > > SENTRY_PERM_CHANGE and SENTRY_PATH_CHANGE tables are cleaned and the > > sequence gap can be larger. > > > > I think we can get rid of this method isClientRequestingNextSeqId and > > be completely dependent on state Client always requests the last seq number it received + 1. SentryAuthzUpdate getUpdates() { if (sentryClient == null) { try { sentryClient = SentryHDFSServiceClientFactory.create(conf); } catch (Exception e) { LOG.error("Error connecting to Sentry !!", e.getMessage()); sentryClient = null; return null; } } try { return sentryClient.getAllUpdatesFrom( authzInfo.getAuthzPermissions().getLastUpdatedSeqNum() + 1, authzInfo.getAuthzPaths().getLastUpdatedSeqNum() + 1, authzInfo.getAuthzPaths().getLastUpdatedImgNum()); } catch (Exception e) { sentryClient = null; LOG.error("Error receiving updates from Sentry", e); return null; } } - kalyan kumar ----------------------------------------------------------- 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 > >
