----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68622/#review208371 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java Line 145 (original), 144 (patched) <https://reviews.apache.org/r/68622/#comment292257> change the message to String.format("(%s) is sending full update to HDFS, Reason: Requested Seq num:(%d) is not available in deltas", retrieverType, seqNum) sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java Lines 52 (patched) <https://reviews.apache.org/r/68622/#comment292258> since you enable the state here, should you disable the state once you are done? sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java Lines 54 (patched) <https://reviews.apache.org/r/68622/#comment292263> disable state once fetching is done sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java Lines 53 (patched) <https://reviews.apache.org/r/68622/#comment292262> disable state once fetching is done sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java Lines 57 (patched) <https://reviews.apache.org/r/68622/#comment292261> should disable the state once fetching is done. sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java Lines 74 (patched) <https://reviews.apache.org/r/68622/#comment292260> we should seperate the logic for path update from perm update. They can be independent. sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java Lines 75 (patched) <https://reviews.apache.org/r/68622/#comment292259> this line is the same as line 74. Why do you duplicate it? sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java Lines 195 (patched) <https://reviews.apache.org/r/68622/#comment292264> you should disable the state SENDING_FULL_UPDATES when you get client request for delta change. Have you realized the state is stored as bitmap? So multiple states can co-exist. You have to disable the state explicitly. In SentryStateBank public static void enableState(String component, SentryState state) { lock.writeLock().lock(); try { states.put(component, states.get(component) | state.getValue()); <- stored as bitmap. Old value is not removed if (LOGGER.isDebugEnabled()) { LOGGER.debug("{} entered state {}", component, state.toString()); } } finally { lock.writeLock().unlock(); } } - Na Li 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 > >
