----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64820/#review194479 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java Line 239 (original), 253 (patched) <https://reviews.apache.org/r/64820/#comment273254> Add another item: "The current notification Id on the HMS is less than the latest processed by Sentry after the configurable number of retries, as detected by isSentryAhead() method" sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java Lines 329 (patched) <https://reviews.apache.org/r/64820/#comment273255> This "else" clause is also engaged as part of the normal flow. Because calling isSentryAhead() method is part of the normal flow and normally "if (latestSentryNotificationId > currentHmsNotificationId)" would be FALSE, so this "else" clause would be engaged. Therefore, we should print this message only if we know we are in the state of trying to recover from "Sentry-ahead" situation. Which is when notificationSyncRetryCount != 0. Therefore, this log should be inside the "if (notificationSyncRetryCount != 0)" clause, it seems. - Vadim Spector On Dec. 24, 2017, 1:50 a.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64820/ > ----------------------------------------------------------- > > (Updated Dec. 24, 2017, 1:50 a.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, kalyan kumar > kalvagadda, Na Li, Sergio Pena, and Vadim Spector. > > > Repository: sentry > > > Description > ------- > > Once SentryHMSNotification table is not empty, there are two cases when a > full snapshot is triggered. > > If first event in list of notifications received from HMS greater than latest > sentry hms notification Id > If latest sentry notification Id is greater than current hms notification Id > The later is a unexpected case where for some reason sentry gets ahead of > HMS. We should add a logic to trigger a full snapshot in case 2 only after a > configurable number of retries. This will avoid unnecessary full snapshot > triggers as they are resource intensive > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java > a9afb151a > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > aa1b6a31c > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java > edde886a7 > > > Diff: https://reviews.apache.org/r/64820/diff/2/ > > > Testing > ------- > > mvn -f sentry-provider/sentry-provider-db/pom.xml test -Dtest=TestHMSFollower > > > Thanks, > > Arjun Mishra > >
